Hi, Tyson!

Thank you for your report. The backtrace is really interesting. I will take a look at it.

Reproducible here.

Crash happens when undoing also the "create selection" step. The parent mask of the selection changes, which poisons the pointer stored in the KisSelection object.

Git commit e6352b1ba31af09e7822388cee9fde3e936436bb by Dmitry Kazakov.
Committed on 09/03/2014 at 19:15.
Pushed by dkazakov into branch 'master'.

Fixed a crash in the Undo/Redo of transformations

This patch:

1) Fixes a crash in KisTransactionBasedCommand. The check for first redo
   got lost somewhere during previous refactoring.

2) Fixes a deadlock in KisNode::prev/nextSibling(). The locks must be
   taken in a descending order only! Otherwise it'll result in a deadlock.
   See a comment in KisNode::prevChildImpl() function.

M  +31   -16   krita/image/kis_node.cpp
M  +2    -0    krita/image/kis_node.h
M  +61   -5    krita/image/tests/kis_node_test.cpp
M  +8    -0    krita/image/tests/kis_node_test.h
M  +2    -2    krita/ui/kis_transaction_based_command.cpp
M  +0    -1    krita/ui/kis_transaction_based_command.h

http://commits.kde.org/calligra/e6352b1ba31af09e7822388cee9fde3e936436bb

Git commit 2d328a990dce57798a7205f0b2413b1a7c2842aa by Dmitry Kazakov.
Committed on 09/03/2014 at 19:15.
Pushed by dkazakov into branch 'calligra/2.8'.

Fixed a crash in the Undo/Redo of transformations

This patch:

1) Fixes a crash in KisTransactionBasedCommand. The check for first redo
   got lost somewhere during previous refactoring.

2) Fixes a deadlock in KisNode::prev/nextSibling(). The locks must be
   taken in a descending order only! Otherwise it'll result in a deadlock.
   See a comment in KisNode::prevChildImpl() function.

M  +31   -16   krita/image/kis_node.cpp
M  +2    -0    krita/image/kis_node.h
M  +61   -5    krita/image/tests/kis_node_test.cpp
M  +8    -0    krita/image/tests/kis_node_test.h
M  +2    -2    krita/ui/kis_transaction_based_command.cpp
M  +0    -1    krita/ui/kis_transaction_based_command.h

http://commits.kde.org/calligra/2d328a990dce57798a7205f0b2413b1a7c2842aa

Thank you for the fix! When I was testing the new revision earlier today, Krita still crashed in the first try. However, it was the only crash I encountered so far. Too bad I clicked the wrong button while sending the bug report. I will report back if there is anything new. Meanwhile, thanks again for fixing this bug!

Another crash when I undid a transformation today. I was doing:

1. Cut from a selection
2. Paste into a new layer
3. Free transform (skew)
4. Undo
5. Free transform (skew)
6. Undo

Krita crashed without showing the crash reporter. This happened multiple times in the past few months and I think it's time to reopen this bug.

The thing that caught me on this bug is that I can never reliably reproduce the crash and the crash report never shows up when I finally reproduced it. It always hit when I was super into my work and lost my progress as a result. Krita's crop and transformation tool have suffered huge regression since 2.9.x. It's a real nasty feeling that I cannot trust the tool I use.

Created attachment 93115 [details]
Krita crash backtrace when redoing transformation after cancelling one

Steps to reproduce (Took me a lunch's time to figure it out XD):
1. Create a 1600x1200 new document;
2. Fill layer 2 with red;
3. Outline selection tool, cut something and paste as a new layer;
4. Cancel selection;
5. Transformation tool (I used rotate/skew/move all in one go), confirm;
6. Ctrl+Z to undo;
7. Transformation tool, make some changes but do not confirm, hit ESC to cancel;
8. Ctrl+Shift+Z to redo;
9. Krita crashed.

It really looked like a history issue to me. I'm sure similar thing happens to Crop tool as well. I will try to reproduce it later.

So the workaround is:
Do not cancel a transformation. Always apply. Undo the change after.

Hi, Tyson!

Thank you for your report! I managed to reproduce the latest crash. I'm wondering, do you still have a crash *without* redoing the action? The one without redoing (if it exists) should be a different one.

Hi Dmitry,
Comment 7 is invalid. It was my vague memory of how the crash happened. Comment 9 is the real steps I found after a series of test.

The only crash I can reliably reproduce at the moment was described Comment 9. There is no crash without redoing. 

I cannot reliably reproduce Crop tool crash at the moment. Maybe a different bug, so please ignore it. I will open a new bug as soon as I narrow down the causes.

Thank you very much for looking into this bug!

Git commit a576bf0b627d4bb66d128eb26d077eb9f74023d9 by Dmitry Kazakov.
Committed on 12/06/2015 at 05:59.
Pushed by dkazakov into branch 'calligra/2.9'.

Fix a crash when trying to redo after canceling a stroke

A stroke uses paint device's undo framework, but can be canceled
in the end and therefore will not add anything into undo stack.
So we need to manually cancel all the hanging redo actions, which
are not valid anymore.

M  +8    -0    krita/image/kis_image.cc
M  +1    -0    krita/image/kis_undo_store.h
M  +13   -0    krita/image/kis_undo_stores.cpp
M  +3    -0    krita/image/kis_undo_stores.h
M  +2    -0    krita/plugins/extensions/dockers/historydocker/KisUndoModel.cpp
M  +5    -0    krita/ui/kis_document_undo_store.cpp
M  +1    -0    krita/ui/kis_document_undo_store.h
M  +28   -0    libs/kundo2/kundo2stack.cpp
M  +2    -0    libs/kundo2/kundo2stack.h

http://commits.kde.org/calligra/a576bf0b627d4bb66d128eb26d077eb9f74023d9

Hi, Tyson!

Could you please check this patch? It should fix the redo problem for you...

Hi Dmitry,
It doesn't crash now, but it brings up another problem. I will describe it below.

This crash happened by the following steps:
1. Transform (A) is applied;
2. Transform (A) is undone;
3. Transform (B) is initiated but cancelled;
4. User tries to redo Transform (A);
5. Krita Crashed

After applying the patch in Comment 13, when Transform (B) is canceled in Step 3, Transform (A) (and any strokes after) also got erased from Krita's history and cannot be redone. But Transform (B) is cancelled and should not be considered as a valid history stroke. I think Transform (A) (and all the strokes after) should be kept after Transform (B) is canceled. Undo/redo to compare before/after is what the user wants to do in this situation. The crash prevented him to do so, but it doesn't help if he still cannot undo/redo transformations with canceled attempts in between anyway.

Created attachment 93130 [details]
Krita crash backtrace when doing brush undo/redo

Also a crash happened right after I applied the patch. I was doing undo/redo of brush strokes.

Created attachment 93131 [details]
Krita crash backtrace when doing brush undo/redo

Now it's clear that after the patch, Krita crashes everytime when I do undo >> redo. This backtrace was captured when I did:

1. Created a new document;
2. Brush stroke;
3. Undo;
4. Redo.

Git commit 6c9bb986ab8fb7fbc2df36a4af65e7763aeb5f3c by Dmitry Kazakov.
Committed on 14/06/2015 at 15:29.
Pushed by dkazakov into branch 'calligra/2.9'.

Fix a crash when redoing actions

We should clear redo stack tail only in case we are running non-undo/redo
stroke.
Related: bug 349108

M  +1    -0    krita/image/commands_new/kis_saved_commands.cpp
M  +4    -1    krita/image/kis_image.cc
M  +11   -0    krita/image/kis_stroke_strategy.cpp
M  +11   -0    krita/image/kis_stroke_strategy.h
M  +5    -0    krita/image/kis_stroke_strategy_undo_command_based.cpp
M  +2    -0    krita/image/kis_stroke_strategy_undo_command_based.h

http://commits.kde.org/calligra/6c9bb986ab8fb7fbc2df36a4af65e7763aeb5f3c

Hi, Tyson!

Could you test now if the bug has gone? I guess it should now work correctly.

Hi Dmitry,
It does not crash now, thanks!

However, the problem in Comment 15 still exists. I don't think a cancelled action should be counted as a history stroke -- it has never been applied. Letting a cancelled action flush out all strokes does not feel logical and kills the convenience to compare 2 different attempts.

Hi, Tyson!

The problem is we use undo framework for cancelling the stroke itself. That is when the stroke is started and the image patch is removed from a layer, so we already need to use undo framework to record this removal, which, in turn, clears redo information. I just checked how PS handles this situation and it seems its behaviour is exactly the same. It clears redo info on transformation cancelling.

Probably you could explain the whole usecase why this feature is needed and how it is going to be used?

Hi Dmitry,

I will explain why we need this feature below:

1. Say, I was drawing a warrior. His pose looked a little too stiff. I thought spreading his legs a little helps to establish his heroic image.
2. I used transform tool for the task. <Transform (A)> happened and was applied.  The result looked okay but I felt it can still be improved.
3. I undid <Transform (A)>, try a different approach. <Transform (B)> is initiated.
4. However, in the middle of fiddling <Transform (B)>, I could not produce anything better than <Transform (A)>. So I canceled <Transform (B)>, then naturally redo <Transform (A)> so I can get it back.

Being able to do Step 4 enables the user to recover an undid transform. The ability to revert your undo is what REDO is all about. That's why I think it's a natural and logical way to allow user to do so. 

Without the ability to do so, I will have to:

1. Current layer (A) needs to be transformed;
2. Duplicate Layer (A) into Layer (B);
3. Hide Layer (A);
4. Transform Layer (B);
5. Hide Layer (B);
6. Duplicate Layer (A) into Layer (C);
7. Transform Layer (C);

This duplicating method is way more flexible, but is also too complicated to execute and breaks the flow. It's more like a trick someone needs to learn outside of the box. Whereas Redo is simply implied by the context of Krita's basic user interaction. 

What we really need in most cases is the chance to regret once or twice, get our stuff back immediately. Being able to redeem the last attempt emboldens the user because they know they can easily get the first version back if the second attempt didn't live up the expectation. A more confident user is more likely to experiment transform tool more. In the process, the potential of both the artist and the tool itself can be realized further, which in my opinion, is a beautiful thing.

Krita crashed again when I was undoing transformation today:
1. I was using horizontal Mirror mode;
2. I selected some area, probably crossing the central line;
3. I transformed the selection, confirmed;
4. I undid the transformation;
5. Krita silently crashed without showing crash report.

I could not reproduce the crash. This is the scariest type of crash --  always caught you offguard and eat you progress but can't find its track afterward. -_- Is there a more reliable way to make sure the crash report shows up when it crashed?

Hi, Tyson!

I will try to reproduce your crash. And with the undo/redo thing. I don't think we can enable 'redo' after cancelling the stroke, but we can do something else: we can allow you just to click on the canvas and continue the transformation from where it has been started :)

Hi Dmitry,
Thank you! I will also report should I manage to capture a backtrace from its next crash.

I don't really understand what you mean for "click on the canvas AND CONTINUE the transformation from where it HAS BEEN STARTED" though. Did you actually meant "click on the canvas TO RESUME a transformation from where it WAS CANCELLED"? In that case, it will be useful! Thank you for thinking into this problem! We probably need a new button to reset that cancelled transformation, as well. Preferably located on canvas next to transformation handles, with an icon looks like the reset brush preset button in toolbar.

Git commit e98e4141e0a370585e77b7bacfb0147485784f8a by Dmitry Kazakov.
Committed on 30/06/2015 at 05:37.
Pushed by dkazakov into branch 'calligra/2.9'.

Implemented continuation of the transform with clicking on canvas

M  +6    -0    krita/image/kis_stroke_strategy_undo_command_based.cpp
M  +2    -0    krita/image/kis_stroke_strategy_undo_command_based.h
M  +27   -1    krita/plugins/tools/tool_transform2/kis_tool_transform.cc
M  +1    -0    krita/plugins/tools/tool_transform2/kis_tool_transform.h
M  +53   -0    krita/plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp
M  +11   -0    krita/plugins/tools/tool_transform2/strokes/transform_stroke_strategy.h
M  +14   -0    libs/kundo2/kundo2stack.cpp
M  +17   -1    libs/kundo2/kundo2stack.h
M  +2    -0    libs/kundo2/kundo2stack_p.h

http://commits.kde.org/calligra/e98e4141e0a370585e77b7bacfb0147485784f8a

Thank you Dmitry! ... how can I use this new function though?

Git commit 1a3e977b1d1d8296f5e089221d2d36ac0cb951e8 by Dmitry Kazakov.
Committed on 04/07/2015 at 10:44.
Pushed by dkazakov into branch 'calligra/2.9'.

Finish continuation of the transformation action

Now the user can edit/compare transformations incrementally:

1) Transform your piece of image
2) Apply transformation
3) Click on the canvas with the transfomration tool again

Now the previously applied transformation is *continued*

If you cancel/reset your transformation, it will be reset to the
correct state, that is to the partial transformation where it has
been started.

If you switch the transformation mode during a continued action,
the previous action will be baked into the image and a new-mode-stroke
will be started.




CC:kimageshop@kde.org

M  +33   -9    krita/plugins/tools/tool_transform2/kis_tool_transform.cc
M  +1    -1    krita/plugins/tools/tool_transform2/kis_tool_transform.h
M  +22   -0    krita/plugins/tools/tool_transform2/tool_transform_args.cc
M  +11   -0    krita/plugins/tools/tool_transform2/tool_transform_args.h

http://commits.kde.org/calligra/1a3e977b1d1d8296f5e089221d2d36ac0cb951e8

Now I see! What a smart solution! Thank you Dmitry! :D

Hi, Tyson!

And here is an example video :)

http://nonaynever.ru/pub/cow_continued_transform/

Thanks! This is awesome!

Git commit c5f13168ca4f0cb88e617b3f9d5078cb712bced7 by Dmitry Kazakov.
Committed on 06/07/2015 at 09:05.
Pushed by dkazakov into branch 'calligra/2.9'.

[FEATURE] Add possibility to continue a Crop Tool action

Now you can continue a finished Crop Tool action

