WebGLRenderer: Copy set of layers when copying 3d texture data #29699
Conversation
|
|
||
| if ( srcTexture.isDepthTexture ) mask = _gl.DEPTH_BUFFER_BIT; | ||
|
|
||
| _gl.blitFramebuffer( minX, minY, width, height, dstX, dstY, width, height, mask, _gl.NEAREST ); |
There was a problem hiding this comment.
If you completely remove the usage of blitFramebuffer(), the method does not support depth attachments anymore. That means the srcTexture.isDepthTexture should be removed and the documentation should be updated.
There was a problem hiding this comment.
Is it possible to have a depth buffer attachment for a 3d texture?
There was a problem hiding this comment.
I believe so. How would depth testing work otherwise?
There was a problem hiding this comment.
I guess I hadn't really considered the case where you'd be rendering to volume textures that way. I can add it back as an option.
To be clear it doesn't look like the previous code path can ever copy the depth buffer for a color render target in the 2d or 3d case:
let mask = _gl.COLOR_BUFFER_BIT;
if ( srcTexture.isDepthTexture ) mask = _gl.DEPTH_BUFFER_BIT;The depth component is only copied when isDepthTexture is on the src texture which will never be the case with normal render render targets. We also have no method for creating 3d depth textures. Is this the kind of logic we want instead?
let mask = _gl.COLOR_BUFFER_BIT | _gl.DEPTH_BUFFER_BIT | _gl.STENCIL_BUFFER_BIT;
if ( srcTexture.isDepthTexture ) mask = _gl.DEPTH_BUFFER_BIT;ie just copy all the attached buffers unless it's a depth texture? I feel we might as well just set all the bits anyway regardless of target type when using the blit path.
There was a problem hiding this comment.
The depth component is only copied when isDepthTexture is on the src texture which will never be the case with normal render render targets.
We already use the following pattern and it works as expected:
renderer.copyTextureToTexture( this.sampleRenderTarget.depthTexture, this.renderTarget.depthTexture );I'm okay with keeping the PR as it is and revisit the issue when a developer requests it. For the normal copyTextureToTexture(), the support is critical though.
There was a problem hiding this comment.
I think I was confusing myself regarding the depth texture. I've added the same branching logic to choose blit vs copy in both 2d and 3d functions for consistency:
if ( srcTexture.isDepthTexture ) {
_gl.blitFramebuffer( minX, minY, width, height, dstX, dstY, width, height, _gl.DEPTH_BUFFER_BIT, _gl.NEAREST );
} else {
_gl.copyTexSubImage2D( _gl.TEXTURE_2D, level, dstX, dstY, minX, minY, width, height );
}Is the use of copyTextureToTexture for your use case tested in the E2E testing? Or is there an easy way I can test that this code path works for you?
There was a problem hiding this comment.
As long as the original blit is retained for the depth texture, I don't think something should break.
There was a problem hiding this comment.
Okay! In that case this should be ready if it looks good to you.
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
Related issue: #29612
Description
Adjust
copyTextureToTexture3Dto copy each layer between the two 3d textures individually since there is no WebGL method for copying a block of 3d data between textures for render targets.Tested in my 3d tiles use case and the code is much simpler, now, and it works great. (see #29688)
--
The last piece of the puzzle is copying 2d textures into 3d texture layers along with their mipmaps.