Bug 438887 - Animation curves: CTRL+Z (in certain condition) generate crash
Summary: Animation curves: CTRL+Z (in certain condition) generate crash
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Animation (show other bugs)
Version: nightly build (please specify the git hash!)
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Krita Bugs
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2021-06-18 19:45 UTC by grum999
Modified: 2021-06-23 16:59 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
CTRL+Z crash (782.48 KB, video/webm)
2021-06-18 19:45 UTC, grum999
Details
Transform tool in step 3 - strange result (1.37 MB, video/webm)
2021-06-18 19:45 UTC, grum999
Details

Note You need to log in before you can comment on or make changes to this bug.
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