Bug 446009

Summary:	Resource: Masked Bush: Preset initially loads incorrect masked brush tip.
Product:	[Applications] krita	Reporter:	Emmet O'Neill <emmetoneill.pdx>
Component:	Resource Management	Assignee:	Eoin O'Neill <eoinoneill1991>
Status:	RESOLVED FIXED
Severity:	major	CC:	dimula73
Priority:	NOR
Version:	5.0.0-beta2
Target Milestone:	---
Platform:	Other
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:
Bug Depends on:	445367
Bug Blocks:

Description Emmet O'Neill 2021-11-24 02:08:24 UTC

SUMMARY
When loading the "v) Texture Impressionism MaskedBig" brush preset from Ramon's impressionism bundle, the wrong brush tip is used. When viewed in the brush editor > brush tip tab, a long vertical rectangle tip appears to be selected, but when you paint the tip appears to be round.


STEPS TO REPRODUCE
1. Import Ramon Miranda's "Impressionism" resource bundle.
2. Select the "v) Texture Impressionism MaskedBig" brush preset.
3. Take note of the selected brush tip in the "Brush Tip" section of the brush editor dialog.
4. Draw with the brush on the canvas.

OBSERVED RESULT
Although the tip appears to be a long rectangle in the brush editor window, the stroke appears to have a round tip on canvas.


EXPECTED RESULT
The tip on canvas should correctly reflect the active tip in the brush editor.

WORKAROUND / NOTE
By switching brush tips away from the rectangular one that's associated with the preset, and then switching back, you can make the brush function as intended. (This might hint that the problem is not with how the brush resource is stored, but instead with how it's being initially loaded.)

Krita 5.0 branch, resource testing.

Comment 1 Emmet O'Neill 2021-11-24 04:20:47 UTC

Correction: It's not the regular brush tip, it's the brush tip associated with masked brushes. 

Brush Editor > Masked Brush section > Brush tip.

Comment 2 Dmitry Kazakov 2021-11-25 12:50:27 UTC

Hi, Emmet!

This bug is tightly related to bug 445367

The problem is that "Krita 4" bundle contains two resources with are the same, but have a different filename. I mean "plain_rake.png" and "chisel_soft.png". When the bundle is imported, then one of them gets disabled ("plain_rake.png"), so it cannot be found by the link defined with filename only. So the solution to this bug should be two-fold:

1) Fix bug 445367 and make sure that the resources are not disabled on import, unless explicitly requested by the user. I'm not sure how to do that technically, because we still don't want to show the user fully-duplicated resources (filename+name+md5) in the GUI

2) Find out why the brushes in the bundle didn't have md5 tag (I have a feeling like this issue might have been fixed already)

Just to make it clear: fixing only the second part will help us only with the presets created after the fix. Legacy presets (including Krita4-era) will still be broken.

Comment 3 Bug Janitor Service 2021-11-25 13:40:02 UTC

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

Comment 4 Dmitry Kazakov 2021-11-25 13:42:31 UTC

Just to make it clear, even though the mentioned merge request fixes the bug, there is still "part 2" to be checked/fixed.

Comment 5 Dmitry Kazakov 2021-11-25 14:32:28 UTC

Git commit aeed954b09cbdb68a6844d2d7b47626f8009c9e4 by Dmitry Kazakov.
Committed on 25/11/2021 at 14:31.
Pushed by dkazakov into branch 'krita/5.0'.

Fix deduplication of resources on import

On importing resource we shouldn't just disable the copies. All the
copies should be available in the database as active, until the user
explicitly disables them. Though we should de-duplicate them
based on their name-filename-md5 signature when listing them in the
model. That can be easily done by simple adding "group by" to the
query.

Such solution both fixes legacy presets (bug 446009), and makes
importing resources less embarrassing (bug 445367).
Related: bug 445367

M  +1    -28   libs/resources/KisResourceCacheDb.cpp
M  +3    -0    libs/resources/KisResourceModel.cpp

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

Comment 6 Dmitry Kazakov 2021-11-25 14:33:09 UTC

Git commit 0b260f71b614e700d6ae9108e31509f81b66fbf2 by Dmitry Kazakov.
Committed on 25/11/2021 at 14:32.
Pushed by dkazakov into branch 'master'.

Fix deduplication of resources on import

On importing resource we shouldn't just disable the copies. All the
copies should be available in the database as active, until the user
explicitly disables them. Though we should de-duplicate them
based on their name-filename-md5 signature when listing them in the
model. That can be easily done by simple adding "group by" to the
query.

Such solution both fixes legacy presets (bug 446009), and makes
importing resources less embarrassing (bug 445367).
Related: bug 445367
(cherry picked from commit aeed954b09cbdb68a6844d2d7b47626f8009c9e4)

M  +1    -28   libs/resources/KisResourceCacheDb.cpp
M  +3    -0    libs/resources/KisResourceModel.cpp

https://invent.kde.org/graphics/krita/commit/0b260f71b614e700d6ae9108e31509f81b66fbf2

Comment 7 Dmitry Kazakov 2021-11-25 14:56:02 UTC

Okay, I have checked the second part of the bug and it seems like all the newly created presets have correct 'md5sum' tag set, so everything is correct. We can close this bug.