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: implement ClippingGroup object #28237

Merged
merged 15 commits into from
Nov 4, 2024

Conversation

aardgoose
Copy link
Contributor

@aardgoose aardgoose commented Apr 29, 2024

Fixed #28838.

As suggested in original renderer

// we might want to call this function with some ClippingGroup

Adds support for ClippingGroup objects. Clipping planes are additive with nested groups as demonstrated in example.

Copy link

github-actions bot commented Jul 10, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 692.35
171.47
692.35
171.47
+0 B
+0 B
WebGPU 822.19
221.91
821.87
221.85
-324 B
-60 B
WebGPU Nodes 821.48
221.76
820.99
221.64
-495 B
-116 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 464.89
112.3
464.89
112.3
+0 B
+0 B
WebGPU 542.77
146.84
542.27
146.74
-505 B
-107 B
WebGPU Nodes 498.71
136.59
498.21
136.47
-505 B
-117 B

@aardgoose aardgoose force-pushed the clipping-context-group branch 2 times, most recently from ad9ee35 to 5ea08b1 Compare July 13, 2024 21:03
@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 16, 2024

When I read the code correctly, this PR removes Renderer.clippingPlanes.

Is it also possible to remove the support for Material.clippingPlanes? When I understand the implementation correctly, ClippingGroup can handle both local and global clipping.

@Mugen87 Mugen87 added this to the r167 milestone Jul 16, 2024
@aardgoose
Copy link
Contributor Author

@Mugen87 I have already removed material clipping planes in my copy. I'll push it here soon. It gets simplifies the Renderer somewhat.

@aardgoose aardgoose force-pushed the clipping-context-group branch 2 times, most recently from bfaed52 to 266a2ef Compare July 18, 2024 13:34
@aardgoose
Copy link
Contributor Author

@Mugen87 @sunag

ClippingGroup() can now replicate the functionality of the existing API without the problems encountered with postprocessing passes etc #28838, the existing example has been updated to use this. The only clipping property now on the material object is 'alphaToCoverage' which is common to other materials.

the planes used for union testing and intersection testing are accumulated separately when inherited.

@sunag To retain the current behaviour where changing 'alphaToCoverage' results in a material rebuild, I have added boolean properties to the RenderObject.getMaterialCacheKey(), I don't know if this is the correct solution here?

@aardgoose aardgoose closed this Jul 20, 2024
@aardgoose aardgoose reopened this Jul 20, 2024
@aardgoose aardgoose marked this pull request as ready for review July 20, 2024 13:16
@mrdoob mrdoob modified the milestones: r167, r168 Jul 25, 2024
@aardgoose aardgoose force-pushed the clipping-context-group branch from 984a1b7 to c4492a3 Compare August 7, 2024 16:02
@sunag
Copy link
Collaborator

sunag commented Aug 7, 2024

To retain the current behaviour where changing 'alphaToCoverage' results in a material rebuild, I have added boolean properties to the RenderObject.getMaterialCacheKey(), I don't know if this is the correct solution here?

I'll check this weekend, sorry for the delay.

@aardgoose
Copy link
Contributor Author

To retain the current behaviour where changing 'alphaToCoverage' results in a material rebuild, I have added boolean properties to the RenderObject.getMaterialCacheKey(), I don't know if this is the correct solution here?

I'll check this weekend, sorry for the delay.

The question is redundant now, fixed by a upstream PR from @Mugen87

aardgoose added 3 commits August 25, 2024 15:11
Replace renderer and material clipping planes with nestable clipping
groups
@mrdoob mrdoob modified the milestones: r168, r169 Aug 30, 2024
@mrdoob mrdoob modified the milestones: r169, r170 Sep 26, 2024
@mrdoob mrdoob modified the milestones: r170, r171 Oct 31, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 2, 2024

@aardgoose Do you have the resources to rebase the PR? If not, I'm going to give it a try in the upcoming days.

We really should merge the PR asap to eventually fix global clipping planes. The implementation can still be refactored after the merge if required.

@aardgoose
Copy link
Contributor Author

Yes, I'll take a look, then I can revisit hardware clipping which is apparently coming in WebGPU in the next chrome version.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 2, 2024

Great! Can't wait to have access to ClippingGroup. It makes things so much nicer.


}

}

}

renderObject( object, scene, camera, geometry, material, group, lightsNode, passId = null ) {
renderObject( object, scene, camera, geometry, material, group, lightsNode, clippingContext = null, passId = null ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

LensflareMesh directly calls renderObject() and can't access clippingContext meaning it can't pass it to renderObject(). I've fixed the resulting runtime error by defining a default parameter.

If we add clippingContext to onBeforeRender(), we could ensure lensflares support clipping as well. Alternatively, we could also use the "current clipping context" for nested renderings if it isn't overwritten with a parameter when calling renderObject().

Revert changes to build files.
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.

WebGPURenderer: global clipping planes applied to color space and tone mapping pass.
4 participants