Bug 448256

Summary:	Versions 4.1.7 onwards do not function properly with an .svg File Layer
Product:	[Applications] krita	Reporter:	Ahab Greybeard <ahab.greybeard>
Component:	Layer Stack	Assignee:	amyspark <amy>
Status:	RESOLVED FIXED
Severity:	normal	CC:	amy, emmetoneill.pdx, ghevan
Priority:	NOR
Version:	nightly build (please specify the git hash!)
Target Milestone:	---
Platform:	Debian stable
OS:	Linux
Latest Commit:	https://invent.kde.org/graphics/krita/commit/1e70e4ed51908c82dc603e3031cbb66701e4afcb	Version Fixed In:

Description Ahab Greybeard 2022-01-11 13:51:06 UTC

SUMMARY
Tested with appimages on Debian 10 and a simple 'Plain' .svg file Saved from Inkscape.

Versions 4.1.0 to 4.1.5 will produce a canvas display of an .svg file as a File Layer.
They show various setup options and ask what the ppi resolution should be.
Then, if the .svg file is edited and Saved by Inkscape, that event is recognised and the user is given the option to change the ppi in a small 'popup window'.

Later versions have loss of functionality as follows:

4.1.6: Not released.

4.1.7, 4.4.8 (intermediate versions not tested):
These provide all user option questions but do not display the .svg file contents.
They recognise when the .svg file has changed and offer a ppi change but do not display contents.

5.0.1, 5.0.2, Jan 10 Nightly:
These offer all user options and also display the .svg file contents in the initial File Layer definition windows.
They do not display .svg content on the canvas and they do not recognise when the .svg file has been updated.

STEPS TO REPRODUCE
See Summary

OBSERVED RESULT
See Summary

EXPECTED RESULT
An .svg file specified during File Layer creation should be displayed.
Its update should be recognised and dealt with.

Comment 1 vanyossi 2022-01-11 15:19:05 UTC

I can reproduce this issue easily, svg as file layer is not loaded anymore.

Comment 2 amyspark 2022-01-27 22:48:51 UTC

List of commits between 4.1.5 and 4.1.7: https://invent.kde.org/graphics/krita/-/compare/v4.1.5...v4.1.7?from_project_id=206

The only one I could find regarding SVG is this one:

commit dc4e44f448ba5fc8379ac7751baf2638593b0ac0
Author: Emmet O'Neill <emmetoneill.pdx@gmail.com>
Date:   Sun Oct 14 22:35:40 2018 +0200

    Improved responsiveness of moving large SVG objects.
    
    Added a KisSignalCompressor to the KisShapeLayerCanvas to reduce the
    number of updates.
    
    BUG:399363

which, when reverted, makes both SVG File Layers and plain SVG documents show a blank canvas on opening.

Further digging around KisSafeDocumentLoader::delayedLoadStart show that the document loads correctly through the SVG impex plugin, but the returned projection is a 0x0 image.

Comment 3 amyspark 2022-01-27 22:50:16 UTC

For the record, the imported image claims to be of the expected size; it's just its projection that returns a broken bounds QRect.

Comment 4 amyspark 2022-01-28 02:26:40 UTC

I found the root cause. Shape layers, when imported via a document's openPath or importDocument and not linked to the GUI, are never painted at all because the strokes are not given a chance to execute. This is caused by a mix of commits.

First, commit dc4e44f448ba5fc8379ac7751baf2638593b0ac0, which was first introduced between 4.1.5 and 4.1.7, as the reporter surmised. The revert didn't fix the paint because there's a second commit compounding this, which removes all avenues of synchronous painting:

commit 5355a56cf0b0eb7844f612d61946f335ce8bac23
Author: Dmitry Kazakov <dimula73@gmail.com>
Date:   Wed Dec 4 17:24:25 2019 +0300

    Remove synchronous update capabilities from KisShapeLayerCanvas
    
    After we fixed KoShapeManager to be thread-safe, we can safely rerender
    the layer in background stroke. It is faster and looks smoother for the
    user.

And finally, the jobs are killed here. This didn't cause an issue pre 4.1.7 because said layers were synchronously painted:

commit 10b7e991fef3cba64c00d61543ad135656071e04
Author: Dmitry Kazakov <dimula73@gmail.com>
Date:   Sun Feb 15 22:10:38 2015 +0400

    Fix applying transform masks to file layers
    
    1) The original() of the file layer should store the correct
       KisDefaultBounds object. That was the exact cause of the bug.
    
    2) KisSafeDocumentLoader should not keep the entire copy of the
       document in memory all the time. Clear the pointer when loaded.
    
    3) Add an kritarc option to enlarge the are available for transform
       mask data fetching outside the image.
    
       Name: transformMaskOffBoundsReadArea
       Type: qreal
    
       Default value is 0.5, which means the image is expanded up to half
       its size into each of the four direction.
    
    4) Added a unittest for KisFileLayer

Assigning to Dmitry as the sum of all commits makes it an async jobs-related issue.

Comment 5 Bug Janitor Service 2022-01-28 16:08:01 UTC

A possibly relevant merge request was started @ https://invent.kde.org/graphics/krita/-/merge_requests/1316

Comment 6 amyspark 2022-01-31 20:24:02 UTC

Git commit 5951c2b8090c7faf2cdee6450ca0500ace09c433 by L. E. Segovia.
Committed on 31/01/2022 at 20:22.
Pushed by lsegovia into branch 'master'.

Force node updates before projecting a newly imported document

M  +5    -0    libs/ui/kis_safe_document_loader.cpp
M  +5    -0    libs/ui/utils/KisClipboardUtil.cpp

https://invent.kde.org/graphics/krita/commit/5951c2b8090c7faf2cdee6450ca0500ace09c433

Comment 7 amyspark 2022-01-31 20:24:33 UTC

Git commit 1e70e4ed51908c82dc603e3031cbb66701e4afcb by L. E. Segovia.
Committed on 31/01/2022 at 20:24.
Pushed by lsegovia into branch 'krita/5.0'.

Force node updates before projecting a newly imported document
(cherry picked from commit 5951c2b8090c7faf2cdee6450ca0500ace09c433)

M  +5    -0    libs/ui/kis_safe_document_loader.cpp
M  +5    -0    libs/ui/utils/KisClipboardUtil.cpp

https://invent.kde.org/graphics/krita/commit/1e70e4ed51908c82dc603e3031cbb66701e4afcb