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 MSAA with Postprocessing #28784

Merged
merged 12 commits into from
Jul 4, 2024

Conversation

RenaudRohlinger
Copy link
Collaborator

@RenaudRohlinger RenaudRohlinger commented Jul 2, 2024

Description

The WebGPU did not support MSAA with a post-processing pipeline. This PR fixes the issue and adds a way to specify options, such as samples, to PassNode.

This contribution is funded by Utsubo

@RenaudRohlinger RenaudRohlinger requested a review from sunag July 2, 2024 09:05
@RenaudRohlinger RenaudRohlinger added this to the r167 milestone Jul 2, 2024
Copy link

github-actions bot commented Jul 2, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
682.2 kB (169 kB) 682.2 kB (169 kB) +0 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
459.4 kB (110.9 kB) 459.4 kB (110.9 kB) +0 B

Mugen87
Mugen87 previously approved these changes Jul 2, 2024
for ( let i = 0; i < this.renderTarget.textures.length; i ++ ) {

const type = this.renderTarget.textures[ i ].type;
this.renderTarget.textures[ i ].isMultisampleRenderTargetTexture = this.renderTarget.samples > 1 && type !== HalfFloatType && type !== FloatType;
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the purpose of type !== HalfFloatType && type !== FloatType?

Does that mean multisampled (half) float textures are not allowed?

@RenaudRohlinger
Copy link
Collaborator Author

RenaudRohlinger commented Jul 3, 2024

I cleaned up and refactored the samples and antialiasing logic. Previously, it was under backend.parameters.antialias and was only handled by the WebGPUBackend.
I moved the antialiasing settings to the Renderer and introduced a renderer.samples property. If this new property is set to a value greater than 0, it will override the antialias logic. The antialias property now simply acts as a shortcut for setting renderer.samples to 4, allowing developers to initialize the sample count directly from the Renderer, for example, by using new WebGPURenderer({ samples: 2 }).

Since WebGPU only supports sample counts of 1 and 4, I refactored the WebGPUUtils.getSampleCount method and used it everywhere it's relevant to ensure the sample count is always translated appropriately in that backend. For the WebGLBackend, it will use the sample count defined in the Renderer, which supports any sample amount, usually with a maximum of 8.

Here is most of the refactor:
811fd9a

Following this refactor, I discovered that in the case of multisampled textures in WebGPU, we were always actually using two textures: one extra that resolves the multisampled one. Based on the specs, it seems that WebGPU supports direct rendering of multisampled 2D and depth 2D textures, as it seems that the GPURenderPassDepthStencilAttachment interface doesn't provide a resolveTarget view and so cannot work. ( 👉 https://www.w3.org/TR/webgpu/#dom-gputexturebindinglayout-multisampled).

Therefore, I added multisample texture support to the WebGPUBackend and updated the WGSLNodeBuilder to support both types: texture_multisampled_2d and texture_depth_multisampled_2d.

Small notes:

  • The multisampling process for textures will continue to work as before if the type is Float or HalfFloat. This is because WebGPU does not support directly rendering to multisampled textures with these types. "unfilterable-float" seems to silent the issue but the antialiasing doesn't seems to get performed in that case. (To answer your comment, also I removed that block /cc @Mugen87)
  • I have not implemented this feature in the WebGLBackend. Given that this is already a substantial PR, I prefer to address this support in a separate, future PR.

Aside from this, while testing most of the examples, I noticed that webgpu_postprocessing_anamorphic.html, webgpu_compute_particles_snow.html all look different from the examples in production of r166, and this issue is also present in dev.

/cc @sunag @mrdoob @Mugen87

@Mugen87 Mugen87 dismissed their stale review July 3, 2024 09:20

New review required.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 3, 2024

(To answer your comment, also I removed that block

It seems the check is still present though.

The logic that configures isMultisampleRenderTargetTexture is something that should not belong to PassNode, imo. How about moving this into the renderer?

isMultisampleRenderTargetTexture is currently only configured by PassNode but it seems that should be done no matter of how a render target was configured.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 3, 2024

I have not implemented this feature in the WebGLBackend. Given that this is already a substantial PR, I prefer to address this support in a separate, future PR.

I don't think it makes sense to do that in the near future since you need the WEBGL_multisampled_render_to_texture extension which is only supported by the Oculus Browser so far. And since we can't use XR with WebGPURenderer right now there is no possibility to test this.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 3, 2024

Sidenote: Post processing in HDR requires half float so THREE.HalfFloatType is the default right now. Rendering multisampled to a color renderable texture is only relevant when the dev configures PassNode as RGBAFormat+ UnsignedByteType + SRGBColorSpace (bgra8unorm-srgb in WebGPU or SRGB8_ALPHA8 in WebGL). And then not all available effects will be compatible with this setup.

I recommend to split the change and add support for texture_multisampled_2d and texture_depth_multisampled_2d with a different PR (independently of PassNode).

@RenaudRohlinger
Copy link
Collaborator Author

The texture_depth_multisampled_2d is necessary to support MSAA with PassNode because GPURenderPassDepthStencilAttachment doesn't support using a resolveTarget:
image
Which means that the depth texture associated with that RenderPass will be multisampled if the ColorBuffer is multisampled.
Unlike the color that is resolved to a normal texture target, the depth will be sent to the GPU as a multisampled texture, which means WGSL should read it as texture_depth_multisampled_2d in order to work and render something. That's why I had to implement this support in WGSL.

Also, I cleaned up PassNode and removed the multisampling for the color passes, which is now reduced to a single line for the isMultisampleRenderTargetTexture flag. Unfortunately, I have to set the depthTexture.isMultisampleRenderTargetTexture flag in the PassNode.setup function if we want to fallback to the renderer.samples, as the texture creation seems to occur after the WGSLNodeBuilder code runs. This would otherwise generate incorrect WGSL for the not-yet-updated texture.

@Mugen87
Copy link
Collaborator

Mugen87 commented Jul 3, 2024

Thanks for the additional information!

Unfortunately, I have to set the depthTexture.isMultisampleRenderTargetTexture flag in the PassNode.setup function if we want to fallback to the renderer.samples, as the texture creation seems to occur after the WGSLNodeBuilder code runs. This would otherwise generate incorrect WGSL for the not-yet-updated texture.

Maybe @sunag has an idea how to hide this bit. Ideally, we evaluate whether a depth texture is multisampled or not inside the renderer and not in components using the renderer.

Apart from that, the PR looks already good!

this.renderTarget.samples = this.options.samples === undefined ? renderer.samples : this.options.samples;

// Disable MSAA for WebGL backend for now
if ( renderer.backend.isWebGLBackend === true ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this exception still required? My understand is you have added this bit since rendering multisampled to a color renderable texture does not yet work in the WebGL backend. But can at least the "normal" MSAA be used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately yes it's still needed. I noticed that using PassNode with samples > 0 doesn't seem to render anything in WebGL. I believe this might be due to the need for an additional frameBuffer somewhere in the render pipeline, which I couldn't easily resolve. That's why I added this comment.

Copy link
Collaborator

@sunag sunag Jul 4, 2024

Choose a reason for hiding this comment

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

Could we add this path in WebGLBackend.beginRender() for now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wouldn't recommend so because basic THREE.RenderTarget setups works just fine with samples > 0. For example webgpu_multisampled_renderbuffers.html.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's merge this PR then! It's possible do it in another PR, currently already a step forward in that sense. So the future goal is for us to move this and isMultisampleRenderTargetTexture into the renderer as @Mugen87 said, when this is ready examples like webgpu_backdrop_water should work too.

@sunag sunag merged commit 913edeb into mrdoob:dev Jul 4, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants