moved QGraphicsItem don't have correct coordinates

[Gilboonet]

@Asperamanca Thank you for your recommendations. I already tried to override itemChange but didn't find what to do to have my item coordinates stay up-to-date.

My problem occur when I have items that have the scene as parent, and that I move one or more of them, then select them and click on a button that runs the code that take the first of them and make it parent of the others, and then compute new positions of the new children items so that they form a kind of triangles ribbon. The problem is that there's an offset for each triangle that was moved before. To be able to move an item then get its correct position I need to clear the selection then redo it, but a scene.clearSelection on my code doesn't do the trick. And this problem doesn't occur when I do the same with an item that has children.

my code was

/*QVariant TriangleItem::itemChange(GraphicsItemChange change, const QVariant &value)
{
    if (change == ItemPositionChange && scene()) {
        QPointF vpf = value.toPointF();
        vpf = mapToParent(vpf);
        return vpf;

    }

    return QGraphicsItem::itemChange(change, value);
}*/

[Pl45m4]

@Gilboonet said in moved QGraphicsItem don't have correct coordinates:

My problem occur when I have items that have the scene as parent

Is this really needed?!
Usually QGraphicsItems only know about parent graphic items, if any.
The scene can be retrieved from items using scene() anyway. So why do you need that parent-child relation?

What does TriangleItem inherit exactly? QObject + QGraphicsItem (making it basically a QGraphicsObject)?

[Gilboonet]

@Pl45m4 Here's how my application works.

User loads a 3d model that is displayed on two QGraphicsScenes, one for 3D and one for 2D
Then he selects triangles to form triangles ribbons

At the beginning of this process, the TriangleItems that are created have the scene as parent and when they become part of a ribbon they become child of another TriangleItem.

You're right, there's no need for TriangleItems to have scene as parent, nullptr is better. I just changed the code, indeed there was no explicit scene2d parenting but they are created with

        gp = new TriangleItem(parent, p, t.id, t.col);
        t2d.push_back(gp);
        gp->setFlag(QGraphicsItem::ItemIsMovable);
        scene2d->addItem(gp);
        deltaW = p.boundingRect().right() + 2;

the parent parameter for TriangleItem constructor is used to keep track of the MainWindow to keep data sync between the two QGraphicsScenes.

TriangleItem inherits QGraphicsPolygonItem

[Pl45m4]

@Gilboonet said in moved QGraphicsItem don't have correct coordinates:

You're right, there's no need for TriangleItems to have scene as parent, nullptr is better.

TriangleItem inherits QGraphicsPolygonItem

But how was this possible anyway?

A QGraphicItem can only have another QGraphicsItem as parent which a QGraphicsScene is not.
That's why I asked how your item is implemented and if it's a QGraphicsObject.

the parent parameter for TriangleItem constructor is used to keep track of the MainWindow to keep data sync between the two QGraphicsScenes.

When your items need to know about what is going on in MainWindow, I think then there is room for design improvements.
Usually you want to avoid that and keep objects without any logic as simple as possible. It might also lead to errors.

Can't think of the perfect solution right now, but what is happening should not happen (positioning/transformation is off, as you describe). And it's definitely doable.

Is every selected and moved item at the wrong position? Or only in some situations, when "pressing your button"/calling some function...?

[Gilboonet]

@Pl45m4

class TriangleItem : public QGraphicsPolygonItem
{
public:
    enum { Type = UserType + 1 };
    int type() const override { return(Type);}
    TriangleItem();
    TriangleItem(MainWindow *, QPolygonF, int, int);
    MainWindow *w;
    int id;
    int col;
    bool estPrem = false;
    bool estLie = false;
    void paint(QPainter *, const QStyleOptionGraphicsItem *, QWidget *);
    //void mousePressEvent(QGraphicsSceneMouseEvent *);
    //void mouseMoveEvent(QGraphicsSceneMouseEvent *);
    //void mouseReleaseEvent(QGraphicsSceneMouseEvent *);
    //QVariant itemChange(GraphicsItemChange change, const QVariant &value);
};

TriangleItem::TriangleItem(MainWindow *window, QPolygonF poly, int id, int col = 0)
{
    w = window;
    this->setParentItem(0);
    this->id = id;
    setData(0, QVariant(id));
    this->col = col;
    setPolygon(poly);
    setFlag(ItemIsSelectable);
    setFlag(ItemIsFocusable);
    setFlag(ItemSendsGeometryChanges);
    setFlag(ItemSendsScenePositionChanges);
    //setCacheMode(NoCache);
    //setZValue(-1);
    estLie = false;
    estPrem = false;
}

The behaviour that I have is : When I move items, then use them into a selection, apply color on that selection, then click on the button that create triangle ribbons from same color items (create items with children), the move items will show an offset with the ribbon that corresponds to their move.

the code that creates a triangle ribbon from same color items : (the first transform is the one that should put an item close to its sibling but fail when the item moved just before)

void MainWindow::couleurAssemble()
{
dep->scene2d->clearSelection();
if (ui->tableCouleurs->rowCount() < 2)
return;

QFont tf = QFont("");
tf.setPointSize(3);

for (int n = 1; n < ui->tableCouleurs->rowCount(); n++) {
if (dep->pool[n].elements.size() == 0)
continue;

QList<int> wPool = dep->pool[n].elements;

for (auto && p : dep->pool[n].pieces){
    for (auto && l : p.lignes) {
        delete(l.li);
    }
}

dep->pool[n].pieces.clear();

// Commencer avec la 1ère face
commencePose(&wPool, n);

int nbKO = 0;
bool ok = false;
int idParentCourant = -1;
while (wPool.size() > 0) { //  Boucler sur ce qui est posé
    //int np = 1;
    for (auto&& p : dep->pool[n].pieces) {
        for (auto && piece : p.attaches) {
            int wP = piece.vers;
            TriangleItem *tSource = dep->t2d[wP];
            if (piece.de == -1){
                idParentCourant = piece.vers;
                QPointF tpos = tSource->scenePos();
                tSource->setParentItem(0);
                tSource->setPos(tpos);
            }
            ok = false;
            for(auto&& v : dep->meshModel->faces[wP].voisins) {
                if (wPool.contains(v.nF)) {
                    p.attaches.push_back(Attache(wP, v.nF));
                    TriangleItem *tCible = dep->t2d[v.nF];
                    tCible->setParentItem(dep->t2d[idParentCourant]);
                    QPolygonF pCible = tCible->polygon();
                    QPolygonF pSource = tSource->polygon();
                    //pCible = dep->t2d[v.nF]->mapToScene(pCible);
                    QPointF ptOrig = pSource[v.id];
                    QPointF delta = ptOrig - pCible[v.idx];
                    QTransform trT;
                    trT.translate(delta.x(), delta.y());
                    pCible = trT.map(pCible);

                    QTransform trR;
                    qreal angle = calc_angle(
                        ptOrig,
                        pSource[next(v.id)],
                        pCible[prev(v.idx)]);
                    trR.translate(ptOrig.x(), ptOrig.y());
                    trR.rotate(-angle);
                    trR.translate(-ptOrig.x(), -ptOrig.y());
                    dep->t2d[v.nF]->setPolygon(trR.map(pCible));
                    dep->t2d[v.nF]->estLie = true;
                    dep->t2d[v.nF]->estPrem = false;
                    dep->t2d[v.nF]->update();

                    TriangleItem *TI = dep->t2d[v.nF];
                    QPolygonF P = TI->polygon();
                    QList<Ligne> L = p.lignes;
                    for (int i = 0; i < 3; i++) {
                        QPointF P1 = P[i];
                        QPointF P2 = P[next(i)];
                        Voisin V = dep->meshModel->faces[v.nF].voisins[i];
                        int nl = -1;
                        int il = 0;
                        for (auto&& R : L) {
                            if (   (eq(R.p1, P1) && eq(R.p2, P2))
                                || (eq(R.p1, P2) && eq(R.p2, P1))) {
                                nl = il;
                                break;
                            } else
                                il++;
                        }
                        if (nl > -1) {
                            p.lignes[nl].nb++;
                        } else {
                            Ligne li (P1, P2, v.nF, V.nF, V.cop);
                            p.lignes.append(li);
                        }
                    }

                    wPool.remove(wPool.indexOf(v.nF));
                    ok = true;
                    break;
                }
            }
            if (ok) {
                break;
            }
        }
        if (ok) {
            break;
        }
    }

    if (!ok)
        nbKO++;

    if (nbKO > 1) { // créer nouvelle pièce
        commencePose(&wPool, n);
        nbKO = 0;
        ok = false;
    }
}

for (auto&& P:dep->pool[n].pieces) {
    TriangleItem *ti0 = dep->t2d[P.attaches[0].vers];
    for (auto&& L:P.lignes) {
        QGraphicsLineItem *li = new QGraphicsLineItem(L.p1.x(), L.p1.y(), L.p2.x(), L.p2.y());
        QPen p = QPen();
        p.setColor(L.nb == 1 ? Qt::black : L.cop < 1 ? Qt::darkRed : Qt::green);
        if (L.nb == 1)
            p.setStyle(Qt::SolidLine);
        else if (L.cop == 0)
            p.setStyle(Qt::NoPen);
        else
            p.setStyle(L.cop < 1 ? Qt::DashLine : Qt::DashDotLine);
        li->setPen(p);
        li->setParentItem(ti0);
        li->setData(0, QVariant::fromValue(-1));
        li->setFlag(QGraphicsItem::ItemIsSelectable, true);
        L.li = li;
        if (L.nb == 1) {
            QGraphicsSimpleTextItem *ti = new QGraphicsSimpleTextItem(QString::number(L.id1));
            ti->setFont(tf);
            ti->setParentItem(li);
            ti->setPos(- ti->boundingRect().width()/2, - ti->boundingRect().height()/2);
            QPointF delta = (L.p1 + L.p2)/2;
            ti->moveBy(delta.x(), delta.y());
        }
    }
}

}
dep->scene2d->update();
}

