Bug 412561 - Crashed when using transform tool. (Safe assert in m_changesTrackerIsEmpty() in kis_tool_transform.cc ln 849)
Summary: Crashed when using transform tool. (Safe assert in m_changesTrackerIsEmpty() ...
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Tools/Transform (show other bugs)
Version: nightly build (please specify the git hash!)
Platform: Microsoft Windows Microsoft Windows
: NOR normal
Target Milestone: ---
Assignee: Dmitry Kazakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-10-03 14:36 UTC by acc4commissions
Modified: 2019-11-11 12:03 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
internal error (28.53 KB, image/png)
2019-10-03 14:36 UTC, acc4commissions
Details
capture 2 (27.55 KB, image/png)
2019-10-03 21:34 UTC, acc4commissions
Details

Note You need to log in before you can comment on or make changes to this bug.
Description acc4commissions 2019-10-03 14:36:32 UTC
Created attachment 122990 [details]
internal error

SUMMARY
It randomly crashes. And since krita doesn't generate crashlog anymore I can't get the backtrace, but I attatch the capture of the popup...

The transform filter is lanczos3.

I'll update more on here as I discover when it happens.

SOFTWARE/OS VERSIONS
Windows: Win7
macOS: 
Linux/KDE Plasma: 
(available in About System)
KDE Plasma Version: 
KDE Frameworks Version: 
Qt Version: 

ADDITIONAL INFORMATION
Comment 1 acc4commissions 2019-10-03 21:34:09 UTC
Created attachment 123001 [details]
capture 2

It crashed when editing global selection mask with select shape tool. It was after I opened and and didn't use the krita application for a long time.
Comment 2 acc4commissions 2019-10-03 21:38:08 UTC
It was not 4.2.7. It's nightly build. (git 835ca2e)
My bad.
Comment 3 Halla Rempt 2019-10-03 21:41:23 UTC
Oh dear, then I might've made 4.2.7.1 for nothing... Our goal was to have these warnings in the nightlies for testing, but not in stable releases.
Comment 4 acc4commissions 2019-10-03 21:45:42 UTC
(In reply to Boudewijn Rempt from comment #3)
> Oh dear, then I might've made 4.2.7.1 for nothing... Our goal was to have
> these warnings in the nightlies for testing, but not in stable releases.

I'm sorry ;-;
Comment 5 wolthera 2019-10-07 15:06:58 UTC
These are two separate safe asserts though, Acc4, can you make a separate bugreport for the second image?
Comment 6 acc4commissions 2019-10-09 04:28:47 UTC
(In reply to wolthera from comment #5)
> These are two separate safe asserts though, Acc4, can you make a separate
> bugreport for the second image?

https://bugs.kde.org/show_bug.cgi?id=412747
Comment 7 wolthera 2019-10-09 10:46:35 UTC
Thanks, adding the safe assert names to the title.
Comment 8 Halla Rempt 2019-10-10 08:34:49 UTC
Assigning to Dmitry. I've seen this safe assert myself sometimes.
Comment 9 Dmitry Kazakov 2019-10-30 16:17:19 UTC
Related merge request:
https://invent.kde.org/kde/krita/merge_requests/178
Comment 10 Dmitry Kazakov 2019-10-30 19:28:18 UTC
Hi, acc4commissions!

Could you please test this package? Does transform tool still work as expected?

https://yadi.sk/d/CddIAfN70Mljzw

(it should fix the reported crash)
Comment 11 acc4commissions 2019-10-30 22:25:38 UTC
(In reply to Dmitry Kazakov from comment #10)
> Hi, acc4commissions!
> 
> Could you please test this package? Does transform tool still work as
> expected?
> 
> https://yadi.sk/d/CddIAfN70Mljzw
> 
> (it should fix the reported crash)

It doesn't crash. But the original crash already hasn't been happening for weeks. I'll try to test the build a bit more to see if there's a difference.
Comment 12 Dmitry Kazakov 2019-10-31 12:27:56 UTC
Thanks, I'll wait a bit! :)

The problem happened when one clicked on the image to actiavte transform and pressed Esc or Enter key concurrently. Basically, when I was testing the issue, I just pressed mouse-button/Esc/Enter randomly and simultaneously a lot of times :)
Comment 13 acc4commissions 2019-10-31 14:16:23 UTC
(In reply to Dmitry Kazakov from comment #12)
> Thanks, I'll wait a bit! :)
> 
> The problem happened when one clicked on the image to actiavte transform and
> pressed Esc or Enter key concurrently. Basically, when I was testing the
> issue, I just pressed mouse-button/Esc/Enter randomly and simultaneously a
> lot of times :)

In the way you described it doesn't crash in both the latast nightly(git 8a22883) and this build.
Comment 14 Dmitry Kazakov 2019-10-31 15:27:13 UTC
Well, master crashes for me. You should just click really fast and a lot :)
Comment 15 acc4commissions 2019-10-31 15:56:03 UTC
(In reply to Dmitry Kazakov from comment #14)
> Well, master crashes for me. You should just click really fast and a lot :)

It doesn't seem crash for me... no matter how crazily I click. 
Since I'm not experincing the crash anymore I'm just gonna assume that it's resolved. :)
Comment 16 Dmitry Kazakov 2019-11-06 15:20:41 UTC
Git commit f336923b879653edfbfc28fa29f839d456699e8c by Dmitry Kazakov.
Committed on 05/11/2019 at 18:41.
Pushed by dkazakov into branch 'master'.

Fix assert and a data-loss in Transform Tool

The patch rewrites the logic of transform stroke completion. Previously,
the decision whether to cancel transformation or to recover continued
state was done by the GUI thread. It cased troubles, because the user
could press esc/enter keys too quickly, even before the stroke was
actually initialized (and before sigTransactionGenerated() was received).
It caused confsion, resulting in the loss of the data of the continued
state.

Now GUI thread doesn't worry about the continued state. All the decisions
are done by the stroke itself, so no races should happen.

M  +24   -20   plugins/tools/tool_transform2/kis_tool_transform.cc
M  +2    -1    plugins/tools/tool_transform2/kis_tool_transform.h
M  +53   -26   plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp
M  +6    -2    plugins/tools/tool_transform2/strokes/transform_stroke_strategy.h

https://invent.kde.org/kde/krita/commit/f336923b879653edfbfc28fa29f839d456699e8c
Comment 17 Dmitry Kazakov 2019-11-06 15:20:42 UTC
Git commit 1fdbf7f5f57a7361bb01d4ef42c83dd2c396cf4a by Dmitry Kazakov.
Committed on 06/11/2019 at 15:17.
Pushed by dkazakov into branch 'master'.

Fix layer data loss when pressing Esc multiple times, while transforming stroke

There were several problems:

1) When the jobs from m_overriddenCommand have been executed,
   they shouldn't be added to the strokes's undo commands queue.
   After the clear-selection job, the paint device gets new
   transaction, therefore all the redo information is lost.
   And commands from the previous stroke are not valid anymore.

2) Since the commands from m_overridenCommand do not take part in
   normal cancel/undo process, in case of stroke cancellation we
   should re-apply them manually. And therefore, we must ensure
   that clear selection and create-preview-device actions are
   executed before cancellation action is performed. Therefore the
   patch introduces a special isCancellable() tag of the stroke jobs.

