Add premult alpha blending to 3D (spatial) shaders #85609

QbieShay · 2023-12-01T15:01:19Z

Follow-up to #68548
Implements godotengine/godot-proposals#3431 (comment)

OhiraKyou · 2023-12-02T18:21:27Z

I suggest converting this PR to an implementation of a general-purpose DELAYED_RGB_MULTIPLIER built-in vec3, as described in my comment on the associated proposal issue. Most of the necessary code is the same (minus the automatic blend mode enabling, which would be omitted).

Example:

#ifdef DELAYED_RGB_MULTIPLIER_USED
    vec3 delayed_rgb_multiplier = vec3(1.0);
#endif // DELAYED_RGB_MULTIPLIER_USED

QbieShay · 2023-12-10T10:00:57Z

@OhiraKyou could you give me an example usecase of this multiplier being a vec3? what kind of effect would it enable?

OhiraKyou · 2023-12-10T20:10:44Z

First, vec3 * float multiplication is per-component anyway. So, it's just good form to expose each component's multiplier to the user by making the multiplier a vec3 as well. That said, here are a couple of specific use cases.

Assumptions

The use cases described below assume the following:

The multiplier is generally available rather than coupled to the premultiplied alpha blend mode—which it really doesn't need to be coupled to.
The multiplier is applied after the sRGB to linear conversion and after built-in lighting.

Use cases

Custom channel mixing

Custom, per-channel post-processing of shader output colors—that includes lighting—may be applied in linear space by assigning a color multiplier to the delayed multiplier.

This includes processing of per-channel, non-color data for custom buffer rendering.

Custom colored lighting

Custom colored lighting (with no actual light objects in a scene) may be implemented in linear space in otherwise sRGB compatibility renderer shaders by ignoring the existing lighting system (i.e., using unshaded shaders) and assigning custom light output to the delayed multiplier.

QbieShay · 2023-12-14T14:14:19Z

We've discussed this PR in the rendering meeting and the design/thinking process around this is the following:

Like everything in Godot, we want to first talk about problems, then solutions.
The usecases you have provided are more along the lines of "if this was there, I could use it like this" rather than "I have this problem and I can't solve it without this".

Light multiplication is something I personally really want to do. I am hoping to be able to eventually have a post-light function (this is a wish, not a promise). But we need to respond with solution when there's a problem to begin with. But this is a whole different problem than what we're trying to solve here.
The problem to begin with here is having premultiplied alpha in the engine, so we should focus of solving that problem.

Additionally, concerns have been brought up about leaving doors open for usecases that are not explicitely supported: this may be a problem in the future when optimizing shaders or refactoring them. I understand that we could say "any use outside of premul alpha is unsupported and you're on your own if you do it" but this historically has never worked: people depend on it, and are disappointed when we "break" things that were never supposed to work to begin with.

I will bring this PR to completion in this dev cycle (4.3), unless something comes up, so that we can finally have premul alpha in 3D ^^ (integrating the render mode too)

scene/resources/material.cpp

clayjohn

Great work! I left a few comments to help clean this up. Most of them are just enforcing a consistent naming of BLEND_MODE_PREMULT_ALPHA

doc/classes/BaseMaterial3D.xml

drivers/gles3/rasterizer_scene_gles3.cpp

scene/resources/material.cpp

servers/rendering/renderer_rd/forward_clustered/scene_shader_forward_clustered.cpp

drivers/gles3/storage/material_storage.cpp

servers/rendering/renderer_rd/forward_clustered/scene_shader_forward_clustered.cpp

servers/rendering/shader_compiler.cpp

servers/rendering/shader_compiler.h

mikejohnstn · 2024-04-12T20:56:59Z

Any idea if PREMUL_ALPHA support is still targeting 4.3? I'm building a tool that depends on it, so it would help to know if I can aim to target a particular release.

clayjohn

Looks great!

doc/classes/BaseMaterial3D.xml

akien-mga · 2024-04-30T14:28:39Z

This PR uses a mix of PREMULT_ALPHA and premul_alpha spelling (note the presence or absence of T). This should be unified to use only one spelling.

See akien-mga's comment after the approval

Co-authored-by: jitspoe <[email protected]> Co-authored-by: Clay John <[email protected]>

akien-mga · 2024-05-01T07:58:17Z

Thanks!

akien-mga · 2024-05-01T07:59:32Z

scene/resources/material.cpp

@@ -688,6 +688,9 @@ void BaseMaterial3D::_update_shader() {
 		case BLEND_MODE_MUL:
 			code += "blend_mul";
 			break;
+		case BLEND_MODE_PREMULT_ALPHA:
+			code += "blend_premul_alpha";


I merged a bit too fast, this one is still wrong.

So it seems like we had this BLEND_MODE_PREMULT_ALPHA but blend_premul_alpha inconsistency in CanvasItem shaders already.

Should we fix it in Spatial shaders and accept that we have an inconsistency in 2D blend_premul_alpha / 3D blend_premult_alpha, or should we do like in 2D?

I noticed that too, but I'd rather not start touching things elsewhere for now

I mean this one is a bug, it's writing blend_premul_alpha in Spatial shaders but what's expected is blend_premult_alpha. I haven't tested but I think the latest update before merge broke the feature.

So we have two options:

Fix this one up to be blend_premult_alpha, and accept that 3D shaders will use a different keyword from 2D shaders

Go back to the previous version of your PR before my review (I didn't know you just replicated the inconsistency of existing 2D keywords) and use blend_premul_alpha in 3D too.

And yes to be clear we shouldn't change 2D shaders, that would break compat.

I didn't remember either, and frankly I wonder if I did it on purpose or not. I'm not sure if we want to keep consistent with 2d and generally inconsistent, or if we should have correct 3d and weird 2D. Is there any precedent we can reference to take the decision?

akien-mga · 2024-05-01T08:03:15Z

doc/classes/BaseMaterial3D.xml

@@ -609,6 +609,9 @@
 		<constant name="BLEND_MODE_MUL" value="3" enum="BlendMode">
 			The color of the object is multiplied by the background.
 		</constant>
+		<constant name="BLEND_MODE_PREMULT_ALPHA" value="4" enum="BlendMode">
+			The color of the object is added to the background and the alpha channel is used to mask out the background. This is effectively a hybrid of the blend mix and add modes, useful for FX like fire where you want the flame to add but the smoke to mix. By default, this works with unshaded materials using premultiplied textures. For shaded materials, use the PREMUL_ALPHA_FACTOR built-in so that lighting can be modulated as well.


Should be [code]PREMULT_ALPHA_FACTOR[/code].

jitspoe · 2024-05-01T09:41:53Z

Does this PR address fog making stuff light up that should be transparent? That was one thing I never got around to fixing on mine.

Macksaur · 2024-05-02T04:08:44Z

scene/resources/material.cpp

+			case BLEND_MODE_PREMULT_ALPHA: {
+				// This is unlikely to ever be used for detail textures, and in order for it to function in the editor, another bit must be used in MaterialKey,
+				// but there are only 5 bits left, so I'm going to leave this disabled unless it's actually requested.
+				//code += "\tvec3 detail = (1.0-detail_tex.a)*ALBEDO.rgb+detail_tex.rgb;\n";


@QbieShay I don't understand why this is commented out. Before the nuance is lost to the sands of time can you expand a little more? 5 bits left where?

godot/scene/resources/material.h

Line 326 in f91db3d

uint64_t detail_blend_mode : get_num_bits(BLEND_MODE_MAX - 1);

The bits as far as I can tell are already spent allocating BLEND_MODE_PREMULT_ALPHA. Commenting it out just seems a little strange.

Ah, that was probably taken from my original PR which didn't have the bits taken. I was hesitant to waste a bit for this, but if the bit is already taken, this should be uncommented.

jitspoe · 2024-05-09T02:16:51Z

Ok, just cherry-picked this and tested and verified that this does NOT properly address fog, at least independently. I did notice there were some other fog related things in the shader file that changed, but I don't think they're related. Guess that should still be addressed.

To better explain, I'm using flame sprites with premult alpha, and if I have fog enabled, the whole quad starts becoming visible, (additively blended with the fog color). I thought maybe the premult_alpha_factor would somehow get used, but looking closer, I'm not really sure what premult_alpha_factor even does, as it seems to just be renamed to premult_alpha, which is a bit confusing.

QbieShay · 2024-05-09T08:55:10Z

@jitspoe it's multiplied at the very end of the shader. Do you mind supplying a test scene with your setup (via DM is fine too) so that u can try and fix the dog issue?

clayjohn · 2024-05-09T16:37:29Z

Ok, just cherry-picked this and tested and verified that this does NOT properly address fog, at least independently. I did notice there were some other fog related things in the shader file that changed, but I don't think they're related. Guess that should still be addressed.

To better explain, I'm using flame sprites with premult alpha, and if I have fog enabled, the whole quad starts becoming visible, (additively blended with the fog color). I thought maybe the premult_alpha_factor would somehow get used, but looking closer, I'm not really sure what premult_alpha_factor even does, as it seems to just be renamed to premult_alpha, which is a bit confusing.

Can you share your shader? Are you writing to PREMUL_ALPHA_FACTOR?

akien-mga · 2024-05-10T08:32:36Z

Make sure to also cherry-pick #91399 if testing on a branch different from upstream master, as this PR wouldn't be functional without that fixup.

QbieShay · 2024-05-10T12:22:22Z

I havent had a chance to boot my dev PC yet but I think I understand the issue, I'll debug it later. If it is what I imagine, I need to multiply fog by alpha if render mode pmul alpha is on.

clayjohn · 2024-05-10T13:49:09Z

I havent had a chance to boot my dev PC yet but I think I understand the issue, I'll debug it later. If it is what I imagine, I need to multiply fog by alpha if render mode pmul alpha is on.

Pmul alpha is already multiplied after fog though. I don't see why you would need to multiply it twice

QbieShay · 2024-05-10T13:52:31Z

Because the fog should be applied only to the non transparent parts of a quad. Alpha zero, factor non zero = additive.

QbieShay · 2024-05-14T17:07:22Z

@jitspoe Please test against latest master, I can't reproduce. Don't forget to use the PREMUL_ALPHA_FACTOR

jitspoe · 2024-05-17T02:57:01Z

So it seems like PREMUL_ALPHA_FACTOR just scales the albedo color value, effectively turning it into a regular alpha, kind of defeating the purpose of the premultiplied aspect. Fixes the fog, but the whole point is to be able to have additive things like flames mixed in with alpha masked stuff like smoke. Unless I'm just not understanding how this is intended to be used? Still trying to understand the reasoning behind PREMUL_ALPHA_FACTOR. How is it different from just scaling ALBEDO?

Building latest main now to do further tests.

jitspoe · 2024-05-17T03:35:12Z

I'm moving the discussion to godotengine/godot-proposals#3431 (comment) so it's in one cohesive location, since there is a lot of other discussion in there as well.

xa8et67 · 2024-06-04T15:07:51Z

太棒了，多年来终于解决了这个问题。

QbieShay mentioned this pull request Dec 1, 2023

Add blend_premul_alpha to SpatialMaterial and shaders godotengine/godot-proposals#3431

Closed

QbieShay force-pushed the qbe/premul-builtin branch from 2f360d9 to f7962aa Compare December 1, 2023 15:22

AThousandShips added enhancement topic:rendering topic:3d labels Dec 1, 2023

AThousandShips added this to the 4.x milestone Dec 1, 2023

YuriSizov modified the milestones: 4.x, 4.3 Dec 1, 2023

QbieShay force-pushed the qbe/premul-builtin branch 2 times, most recently from 8d508fa to b305c5c Compare December 2, 2023 13:49

YuriSizov mentioned this pull request Dec 13, 2023

Add premul alpha blending to 3D (spatial) shaders #68548

Closed

QbieShay force-pushed the qbe/premul-builtin branch 2 times, most recently from 5a6fc8a to 25f6dbc Compare December 15, 2023 20:44

QbieShay marked this pull request as ready for review December 15, 2023 20:47

QbieShay requested review from a team as code owners December 15, 2023 20:47

AThousandShips reviewed Dec 15, 2023

View reviewed changes

scene/resources/material.cpp Outdated Show resolved Hide resolved

QbieShay force-pushed the qbe/premul-builtin branch from 25f6dbc to facda0d Compare December 15, 2023 23:07

clayjohn reviewed Dec 16, 2023

View reviewed changes

QbieShay force-pushed the qbe/premul-builtin branch from 7a38045 to 738c28f Compare December 16, 2023 10:26

QbieShay force-pushed the qbe/premul-builtin branch from 738c28f to d6335bc Compare April 26, 2024 07:14

clayjohn previously approved these changes Apr 30, 2024

View reviewed changes

AThousandShips approved these changes Apr 30, 2024

View reviewed changes

doc/classes/BaseMaterial3D.xml Outdated Show resolved Hide resolved

QbieShay force-pushed the qbe/premul-builtin branch 2 times, most recently from 2a0a535 to b87eeeb Compare April 30, 2024 22:52

Added premult alpha blending to 3D (spatial) shaders.

41a2b0e

Co-authored-by: jitspoe <[email protected]> Co-authored-by: Clay John <[email protected]>

QbieShay force-pushed the qbe/premul-builtin branch from b87eeeb to 41a2b0e Compare April 30, 2024 22:53

akien-mga changed the title ~~Add PREMUL_ALPHA builtin~~ Add premult alpha blending to 3D (spatial) shaders Apr 30, 2024

akien-mga merged commit e950d7a into godotengine:master May 1, 2024
16 checks passed

akien-mga reviewed May 1, 2024

View reviewed changes

QbieShay mentioned this pull request May 1, 2024

Revert premul alpha to spell without the T #91399

Merged

Macksaur reviewed May 2, 2024

View reviewed changes

fire mentioned this pull request Nov 6, 2024

[3.x] Add blend_premul_alpha support to scene shaders. #36747

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add premult alpha blending to 3D (spatial) shaders #85609

Add premult alpha blending to 3D (spatial) shaders #85609

QbieShay commented Dec 1, 2023 •

edited by YuriSizov

Loading

OhiraKyou commented Dec 2, 2023 •

edited

Loading

QbieShay commented Dec 10, 2023

OhiraKyou commented Dec 10, 2023

QbieShay commented Dec 14, 2023 •

edited

Loading

clayjohn left a comment

mikejohnstn commented Apr 12, 2024

clayjohn left a comment

akien-mga commented Apr 30, 2024

akien-mga commented May 1, 2024

akien-mga May 1, 2024

akien-mga May 1, 2024

QbieShay May 1, 2024

akien-mga May 1, 2024 •

edited

Loading

QbieShay May 1, 2024

akien-mga May 1, 2024

jitspoe commented May 1, 2024

Macksaur May 2, 2024

jitspoe May 2, 2024

jitspoe commented May 9, 2024

QbieShay commented May 9, 2024

clayjohn commented May 9, 2024

akien-mga commented May 10, 2024

QbieShay commented May 10, 2024

clayjohn commented May 10, 2024

QbieShay commented May 10, 2024

QbieShay commented May 14, 2024

jitspoe commented May 17, 2024

jitspoe commented May 17, 2024

xa8et67 commented Jun 4, 2024

Add premult alpha blending to 3D (spatial) shaders #85609

Add premult alpha blending to 3D (spatial) shaders #85609

Conversation

QbieShay commented Dec 1, 2023 • edited by YuriSizov Loading

OhiraKyou commented Dec 2, 2023 • edited Loading

QbieShay commented Dec 10, 2023

OhiraKyou commented Dec 10, 2023

Assumptions

Use cases

Custom channel mixing

Custom colored lighting

QbieShay commented Dec 14, 2023 • edited Loading

clayjohn left a comment

Choose a reason for hiding this comment

mikejohnstn commented Apr 12, 2024

clayjohn left a comment

Choose a reason for hiding this comment

akien-mga commented Apr 30, 2024

akien-mga commented May 1, 2024

akien-mga May 1, 2024

Choose a reason for hiding this comment

akien-mga May 1, 2024

Choose a reason for hiding this comment

QbieShay May 1, 2024

Choose a reason for hiding this comment

akien-mga May 1, 2024 • edited Loading

Choose a reason for hiding this comment

QbieShay May 1, 2024

Choose a reason for hiding this comment

akien-mga May 1, 2024

Choose a reason for hiding this comment

jitspoe commented May 1, 2024

Macksaur May 2, 2024

Choose a reason for hiding this comment

jitspoe May 2, 2024

Choose a reason for hiding this comment

jitspoe commented May 9, 2024

QbieShay commented May 9, 2024

clayjohn commented May 9, 2024

akien-mga commented May 10, 2024

QbieShay commented May 10, 2024

clayjohn commented May 10, 2024

QbieShay commented May 10, 2024

QbieShay commented May 14, 2024

jitspoe commented May 17, 2024

jitspoe commented May 17, 2024

xa8et67 commented Jun 4, 2024

QbieShay commented Dec 1, 2023 •

edited by YuriSizov

Loading

OhiraKyou commented Dec 2, 2023 •

edited

Loading

QbieShay commented Dec 14, 2023 •

edited

Loading

akien-mga May 1, 2024 •

edited

Loading