Page MenuHome

Code Cleanup: de-duplicate blend modes formulas
Needs ReviewPublic

Authored by Kévin Dietrich (kevindietrich) on May 21 2014, 4:56 PM.

Details

Summary

Title says it all. Formulas for Blender internal render engine, the compositor, texture, vertex and weight painting are now located in math_color_blend_inline.c. To that end the functions for floating point blending were split into a 'single channel' version, and a 'multi channel' version. Bytes blend mode were left untouched, apart from one minor correction.

Also makes the formulas to be more consistent as their implementations would slightly vary depending on where/when they were added to the code base.

Diff Detail

Repository
rB Blender
Branch
blend_modes

Event Timeline

Also, quick inline question:

source/blender/render/intern/source/render_texture.c
1392

What is the use of this function ? Strangely, I couldn't find how/where it is used in Blender, so could not really test the changes, which are to be added to this here patch.

Kévin Dietrich (kevindietrich) updated this revision to Unknown Object (????).May 22 2014, 4:10 PM

Commit log:

  1. Use enums instead of definitions for blendtype flags
  2. Blender: use interpf to further simplify formula reading
  3. Blender: texture_value_blend() function cleanup
  4. Blender: GLSL blendmodes cleanup, use mix() function
  5. Cycles: OSL blendmodes cleanup, use mix() function
  6. Cycles: SVM blendmodes cleanup, use interp() wherever possible, correct Linear Light formula
  7. Compositor: use col_mix_blend in MixBaseOperation::executePixelSampled()
Kévin Dietrich (kevindietrich) updated this revision to Unknown Object (????).May 31 2014, 3:11 PM

Rewrite H, S, V, C blend mode (give speedups)

Kévin Dietrich (kevindietrich) updated this revision to Unknown Object (????).Jun 1 2014, 7:44 PM

We don't have an issue with the patch; but have the users also accepted the changes (like fsiddi, venomgfx or sebastian_k) at least for the compositor.
Perhaps sergey could have a look to the other code.

Sergey, perhaps you have an opinion on the cycles part.

Will have a look tomorrow morning. Kinda late here already.

Kévin Dietrich (kevindietrich) updated this revision to Unknown Object (????).Jun 5 2014, 10:08 AM

Fix wrong formula for Color and Value: they would mimic the YUV colorspace instead of the HSV.

Note that I have not changed HSV blending modes in Cycles yet, for I have only done a rough cleanup there. Also, I think it could be good to rework the bytes formulas according to what I have done for the float ones. In that scheme, dealing with the blend operations for vertex and weight painting is also considered.

@Jeroen Bakker (jbakker): I was also thinking that, I might send a mail to bf-compositor to get some feedback from the users.

Nice work. My thoughts:

  • Color looks better. Almost like a Vibrance adjustment.
  • Hue looks the same?
  • Saturation also looks better, because it seems to be both saturating and desaturating according to the image. Some users might like the old mode better, but it can probably be reproduced by bumping the sat on an HSV node.
  • Value looks weird. Looks like it's also affecting the saturation and hue. This one would my only complain.

It's hard to say without looking at the original image though.

@Diego Gangl (januz): I uploaded the original image (as well the blendfile used, just for the hell of it).

Hue: almost the same, the colors are just smoother (no artifacts); will try to find a good image to demonstrate that.
Value: I still can't find what exactly is going wrong, as it is basically the same formula as Color but with the input swapped. So this will most likely change.

Thanks. Yeah, it all looks ok to me.

source/blender/blenlib/intern/math_color_blend_inline.c
566–568

This is the only part that doesn't look reversed from the color blend function. Maybe it has something to do with the problem?

source/blender/blenlib/intern/math_color_blend_inline.c
566–568

No, that part handles the mixing of the images from 0 to 1. If I reverse this, we will get a full mix with a factor of 0 and no mix at 1.

Kévin Dietrich (kevindietrich) updated this revision to Unknown Object (????).Jun 14 2014, 7:13 AM

I decided to revert the changes made to some of formulas, and other small ones, for now as these should (and will) be dealt with in a separate patch. Also, there were a bit problematic in some cases, so let's not make things worse.

In all, I think this is ready for final review.

@Antony Riakiotakis (psy-fi), mind having your eyes here? :)

I'll check the patch a nit later.

Kévin Dietrich (kevindietrich) planned changes to this revision.Feb 6 2015, 8:57 AM

Planning changes, since the last time I worked on this soc-2013-paint was merged and introduce some new blend modes to the paint system, will check on de-duplicating those along the ones in this patch.

Kévin Dietrich (kevindietrich) edited edge metadata.

De-duplicate vertex and weight paint blend modes.

  • Fix stupid bug.
  • Port rest of the blending modes to single channel versions.
  • Fix glsl shader compile error, and some bugs.
  • blend_color_* -> blend_value_*

Generally seems good, though didn't check OSL changed much.

One think I'm not so keen on is adding the n argument all over which is only ever checked for 4,
Why not have blend_color_screen_float_v3 and blend_color_screen_float_v4? (the v4 version can just call the v3 one in many cases).

  • Split funcs into *_v3 and *_v4 variants.
This revision is now accepted and ready to land.Jan 27 2016, 9:20 AM
Thomas Dinges (dingto) requested changes to this revision.Jan 27 2016, 9:33 AM

Cycles changes look ok (didn't test though) but one thing I'd like to see is consistency between SVM and OSL.
E.G. svm_mix_overlay() uses a for loop in OSL, in SVM it's separate. I want OSL and SVM code to match as closely as possible, which is important when debugging e.g. SVM/OSL render differences. Please change that before commiting.

This revision now requires changes to proceed.Jan 27 2016, 9:33 AM

It's indeed important to have OSL and SVM matching as close as possible. It also worth checking if wrapping stuff into for-loops doesn't result in changes in performance.

But IMO main quesiton here should be, how did all this was verified and checked against regressions?

Well, this is quite an old patch (first revision submitted ~20 months ago), so by memory I did some tests by comparing a handful of images. But, apart from an obvious call to a linear interpolation function (e.g. (1.0 - fac) * x + fac * y -> lerp(x, y, fac)) to make the code a bit more readable, only a few functions got the logic rewritten. Namely dodge, burn and soft light. And that's only for Cycles and the GLSL code for some reason (I forgot why). screen also got the math simplified. The Blender side is using the same formulas as master. If you feel a bit worried about it I can revert the formulas in Cycles and GLSL, but still use the lerp function. After all, the goal was de-duplication.

Then for speed regression, I haven't checked though.

Kévin Dietrich (kevindietrich) edited edge metadata.
  • Revert some changes to make to functions be the same accross the board