Skip to content

Simplify softlight glow blending math.#110239

Closed
allenwp wants to merge 1 commit into
godotengine:masterfrom
allenwp:simplify-softlight
Closed

Simplify softlight glow blending math.#110239
allenwp wants to merge 1 commit into
godotengine:masterfrom
allenwp:simplify-softlight

Conversation

@allenwp
Copy link
Copy Markdown
Contributor

@allenwp allenwp commented Sep 3, 2025

Would someone mind checking my math on this one? It seems that the softlight blend mode has a number of math operations that will never happen.

I've taken a screenshot with this blend mode before and after the change and it resulted in the exact same render.

(I also fixed a comment in another file that incorrectly references the "softlight" term.)

Edit: I've updated the PR to use a better simplification from @Lielay9

@Lielay9
Copy link
Copy Markdown
Contributor

Lielay9 commented Sep 4, 2025

Is there really a problem with 0.5; won't both branches simplify to color as (2.0f * glow.r - 1.0f) becomes 0?

Otherwise the function looks correct but the form is a bit odd. Personally I'd move to use the more widely known version (unless there's a precision benefit or smth that I'm unaware of) for the left side:

(color.r + (2.0f * glow.r - 1.0f) * (((16.0f * color.r - 12.0f) * color.r + 4.0f) * color.r - color.r))

If you want to be space efficient combine the common parts and place the condition in the middle:

(color.r + (2.0f * glow.r - 1.0f) * ((color.r <= 0.25f ? ((16.0f * color.r - 12.0f) * color.r + 4.0f) * color.r : sqrt(color.r)) - color.r))

EDIT: Concentrated on the functions at first but if you want to simplify further:

glow = glow * vec3(0.5f) + vec3(0.5f);
(2.0f * glow.r - 1.0f);

these lines are inverse of one another, so the combined function (the short version for example) could become:

(color.r + glow.r * ((color.r <= 0.25f ? ((16.0f * color.r - 12.0f) * color.r + 4.0f) * color.r : sqrt(color.r)) - color.r))

@allenwp
Copy link
Copy Markdown
Contributor Author

allenwp commented Sep 4, 2025

Is there really a problem with 0.5; won't both branches simplify to color as (2.0f * glow.r - 1.0f) becomes 0?

Ah, I think you're right; I had just seen (0,undefined) in the graphing calculator and blindly trusted it instead of checking the math myself.

@clayjohn
Copy link
Copy Markdown
Member

clayjohn commented Sep 4, 2025

Let's look at the case where glow.r is < 0.5.

Assume glow.r = 0.4 and color.r = 0.8

Before:
(color.r - (1.0f - 2.0f * glow.r) * color.r * (1.0f - color.r))
result = (0.8 - (1.0f - 2.0f * 0.4) * 0.8 * (1.0f - 0.8))
result = (0.8 - (0.2) * 0.8 * (0.2))
result = 0.768

After
result = 0.8

@Lielay9
Copy link
Copy Markdown
Contributor

Lielay9 commented Sep 4, 2025

Let's look at the case where glow.r is < 0.5.

Assume glow.r = 0.4 and color.r = 0.8

Before: (color.r - (1.0f - 2.0f * glow.r) * color.r * (1.0f - color.r)) result = (0.8 - (1.0f - 2.0f * 0.4) * 0.8 * (1.0f - 0.8)) result = (0.8 - (0.2) * 0.8 * (0.2)) result = 0.768

After result = 0.8

Glow can never be below 0.5 as it is first clamped to 0 to 1 range and then 0.5 is added. The path for below 0.5 is completely unnecessary as we only want to use the part of soft-light blending that lightens (i.e > 0.5 range).

Edit: For further clarity: even in your example it doesn't make sense that glow 0.4 + color 0.8 would result in darker color (0.768) i.e. negative glow.

@clayjohn
Copy link
Copy Markdown
Member

clayjohn commented Sep 4, 2025

Aaaaaaah right. I should have read the surrounding code

@AThousandShips AThousandShips added this to the 4.x milestone Sep 4, 2025
Comment thread drivers/gles3/shaders/effects/post.glsl
@allenwp
Copy link
Copy Markdown
Contributor Author

allenwp commented Sep 4, 2025

these lines are inverse of one another, so the combined function (the short version for example) could become:

(color.r + glow.r * ((color.r <= 0.25f ? ((16.0f * color.r - 12.0f) * color.r + 4.0f) * color.r : sqrt(color.r)) - color.r))

Wow, I didn't intend to nerd-snipe like this, but this new function looks great to me! I've pushed the changes to this PR with you as co-author.

@allenwp
Copy link
Copy Markdown
Contributor Author

allenwp commented Sep 30, 2025

I've included the changes from this PR in #110671 and added co-authors to it as well.

Co-authored-by: Hei <40064911+Lielay9@users.noreply.github.com>
@clayjohn
Copy link
Copy Markdown
Member

Superseded by #110671

@clayjohn clayjohn closed this Oct 15, 2025
@clayjohn clayjohn removed this from the 4.x milestone Oct 15, 2025
@allenwp allenwp deleted the simplify-softlight branch October 15, 2025 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants