Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undo/redo buttons in Editor widget are not functional #135

Open
fedorov opened this issue Jul 23, 2016 · 11 comments
Open

Undo/redo buttons in Editor widget are not functional #135

fedorov opened this issue Jul 23, 2016 · 11 comments
Labels

Comments

@fedorov
Copy link
Member

fedorov commented Jul 23, 2016

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

@fedorov fedorov added the bug label Jul 23, 2016
@che85
Copy link
Member

che85 commented Jul 25, 2016

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

@fedorov
Copy link
Member Author

fedorov commented Jul 25, 2016

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
Copy link

pieper commented Jul 25, 2016

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

@pieper
Copy link

pieper commented Jul 25, 2016

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

@che85
Copy link
Member

che85 commented Jul 25, 2016

@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
Copy link

pieper commented Jul 25, 2016

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
#135 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAHsfYgsminiCtHejd3gGXgEt2ZrVopWks5qZM86gaJpZM4JTbtQ
.

@che85
Copy link
Member

che85 commented Jul 25, 2016

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
Copy link
Member

che85 commented Jul 25, 2016

  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
Copy link

pieper commented Jul 26, 2016

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
#135 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAHsfVPKEiN7mtQiuciiHfyr8VkxLxTjks5qZNvggaJpZM4JTbtQ
.

@fedorov
Copy link
Member Author

fedorov commented Jul 26, 2016

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
Copy link

pieper commented Jul 27, 2016

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
#135 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAHsfRgFw5rhjBlBkyapG69KofN19lX7ks5qZkfygaJpZM4JTbtQ
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants