#2503 closed defect (fixed)
wxdigit: wrong undo & redo
| Reported by: | mlennert | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.0.0 |
| Component: | wxGUI | Version: | svn-trunk |
| Keywords: | digitizer | Cc: | martinl |
| CPU: | Unspecified | Platform: | Unspecified |
Description
The undo and redo buttons in the digitizer (accessed via the Map Display) seem to work well for simple features, but as soon as I use tools like split line or similar, the results of undo and redo become somewhat arbitrary (at least to the innocent user).
See a screencast of the issue.
Change History (18)
follow-up: 2 comment:1 by , 9 years ago
follow-up: 3 comment:2 by , 9 years ago
Replying to mmetz:
Replying to mlennert:
The undo and redo buttons in the digitizer (accessed via the Map Display) seem to work well for simple features, but as soon as I use tools like split line or similar, the results of undo and redo become somewhat arbitrary (at least to the innocent user).
Much better, thanks !
There still is some confusion when one goes back one or two steps, then digitizes something else. When you then undo and redo again, the stack seems to be mixed between the different paths, sometimes leading to objects left on screen even when going all the way back to the last possible undo.
You can check a short example screencast and a longer example screencast.
Moritz
follow-up: 4 comment:3 by , 9 years ago
Replying to mlennert:
Replying to mmetz:
Replying to mlennert:
The undo and redo buttons in the digitizer (accessed via the Map Display) seem to work well for simple features, but as soon as I use tools like split line or similar, the results of undo and redo become somewhat arbitrary (at least to the innocent user).
Much better, thanks !
There still is some confusion when one goes back one or two steps, then digitizes something else.
The question is what should happen with the available redo steps if digitize something new after some undo steps. Eliminate the redo steps or insert the new changeset before the first redo step? This is handled by the wx digitizer, not the vector lib.
When you then undo and redo again, the stack seems to be mixed between the different paths, sometimes leading to objects left on screen even when going all the way back to the last possible undo.
It seems that a new changeset is appended after the last redo step if available, leading to a mix-up.
follow-up: 5 comment:4 by , 9 years ago
Replying to mmetz:
Replying to mlennert:
Replying to mmetz:
Replying to mlennert:
The undo and redo buttons in the digitizer (accessed via the Map Display) seem to work well for simple features, but as soon as I use tools like split line or similar, the results of undo and redo become somewhat arbitrary (at least to the innocent user).
Much better, thanks !
There still is some confusion when one goes back one or two steps, then digitizes something else.
The question is what should happen with the available redo steps if digitize something new after some undo steps. Eliminate the redo steps or insert the new changeset before the first redo step? This is handled by the wx digitizer, not the vector lib.
I've tried three different programs (LibreOffice Writer, Inkscape and GIMP) and all discard these redo steps,i.e. when you do A, then B, then undo B, then do C, you cannot find B again.
This seems to be the most intuitive behaviour to me.
When you then undo and redo again, the stack seems to be mixed between the different paths, sometimes leading to objects left on screen even when going all the way back to the last possible undo.
It seems that a new changeset is appended after the last redo step if available, leading to a mix-up.
Yes.
Moritz
follow-up: 6 comment:5 by , 9 years ago
Replying to mlennert:
Replying to mmetz:
Replying to mlennert:
There still is some confusion when one goes back one or two steps, then digitizes something else.
The question is what should happen with the available redo steps if digitize something new after some undo steps. Eliminate the redo steps or insert the new changeset before the first redo step? This is handled by the wx digitizer, not the vector lib.
I've tried three different programs (LibreOffice Writer, Inkscape and GIMP) and all discard these redo steps,i.e. when you do A, then B, then undo B, then do C, you cannot find B again.
This seems to be the most intuitive behaviour to me.
OK.
When you then undo and redo again, the stack seems to be mixed between the different paths, sometimes leading to objects left on screen even when going all the way back to the last possible undo.
It seems that a new changeset is appended after the last redo step if available, leading to a mix-up.
Yes.
Fixed in trunk r63341, please test.
follow-up: 7 comment:6 by , 9 years ago
Replying to mmetz:
Replying to mlennert:
Replying to mmetz:
Replying to mlennert:
There still is some confusion when one goes back one or two steps, then digitizes something else.
The question is what should happen with the available redo steps if digitize something new after some undo steps. Eliminate the redo steps or insert the new changeset before the first redo step? This is handled by the wx digitizer, not the vector lib.
I've tried three different programs (LibreOffice Writer, Inkscape and GIMP) and all discard these redo steps,i.e. when you do A, then B, then undo B, then do C, you cannot find B again.
This seems to be the most intuitive behaviour to me.
OK.
When you then undo and redo again, the stack seems to be mixed between the different paths, sometimes leading to objects left on screen even when going all the way back to the last possible undo.
It seems that a new changeset is appended after the last redo step if available, leading to a mix-up.
Yes.
Fixed in trunk r63341, please test.
Much better !
There is still one issue I've come upon: when features are automatically modified from the form they are digitized in because of overlaps, the second original feature causing the overlap seems to be erased from the undo stack, meaning that it remains, even if you undo all the way to origin or if you close without saving.
To reproduce, digitize a line and then a second line that crosses that line. The lines are correctly split at the intersection. If you then undo the second line remains in its original (unsplit) form. The same is true for two overlapping polygons or a line crossing a polygon.
follow-up: 8 comment:7 by , 9 years ago
Replying to mlennert:
There is still one issue I've come upon: when features are automatically modified from the form they are digitized in because of overlaps, the second original feature causing the overlap seems to be erased from the undo stack, meaning that it remains, even if you undo all the way to origin or if you close without saving.
To reproduce, digitize a line and then a second line that crosses that line. The lines are correctly split at the intersection. If you then undo the second line remains in its original (unsplit) form. The same is true for two overlapping polygons or a line crossing a polygon.
The list of updated lines as returned by the vector lib was incomplete. Fixed in r63349,50 (trunk, relbr70), please test.
follow-up: 9 comment:8 by , 9 years ago
Replying to mmetz:
Replying to mlennert:
There is still one issue I've come upon: when features are automatically modified from the form they are digitized in because of overlaps, the second original feature causing the overlap seems to be erased from the undo stack, meaning that it remains, even if you undo all the way to origin or if you close without saving.
To reproduce, digitize a line and then a second line that crosses that line. The lines are correctly split at the intersection. If you then undo the second line remains in its original (unsplit) form. The same is true for two overlapping polygons or a line crossing a polygon.
The list of updated lines as returned by the vector lib was incomplete. Fixed in r63349,50 (trunk, relbr70), please test.
Still not quite: http://tomahawk.ulb.ac.be/moritz/wxdigit_undo_redo4.ogv.
If you need more explanation, I can write some, but right now I have to go.
Moritz
follow-up: 10 comment:9 by , 9 years ago
Replying to mlennert:
Replying to mmetz:
The list of updated lines as returned by the vector lib was incomplete. Fixed in r63349,50 (trunk, relbr70), please test.
Still not quite: http://tomahawk.ulb.ac.be/moritz/wxdigit_undo_redo4.ogv.
If you need more explanation, I can write some, but right now I have to go.
Got it. This bug (yet another one) should be fixed in r63364,5 (trunk only).
follow-up: 11 comment:10 by , 9 years ago
Replying to mmetz:
Replying to mlennert:
Replying to mmetz:
The list of updated lines as returned by the vector lib was incomplete. Fixed in r63349,50 (trunk, relbr70), please test.
Still not quite: http://tomahawk.ulb.ac.be/moritz/wxdigit_undo_redo4.ogv.
If you need more explanation, I can write some, but right now I have to go.
Got it. This bug (yet another one) should be fixed in r63364,5 (trunk only).
Wow, great detective work there ! As of now I haven't been able to find any other bugs linked to undo/redo. This fix should be backported to grass7release and then I think we can close this bug for now.
Thanks a lot for the fixes !
Moritz
follow-up: 12 comment:11 by , 9 years ago
Replying to mlennert:
Replying to mmetz:
Replying to mlennert:
Replying to mmetz:
The list of updated lines as returned by the vector lib was incomplete. Fixed in r63349,50 (trunk, relbr70), please test.
Still not quite: http://tomahawk.ulb.ac.be/moritz/wxdigit_undo_redo4.ogv.
If you need more explanation, I can write some, but right now I have to go.
Got it. This bug (yet another one) should be fixed in r63364,5 (trunk only).
Wow, great detective work there ! As of now I haven't been able to find any other bugs linked to undo/redo. This fix should be backported to grass7release and then I think we can close this bug for now.
I do not fully understand 1) why the fix is needed, 2) why the fix is working. The GRASS vector library usually employs Copy On Write (COW) when a feature is modified. Sometimes a feature is modified in place, overwriting the previous version. The digitizer's undo/redo mechanism relies on COW, but the move tool seems to replace an older version of the feature, thus undo/redo should not work with the move tool (and maybe other tools, too).
follow-up: 13 comment:12 by , 9 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Replying to mmetz:
Replying to mlennert:
Replying to mmetz:
Replying to mlennert:
Replying to mmetz:
The list of updated lines as returned by the vector lib was incomplete. Fixed in r63349,50 (trunk, relbr70), please test.
Still not quite: http://tomahawk.ulb.ac.be/moritz/wxdigit_undo_redo4.ogv.
If you need more explanation, I can write some, but right now I have to go.
Got it. This bug (yet another one) should be fixed in r63364,5 (trunk only).
Wow, great detective work there ! As of now I haven't been able to find any other bugs linked to undo/redo. This fix should be backported to grass7release and then I think we can close this bug for now.
I do not fully understand 1) why the fix is needed, 2) why the fix is working.
The fix is working because V2_rewrite_line_nat() does not work as expected, it does not rewrite, it always deletes the line and writes out a new line (COW). All fixes related to this ticket have been backported to relbr70 in r63397,8 after testing.
follow-up: 14 comment:13 by , 9 years ago
Replying to mmetz:
Replying to mmetz:
Replying to mlennert:
Replying to mmetz:
Replying to mlennert:
Replying to mmetz:
The list of updated lines as returned by the vector lib was incomplete. Fixed in r63349,50 (trunk, relbr70), please test.
Still not quite: http://tomahawk.ulb.ac.be/moritz/wxdigit_undo_redo4.ogv.
If you need more explanation, I can write some, but right now I have to go.
Got it. This bug (yet another one) should be fixed in r63364,5 (trunk only).
Wow, great detective work there ! As of now I haven't been able to find any other bugs linked to undo/redo. This fix should be backported to grass7release and then I think we can close this bug for now.
I do not fully understand 1) why the fix is needed, 2) why the fix is working.
The fix is working because V2_rewrite_line_nat() does not work as expected, it does not rewrite, it always deletes the line and writes out a new line (COW). All fixes related to this ticket have been backported to relbr70 in r63397,8 after testing.
You just signficantly improved the use of the digitizer, thanks a lot !
Just one question concerning r63398: shouldn't the code just be deleted instead of using 'if 0' ?
Moritz
comment:14 by , 9 years ago
Replying to mlennert:
Just one question concerning r63398: shouldn't the code just be deleted instead of using 'if 0' ?
I am not sure if the new PostGIS interface needs this, Martin would know more about this, therefore I deactivated the code instead of deleting the code. The same applies to the list of updated nodes (not used by the digitizer, but by the PostGIS functions in the vector lib, I think).
Markus
comment:15 by , 9 years ago
| Cc: | added |
|---|
comment:16 by , 9 years ago
Martin,
Any opinion on the question concerning the ifdef'ed out code ?
For the rest we can close this ticket.
follow-up: 18 comment:17 by , 9 years ago
Using current trunk, any redo operation in wxdigit makes the GUI crash. Don't know if this is related to this ticket or whether I should open a new one. Can anyone confirm the behaviour ?
comment:18 by , 9 years ago
Replying to mlennert:
Using current trunk, any redo operation in wxdigit makes the GUI crash. Don't know if this is related to this ticket or whether I should open a new one. Can anyone confirm the behaviour ?
Yes, confirmed here (I have Ubuntu).
Replying to mlennert:
Please try r63265 (trunk) or r63266 (relbr70).