Authored by Reptorian on Jun 6 2018, 4:56 PM.

# Details

Reviewers
 dkazakov
Group Reviewers
 Krita
Commits
Summary

During the creation of Quadratic Blending Modes, I have did the following:

• I copied and paste existing codes, and created new codes used to calculate blending of images accordingly to the Pegtop formula reference sheet
• I have tested the 4 new blending modes, and keep adjusting until the blending modes matches that of the Pegtop blending modes

• Before the testing plan, find a way to make freeze and heat with acceptable coding. Reflect Blend Mode has now been solved.
Test Plan
• Have artists find usage of quadratic blend modes, and see if they find the blending modes useful
• Test results of quadratic blend modes with existing programs (As far as I"m concerned, only reflect and glow are available for testing)

-If all seem sounds, and ready to go, then maybe it can be patched in Krita

# Diff Detail

Repository
R37 Krita
Lint
 Automatic diff as part of commit; lint not applicable.
Unit
 Automatic diff as part of commit; unit tests not applicable.
Reptorian created this revision.Jun 6 2018, 4:56 PM
Restricted Application added a reviewer: Krita. Jun 6 2018, 4:56 PM
Restricted Application added a project: Krita.
Reptorian requested review of this revision.Jun 6 2018, 4:56 PM
Reptorian updated this revision to Diff 35701.Jun 6 2018, 6:56 PM

Decided to add information about the new blending modes into the diff

I will test the patch a bit later :)

dkazakov requested changes to this revision.Jun 9 2018, 7:57 AM

Hi, @Reptorian!

The patch looks almost correct, except the fact that the values may become denormalized for integer color spaces during multiplications and divisions. Please use functions from Arithmetc namespace for all the operations that involve multiplication and division. It will make the code support all the types of color spaces.

libs/pigment/compositeops/KoCompositeOpFunctions.h
464

Instead of direct multiplication and division operations (and pow() function), one should use functions from Arithmetic namespace here. Otherwise, the values may become denormalized (e.g. int8 colorspaces are normalized in range 0...255 instead of standard 0...1.0). E.e. this line should look like:

`return clamp<T>(div(mul(src, src), inv(dst)));`
This revision now requires changes to proceed.Jun 9 2018, 7:57 AM
Reptorian updated this revision to Diff 35893.Jun 9 2018, 4:00 PM
This comment was removed by Reptorian.
Reptorian updated this revision to Diff 35895.Jun 9 2018, 4:56 PM
This comment was removed by Reptorian.
Reptorian updated this revision to Diff 35959.Jun 10 2018, 6:10 PM
1. I have fixed the Reflect Blend Mode, and Glow+Reflect are now acceptable as they follow the form the code reviewer request. Reflect Blend mode no longer have an ugly black splash.
2. I cannot avoid the usage of unitValue<T>() as it's the only way the code will compile. I need to know what to do here.
Reptorian edited the summary of this revision. (Show Details)Jun 10 2018, 6:29 PM
Reptorian edited the test plan for this revision. (Show Details)
Reptorian updated this revision to Diff 35982.Jun 11 2018, 12:10 AM

-I have attempted to solve the issue of code formatting by using a helper blending mode, and the formatting goes on the line of inv(x) where x is the base equation used for blending mode calculation. So, thus, it fit the code requirement. There is no other approach that doesn't have the issue of not fitting the code and avoids long long int error.

• What's next? It is to solve the discrepancies between color depth for Heat, and Freeze as it's pretty severe now. And then, the testing phase can continue.
Reptorian added a comment.EditedJun 11 2018, 12:25 AM

Negative colors explains the discrepancies

I'm not sure how to fix this issue, but if it can be fixed, then I believe the discrepancies between depth would be gone. Some even dip to -150, but this doesn't happen with 8-bit integer.

A theoretical way to solve this issue is to somehow have src and dst on the range of 0,255 with decimals restricted to .5 steps. Like src and dst are forced in those range, and estimated to .5 steps. I am not sure how to code that.

Hi, @Reptorian!

I have made a small patch (should be applied on the top of your patch) that fixes a few issues in the code. Please check if it behaves correct. If so, I will push it either to master or to some branch so painters could test it.

