Undo LPE editing in node tool crashes

[Johan Engelen]

1. Start Inkscape
2. Open history dialog (ctrl+shift+L), open LPE dialog (ctrl+shift+&)
3. Draw freehand shape (without spiro, shape: none)
4. Add LPE Bend
5. Click "edit on-canvas" button for the bend path
6. Change the bend path a bit by dragging it
7. In the history dialog, click "draw path" (2 steps back) ==> a knot appears at the top-left of the document page. Shouldn't be there!
8. In the history dialog, click "drag curve" (2 steps forward again) ==> crash

Backtrace in GDB:
(inkscape.exe:2472): GLib-GObject-WARNING **: invalid uninstantiatable type `(null)' in cast to `LivePathEffectObject'

Program received signal SIGSEGV, Segmentation fault.
0x009f2a93 in Inkscape::UI::PathManipulator::_createControlPointsFromGeometry ()
(gdb) bt
#0 0x009f2a93 in Inkscape::UI::PathManipulator::_createControlPointsFromGeometry ()
#1 0x009f33d8 in Inkscape::UI::PathManipulator::_externalChange ()
#2 0x009aedac in Inkscape::XML::CompositeNodeObserver::notifyAttributeChanged ()
#3 0x00615bb6 in Inkscape::XML::SimpleNode::setAttribute ()
#4 0x00629ecd in Inkscape::XML::EventChgAttr::_replayOne ()
#5 0x0062a55c in Inkscape::XML::replay_log_to_observer ()
#6 0x0062ad7f in sp_repr_replay_log ()
#7 0x0051f9d8 in sp_document_redo ()
#8 0x007ad97a in Inkscape::UI::Dialog::UndoHistory::_onListSelectionChange ()
#9 0x664d7412 in Glib::SignalProxyNormal::slot0_void_callback () from D:\inkscape\trunk\inkscape\libglibmm-2.4-1.dll
#10 0x63a44124 in g_closure_invoke () from D:\inkscape\trunk\inkscape\libgobject-2.0-0.dll
#11 0x63a57c7a in signal_emit_unlocked_R ()

Inkscape version 0.47+dev r9199. Windows XP

[Krzysztof Kosinski (tweenk)]

The fundamental cause of this crash is a design error: there is no common superclass for SPPath and LivePathEffectObject, and it's not possible to access the LPEObject from the SP tree.

During undo, the selection modification signal doesn't get called, and neither does the LivePathEffectObject deletion signal. At the moment I have no idea how to fix this short of rewriting a significant portion of the SP tree related to LPEs, but maybe I'll find some way.

[Krzysztof Kosinski (tweenk)]

I re-analyzed this problem once more and I still think it's not solvable without invasive changes to the XML and/or SP subsystems, so it's definitely not going to be in 0.48.1.

[ScislaC (scislac)]

Krzysztof: Is this actually "in progress"?

[su_v (suv-lp)]

Reverting status to 'Triaged' (no response to earlier question).

[Jabiertxof (jabiertxof)]

This fix the bug by other way.

[Jabiertxof (jabiertxof)]

This fix a hunk

[Jabiertxof (jabiertxof)]

This cleanup the patch. Also put in a better place position of parameter on paste

[jazzynico (jazzynico)]

Patch tested on Windows XP, Inkscape trunk rev. 14968.

The crash is fixed, but the LPE curve doesn't apply to the path when reaching step 8.

A console message shows at step 7 (Document was modified while being updated after undo operation), and an empty line is added in the Undo history dialog when clicking again on "Draw path" after step 8 (the console then shows a "Incomplete undo transaction" warning).

[jazzynico (jazzynico)]

Tested again with the V2 patch. Same issue, but the "Document was modified while being updated after undo operation" no longer shows.

[Jabiertxof (jabiertxof)]

Diferent approach. less chages

[Jabiertxof (jabiertxof)]

Fixes position of shapes if the LPE item has transforms

[jazzynico (jazzynico)]

V5 tested on Windows XP. Almost good. Now the path is correct after step 8, but the original path is still visible (see screenshot).

Note that the "Document was modified while being updated after undo operation" message is back when reaching step 7.

Maybe unrelated:
After step 8, switch the the selector tool, and back to the node tool. Then go back to step 7 to trigger a crash.

[Jabiertxof (jabiertxof)]

This patch fix some positioning on paste from clipboard and also hope fixes the node tool crash pointed by Jazinico.
The other two questions about warnings and state of the path after unodo seems difuicult to fix if not "imposilbe" without a hard rework of all.

[Jabiertxof (jabiertxof)]

Im hurry on previous comment.
I Just fixed the warnings but no warnings but sometime crash, so I thik is beter warnings. I think get the path parameter selected trought undo is not posible actualy.

[Jabiertxof (jabiertxof)]

@jazzinico Is ok this.
-----------------------
V5 tested on Windows XP. Almost good. Now the path is correct after step 8, but the original path is still visible (see screenshot).
-----------------------

Also happends when select the path with the node tool. Only hides on activate pathparameter, and this I think couldent be fired on undo

[Jabiertxof (jabiertxof)]

Minor things

[Jabiertxof (jabiertxof)]

Remove unwanted comment

[jazzynico (jazzynico)]

Patch v8 tested on Xubuntu 16.04, with inkscape rev. 14974.

The crash is fixed (even the residual one explained comment #12). The original path is still visible, but it's not a big issue in my humble opinion.
Also tested with some other LPE, and found no obvious regression.

Thanks Jabier!

[Jabiertxof (jabiertxof)]

You are welcome jazzynico!

[Jabiertxof (jabiertxof)]

fixed on r.14975