Bug 478225 - Crash when undoing "New layer from visible" if it's animated
Summary: Crash when undoing "New layer from visible" if it's animated
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Animation (show other bugs)
Version: git master (please specify the git hash!)
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Dmitry Kazakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-12-07 18:26 UTC by paleh
Modified: 2024-02-28 18:10 UTC (History)
2 users (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description paleh 2023-12-07 18:26:20 UTC
SUMMARY


STEPS TO REPRODUCE
1. Create a new document
2. Create a frame on a paint layer (P)
3. Call "New layer from visible". A new layer "Visible" appears
4. Turn off the visibility of Visible
5. Select P in the layers docker
6. Press Undo twice

OBSERVED RESULT
Crash after second undo.
Additionally, If layer P has onion skins active before step 3, creating a new layer from visible turns it off.

EXPECTED RESULT
Document returns to the state before step 3.
Additionally, calling "new layer from visible" doesn't deactivate the onion skins.

SOFTWARE/OS VERSIONS
Linux: Ubuntu 22.04
Qt Version: 5.15.7
Appimage

ADDITIONAL INFORMATION
present in git-5ffd6bebd0, as well as 5.2.2
was already present in 5.1.5
Comment 1 Freya Lupen 2023-12-07 18:45:24 UTC
Confirmed, here's the crash backtrace:

> 0   QtCore                        	       0x10661b378 QMetaObject::cast(QObject*) const + 28
> 1   kritaanimationdocker.so       	       0x127dd1d4c KisNodeDummy* qobject_cast<KisNodeDummy*>(QObject*) + 12 (qobject.h:524) [inlined]
> 2   kritaanimationdocker.so       	       0x127dd1d4c TimelineNodeListKeeper::slotUpdateDummyContent(QObject*) + 36 (timeline_node_list_keeper.cpp:129)
> 3   QtCore                        	       0x1066436a0 0x106444000 + 2094752
> 4   libkritaglobal.20.0.0.dylib   	       0x1025ca3f4 KisSignalMapper::mapped(QObject*) + 28 (moc_KisSignalMapper.cpp:200) [inlined]
> 5   libkritaglobal.20.0.0.dylib   	       0x1025ca3f4 KisSignalMapper::map(QObject*) + 768 (KisSignalMapper.cpp:231)
> 6   QtCore                        	       0x10663c18c QObject::event(QEvent*) + 596
Comment 2 Dmitry Kazakov 2024-01-16 14:55:49 UTC
Git commit a60beeadbdf24c7ccb4ee1bfbc204d46f2081e0d by Dmitry Kazakov.
Committed on 16/01/2024 at 15:49.
Pushed by dkazakov into branch 'master'.

Fix a crash when undoing "new from visible" of an animated layer

M  +1    -0    plugins/dockers/animation/timeline_node_list_keeper.cpp

https://invent.kde.org/graphics/krita/-/commit/a60beeadbdf24c7ccb4ee1bfbc204d46f2081e0d
Comment 3 Dmitry Kazakov 2024-01-16 15:18:27 UTC
The second part of the bug (onion skins switch) is still to be fixed.
Comment 4 Dmitry Kazakov 2024-01-19 12:49:35 UTC
Git commit 8dc97144531e42055a2d31918c4674e30e1a7026 by Dmitry Kazakov.
Committed on 19/01/2024 at 09:19.
Pushed by dkazakov into branch 'master'.

Fix a crash when quickly undo/redo new-from-visible+visibility-change

This patch introduce multiple changes to fix the original bug:

1) KisSynchronizedConnection no longer behaves as if it were
an auto-connection. Instead, **all** signals are now linearized
via the qApp events queue. This fixes the reordering issue when two
actions are undone/redone too quickly and one action is a legacy
action that executes in the GUI thread (e.g. visibility change) and
the other action is a "processing" action that executes in a worker
thread (e.g. new-from-visible). "Auto"-style connection would reorder
the actions on the basis of their execution thread.

2) Unittests still explicitly activate the "auto"-style connection,
because they don't have KisApplication and the corresponding handler
for KisSynchronizedConnectionEvent.

3) Fixes a bug in KisDummiesFacadeBase that caused double addition
of the dummies on loading of the document. It could happen that
two setImage() calls were called in the GUI thread **before** the
addDummyImpl() had a chance to execute.