1 diff --git a/libs/pigment/compositeops/KoCompositeOpFunctions.h b/libs/pigment/compositeops/KoCompositeOpFunctions.h index 8f8a236..9a97834 100644 --- a/libs/pigment/compositeops/KoCompositeOpFunctions.h +++ b/libs/pigment/compositeops/KoCompositeOpFunctions.h @@ -460,6 +460,9 @@ inline T cfGlow(T src, T dst) { if(dst == unitValue()) return unitValue(); + + if(src == zeroValue()) + return zeroValue(); return clamp(div(mul(src, src), inv(dst))); } @@ -468,22 +471,7 @@ template inline T cfReflect(T src, T dst) { using namespace Arithmetic; - if(dst == unitValue()) - return unitValue(); - - return clamp(cfGlow(dst,src)); - -} - -template -inline T cfUQuad1(T src, T dst) { - using namespace Arithmetic; - // Helper Blender Mode for Heat, and Freeze - Reptorian - - if(dst == zeroValue()) - return zeroValue(); - - return clamp(div(mul(inv(src), inv(src)),dst)); + return cfGlow(dst,src); } template @@ -492,18 +480,18 @@ inline T cfHeat(T src, T dst) { if(dst == zeroValue()) return zeroValue(); - - return clamp(inv(cfUQuad1(src,dst))); + + if(src == unitValue()) + return unitValue(); + + return inv(clamp(div(mul(inv(src), inv(src)),dst))); } template inline T cfFreeze(T src, T dst) { using namespace Arithmetic; - if(dst == zeroValue()) - return zeroValue(); - - return clamp(cfHeat(dst,src)); + return cfHeat(dst,src); }

libs/pigment/compositeops/KoCompositeOpFunctions.h
461

I would also add to make the code a bit faster:

```if (src == zeroValue<T>()) {
return zeroValue<T>();
}```
471

This check is not needed here, because there is already a check in cfGlow. More than that, it is incorrect, when dst is a unit value, the result may have any value in range 0...1.

474

Second clamp is not needed here

Reptorian updated this revision to Diff 36062.Jun 12 2018, 5:16 PM
Reptorian marked 2 inline comments as done.
• The changes are based on the dkazakov's patch with a slight change. I removed the additional line of glow as it leads to 0,1,1 result on areas where they are not supposed to show up on. I also applied the break bracket on Glow, and Heat to make the code faster. Everything works as expected.

-Finally, there's a comment on the code as there are issues with Heat, and Freeze.

Sidenote: Now, I learned how to get inv(with long formula here) working thanks to Dmitry.

Reptorian added a comment.EditedJun 13 2018, 1:58 AM

I believe I may have found a formula that may lead to an answer for 16-bit, and 32-bit images with respect to Heat and Freeze Blend Mode.

It's either Formula 1 or Formula 2
F1 -> (255-(255-a)^2/b^((a*b)^.04))
F2 -> (255-(255-a)^2/b^(a^.04))

The main thing we need to calibrate is the .04 number. I am leaning to Formula 2. I'll start on working on making 2 new blending modes, and they'll be called F-Heat, and F-Freeze. F should indicates that they can be used for 16-bit and higher.

Oh, and for the note, 255 is not 255. 255 is unitValue.

libs/pigment/compositeops/KoCompositeOpFunctions.h
461

I decided to remove this as there was issues with the rendering result that leads to undesirable result like 0, 1, 1 among many areas. Now, I followed your suggestion of (Code()) {return;} for heat and freeze during the meantime.

Hi, @Reptorian!

I have tested your patch. I think it is somehow expected that Freeze and Heat modes work this way in F32 and F16 modes. The same applies to Linear Burn composite mode, which has a formula like that.

Please answer the question about zero value shortcut (you can catch me on IRC) and then we will push your patch into master

libs/pigment/compositeops/KoCompositeOpFunctions.h
461

How can it cause flipping problems? If I understand the formula correctly

`return clamp<T>(div(mul(src, src), inv(dst)));`

when src is null, then the result will also be null:

`clamp((0*0) / inv(dst)) -> 0 for any dst`

Or I don't understand something :)

dkazakov accepted this revision.Jun 15 2018, 7:37 PM

I will push the patch now! :)

This revision is now accepted and ready to land.Jun 15 2018, 7:37 PM
This revision was automatically updated to reflect the committed changes.