Update Alpha Darken Composite Operation to use flow more reasonably
Details
Details
- Reviewers
dkazakov - Group Reviewers
Krita - Maniphest Tasks
- T8576: Improve Brush Opacity handling.
- Commits
- R37:d4a0c1f9fb52: Flow Update
Diff Detail
Diff Detail
- Repository
- R37 Krita
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
Forum thread discussing issues:
https://forum.kde.org/viewtopic.php?f=288&t=136165&sid=b0322cc336861566b625af479805062b
Comment Actions
Okay, this builds properly for me, it'll require a look over by @dkazakov to check if the maths is ok.
@Deevad, @kamathraghavendra, @ramonmiranda will need to test if it is desirable behaviour.
Pro: it would make Krita's behaviour much more consistent with other applications.
Con: We'll need to adjust the presets that depended on the old behaviour.
| libs/pigment/compositeops/KoCompositeOpAlphaDarken.h | ||
|---|---|---|
| 106 | please don't add newlines :) | |
Comment Actions
Hey William Brown, I am going to push this patch temporarily so artists can test, could you provide an email address so I can credit you properly?
Comment Actions
Okay, I've temporarily pushed it in https://phabricator.kde.org/R37:e3c2f5bed0c83462f893919098dad16d37263698 , we'll revert it on Monday 28th.
Comment Actions
Here are the first survey results: https://www.dropbox.com/s/8ispua7oo6er1z4/survey_2019-1.pdf?dl=0 -- people are _very_ happy :-)
Comment Actions
Hi, all!
I have checked the maths of the new algorithm. It looks logical and should work fine. I have also added a few comments to the code to make it easier to decrypt later. See patch snippet below.
The only problems I see with the patch are 1) the transition process for existing presets and 2) the old effect is not available in the new version. I have a feeling that we have people who like the old effect, so it would be better to keep the old version as well.
I guess it is not very difficult to make both composite ops available. We just need to do the following:
- Implement a new composite op, say, "alpha-darken-creamy".
- Modify KisPaintOpSettings::indirectPaintingCompositeOp() to fetch the selected composite op from the preset itself
- Add a switch to Flow section to select between "sharp" and "creamy" variants of flow and save it into the preset.
- Modify all our default presets to use the new "creamy" version of flow :)
If needed, I can help with points 1 and 2.
Comment Actions
The only problems I see with the patch are 1) the transition process forexisting presets and
That's what Deevad was worried about, too, but it turned out not to be a
problem.
- the old effect is not available in the new version. I have a feeling that we have people who like the old effect, so it would be better to keep the old version as well.
That is not a problem either. As Deevad has shown in his video, the old effect
is still achievable.
Note that only one person in the survey, who had not seen Deevad's video,
asked for the old behaviour to be retained as an option; everyone else wanted
the new effect, and couldn't care less about the old effect.
Comment Actions
That is not a problem either. As Deevad has shown in his video, the old effect
is still achievable.
Okay, now I agree with the point that the old effect is achievable. With breaking old presets, I'm not sure. Perhaps, we could make some compatibility mode, that is, if you activate some global config option (which is off by default), the presets will behave as before? And the option will be removed in, e.g. Krita 5?
Implementing one more composite op and doing a switch based on KConfigGroup is not a big task (we already do that for AVX optimizations). And this way people, who rely on their custom presets in production work will not suffer much :)
Comment Actions
< Implementing one more composite op and doing a switch based on KConfigGroup is not a big task (we already do that for AVX optimizations). And this way people, who rely on their custom presets in production work will not suffer much :)
I'm sure it could be done -- but I don't think there's a huge need. I'd like this patch to be pushed asap, so our git master users have the new behaviour.