I keep track of MainWindow because I need to update one of its members where are the application data, it would be better to only keep track of this data member, it's what I did on my first version of that application. And on that first version, to be able to keep track of items moves I also override the mouse events, I'm still hoping to be able do avoid that.

[Asperamanca]

@Gilboonet said in moved QGraphicsItem don't have correct coordinates:

my code was

/*QVariant TriangleItem::itemChange(GraphicsItemChange change, const QVariant &value)
{
    if (change == ItemPositionChange && scene()) {
        QPointF vpf = value.toPointF();
        vpf = mapToParent(vpf);
        return vpf;

    }

    return QGraphicsItem::itemChange(change, value);
}*/

That code would do some damage if you enabled it. "ItemPositionChange" allows you to manipulate the position of an item as the position is changed.
I'm pretty should it would confuse the default move mechanism.

Positions are item coordinates. Item coordinates are always relative to the parent item (or to the scene, if parentItem == nullptr).

If I understood correctly, you have a bunch of items that have no parent (directly in scene), and you want to make one of them a parent of the others, without changing visual positions.

1. The new parent item would be unchanged.
2. You set that parent item to all other items
3. Because their coordinate system has changed, the items will be moved by the base position of the new parent item
4. Don't be tempted to calculate this yourself. Use the "map" functions provided by QGraphicsItem. I'll figure out the exact functions to use tomorrow (they are a bit confusing, and I have to leave, unfortunately), unless you find them by then

[Gilboonet]

@Asperamanca Yes my code created a mess into the Scene, that's why I commented it.

For the moment my code to create a triangle ribbon work, it does so from one of the items and then attach others items to it by making it their parent, and compute their coordinates. But if I moved one or more items with the mouse into the scene, their new coordinates are not updated.

