-
-
Notifications
You must be signed in to change notification settings - Fork 35.3k
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
Adds support for combined Depth+Stencil Texture #9368
Conversation
…epthStencilFormat
…t and DepthStencilFormat
@mattdesl looks good? |
I resolved all merge conflicts. Ping @mattdesl ;) |
Merge conflicts resolved again. (And yeah, ping @mattdesl ) |
When using depth only attachments, the paradigm I have observed in THREE.js is that the user creates a depth texture with an empty constructor as follows:
then the depth texture is set to default values and later updated with framebuffer size information in the function setupDepthTexture()
Since the texture is set to be a depth only texture by default, it naturally needs to be called using the paradigm of a complete constructor as in your usage comment at the start of this discussion:
I think that it would be more consistent if THREE.js constructed a texture with a depth_stencil attachment once it sees that its target buffer has the stencil flag set to true or better yet new in addition to THREE.DepthTexture(), we could also have a THREE.DepthStencilTexture() class to preserve the THREE.js paradigm of allocating textures using default constructors and allowing the rendering targets to fill them in as needed. |
I personally dislike the idea of automatically deciding which texture type to use based on the stencil flag, since this means changing/overriding other values like the pixel format, which could have been set by users. Using a separate class is probably feasible, but creates duplicate code. I think a clean approach would be to change the constructor to an options-object styled constructor (since missing parameters are filled in with defaults anyways), this would then result in
and
@mrdoob , @Bryce-Summers what do you think? |
Looks good! |
Thanks! |
Any chance you could also provide an example for this? That way we'll make sure that we don't break it in the future. |
Thanks! I hope to find time on the weekend, created issue #9417 for that. (Assign to me pls :)?) |
@AtiX I agree with you. By the way, I integrated your PR into an older version of three.js and it works great for image space CSG! |
* Adds THREE.UnsignedInt248Type and THREE.DepthStencilFormat constants * Extends DepthTexture to allow for either THREE.DepthFormat or THREE.DepthStencilFormat * Extends depthTexture initialization to support both DepthTextureFormat and DepthStencilFormat * Fixes typos * Ensures correct internal format for DepthStencil textures * Allows to set depthTexture via options.depthTexture in constructor
@Bryce-Summers do you have a demo up anywhere ? |
I wish I could point you to my demo code, but at that time I was working with a team with a closed source policy. |
Extends DepthTexture to not only support the depth component, but also depth+stencil component.
Two new constants (
THREE.DepthStencilFormat
andTHREE.UnsignedInt248Type
) are introduced.The WebGLRenderTarget is extended to support setting the depthTexture in the options instead of currently setting it manually afterwards:
Create a depth + stencil texture the following way:
(see also: https://www.khronos.org/registry/webgl/extensions/WEBGL_depth_texture/)