3) Since finishStrokeCallback() now adds more jobs to the strokes
   queue, we need some way to mark them non-cancellable. It is done
   by the same isCancellable() tag.

M  +5    -3    libs/image/commands_new/kis_saved_commands.cpp
M  +1    -1    libs/image/commands_new/kis_saved_commands.h
M  +2    -1    libs/image/kis_stroke_job.h
M  +14   -2    libs/image/kis_stroke_job_strategy.cpp
M  +4    -0    libs/image/kis_stroke_job_strategy.h
M  +5    -1    libs/image/kis_stroke_strategy_undo_command_based.cpp
M  +9    -4    libs/image/kis_stroke_strategy_undo_command_based.h
M  +31   -14   plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp
M  +2    -0    plugins/tools/tool_transform2/strokes/transform_stroke_strategy.h

https://invent.kde.org/kde/krita/commit/1fdbf7f5f57a7361bb01d4ef42c83dd2c396cf4a
Comment 18 Dmitry Kazakov 2019-11-11 12:03:12 UTC
Git commit cd21bb33b70e0c06426926c8c6c707cc540df464 by Dmitry Kazakov.
Committed on 11/11/2019 at 12:01.
Pushed by dkazakov into branch 'krita/4.2'.

Fix assert and a data-loss in Transform Tool

The patch rewrites the logic of transform stroke completion. Previously,
the decision whether to cancel transformation or to recover continued
state was done by the GUI thread. It cased troubles, because the user
could press esc/enter keys too quickly, even before the stroke was
actually initialized (and before sigTransactionGenerated() was received).
It caused confsion, resulting in the loss of the data of the continued
state.

Now GUI thread doesn't worry about the continued state. All the decisions
are done by the stroke itself, so no races should happen.

M  +24   -20   plugins/tools/tool_transform2/kis_tool_transform.cc
M  +2    -1    plugins/tools/tool_transform2/kis_tool_transform.h
M  +53   -26   plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp
M  +6    -2    plugins/tools/tool_transform2/strokes/transform_stroke_strategy.h

https://invent.kde.org/kde/krita/commit/cd21bb33b70e0c06426926c8c6c707cc540df464
Comment 19 Dmitry Kazakov 2019-11-11 12:03:13 UTC
Git commit 393315265d7647afc876a1db0e2e0776c416b240 by Dmitry Kazakov.
Committed on 11/11/2019 at 12:01.
Pushed by dkazakov into branch 'krita/4.2'.

Fix layer data loss when pressing Esc multiple times, while transforming stroke

There were several problems:

1) When the jobs from m_overriddenCommand have been executed,
   they shouldn't be added to the strokes's undo commands queue.
   After the clear-selection job, the paint device gets new
   transaction, therefore all the redo information is lost.
   And commands from the previous stroke are not valid anymore.

2) Since the commands from m_overridenCommand do not take part in
   normal cancel/undo process, in case of stroke cancellation we
   should re-apply them manually. And therefore, we must ensure
   that clear selection and create-preview-device actions are
   executed before cancellation action is performed. Therefore the
   patch introduces a special isCancellable() tag of the stroke jobs.

3) Since finishStrokeCallback() now adds more jobs to the strokes
   queue, we need some way to mark them non-cancellable. It is done
   by the same isCancellable() tag.

M  +5    -3    libs/image/commands_new/kis_saved_commands.cpp
M  +1    -1    libs/image/commands_new/kis_saved_commands.h
M  +2    -1    libs/image/kis_stroke_job.h
M  +14   -2    libs/image/kis_stroke_job_strategy.cpp
M  +4    -0    libs/image/kis_stroke_job_strategy.h
M  +5    -1    libs/image/kis_stroke_strategy_undo_command_based.cpp
M  +9    -4    libs/image/kis_stroke_strategy_undo_command_based.h
M  +31   -14   plugins/tools/tool_transform2/strokes/transform_stroke_strategy.cpp
M  +2    -0    plugins/tools/tool_transform2/strokes/transform_stroke_strategy.h

https://invent.kde.org/kde/krita/commit/393315265d7647afc876a1db0e2e0776c416b240