Indeed, I'm not using pos but directly manipulate the polygon of the item, its maybe here that lies the problem if the default mouse handler of an overriden QGraphicsPolygonItem doesn't change its polygon, I might need to add the code to handle that. I can try to do that into the itemchange function.

[Asperamanca]

@Gilboonet That sounds plausible. Mixing QGraphicsItem::pos() and positions of polygon points is a popular source of confusion.

[Gilboonet]

@Asperamanca I only change polygon points. What I do is identify a source item, then its target item, find on each of their polygons points that correspond and compute the offset between them, then create a transform that translate of the amount of this offset, then map the target polygon using the transform. This doesn't involve pos().

I tried to use itemchange to update the polygon points using the offset of newpos and oldpos of the item, but it didn't work either. Maybe is it because i finish my code with a return value ?

/*QVariant TriangleItem::itemChange(GraphicsItemChange change, const QVariant &value)
{
    if (change == ItemPositionHasChanged && scene()) {
        QPointF delta = value.toPointF() - this->pos();
        QPolygonF tp = this->polygon();
        QTransform tr;
        tr.translate(delta.x(), delta.y());
        this->setPolygon(tr.map(tp));
    }

    return value;
    //return QGraphicsItem::itemChange(change, value);
}*/

[Asperamanca]

If you want to modify the value as it is in the process of changing, you need to use "ItemPositionChange".
"ItemPositionHasChanged" is just a read-only notification and will ignore the return value.

However, you have to check if the default moving logic of QGraphicsItem can handle you manipulating positions on the fly. If not, you'll have to wait for the move to be finished (that's actually tricky to find out) and update things after. Post if you run into this problem, I'll keep an eye on this thread.

[Gilboonet]

@Asperamanca I'm ok with ItemPositionChange and ItemPositionHasChanged, here what I wanted to do is updating the polygon points using the offset of the item positions changes, but it didn't change anything and the moved item keep its gap. I don't see how I could use ItemPosition as it only is useful when you want to limit the moves to an area or things like that, here my problem is that the item polygon data don't sync when it is moved. I already have a working solution from my first version of this application where I handle mouse events and keep the items position up-to-date, but I would like to use the Graphics View Framework default behaviour as much as possible.

[Asperamanca]

@Gilboonet Right, I overlooked that you don't change the return value anyway.
But I noticed something else: You change the polygon points, which is likely to change the bounding rect. But you need to call prepareGeometryChange() before doing anything that will change the bounding rect.
Now, I don't know if that doesn't cause trouble when called in the context of a position change...

[Gilboonet]

@Asperamanca I had a prepareGeometryChange() on the item before I change it's polygon, but it fired error :

void QGraphicsItem::prepareGeometryChange()' is protected within this context

my assembly function is a slot of my MainWindow class, not inside the TriangleItem class, I maybe need to rethink this function and make it as member of TriangleItem, but then I would need TriangleItem to be aware of my dataclass (who is currently a member of my MainWindow class). On my first version of this application, what I did was adding a pointer to this data class on every class constructor that needed to use it.

[Asperamanca]

@Gilboonet Ah....setting the polygon points probably does it internally. Sorry for the confusion.

[Asperamanca]

@Gilboonet said in moved QGraphicsItem don't have correct coordinates:

my assembly function is a slot of my MainWindow class, not inside the TriangleItem class, I maybe need to rethink this function and make it as member of TriangleItem, but then I would need TriangleItem to be aware of my dataclass (who is currently a member of my MainWindow class). On my first version of this application, what I did was adding a pointer to this data class on every class constructor that needed to use it.

My approach would be to have an implicitly shared data class with the data necessary for one item, have a model that contains all that data, and simply set copies of that data to the items (cheap because implicitly shared).
But that depends on your structure and data throughput.

[JonB]

@Asperamanca
Yes, I think when you call QGraphicsPolygonItem::setPolygon() or similar you can assume the Qt code for that calls prepareGeometryChange() for you.. That method is protected, so it is for people subclassing QGraphicsItem themselves to call, which QGraphicsPolygonItem itself should do.