Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebGPURenderer: Make material.transparent behave as in WebGLRenderer #30862

Merged
merged 2 commits into from
Apr 8, 2025

Conversation

rkreis-v
Copy link
Contributor

@rkreis-v rkreis-v commented Apr 4, 2025

Fixed #30861

Description

Make WebGPU backend apply material.blending even when material.transparent is false, just like WebGL backend.

@rkreis-v
Copy link
Contributor Author

rkreis-v commented Apr 4, 2025

Can also be fixed by changing WebGL backend to behave like WebGPU, but I find the behavior of the WebGL backend more intuitive - it wasn't apparent to me why WebGPU backend would ignore an explicitly set blending property.

Copy link

github-actions bot commented Apr 4, 2025

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 336.49
78.38
336.49
78.38
+0 B
+0 B
WebGPU 542.03
150.12
542.11
150.15
+74 B
+28 B
WebGPU Nodes 541.5
150.02
541.57
150.05
+74 B
+28 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Before After Diff
WebGL 465.42
112.22
465.42
112.22
+0 B
+0 B
WebGPU 614.87
166.17
614.95
166.19
+74 B
+23 B
WebGPU Nodes 569.86
155.58
569.93
155.6
+74 B
+21 B

@sunag sunag changed the title WebGPU: Make material.transparent behave as in WebGL WebGPURenderer: Make material.transparent behave as in WebGL Apr 4, 2025
@sunag sunag changed the title WebGPURenderer: Make material.transparent behave as in WebGL WebGPURenderer: Make material.transparent behave as in WebGLRenderer Apr 4, 2025
@sunag sunag added this to the r176 milestone Apr 4, 2025
@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 4, 2025

Aligning both backends makes of course sense. However, I find the WebGL behavior more confusing. It seems more intuitive to me that blending only works when transparent is set to true.

@WestLangley What is the reason for implementing this aspect in WebGLRenderer differently?

@WestLangley
Copy link
Collaborator

@Mugen87 See if the discussion in #14171 answers your question.

@rkreis-v
Copy link
Contributor Author

rkreis-v commented Apr 5, 2025

@sunag thanks for the fixup! Clarification: It's specifically about the WebGPU backend, WebGPURenderer with forceWebGL: true behaves like WebGLRenderer already. Ok with you to revert the title?

Regarding expected behavior, I discovered the mismatch implementing a specific use case where I need blending without moving objects to the second render pass for transparent objects. I see no way to do that with current WebGPU backend.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 6, 2025

How can you ensure that objects are properly sorted for blending if you don't set transparent to true?

@rkreis-v
Copy link
Contributor Author

rkreis-v commented Apr 7, 2025

I'm using a full screen pass to draw an overlay on existing opaque geometry, which has to be rendered before other opaque geometry. I'd prefer not to derail the discussion, but focus on the bug and API design instead. Can we agree that there's a legitimate use case to have blending set without transparent: true, and there's existing code using that combination on WebGL backend?

@sunag
Copy link
Collaborator

sunag commented Apr 7, 2025

... but focus on the bug and API design instead.

This is exactly what @Mugen87 is doing. The ordering works in reverse to transparent objects, in opaque rendering from closest to furthest for early-z rejection to work in an optimized way which seems like a real problem to me. We need to make sure that users are using the correct approach for each purpose so that the API design is not mistaken for a bug.

@Mugen87 Do you remember when we started using material.id difference to sort the render-order?

} else if ( a.material.id !== b.material.id ) {
return a.material.id - b.material.id;

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 7, 2025

Using the material ID in painterSortStable() was introduced long time ago in WebGLRenderer with the purpose to minimize shader program state changes. At that time it was assumed that minimizing the overhead of switching shader programs is more important than a strict depth-based sort.

I have never seen a performance measurement that has proven the current approach is faster than without using the material ID. I'm also not sure if the current sort is still beneficial for WebGPU.

@sunag
Copy link
Collaborator

sunag commented Apr 7, 2025

Hmm.. I understand that minimizing the overhead of switching shader programs is important, but I think this can be improved, the material.id does not bring any relevant information that the material is the same as the next one, and in this context all the ids are different from each other, making the following conditionals ignored. And it would be considering the user to be lucky to create the same type of material right after.

Another example of what could happen, the sort is based on which material was created last, if you uncomment lines 63 and 64 from the example below you will have a different sort, making the API unstable if transparent:false and is blending:add that confirm #30862 (comment)

Example
https://jsfiddle.net/rn7j4yse/2/

The idea is to resolve similar objects in the backend itself to minimizing the overhead of switching gpu states, I was developing something in this sense, soon I will continue this work as soon as I resolve some other PRs. This should solve this issue too #30560

@sunag sunag removed this from the r176 milestone Apr 7, 2025
@sunag sunag closed this Apr 7, 2025
@WestLangley WestLangley reopened this Apr 7, 2025
@WestLangley
Copy link
Collaborator

WestLangley commented Apr 7, 2025

There are so many issues with this thread...

@Mugen87 and @sunag The use of blending does not necessarily have anything to do with transparency. There are a lot of blending functions other than the familiar Porter-Duff formula, which is used to model physical transparency.

@rkreis-v You are correct that the WebGLRenderer approach should be followed, but your formula is not quite correct. transparent === true is not the same as transparent !== false.

@sunag In the existing code (prior to this PR), blending can remain undefined. Is that intentional?

@sunag
Copy link
Collaborator

sunag commented Apr 7, 2025

@WestLangley Currently the blending was working according to the user's luck creating one material after another as I demonstrated in the fiddle, so painterSortStable() will be redone and the opaque materials to favor early-z rejection then the front objects will be rendered first as it should be for opaque materials. Are you considering this?

@WestLangley
Copy link
Collaborator

@sunag My previous comments have nothing to do with sorting. I do not want to get sidetracked here. This PR is about making sure the WebGPURenderer blending formula matches WebGLRenderer. We can't require users to set transparent = true to get their custom blending to be applied.

Please do not assume the use of blending means the material is transparent. Doing so is a misconception.

@sunag
Copy link
Collaborator

sunag commented Apr 8, 2025

It seems like you are linking transparent to the Porter-Duff formula, but practically all the transparent code is related to sorting, what about materials that use custom-blending or refraction that need transparent to do the correct sorting, they certainly don't use the Porter-Duff formula but they are still correct. What I mean is that when we fix painterSortStable(), material.blending won't work without considering transparent flag #30862 (comment), either through the material.transparent or inside the RenderList through some extra conditional or custom sorting, somehow we will need to define it. All this can be discussed once we are considering all the limitations.

@rkreis-v
Copy link
Contributor Author

rkreis-v commented Apr 8, 2025

I'm setting renderer.sortObjects = false, so none of the object sorting discussion is relevant to my use case or how we should align WebGPU and WebGL backend behaviour. Because many of my remarks and questions are ignored anyway, I don't have anything else to say at this point.

@Mugen87
Copy link
Collaborator

Mugen87 commented Apr 8, 2025

I'm afraid I still don't understand why blending should work without setting transparent to true, sorry. #14171 isn't a help here because all the discussion says it's incorrect otherwise. I also don't understand what the "other blending functions" mentioned in #30862 (comment) are. Maybe a fiddle could help clarifying this aspect?

Besides, answers to the following questions would also help:

  • Before Allow Additive, CustomBlending when material.transparent is false #14171 was implemented, developers eventually used the approach we have implemented in WebGPURenderer (WebGPU backend) for eight years. Are there any user issues that requested a change?
  • Why is it wrong to couple Material.transparent with Material.blending?
  • Would it break user code if we ask users to set Material.transparent to true if blending is used? Would this be an extensive migration task?

@sunag
Copy link
Collaborator

sunag commented Apr 8, 2025

@Mugen87 I'm merging, I think we've now fixed painterSortStable() this will no longer present bugs like the ones I mentioned here

This is certainly not compatible with depthTest or it would be relying on the luck of the object behind being rendered first with renderer.sortObjects=false, but through the manipulation of the depth-buffer or create a custom-sorting this would be possible in a stable way.

@sunag sunag merged commit 075e1c9 into mrdoob:dev Apr 8, 2025
22 checks passed
@sunag sunag added this to the r176 milestone Apr 8, 2025
@@ -88,7 +88,7 @@ class WebGPUPipelineUtils {

let blending;

if ( material.transparent === true && material.blending !== NoBlending ) {
if ( material.blending !== NoBlending && ( material.blending !== NormalBlending || material.transparent === true ) ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rkreis-v @sunag @Mugen87

As I mentioned in the discussion, to replicate the WebGLRenderer logic exactly, I think it should be:

if ( material.blending !== NoBlending && ( material.blending !== NormalBlending || material.transparent !== false ) ) {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated: 4e3db42

@WestLangley
Copy link
Collaborator

@sunag I think you missed my question in #30862 (comment). Can you please have a look? 🙏

Mugen87 added a commit that referenced this pull request Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebGPU and WebGL backends handle blending and transparency properties differently
4 participants