M  +14   -1    libs/global/KisSynchronizedConnection.cpp
M  +11   -0    libs/global/KisSynchronizedConnection.h
M  +1    -1    libs/store/tests/CMakeLists.txt
M  +10   -12   libs/ui/flake/kis_dummies_facade_base.cpp
M  +21   -11   plugins/dockers/animation/timeline_node_list_keeper.cpp
M  +1    -1    plugins/dockers/animation/timeline_node_list_keeper.h
M  +2    -0    sdk/tests/kistest.h
M  +2    -0    sdk/tests/simpletest.h

https://invent.kde.org/graphics/krita/-/commit/8dc97144531e42055a2d31918c4674e30e1a7026
Comment 5 Dmitry Kazakov 2024-01-19 12:49:43 UTC
Git commit 61d78cc9288f9a575e6d3ba4c6fd4d387492dd04 by Dmitry Kazakov.
Committed on 19/01/2024 at 09:19.
Pushed by dkazakov into branch 'master'.

Fix restoring layer's properties after new-layer-from-visible action

M  +63   -4    libs/image/kis_layer_utils.cpp

https://invent.kde.org/graphics/krita/-/commit/61d78cc9288f9a575e6d3ba4c6fd4d387492dd04
Comment 6 Dmitry Kazakov 2024-02-28 18:09:34 UTC
Git commit 5daea563abf05caf0bb96f4522393f3d40f09cdb by Dmitry Kazakov.
Committed on 28/02/2024 at 12:55.
Pushed by dkazakov into branch 'kazakov/for-5.2'.

Fix restoring layer's properties after new-layer-from-visible action

M  +63   -4    libs/image/kis_layer_utils.cpp

https://invent.kde.org/graphics/krita/-/commit/5daea563abf05caf0bb96f4522393f3d40f09cdb
Comment 7 Dmitry Kazakov 2024-02-28 18:09:58 UTC
Git commit c49291363eaf981fb4306b9f1f34da2e7aca7ce0 by Dmitry Kazakov.
Committed on 28/02/2024 at 12:55.
Pushed by dkazakov into branch 'kazakov/for-5.2'.

Fix a crash when undoing "new from visible" of an animated layer

M  +1    -0    plugins/dockers/animation/timeline_node_list_keeper.cpp

https://invent.kde.org/graphics/krita/-/commit/c49291363eaf981fb4306b9f1f34da2e7aca7ce0
Comment 8 Dmitry Kazakov 2024-02-28 18:10:22 UTC
Git commit 426f99638a1fa712a6d8ded0eb9a523ef160551b by Dmitry Kazakov.
Committed on 28/02/2024 at 12:55.
Pushed by dkazakov into branch 'kazakov/for-5.2'.

Fix a crash when quickly undo/redo new-from-visible+visibility-change

This patch introduce multiple changes to fix the original bug:

1) KisSynchronizedConnection no longer behaves as if it were
an auto-connection. Instead, **all** signals are now linearized
via the qApp events queue. This fixes the reordering issue when two
actions are undone/redone too quickly and one action is a legacy
action that executes in the GUI thread (e.g. visibility change) and
the other action is a "processing" action that executes in a worker
thread (e.g. new-from-visible). "Auto"-style connection would reorder
the actions on the basis of their execution thread.

2) Unittests still explicitly activate the "auto"-style connection,
because they don't have KisApplication and the corresponding handler
for KisSynchronizedConnectionEvent.

3) Fixes a bug in KisDummiesFacadeBase that caused double addition
of the dummies on loading of the document. It could happen that
two setImage() calls were called in the GUI thread **before** the
addDummyImpl() had a chance to execute.

M  +14   -1    libs/global/KisSynchronizedConnection.cpp
M  +11   -0    libs/global/KisSynchronizedConnection.h
M  +1    -1    libs/store/tests/CMakeLists.txt
M  +10   -12   libs/ui/flake/kis_dummies_facade_base.cpp
M  +21   -11   plugins/dockers/animation/timeline_node_list_keeper.cpp
M  +1    -1    plugins/dockers/animation/timeline_node_list_keeper.h
M  +2    -0    sdk/tests/kistest.h
M  +2    -0    sdk/tests/simpletest.h

https://invent.kde.org/graphics/krita/-/commit/426f99638a1fa712a6d8ded0eb9a523ef160551b