Undo/redo buttons in Editor widget are not functional

[fedorov]

While using Pencil tool, undo doesn't seem to work.

[che85]

@fedorov Undo/Redo anyway behaves weird even when using Editor only module ....

[fedorov]

right, I was going to test outside SliceTracker and confirm. If this is a Slicer issue, it should be reported separately. I believe in the past it was possible to undo individual strokes while painting. Maybe it is not supposed to work anymore. cc: @pieper

[pieper]

Can you explain what you mean by behaving weird? Nothing should have changed recently.

[pieper]

I just tried a recent build and undo/redo of paint and draw operations work fine for me.

[che85]

@pieper I just tried nightly from 07/12/2016 and it works better. Other than that, when creating a structure, painting something and then deleting the structure, I can still use undo/redo buttons and nothing happens. The history should be cleared or if after deleting the structure "undo" is pushed, the structure should be reverted.

[pieper]

Hmm, nothing should have changed in years with this behavior so I'm not sure why the nightly from 7/12 would work differently than any other recent version. When you say "it works better" what are you comparing it to? Earlier versions or your locally modified version?

Also, it would help if you clearly stated the sequence of steps to reproduce the behavior you find weird.

Internally the undo/redo list includes a handle of the volume node it applies to, so if you have deleted the volume from the scene then the undo operation is happening to a volume node that is essentially in the recycle bin. Also if you are doing undo/redo operations when switching between per-structure volumes then it's possible to modify a volume that's not currently active. Is this what you are seeing? It might make sense to invalidate the list everytime a new label is selected.

The undo/redo code is pretty simple:

https://github.com/Slicer/Slicer/blob/master/Modules/Scripted/EditorLib/EditUtil.py#L509-L617

On Mon, Jul 25, 2016 at 10:59 AM, Christian Herz notifications@github.com wrote:

@pieper https://github.com/pieper I just tried nightly from 07/12/2016 and it works better. Other than that, when creating a structure, painting something and then deleting the structure, I can still use undo/redo buttons and nothing happens. The history should be cleared or if after deleting the structure "undo" is pushed, the structure should be reverted.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/SlicerProstate/mpReview/issues/135#issuecomment-234978776, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHsfYgsminiCtHejd3gGXgEt2ZrVopWks5qZM86gaJpZM4JTbtQ .

[che85]

Oh sorry the other nightly version that I tried is from 04/26/2016. When creating a structure with that version and using PaintEffect/DrawEffect, the undo/redo buttons are not even enabled.

I will add a list of steps, so that you can hopefully reproduce it.

[che85]

1. Start Slicer and load some DICOM
2. Switch to Editor Module
3. Add structure
4. Use PaintEffect/DrawEffect
5. Draw something
6. Use undo/redo (several times to check if it works)

Until that point everything works just fine.

7. Add another structure (undo/redo buttons are still enabled)
8. Using undo/redo works on the previously created structure which is not anymore visible since the new one is selected
9. Draw something for new structure and undo redo those steps, which should just work fine
10. Redo as many times as possible so that you can see the most recent selected structure

Until that point everything works just fine for second created structure

11. Select first selected structure
12. Click undo, then redo
13. select second created structure (nothing is displayed anymore)

I have the feeling that the history of several structures is somehow mixed up

I hope that works for reproducing

[pieper]

Yes, I can see how that would get screwed up.

I think the change required would be to add a method:

def reset(self): self.undoList = [] self.redoList = []

at line 552 in [1] and then call it each time a different structure is selected.

Bigger question: are you planning to switch over to the Segment Editor soon? If so is this issue in the critical path? (It sounds like it might be more of an annoyance than anything else but if it's critical it shouldn't be too hard to fix).

[1] https://github.com/Slicer/Slicer/blob/master/Modules/Scripted/EditorLib/EditUtil.py#L509-L617

On Mon, Jul 25, 2016 at 11:53 AM, Christian Herz notifications@github.com wrote:

1. Start Slicer and load some DICOM
2. Switch to Editor Module
3. Add structure
4. Use PaintEffect/DrawEffect
5. Draw something
6. Use undo/redo (several times to check if it works)

Until that point everything works just fine.

7. Add another structure (undo/redo buttons are still enabled)
8. Using undo/redo works on the previously created structure which is not anymore visible since the new one is selected
9. Draw something for new structure and undo redo those steps, which should just work fine
10. Redo as many times as possible so that you can see the most recent selected structure

Until that point everything works just fine for second created structure

11. Select first selected structure
12. Click undo, then redo
13. select second created structure (nothing is displayed anymore)

I hope that works for reproducing

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/SlicerProstate/mpReview/issues/135#issuecomment-234995578, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHsfVPKEiN7mtQiuciiHfyr8VkxLxTjks5qZNvggaJpZM4JTbtQ .

[fedorov]

Steve, Ron was telling me there is no Undo/Redo in Segmentations.

In any case, we will definitely wait for few months before considering to integrate. When I tried it, I had quite a few questions about how things worked there, and we can't commit resources to integrate it now anyway.

[pieper]

True, there was no undo/redo in the segment editor last I talked with Andras. But he's quick : )

Anyway it's a very simple change to the Editor to implement what I suggested above. Would you be able to make and test the changes based on my suggestions? Or, maybe we should sit together and do it (or hangout) to avoid a lot of back-and-forth where we try to describe code edits and user interface actions in text.

On Tue, Jul 26, 2016 at 1:46 PM, Andrey Fedorov notifications@github.com wrote:

Steve, Ron was telling me there is no Undo/Redo in Segmentations.

In any case, we will definitely wait for few months before considering to integrate. When I tried it, I had quite a few questions about how things worked there, and we can't commit resources to integrate it now anyway.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/SlicerProstate/mpReview/issues/135#issuecomment-235347962, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHsfRgFw5rhjBlBkyapG69KofN19lX7ks5qZkfygaJpZM4JTbtQ .