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: Support for multiple render targets #26409

Merged
merged 6 commits into from
Aug 30, 2023
Merged

Conversation

aardgoose
Copy link
Contributor

First attempt at getting multiple render targets working in WebGPU/Nodes using WebGLMultipleRenderTargets, lots of cleanup work required if this looks like the right direction. Probably cleaner to use renderTargets.textures[] though, rather than special cases for multiple targets.

In the nodes material the returned outputStruct( outputColor1, outputColor2....) writes to the attached textures.

Example from webgl2 ported and working (you need to move the mouse around to get the first render. related to another async problem I have a patch for).

@sunag
Copy link
Collaborator

sunag commented Jul 12, 2023

Very good, I just think that we can use NodeMaterial.outputNode instead of a new material:

const material = new NodeMaterial();
material.outputNode = outputStruct( [ diffuseNode, normalNode ] );

So users also use GBuffer to create masks for post processing preserving the mainly material, without having to create other render-pass.

import { materialOutput } = 'three/nodes'

const standardMaterial = new MeshStandardNodeMaterial();
standardMaterial.outputNode = outputStruct( [ materialOutput, maskNode ] );

And for Deferred Shading internally approach as:

material.outputNode = outputStruct( [ materialPosition, materialDiffuse, materialNormal, ... ] );

--

I will create materialOutput I think it can help for this design.

@aardgoose
Copy link
Contributor Author

Using material.outputNode to avoid hacking nodeMaterial internals, ( that was only intended as a workaround).

@aardgoose aardgoose force-pushed the multi branch 2 times, most recently from 8e9a104 to 53b660d Compare July 17, 2023 11:07
add support for webglmultiplerendertargets wip
@aardgoose
Copy link
Contributor Author

@sunag

Updated to handle antialiasing. Do you want this reworked to handle all RTT cases with the same path, that is a textures array, and avoid all the special casing of Array.isArray()? This would agree with the approach taken in #26427.

@sunag
Copy link
Collaborator

sunag commented Jul 26, 2023

Do you want this reworked to handle all RTT cases with the same path, that is a textures array, and avoid all the special casing of Array.isArray()?

Sounds good for me 👍

@mrdoob mrdoob modified the milestones: r155, r156 Jul 27, 2023
@sunag
Copy link
Collaborator

sunag commented Aug 27, 2023

@aardgoose What would be missing here? It would be very good if we could merge this PR in this r156.

@aardgoose
Copy link
Contributor Author

The only thing I see as significant are the changes required if #26427. was accepted. Otherwise it should be fine.

@sunag sunag changed the title WIP support for multiple render target in WebGPU/Nodes WebGPURenderer: Support for multiple render targets Aug 30, 2023
@sunag
Copy link
Collaborator

sunag commented Aug 30, 2023

The only thing I see as significant are the changes required if #26427. was accepted. Otherwise it should be fine.

Thank you, I thought it was a great job, about the changes required in the #26427 I think we can wait, it will be backwards compatible, and we will need to rename WebGLMultipleRenderTargets to MultipleRenderTargets soon too, I think we can merge it now, this PR will be important for planning new features of the architecture.

@sunag sunag merged commit 1f43ebe into mrdoob:dev Aug 30, 2023
17 checks passed
@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 31, 2023

and we will need to rename WebGLMultipleRenderTargets to MultipleRenderTargets soon too

Maybe we should consider #26427 before doing this. If @mrdoob approves the general direction of the PR, we don't need a MultipleRenderTargets class.

@aardgoose aardgoose deleted the multi branch September 5, 2023 10:15
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.

4 participants