#2503: wxdigit: wrong undo & redo
-------------------------+--------------------------------------------------
Reporter: mlennert | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: wxGUI | Version: svn-trunk
Keywords: digitizer | Platform: Unspecified
Cpu: Unspecified |
-------------------------+--------------------------------------------------
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).
Replying to [ticket:2503 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).
Replying to [comment:1 mmetz]:
> Replying to [ticket:2503 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).
>
> Please try r63265 (trunk) or r63266 (relbr70).
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.
Replying to [comment:2 mlennert]:
> Replying to [comment:1 mmetz]:
> > Replying to [ticket:2503 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).
> >
> > Please try r63265 (trunk) or r63266 (relbr70).
>
> 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.
Replying to [comment:3 mmetz]:
> Replying to [comment:2 mlennert]:
> > Replying to [comment:1 mmetz]:
> > > Replying to [ticket:2503 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).
> > >
> > > Please try r63265 (trunk) or r63266 (relbr70).
> >
> > 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.
>
Replying to [comment:4 mlennert]:
> Replying to [comment:3 mmetz]:
> > Replying to [comment:2 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.
Replying to [comment:5 mmetz]:
> Replying to [comment:4 mlennert]:
> > Replying to [comment:3 mmetz]:
> > > Replying to [comment:2 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.
Replying to [comment:6 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.
Replying to [comment:7 mmetz]:
> Replying to [comment:6 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.
Replying to [comment:8 mlennert]:
> Replying to [comment:7 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).
Replying to [comment:9 mmetz]:
> Replying to [comment:8 mlennert]:
> > Replying to [comment:7 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.
Replying to [comment:10 mlennert]:
> Replying to [comment:9 mmetz]:
> > Replying to [comment:8 mlennert]:
> > > Replying to [comment:7 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).
Replying to [comment:11 mmetz]:
> Replying to [comment:10 mlennert]:
> > Replying to [comment:9 mmetz]:
> > > Replying to [comment:8 mlennert]:
> > > > Replying to [comment:7 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.
Replying to [comment:12 mmetz]:
> Replying to [comment:11 mmetz]:
> > Replying to [comment:10 mlennert]:
> > > Replying to [comment:9 mmetz]:
> > > > Replying to [comment:8 mlennert]:
> > > > > Replying to [comment:7 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' ?
Replying to [comment:13 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).
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 ?
Replying to [comment:17 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 ?