Search code examples
qtundoqgraphicsitemqgraphicssceneredo

Issue with Qt Undo Framework example: add / remove item


I was using the QT undo framework example as reference to implement this functionality in my tool. However, it seems to have a bug with the way that it calls the destructor of the items.

I understand that QGraphicsScene will assume ownership of the items while they are in the scene. However, both undo objects: AddCommand and RemoveCommand should assume ownership of these items when they remove them from the scene.

In the Qt undo framework example, only AddCommand tries to delete the object in its destructor, but will not do it if the item is still in the scene.

AddCommand::~AddCommand()
{
    if (!myDiagramItem->scene())
        delete myDiagramItem;
}

In this case, if we remove the item from the scene after the corresponding AddCommand object leave the stack (when using an undo limit), the item will never be deleted again since RemoveCommand destructor doesn't do it.


Solution

  • I fixed it using a flag in both AddCommand and RemoveCommand classes. It informs when this objects should be responsible for the item destruction. When they remove the item from the scene, I set this flag to true, and I test this flag in the undo object destructor before calling the item's destructor:

    AddCommand::AddCommand(QGraphicsScene *scene, DraftGraphicItem* item, QUndoCommand *parent):
        scene(scene), item(item), QUndoCommand(parent){
        setText("Add item to scene");
    }
    
    AddCommand::~AddCommand(){
        if(isItemOwner)
            delete item;
    }
    
    void AddCommand::undo(){
        Q_ASSERT(item->scene()); 
        scene->removeItem(item);
        isItemOwner = false;
    }
    
    void AddCommand::redo(){
        Q_ASSERT(!item->scene()); 
        scene->addItem(item);
        isItemOwner = true;
    }
    

    and the same with RemoveCommand, just inverting redo() and undo() methods.