Undo/redo for moving QGraphicsLineItem

I am moving different entities like point, line, circle and ellipse in my scene. I am implementing undo-redo using the Undo-Redo Framework provided in Qt. I get correct coordinates for point when it is moved from one position to another i.e. I get its scene coordinates.

When I move onto other entities, I get coordinates in the stack in the terms of their local coordinates, not the scene coordinates. How do I get the scene coordinates of other entities?

My move command is defined as:

Code:

---

class CommandMove : public QUndoCommand
{
public:
    CommandMove(QGraphicsItem *item, qreal fromX, qreal fromY,
                   qreal toX, qreal toY)
    {
        m_item = item;
        mFrom = QPointF(fromX, fromY);
        mTo = QPointF(toX, toY);
        setText(QString("Point move (%1,%2) -> (%3,%4)").arg(fromX).arg(fromY)
                .arg(toX).arg(toY));
    }
 
    virtual void undo()
    {
        m_item->setPos(mFrom);
    }
 
    virtual void redo()
    {
        m_item->setPos(mTo);
    }
 
private:
    QGraphicsItem *m_item;
    QPointF mFrom;
    QPointF mTo;
};

---

What should be added/edited in this piece of code so that I get the scene coordinates for all entities?

Also I have set the position of point in mousePressEvent using:

Code:

---

setPos(mouseEvent->scenePos);

---

For line, I do using setLine() function but it doesn't give the scene coordinates of the end points of the line. Help me solve this issue.

Thanks in advance!

If part of the process of moving items is reparenting them to an item that is underneath then you will need to store the parent item as well as the position relative to that item. So the whole move operation will consist of changing {par1, pos1} to {par2, pos2}. If moving items does not change their parents then I'm not sure what is the problem you are having as moving the item would not result in changing the parent and again using pos() and setPos() should be enough.

For completeness -- there is QGraphicsItem::scenePos() to reed the position relative to the scene and there is a family of mapTo*() and mapFrom*() (including QGraphicsItem::mapFromScene()) to map coordinates between different systems.

By the way -- if you have a "delete" among the commands you handle in your undo/redo system, be sure that you think twice how to handle that in combination of storing pointers to items in other commands -- if you really delete an item, its pointer will become invalid and all other stored commands dealing with that item will contain dangling pointers.

Quote:

---

Originally Posted by wysota

If part of the process of moving items is reparenting them to an item that is underneath then you will need to store the parent item as well as the position relative to that item. So the whole move operation will consist of changing {par1, pos1} to {par2, pos2}. If moving items does not change their parents then I'm not sure what is the problem you are having as moving the item would not result in changing the parent and again using pos() and setPos() should be enough.

---

There is nothing like changing parents in my implementation.

Quote:

---

For completeness -- there is QGraphicsItem::scenePos() to reed the position relative to the scene and there is a family of mapTo*() and mapFrom*() (including QGraphicsItem::mapFromScene()) to map coordinates between different systems.

---

I have also come across this. But I am not able to produce the expected results.

Quote:

---

By the way -- if you have a "delete" among the commands you handle in your undo/redo system, be sure that you think twice how to handle that in combination of storing pointers to items in other commands -- if you really delete an item, its pointer will become invalid and all other stored commands dealing with that item will contain dangling pointers.

---

Ah! Thank you for pointing this out.

I will soon be posting a minimal example to show my implementation so that the problem can be rectified after scrutinizing.

Quote:

---

Originally Posted by Kamalpreet

There is nothing like changing parents in my implementation.

---

I don't know what the following means then:

Quote:

---

Originally Posted by Kamalpreet

When I move onto other entities, I get coordinates in the stack in the terms of their local coordinates, not the scene coordinates.

---

I have managed to obtain the old and updated coordinates of line using line().

In my move command, I have defined my undo() and redo() as follows:

Code:

---

virtual void undo()
{
    m_item->setLine(oldStart.x(), oldStart.y(), oldEnd.x(), oldEnd.y());
    qDebug() << m_item->line();
}
 
virtual void redo()
{
    m_item->setLine(newStart.x(), newStart.y(), newEnd.x(), newEnd.y());
    qDebug() << m_item->line();
}

---

The debugging gives me correct coordinates, both old and new. However, the line stays at the same position in the graphics scene. Does undo() and redo() update the position using setPos() only?
What kind of implementation can be done in the case of line so that it is updated in the scene?

setLine() does not modify the item's position! It only modifies the line (and implicitly the bounding rect) the item represents. The item is still "anchored" where it was (by default at (0,0) in its parent's coordinate system).

Quote:

---

Originally Posted by wysota

setLine() does not modify the item's position! It only modifies the line (and implicitly the bounding rect) the item represents. The item is still "anchored" where it was (by default at (0,0) in its parent's coordinate system).

---

So for undo(), redo() I can use only setPos()? Is it so? I am unable to figure out an alternative.

It really depends on your application logic. I always opt for adjusting item position to implement the "move" operation. Of course often you have to adjust the bounding rect too.