1) Activate Crop Tool
2) Choose an area
3) Press Enter to crop the image

Now your image is cropped

4) Single-click on the canvas

Now the previous crop action is continued

CC:kimageshop@kde.org

M  +1    -0    krita/image/CMakeLists.txt
A  +35   -0    krita/image/kis_crop_saved_extra_data.cpp     [License: GPL (v2+)]
A  +61   -0    krita/image/kis_crop_saved_extra_data.h     [License: GPL (v2+)]
M  +13   -2    krita/image/kis_image.cc
M  +6    -1    krita/image/kis_processing_applicator.cpp
M  +3    -1    krita/image/kis_processing_applicator.h
M  +14   -1    krita/image/kis_stroke_strategy_undo_command_based.cpp
M  +16   -1    krita/image/kis_stroke_strategy_undo_command_based.h
M  +42   -2    krita/plugins/tools/tool_crop/kis_tool_crop.cc
M  +2    -0    krita/plugins/tools/tool_crop/kis_tool_crop.h
M  +2    -2    krita/plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp
M  +1    -1    libs/kundo2/CMakeLists.txt
A  +24   -0    libs/kundo2/kundo2commandextradata.cpp     [License: GPL (v2+)]
A  +31   -0    libs/kundo2/kundo2commandextradata.h     [License: GPL (v2+)]
M  +2    -6    libs/kundo2/kundo2stack.cpp
M  +4    -6    libs/kundo2/kundo2stack.h
M  +1    -1    libs/kundo2/kundo2stack_p.h

http://commits.kde.org/calligra/c5f13168ca4f0cb88e617b3f9d5078cb712bced7

Thank you Dmitry! This addition surely brought better user experience on continuous, multiple crops! :D