How to use Undo-Redo functionality in Schematic feature ?

[tushu]

I have a following schematic. After double clicking on line d, it gets expanded.

After expanding

For this schematic, I want to give Undo - Redo effect. For that I have already created buttons. ( in 1st pic you can see)
Through this , I have already given undo-redo effect for "Zoom-in" "Zoom-out", "Fit-in" feature using "QUndoCommand" .

But in this feature, how to give undo-redo feature, I am confused.
This is the way I tried but not understanding, how to proceed further.

In guiSchematicViewer class I have already created object of "QUndoStack" like this

_undoStack = new QUndoStack(this);

How to proceed further, can anyone help me ?

[tushu]

@SGaist , @JonB
Can you help me in this ?

[JonB]

@tushu
You show a (lovely but) complicated picture & code for what you are trying to do, I'm afraid the details are beyond me! The basics of Qt undo/redo however are pretty simple.

Have you started by looking through Overview of Qt's Undo Framework and Undo Framework Example? Notre, say, AddCommand Class Definition. That is like your "expand" as it "adds" something to the scene, and undoing will remove it. You create a subclass of QUndoCommand for each "action". Your subclass has enough private variables to be able to perform the action in the first place, and thereby enough to revert it if requested. This class is what you push to the QUndoStack.

To keep your code "clean", write the redo() so that it can be called the very first time to do the action in the first place, and call that to perform it. Otherwise you end up with code duplicated for "first time" and "redo". In your case that might be the bool reBuild = ... statement, which should be moved to a redo() function. You have to do this because void QUndoStack::push(QUndoCommand *cmd)

Pushes cmd on the stack or merges it with the most recently executed command. In either case, executes cmd by calling its redo() function.

So when you first push the undo command onto the stack it will execute redo() (whether you like it or not).

[tushu]

@JonB Thanks for your reply.

I have written redo() but not understanding what to write in undo() ?

void guiUndoExpansionInIncrementalSchematic::undo()
{

}

void guiUndoExpansionInIncrementalSchematic::redo()
{
    bool reBuild = _ctxt->ReBuildIncrSchematic(_sView, _bGraph, _vd, _utObj,
                                                _stepOver->isChecked() ? true : false, _boostGraph);
    
}

[tushu]

@JonB

I have tried this way

But currently it is crashing. I am finidng out the reason.

My main function ( from where schematic call is coming) is like this now

Here I am not understanding how to handle

return false

Because redo() function does not return bool value.

[JonB]

@tushu
Yes, undo/redo() do not return a value, so what? I don't what else there is to say. redo() should do whatever, and undo() should undo whatever redo() did. Save whatever state is necessary in the QUndoCommand-derived class. The docs/examples tell you everything, did you take the time to study them?

[tushu]

@JonB Thanks for your reply.

I have some doubts.

When I will create expand object, and will push into _undoStack, redo() will get called and in redo I am executing original API, which will create schematic. ( It is working properly)

So I understood what redo() will do.

And what undo() will do ? It will undo what redo() has done.

So what I am considering , undo() will clear the screen , but i want previous data ( here schematic).

So basically I am confused. What should be the functionality of undo() ?

Can you help me in it ?

[JonB]

@tushu
undo() must undo precisely what redo() did, no more and no less. If the original action was doing something about "expanding" which added or made another item visible then the undo should remove just that. It should not clear the whole diagram. You should save state in the QUndoCommand-derived class about adding the item or whatever you do, or if necessary the state of the previous schematic (not a great implementation, but possible), so that you can back out just that change.

[jsulm]

@tushu Please post code as text, not pictures

[tushu]

@jsulm , @JonB

Thanks for reply.

In my code,when I expand schematic ( i.e. all objects in the scene ) by double clicking, I draw all the objects in the scene once again.

my current code :

bool guiSchematicViewer::ExpandOnDoubleClick(BGType* bGraph, VertexDescriptor vd, utDbObj* utObj, bool instDoubleClick)
{
    if (_schType == SCH_TYPE::DESIGN_INCR_SCH) {
        if (auto schCtxt = schContextManager::GetContext(guiMgr::GetInstance()->GetContextName())) {
            if (guiVerificCtxt* vfCtxt = DynamicCast<guiVerificCtxt*>(schCtxt)) {
              
                guiUndoExpansionInIncrementalSchematic* undoExpand = new
                        guiUndoExpansionInIncrementalSchematic(
                            this,bGraph,vd,utObj,vfCtxt,instDoubleClick,
                            _boostGraph);
 
                if (instDoubleClick) {
                    vd = vfCtxt->AddItemInBG(bGraph, utObj);
                }
                _currentSelectedObject.clear();
                std::string hierName = (*bGraph)[boost::graph_bundle]._name;
                guiUndoExpansionInIncrementalSchematic* expand = new
                        guiUndoExpansionInIncrementalSchematic(
                            vfCtxt, this, bGraph, vd, utObj, _stepOver->isChecked() ? true : false, _boostGraph,
                            _stepOver, _view, _overlappingDialog, _hideUnhideDialog, hierName);
                _undoStack->push(expand);
                return true;
            }
        }
    }
    return false;
}

