Bug 325686

Summary:	Krita crashes when an image used in a File Layer is updated outside of Krita
Product:	[Applications] krita	Reporter:	Ragnar Brynjúlsson <ragtag>
Component:	Layer Stack	Assignee:	Dmitry Kazakov <dimula73>
Status:	RESOLVED FIXED
Severity:	crash	CC:	dimula73, ragtag, sven.langkamp
Priority:	NOR
Version:	git master (please specify the git hash!)
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:	https://commits.kde.org/krita/40a1a0c0999a20744ae08244df10b679f172b724	Version Fixed In:
Attachments:	File Layer bug GDB output

Description Ragnar Brynjúlsson 2013-10-06 01:11:39 UTC

When you edit an image used in a File Layer outside Krita (such as in Gimp), and save it to disk, Krita pops up three error windows with "Could not open, /your/file.png, Internal error", and one with Choose Filter windows with a list of file formats. Nothing is clickable in any of these windows, and you basically have to kill Krita.

Reproducible: Always

Steps to Reproduce:
1. Add a File Layer.
2. Load a picture to use in the File Layer (I used .png in this test)
3. Edit the picture outside Krita, and save over it.
Actual Results:  
Krita pops up error messages, and nothing is clickable. You need to kill Krita to recover from the error.

Expected Results:  
That the image used in the File Layer would be updated on the canvas.

Tested on Ubuntu 12.04 LTS 64-bit, but others on IRC reported the same issue.

If detecting that a file has changed on disk is problematic, or it's trying to read it before it's fully written, maybe a workaround would be to have a refresh/reload button on the File Layer, to manually reload the image when needed.

Comment 1 Sven Langkamp 2013-10-06 11:35:32 UTC

Did you get a backtrace?

Comment 2 Ragnar Brynjúlsson 2013-10-06 19:48:56 UTC

Created attachment 82695 [details]
File Layer bug GDB output

I've attached the output from the terminal and gdb.

It seems like Krita starts reading the image in, before Gimp has finished writing it to disk.

This second test I did with Krita 2.7.3, using both .png and .jpg files, and both Gimp, Blender and another instance of Krita to save the image outside of Krita.

Comment 3 Dmitry Kazakov 2013-12-13 20:56:13 UTC

I have a very dirty patch for it. Now I just need to implement it cleanly and it'll work :)

Comment 4 Dmitry Kazakov 2013-12-17 09:52:59 UTC

Git commit e4fc9d4ee891b075a1c70f06d7fce3ebbb59982f by Dmitry Kazakov.
Committed on 17/12/2013 at 09:47.
Pushed by dkazakov into branch 'master'.

Fixed a concurrent File Layer source image change

Now when the source image is changed we use an iterational algorithm
which is usually applied to a SeqLock problem:

1) Save the size and lastModified tag of the image
2) Copy the image to the temporary directory
3) Check whether the image has changed during the copy operation
4) If everything is ok load the image from a temporary copy. If not,
   restart.

M  +1    -0    krita/ui/CMakeLists.txt
M  +14   -24   krita/ui/kis_file_layer.cpp
M  +3    -10   krita/ui/kis_file_layer.h
A  +172  -0    krita/ui/kis_safe_document_loader.cpp     [License: GPL (v2+)]
A  +53   -0    krita/ui/kis_safe_document_loader.h     [License: GPL (v2+)]

http://commits.kde.org/calligra/e4fc9d4ee891b075a1c70f06d7fce3ebbb59982f

Comment 5 Halla Rempt 2013-12-18 11:52:43 UTC

Git commit 0e5a207a3d301860a5802d17a61ce9c4f5628790 by Boudewijn Rempt, on behalf of Dmitry Kazakov.
Committed on 17/12/2013 at 09:47.
Pushed by rempt into branch 'calligra/2.8'.

Fixed a concurrent File Layer source image change

Now when the source image is changed we use an iterational algorithm
which is usually applied to a SeqLock problem:

1) Save the size and lastModified tag of the image
2) Copy the image to the temporary directory
3) Check whether the image has changed during the copy operation
4) If everything is ok load the image from a temporary copy. If not,
   restart.

Conflicts:
	krita/ui/kis_file_layer.cpp

M  +1    -0    krita/ui/CMakeLists.txt
M  +13   -22   krita/ui/kis_file_layer.cpp
M  +3    -10   krita/ui/kis_file_layer.h
A  +172  -0    krita/ui/kis_safe_document_loader.cpp     [License: GPL (v2+)]
A  +53   -0    krita/ui/kis_safe_document_loader.h     [License: GPL (v2+)]

http://commits.kde.org/calligra/0e5a207a3d301860a5802d17a61ce9c4f5628790

Comment 6 Halla Rempt 2018-06-05 14:33:35 UTC

Git commit 6828f33fe3e92fd4ca151bd44ce1886118d01ed6 by Boudewijn Rempt.
Committed on 05/06/2018 at 14:32.
Pushed by rempt into branch 'krita/4.0'.

Do not allow .kra files as file layers

Nesting .kra files is dangerous, since a .kra file can contain
file layers itself, which is not supported.
Related: bug 386515, bug 394211

M  +4    -1    libs/ui/dialogs/kis_dlg_file_layer.cpp

https://commits.kde.org/krita/6828f33fe3e92fd4ca151bd44ce1886118d01ed6

Comment 7 Halla Rempt 2018-06-05 14:33:41 UTC

Git commit 40a1a0c0999a20744ae08244df10b679f172b724 by Boudewijn Rempt.
Committed on 05/06/2018 at 14:33.
Pushed by rempt into branch 'master'.

Do not allow .kra files as file layers

Nesting .kra files is dangerous, since a .kra file can contain
file layers itself, which is not supported.
Related: bug 386515, bug 394211
(cherry picked from commit 6828f33fe3e92fd4ca151bd44ce1886118d01ed6)

M  +4    -1    libs/ui/dialogs/kis_dlg_file_layer.cpp

https://commits.kde.org/krita/40a1a0c0999a20744ae08244df10b679f172b724