Bugs item #1684042, was opened at 2007-03-20 01:47
Message generated for change (Comment added) made by gustav_b
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=604306&aid=1684042&group_id=93438
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
>Category: Other
Group: None
>Status: Closed
>Resolution: Fixed
Priority: 9
Private: No
Submitted By: Simon (simon80)
>Assigned to: Gustav Broberg (gustav_b)
Summary: undo while adding a shape with the mouse results in a crash
Initial Comment:
1. Pick a shape tool like rectangle or circle, and click and drag to place one on the page. Do not release the mouse button.
2. With the left mouse button still down, hit Ctrl+Z to undo.
fun ensues :)
----------------------------------------------------------------------
>Comment By: Gustav Broberg (gustav_b)
Date: 2007-03-30 01:07
Message:
Logged In: YES
user_id=1051104
Originator: NO
Ah, I guess you're right. I've added clear undo/redo notifications to the
UndoStackObserver base and hooked them up + added the commit on #1.
Closing this as fixed.
----------------------------------------------------------------------
Comment By: MenTaLguY (mental)
Date: 2007-03-29 05:50
Message:
Logged In: YES
user_id=791
Originator: NO
My feeling is that it would be best to add notifications for #1 and #3,
rather than hiding notifications of all the partial-transaction events.
----------------------------------------------------------------------
Comment By: Gustav Broberg (gustav_b)
Date: 2007-03-28 22:48
Message:
Logged In: YES
user_id=1051104
Originator: NO
I found two problems here.
The first one is that we're prepending an Inkscape::XML::Event to the
document's undo stack which should only contain Inkscape::Events, that
happens in finish_incomplete_transaction (document-undo.cpp:204), it was
my fault, and easy to fix.
The second problem is that the UndoStackObservers get notified about
undos/redos of incomplete transactions, but not the commits of them.
What happens when an ongoing transaction is interrupted with an undo
is the following:
1. The incomplete transaction is pushed on the undo stack by
finish_incomplete_transaction
The UndoStackObservers are _not_ notified about this commit.
2. Immediately afterwards, the incomplete transaction is undone, removed
from the stack, and pushed on the redo stack.
The UndoStackObservers _are_ notified about this undo.
3. The transaction finally finishes in sp_document_(maybe)_done and
the redo log is cleared.
The UndoStackObservers are _not_ notified about this clearing.
So, there's a big difference how this operation is seen by the
undo handling in the document and how it's seen by the
UndoStackObservers.
The handling document will cancel out the commit with an undo, but also
clear the redo stack. The UndoStackObservers, on the other hand, will
see the complete operation as a regular undo (on an transaction that
hasn't even been done).
To fix this difference, one could probably prevent the undo
notifications to be sent if an incomplete transaction just has been
pushed on the undo stack by finish_incomplete_transaction. Mental, is
this a reliable way to solve it?
For now, I've added a check in my undo history's UndoStackObserver to
prevent it from accepting undos or redos of events that aren't the
same as the previous/next events in its own event list. This fixes
the crash for me and is probably a good check to do anyway...
The problem with the clearing of the redo log in step 3 is still left
to solve, though. If there are previously undone events when the
incomplete transaction+undo takes place, the redo stack will be
cleared (which is kind of bad for the end-user, by the way), but the
UndoStackObservers won't know about this. I guess we could add methods
like UndoStackObserver::notify(Redo/Undo)Cleared(); to get around
it.
But for now I think I'll just solve the crash. After all, the redo-clear
problem just results in displaying an Undo History with events that
aren't
redoable, and that's the worst case.
(As a side note, doing the same thing with the star tool results in
another crash. This has been the case since at least 0.44, so that's
most likely a different bug.)
I've committed a fix in rev 14640, could you just double-check that it
fixes it for you too?
----------------------------------------------------------------------
Comment By: bulia byak (buliabyak)
Date: 2007-03-20 21:49
Message:
Logged In: YES
user_id=741217
Originator: NO
Mental, this is the same crash as the one I assigned to you yesterday, in
undo code. I remember we discussed it long ago that it should not crash
when trying to undo with incomplete transaction, and I just checked that
0.43 does not crash in this situation. So this is a regression.
----------------------------------------------------------------------
Comment By: Zounas (zounas)
Date: 2007-03-20 09:24
Message:
Logged In: YES
user_id=1735692
Originator: NO
Happens with Windows version too ;)
----------------------------------------------------------------------
Comment By: Simon (simon80)
Date: 2007-03-20 01:51
Message:
Logged In: YES
user_id=1253354
Originator: YES
Sorry, I should add that I'm running Inkscape 0.45 on Ubuntu 6.10.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=604306&aid=1684042&group_id=93438
|