Following piece of code (from above code) simply tries to pass some states ( parameter ) to undo().

Note that, I have not pushedundoExpand object into stack.

But problem is that, at undo() side, by using these values I want to re draw previous state but these parameter are not showing correct values. So I can not re draw previous state.

guiUndoExpansionInIncrementalSchematic* undoExpand = new
                       guiUndoExpansionInIncrementalSchematic(
                           this,bGraph,vd,utObj,vfCtxt,instDoubleClick,
                           _boostGraph);

Now below code ( from above code only ) is for redo(). In redo() I am using these values and drawing schematic, which works perfectly.

guiUndoExpansionInIncrementalSchematic* expand = new
                        guiUndoExpansionInIncrementalSchematic(
                            vfCtxt, this, bGraph, vd, utObj, _stepOver->isChecked() ? true : false, _boostGraph,
                            _stepOver, _view, _overlappingDialog, _hideUnhideDialog, hierName);
                _undoStack->push(expand);

Note that, here I have pushed object into stack.

Doubt 1 )
Why undoExpand objects parameters are not showing correct values at undo() side ?
( I am trying to draw schematic with old values through undo() and with new values through redo() )

Doubt 2 : If above way is wrong, then how to store, current view's value ? Means thorugh undo() I will show previous view's value ?

Please help me in understanding this.

[JonB]

@tushu said in How to use Undo-Redo functionality in Schematic feature ?:

But problem is that, at undo() side, by using these values I want to re draw previous state but these parameter are not showing correct values.

Why undoExpand objects parameters are not showing correct values at undo() side ?

( I am trying to draw schematic with old values through undo() and with new values through redo() )

You keep showing complex code to do with whatever your "schematics" library is/does. How can anybody but you know what this does, how this works or what you have to do? Only you know this. As I have said repeatedly, it is your responsibility to do whatever is necessary to either undo a specific action or restore to a fully-defined state which was in place at the time of the redo(). Either way you must discover and save-for-undo whatever information is necessary to restore to previous full state.

then how to store, current view's value ? Means thorugh undo() I will show previous view's value ?

That is your job, whatever is necessary. Any variables for state should be stored in your class derived from QUndoCommand, in member variables you define and populate. It will need to hold whatever so that it can both redo() and undo() the particular action.

[SimonSchroeder]

From the last code you have shown it looks like you don't want the regular undo/redo feature. Here is the regular idea of a QUndoCommand: When the user does an action you create a new command and push it onto the QUndoStack. Normally, the actions are undone through the undo stack. This might be through a shortcut (Ctrl+Z) or the menu. The QUndoCommand has both a redo() and an undo() function. When you push the command on the stack it will execute the redo function. When you undo the stack, you don't create a new command, but the undo stack will just execute the undo function of the same command that is already stored on the stack. And the stack does not drop your command immediately, so you could actually undo the undo operation (i.e. the redo). It allows you to redo the exact same command which is stored on the undo stack.

I am not sure if your double click action would fit right in to the undo/redo functionality. It is probably not what a user expects. The only way this could work out is if you have both an expand undo command and a collapse undo command. So, if you double click to expand you push the expand undo command to the undo stack. The use could now undo to collapse again and after redo to expand again. However, if the user double clicks again you would push a collapse undo command on the undo stack. What that means is that your implementation for redo() in your expand undo command is the same as the undo() in your collapse undo command and vice versa.

Before you write any undo commands you should figure out what to do when double clicking to expand or collapse. If this code works without first creating an undo command you can then put the code for expansion into the redo() function and the code for collapse into the undo() function of your expand undo command (and the other way around for your collapse command -- you might want to have ExpandCollapseUndoCommand which receives an additional boolean (or better an enum) to choose between expansion or collapse). If you can't figure out what to do without the undo command you will not be able to write the undo command.