Bug 438887

Summary:	Animation curves: CTRL+Z (in certain condition) generate crash
Product:	[Applications] krita	Reporter:	grum999
Component:	Animation	Assignee:	Krita Bugs <krita-bugs-null>
Status:	RESOLVED FIXED
Severity:	normal	CC:	emmetoneill.pdx, eoinoneill1991, tamtamy.tymona
Priority:	NOR	Keywords:	regression
Version:	nightly build (please specify the git hash!)
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:	https://invent.kde.org/graphics/krita/commit/583a1a7a366094f857d2b312333216772a85ff8c	Version Fixed In:
Attachments:	CTRL+Z crash Transform tool in step 3 - strange result

Description grum999 2021-06-18 19:45:08 UTC

Created attachment 139504 [details]
CTRL+Z crash

SUMMARY

Following this comment:
https://bugs.kde.org/show_bug.cgi?id=438341#c5

I create a dedicated bug.


STEPS TO REPRODUCE

1. Create a new interpolated keyframe by selection transform tool

2. validate it by Enter

3. Reclick on layer with transform tool
   => key frame disappear from curves graph

4. Press ESCAPE twice
   => keyframe is not available anymore

5. Do CTRL+Z
   => Krita crash with a segmentation fault


Note: in step 3, 



OBSERVED RESULT
In step 3: may
- No more keyframes visible in animation curves (but should be still there)
- Also, if you try to work with transform tool (rotation, move), graphical result is weird: only tool shape is visible, there's no bitmap preview (not in attached video)

In step 4:
- Keyframe is still not visible anymore

In step 5:
- Krita badly crash


EXPECTED RESULT
In step 3:
- Keyframe should be visible
- Working with transform tool, content should be visible

In step 4:
- As there's not explicit action to remove keyframes, keyframes should be there

In step 5:
- no crash 




SOFTWARE/OS VERSIONS
krita-5.0.0-prealpha-4c233e3-x86_64.appimage

ADDITIONAL INFORMATION
Description on all steps maybe origin of crash

Comment 1 grum999 2021-06-18 19:45:54 UTC

Created attachment 139505 [details]
Transform tool in step 3 - strange result

Add a video of step 3 with strange result on transform tool usage

Comment 2 Tiar 2021-06-20 21:57:17 UTC

Not sure if this is a release blocker, but let's mark it at least as a regression...

Comment 3 Emmet O'Neill 2021-06-22 21:54:38 UTC

Confirmed.

Comment 4 Emmet O'Neill 2021-06-22 21:57:04 UTC

Popping this backtrace here for now while I debug:

#0  0x00007ffff28e0fff in void doActivate<false>(QObject*, int, void**) () at /lib64/libQt5Core.so.5
#1  0x00007ffff594e10f in KisScalarKeyframe::sigChanged(KisScalarKeyframe const*) (this=<optimized out>, _t1=<optimized out>)
    at /home/emmet/Code/krita/build/libs/image/kritaimage_autogen/EWIEGA46WW/moc_kis_scalar_keyframe_channel.cpp:134
#2  0x00007ffff5c7ce27 in KisScalarKeyframeUpdateCommand::undo() (this=<optimized out>)
    at /home/emmet/Code/krita/source/libs/image/kis_keyframe_commands.cpp:101
#3  0x00007ffff4f0e1d6 in KUndo2Command::undo() (this=0x7fff4004c940) at /home/emmet/Code/krita/source/libs/command/kundo2stack.cpp:242
#4  0x00007ffff5b101fa in KisStrokeStrategyUndoCommandBased::doStrokeCallback(KisStrokeJobData*) (this=0x1a48ea0, data=0xc505cb0)
    at /home/emmet/Code/krita/source/libs/image/kis_stroke_strategy_undo_command_based.cpp:115
#5  0x00007ffff5956632 in KisStrokeJob::run() (this=<optimized out>)
    at /home/emmet/Code/krita/build/libs/image/kritaimage_autogen/EWIEGA46WW/../../../../../source/libs/image/kis_stroke_job.h:32
#6  KisUpdateJobItem::run() (this=0x5f79a00)
    at /home/emmet/Code/krita/build/libs/image/kritaimage_autogen/EWIEGA46WW/../../../../../source/libs/image/kis_update_job_item.h:90
#7  0x00007ffff271b740 in QThreadPoolThread::run() () at /lib64/libQt5Core.so.5
#8  0x00007ffff2718751 in QThreadPrivate::start(void*) () at /lib64/libQt5Core.so.5
#9  0x00007ffff1e73299 in start_thread () at /lib64/libpthread.so.0
#10 0x00007ffff21e5353 in clone () at /lib64/libc.so.6

Comment 5 Eoin O'Neill 2021-06-22 23:36:46 UTC

NOTE:

This seems to be related to Krita's attempt to merge multiple transform operations. Basically:

1) You begin transforming, which creates keyframes. When finished, the transform is committed to the undo stack -- we'll call this "Transform Command A".

2) You then begin a new transformation, calling InplaceTransformStroke which verifies existing keyframes and skips creating new ones. This is "Transform Command B".

3) Somehow / somewhere, Transform Command A is seen as the most recent command, and Krita decides to undo Transform Command A. Because Transform Command B skipped creating new keyframes, it continues to believe that Transform Command A's keyframes still exist. This leads to a nullptr situation, causing a crash. 

Possible solution -- maybe we should always override existing keyframes and create new ones so that this type of thing can't occur?

Comment 6 Emmet O'Neill 2021-06-23 05:15:20 UTC

Git commit 583a1a7a366094f857d2b312333216772a85ff8c by Emmet O'Neill.
Committed on 23/06/2021 at 05:12.
Pushed by emmetoneill into branch 'master'.

Fix crash when undoing a modified transform mask keyframe.

M  +10   -8    plugins/tools/tool_transform2/strokes/inplace_transform_stroke_strategy.cpp

https://invent.kde.org/graphics/krita/commit/583a1a7a366094f857d2b312333216772a85ff8c

Comment 7 grum999 2021-06-23 16:59:24 UTC

(In reply to Emmet O'Neill from comment #6)
> Git commit 583a1a7a366094f857d2b312333216772a85ff8c by Emmet O'Neill.
> Committed on 23/06/2021 at 05:12.
> Pushed by emmetoneill into branch 'master'.
Just tested, it seems Ok!
1) keyframes are still visible
2) Undo is working properly, no crash


Concerning applied solution used to fix problem
""
Possible solution -- maybe we should always override existing keyframes and create new ones so that this type of thing can't occur?
""
Don't know if it what that has been applied, but it works.

The only thing is during some millisecond, we can see the animation curve flickering: reset to default values, and then right values applied.

Some could be disturbed by this, but that's not blocking for me.

Can consider that's fixed :) 


Grum999