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

RFC: Multiple bindgroups for WebGPU - breaks WebGL currently #28665

Closed
wants to merge 4 commits into from

Conversation

aardgoose
Copy link
Contributor

@aardgoose aardgoose commented Jun 15, 2024

Utilise the shared uniform group mechanism #27134 to enable shared uniform buffers for WebGPU - should be extendable to UBO in WebGL.

Camera data is only stored once in the GPU side rather than in a uniform buffer per pipeline

Bind group 0: camera uniforms
Bind group 1: object and material uniforms

Because until all materials have been seen, we don't know which camera uniforms are required. The binding is set up for all camera uniforms and only those in use are attached on the fly to the correct update mechanism. To keep single pass frame rendering working (for cube maps etc), a NodeUpdate type of ONCE is added that automatically resets to NONE after the first update.

Most examples work except BatchMesh. The screenshot test failures are a symptom of the WebGL breakage.

Bind group 0: camera uniforms
Bind group 1: object and material uniforms
@RenaudRohlinger
Copy link
Collaborator

That's awesome! I also started working on a similar PR, but your dynamic attachment approach is even better!

Since I'm really looking forward to this feature, I can work on the WebGL implementation if you’d like!

@sunag
Copy link
Collaborator

sunag commented Jun 16, 2024

I was thinking of something closer to this #28347 (comment) for uniforms groups.

@@ -1,8 +1,14 @@
import DataMap from '../DataMap.js';
import ChainMap from '../ChainMap.js';
import NodeBuilderState from './NodeBuilderState.js';
import { EquirectangularReflectionMapping, EquirectangularRefractionMapping, NoToneMapping, SRGBColorSpace } from 'three';
import { NodeFrame, vec4, objectGroup, renderGroup, frameGroup, cubeTexture, texture, rangeFog, densityFog, reference, viewportBottomLeft, normalWorld, pmremTexture, viewportTopLeft } from '../../../nodes/Nodes.js';
import { EquirectangularReflectionMapping, EquirectangularRefractionMapping, NoToneMapping, SRGBColorSpace, Vector3 } from 'three';

Check notice

Code scanning / CodeQL

Unused variable, import, function or class Note

Unused import Vector3.
@aardgoose
Copy link
Contributor Author

@sunag This is just getting a working mechanism. Addition of per material uniform groups and containing bind groups should be more straightforward, because we obviously know what uniforms, textures etc a material uses.

Re your stage 2. With materials using different combinations of camera uniforms in different orders and supporting multiple camera uniform Groups which that implies, adds more state changes and complexity per draw call, I was trying to avoid (I attempted that approach first but wanted to avoid the potential overhead and complexity that created).

In your model, would it be better to have scene related items like fog etc in a separate uniform group that could share bind group 0 with the camera?

Incidentally, the batch mesh failure to render is actually in the base code, rather than with this PR.

@sunag
Copy link
Collaborator

sunag commented Jun 21, 2024

Thank you for the initiative and sorry maybe I couldn't explain all the details in the previous message, so I created the PR.

Theoretically we can organize camera, material, model and others using shared groups. This will make it easier to determine what can be shared, and what will be shared with, and what not in the next stages.

The bind set definitions that I referred to would only be at a redundancy level, for example if ( currentBindGroup[ 0 ] !== previousBindGroup[ 0 ] ), to help in this stage we can do a sort at the time of construction of the shader.

There are also some offset details that I would like to check in the future, to make the bind sequences more organized related to the vertex stages, but it is not something crucial for operation.

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.

None yet

3 participants