Undo and Redo button

AlvaroS

Hello to all.
First of all thank for helping me and reading this post.

I would like to do a undo and redo button in a Scene and QGraphicsView.

I did this:

void MainWindow::on_MoveLines_clicked()
{
    sceneOld=scene;

    for (int i=0; i<num_item_selected; i++) //We get the values of X1,x2,y1,y2 of each line.
    {
..................
..................
        scene->removeItem(my_item); //Remove the item to add the new item moved.
        linea_moved = scene->addLine(.....)
        linea_moved ->setFlag(QGraphicsItem::ItemIsSelectable, true); // Every items can be selected
          }
}


void MainWindow::on_Undo_Button_clicked()
{
    scene=sceneOld;
    ui->graphicsView->setScene(sceneOld);
}

So when I click on MoveLines button I move lines selected.
Then when I click on Undo_Button I would like to undo the line selected and come back to the last scene.

I save the scene in sceneOld before to move lines. Then when I click on Undo_button I set the sceneOld in graphicsView.
It compiles fine but it does not work.
When I click the undo_button it happens nothing... the scene is the same...

How can I do that or what I am doing wrong¿

thanks a lot.

jsulm

Since scene is a pointer this:

sceneOld=scene;

just stores pointer to the same scene in sceneOld.
If you want to do it this way you need to make a copy of the scene and handle memory management to avoid memory leaks.

AlvaroS

@jsulm Yes. Tha is what happen.
How can I do a copy of the scene?

jsulm

I think you should not do it this way. Else memory consumption will grow and you have to ensure the correct scene is used. And I don't know whether you can have more than one scene.
You should record changes you did to the scene and undo them (move the lines back to their old positions).
Especially if you're going to have more than one undo steps.

AlvaroS

@jsulm Yes but How can I undo the accions? I have just one scene.
I have had a look to http://doc.qt.io/qt-4.8/qundocommand.html#undo
should I use that?

thanks a lot.

jsulm

You can undo the actions in the same way you change the scene in on_MoveLines_clicked().
You need to know how you changed it before to undo the change.
I think you can use QUndoCommand.

mrjj

HI
as @jsulm mentions you should have a look at
http://doc.qt.io/qt-5/qundo.html

Keeping a copy of scene for each change will fast be
non optimal.

Also user expect undo to undo more than a line move.
Like on delete and other changes so handling all operations
that should be undoable will be complex.
The Undo Framework helps manage this but
it does takes some reading and setting up.

AlvaroS

@mrjj I do not understand at all..

I imagine that I have to save the position of Stack before moving a line and when I click on undoButton the application has to come back to the oldPosition.
I have had a look at this example but I can not understand at all... http://doc.qt.io/qt-5/qtwidgets-tools-undoframework-example.html

mrjj

@AlvaroS
hi
Good example.
Yes, you will implement all operations as QUndoCommand childs
and undoStack->push them when applied.

void MainWindow::addTriangle()
{
    QUndoCommand *addCommand = new AddCommand(DiagramItem::Triangle,
                                              diagramScene);
    undoStack->push(addCommand);
}

jsulm

"when I click on undoButton the application has to come back to the oldPosition" - you are responsible for that. The undo/redo framework only provides some help to simplify it, but it cannot know what undo/redo in each and every application means.
So, before changing something you store the information about what you change on the stack. If undo button is clicked you get this information from the stack and change the scene, so it is like it was before that change.
From the example:

MoveCommand::MoveCommand(DiagramItem *diagramItem, const QPointF &oldPos,
                 QUndoCommand *parent)
    : QUndoCommand(parent)
{
    myDiagramItem = diagramItem;
    newPos = diagramItem->pos();
    myOldPos = oldPos;
}

We save both the old and new positions for undo and redo respectively.

void MoveCommand::undo()
{
    myDiagramItem->setPos(myOldPos);
    myDiagramItem->scene()->update();
    setText(QObject::tr("Move %1")
        .arg(createCommandString(myDiagramItem, newPos)));
}

SGaist

Hi,

In the absolute, you should rather store both parameters and do the initial move in the redo function reimplementation. That way you can go back and forth in the undo stack.

i.e.:

MoveCommand::MoveCommand(DiagramItem *diagramItem, const QPointF &newPosition,
                 QUndoCommand *parent)
    : QUndoCommand(parent)
   ,  newPosition(newPosition)
   , oldPosition(diagramItem->pos())
   , diagramItem(diagramItem)
{
}

void MoveCommand::redo()
{
    diagramItem->setPos(newPosition);
    diagramItem->scene()->update();
}


void MoveCommand::undo()
{
    diagramItem->setPos(oldPosition);
    diagramItem->scene()->update();
}

AlvaroS

@SGaist @jsulm @mrjj Thanks a lot for answering. I am going to take the tips that you gave me in this post. When I achieve it I will post here!!! Thanks!!