Bug 446623 - FGtoBG gradient has incorrect preview in the gradient chooser
Summary: FGtoBG gradient has incorrect preview in the gradient chooser
Status: ASSIGNED
Alias: None
Product: krita
Classification: Applications
Component: Resource Management (other bugs)
Version First Reported In: 5.0.0-beta5
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: wolthera
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2021-12-07 14:22 UTC by Dmitry Kazakov
Modified: 2021-12-14 20:38 UTC (History)
1 user (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 Dmitry Kazakov 2021-12-07 14:22:03 UTC
1) Select Red and Blue colors for Fg and Bg colors correspondingly
2) Open any gradient chooser, see that the BGtoFG gradient is still painted as white-to-black

This is a regression. It used to work in the commit below, but some later refactorings removed cloning and gradient->updateVariableColors() call from the update finction.

Last known to work commit:

commit fe4744f96184084778681ed731ab687443a371d4 
Author: Dmitry Kazakov <dimula73@gmail.com>
Date:   Fri, 24 Jul 2020 00:09:31 +0300

    Fix cached thumbnails for gradient resources
    
    The solution is the following:
    
    1) Gradient resources' cached thumbnail now has size 2048x1 px
    
    2) KoAbstractGradient::generatePreview() does not include
       checkers anymore (obviously, how would it render it in
       1 px line?)
    
    3) Therefore, all widgets that show gradients should now
       render checkers themselves (that is, KisCmbGradient,
       KisControlFrame, KisIconToolTip and KisResourceItemDelegate)
    
    2) All widgets drawing gradients should also stretch the
       thumbnail into desired size.
Comment 1 Bug Janitor Service 2021-12-08 10:36:07 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/krita/-/merge_requests/1211
Comment 2 Dmitry Kazakov 2021-12-08 10:57:40 UTC
Git commit 98502d4f1dae30780cc6121274ef0a43c5408057 by Dmitry Kazakov.
Committed on 08/12/2021 at 10:55.
Pushed by dkazakov into branch 'master'.

Fix layer styles dialog breaking the global Fg->Bg gradient

It looks like we don't create copies of the required resources
when putting them in the local storage. Whether we should do that
or not is a rather disputable topic (because it can cause
efficiency problems). So right now we should just be careful when
baking the gradients, and make copies explicitly.

The patch is also partially related to the problem mentioned in
the bug 445390 (though it doesn't fix it):
Related: bug 445390
(cherry picked from commit 0cdacdf4d5d60703b652a36465df5e04a3d35da8)

M  +17   -5    libs/image/kis_psd_layer_style.cpp
M  +6    -0    libs/resources/KisLocalStrokeResources.cpp
M  +1    -0    libs/resources/KisLocalStrokeResources.h

https://invent.kde.org/graphics/krita/commit/98502d4f1dae30780cc6121274ef0a43c5408057
Comment 3 Dmitry Kazakov 2021-12-08 10:59:23 UTC
Git commit 7d4a3f67356fec178e6cadd734ede1414c72a987 by Dmitry Kazakov.
Committed on 08/12/2021 at 10:58.
Pushed by dkazakov into branch 'krita/5.0'.

Fix layer styles dialog breaking the global Fg->Bg gradient

It looks like we don't create copies of the required resources
when putting them in the local storage. Whether we should do that
or not is a rather disputable topic (because it can cause
efficiency problems). So right now we should just be careful when
baking the gradients, and make copies explicitly.

The patch is also partially related to the problem mentioned in
the bug 445390 (though it doesn't fix it):
Related: bug 445390

M  +17   -5    libs/image/kis_psd_layer_style.cpp
M  +6    -0    libs/resources/KisLocalStrokeResources.cpp
M  +1    -0    libs/resources/KisLocalStrokeResources.h

https://invent.kde.org/graphics/krita/commit/7d4a3f67356fec178e6cadd734ede1414c72a987
Comment 4 wolthera 2021-12-14 20:38:55 UTC
I am wondering how this ever worked in 5.0, because gradients need to be loaded to get updated, which is kinda against the the whole point of the rewrite... The thumbnail inside the resource database doesn't get updated either.

I think what might be wisest is that we delay this until after 5.0, then we implement a way for the resource database to know whether a thumbnail is dependent on things happening inside the canvas provider, and then use changes in the canvas provider to update thumbnails.

Doesn't work on Version: 5.0.0-beta1 (git 0ff34e9), which is from early november.