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

WebGLRenderTarget: Add support for swapping textures #19851

Closed
wants to merge 3 commits into from

Conversation

Oletus
Copy link
Contributor

@Oletus Oletus commented Jul 16, 2020

This can be used to save GPU memory or to avoid texture copy operations in situations where multiple textures need to be used as render targets but one shared depth/stencil buffer is enough.

The downside is that the WebGL implementation needs to validate the framebuffer again for completeness.

WebGLCubeRenderTarget returns Textures from this function, not CubeTextures.

This can be used to save GPU memory or to avoid texture copy operations in situations where multiple textures need to be used as render targets but one shared depth/stencil buffer is enough.

The downside is that the WebGL implementation needs to validate the framebuffer again for completeness.

WebGLCubeRenderTarget returns Textures from this function, not CubeTextures.
@Oletus
Copy link
Contributor Author

Oletus commented Jul 16, 2020

Thoughts on this @mrdoob @Mugen87 ? The API is a bit ugly since not just any Texture is suitable for passing in and because WebGLCubeRenderTarget doesn't use CubeTextures, but I'm quite sure that this is useful functionality to have.

@gkjohnson
Copy link
Collaborator

I recently ran into this again when trying to implement an effect that uses depth peeling. I wound up creating two unused color buffers just because I wanted to write to a depth texture. In my opinion it would be more intuitive to be able to replace the depth buffer of a WebGLRenderTarget especially considering a field already exists for setting the depthTexture. I think being able to change depthTexture or set it to a shared depth attachment would let you achieve the same effects.

I previously created #19447 about the current inability to update depthTexture after the first render and briefly outlined some thoughts an idea for enabling updating the depth / color attachments for devices that don't support depth textures here (#15490 (comment)) that I think fit into the existing three.js patterns for updating fields.

It's been on my list to address it at some point but I'm definitely interested in your thoughts!

@Oletus
Copy link
Contributor Author

Oletus commented Jul 20, 2020

@gkjohnson I think it's important that both depth textures and depth renderbuffers are supported. WEBGL_depth_texture is not quite universally supported, and depth renderbuffers can be more efficient than depth textures. If both could be swapped in and out of WebGLRenderTargets I think I'd be happy. Still a bit awkward to create extra framebuffer objects, but that's acceptable.

It would have one advantage over this: if a depth buffer could be kept attached to multiple FBOs, the WebGL implementation doesn't have to revalidate framebuffer completeness.

I'd like to hear what other maintainers think!

@gkjohnson
Copy link
Collaborator

I think it's important that both depth textures and depth renderbuffers are supported. WEBGL_depth_texture is not quite universally supported, and depth renderbuffers can be more efficient than depth textures.

Yeah I definitely agree. #19447 was created to address the inability to swap depth textures which I think would make adding custom depth buffers easier. I referenced #15490 (comment) to show how adding a frame buffer API for depth might look, which I think is more what you're looking for.

@Oletus
Copy link
Contributor Author

Oletus commented Jul 23, 2020

So what are the next steps on this? Do you think it would get traction if I proposed a patch adding a renderbuffer API, or is someone else planning to work on this? Or should this texture swapping API still be considered as well? The two are not mutually exclusive.

@gkjohnson
Copy link
Collaborator

As far as I'm aware no one is working on this -- I suggested the frame buffer API previously because I expected it fit the existing three.js patterns more closely and would be more agreeable but there wasn't any feedback from maintainers. I think it would be nice to get confirmation that this is something worth doing before putting the work in. Maybe @mrdoob or @Mugen87 have some thoughts?

@pailhead
Copy link
Contributor

pailhead commented Jul 28, 2020

Since #15490 is still open could you consider pinging the maintainers there? Technically i think this should all be filed under this issue #15440 but since it was closed in favor of having the discussion in #15490, i'd suggest to keep it in the same place. I don't like talking about the same issue but over several different implementations.

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.

3 participants