Bug 460461

Summary:	Using line tool repeatedly and undoing results in irrecoverable data loss of tile data.
Product:	[Applications] krita	Reporter:	Raghavendra kamath <raghu>
Component:	OpenGL Canvas	Assignee:	Dmitry Kazakov <dimula73>
Status:	RESOLVED FIXED
Severity:	normal	CC:	dimula73, halla, wojtryb, ww123tdw
Priority:	NOR
Version:	5.1.1
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:

Description Raghavendra kamath 2022-10-15 09:02:53 UTC

SUMMARY
I think this bug is of a bit serious nature as it results in data loss. 
We got two users complaining on KA about tile crashing and resulting in data loss while using the line tool and undoing the actions repeatedly.

I am copy pasting Freyalupens remarks here :
"For demonstration, here’s a video I recorded on 5.2.x (git 939408c), just repeatedly using the line tool for a few minutes; brush is “b) Basic-2 Opacity”. Before that I was using “b) Basic-1”, but nothing was happening, so I switched brushes. Could be it only happens with certain brushes, could be the act of switching brushes somehow triggers it, could be I was just (un)lucky. After a while it just randomly happens, trying to undo it causes a safe assert in libs/image/tiles3/kis_memento_manager.cc, line 275, and redoing in line 332.

(For fun I turned on all the logging options. The Unknown transform parameter : "" being logged is from kis_dynamic_sensor::id2sensor(). The brush is trying to use some sort of incorrect sensor or something, but the message is very unhelpfully worded.)"

You can check the thread where this bug is discussed here - https://krita-artists.org/t/using-brush-or-line-tools-sometimes-delete-part-of-the-layer-in-a-square-rectangle-pattern/48985


There is also a video of the bug happening in the thread linked above.

Comment 1 Wojtek Trybus 2022-10-15 14:06:09 UTC

Hi, I confirm this bug on kubuntu 20.04, AMD Ryzen CPU and Nvidia GTX1050.

For me this happens using freehand brush tool, after undoing very fast after finishing a stroke, so I believe it could be more related to some "undo before finished processing" thing than to the line tool which I don't use.

Still, this is very hard to reproduce, and I never managed to get this on demand - it's really rare in everyday work.

Comment 2 Dmitry Kazakov 2022-10-26 12:05:10 UTC

Git commit fac1d60bd99f811fd7949a3d3a69ced92404750c by Dmitry Kazakov.
Committed on 26/10/2022 at 12:04.
Pushed by dkazakov into branch 'master'.

Fix a hangup+crash in the line tool when the line is too short

The fix might be related to the bug below, though I'm not very sure

M  +21   -12   plugins/tools/basictools/kis_tool_line_helper.cpp

https://invent.kde.org/graphics/krita/commit/fac1d60bd99f811fd7949a3d3a69ced92404750c

Comment 3 Dmitry Kazakov 2022-10-27 11:57:54 UTC

Git commit 6d87f89408cec4e9b302199b888083249a934e77 by Dmitry Kazakov.
Committed on 27/10/2022 at 11:57.
Pushed by dkazakov into branch 'master'.

Possible fix for the artifacts when using the line tool

There was a theoretical race condition possible in
KisAsyncMerger::setupProjection:

Preconditions:

1) The image has only one layer with fully transparent default pixel
   (to trigget tryOblidgeChild mechanism)

Mecanics of the issue:

1) KisAsyncMerger requests currentLeaf->parent()->original(), which points
   to the projection() of the only child layer (which equals to
   its paintDevice().

2) KisAsyncMerger thread sleeps

3) Some other thread starts an indirect painting stroke over this
   layer, basically changing projection() of the layer to be separate
   from its paintDevice()

4) KisAsyncMerger thread wakes up

5) The condition `parentOriginal != currentLeaf->projection()` succeeds
   because parentOriginal still points to the layer's paintDevice

6) KisAsyncMerger starts to write into paint layer's paint device,
   which breaks its undo history.

Lots of thanks go to freyalupen for debugging this issue locally and
providing the valuable debugging info.

M  +2    -2    libs/image/kis_async_merger.cpp
M  +20   -2    libs/image/kis_group_layer.cc
M  +8    -0    libs/image/kis_group_layer.h
M  +6    -0    libs/image/kis_projection_leaf.cpp
M  +1    -0    libs/image/kis_projection_leaf.h

https://invent.kde.org/graphics/krita/commit/6d87f89408cec4e9b302199b888083249a934e77

Comment 4 Dmitry Kazakov 2022-10-28 06:19:30 UTC

The latest patch is reported to fix the bug :)

Comment 5 Dmitry Kazakov 2022-10-28 06:29:44 UTC

Git commit 0641f768bd3d4a712f465a32a3cf4304457f66cb by Dmitry Kazakov.
Committed on 28/10/2022 at 06:20.
Pushed by dkazakov into branch 'krita/5.1'.

Possible fix for the artifacts when using the line tool

There was a theoretical race condition possible in
KisAsyncMerger::setupProjection:

Preconditions:

1) The image has only one layer with fully transparent default pixel
   (to trigget tryOblidgeChild mechanism)

Mecanics of the issue:

1) KisAsyncMerger requests currentLeaf->parent()->original(), which points
   to the projection() of the only child layer (which equals to
   its paintDevice().

2) KisAsyncMerger thread sleeps

3) Some other thread starts an indirect painting stroke over this
   layer, basically changing projection() of the layer to be separate
   from its paintDevice()

4) KisAsyncMerger thread wakes up

5) The condition `parentOriginal != currentLeaf->projection()` succeeds
   because parentOriginal still points to the layer's paintDevice

6) KisAsyncMerger starts to write into paint layer's paint device,
   which breaks its undo history.

Lots of thanks go to freyalupen for debugging this issue locally and
providing the valuable debugging info.

M  +2    -2    libs/image/kis_async_merger.cpp
M  +20   -2    libs/image/kis_group_layer.cc
M  +8    -0    libs/image/kis_group_layer.h
M  +6    -0    libs/image/kis_projection_leaf.cpp
M  +1    -0    libs/image/kis_projection_leaf.h

https://invent.kde.org/graphics/krita/commit/0641f768bd3d4a712f465a32a3cf4304457f66cb

Comment 6 Dmitry Kazakov 2022-10-28 06:29:52 UTC

Git commit 9efeb7677a5a05017d6749653144327f11eb5141 by Dmitry Kazakov.
Committed on 28/10/2022 at 06:29.
Pushed by dkazakov into branch 'krita/5.1'.

Fix a hangup+crash in the line tool when the line is too short

The fix might be related to the bug below, though I'm not very sure

# Conflicts:
#	plugins/tools/basictools/kis_tool_line_helper.cpp

M  +17   -12   plugins/tools/basictools/kis_tool_line_helper.cpp

https://invent.kde.org/graphics/krita/commit/9efeb7677a5a05017d6749653144327f11eb5141