WebGLBackend: Bring back 3D functionality for copyTextureToTexture #30584
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
zonkypop
changed the title
| * | ||
| * @param {Texture} srcTexture - The source texture. | ||
| * @param {Texture} dstTexture - The destination texture. | ||
| * @param {?Vector4} [srcRegion=null] - The region of the source texture to copy. |
There was a problem hiding this comment.
Do you mind fixing the parameter type of srcRegion?
| * @param {number} [dstLevel=0] - The destination mip level to copy to. | ||
| */ | ||
| copyTextureToTexture( srcTexture, dstTexture, srcRegion = null, dstPosition = null, level = 0 ) { | ||
| copyTextureToTexture( srcTexture, dstTexture, srcRegion = null, dstPosition = null, srcLevel = 0, dstLevel = 0 ) { |
There was a problem hiding this comment.
Do you mind updating all copyTextureToTexture() signatures in WebGPURenderer? For that, you have to update Renderer.copyTextureToTexture(), Backend.copyTextureToTexture() (the interface definition) and both backends.
There was a problem hiding this comment.
Cool I've updated the signatures too, hopefully it's looking alright
zonkypop
force-pushed
the
WebGLBackend-copyTextureToTexture-3D
branch
from
3cb654b to
b72a191
Compare
src/renderers/common/Renderer.js
Outdated
| * @param {number} [dstLevel=0] - The destination mip level to copy to. | ||
| */ | ||
| copyTextureToTexture( srcTexture, dstTexture, srcRegion = null, dstPosition = null, level = 0 ) { | ||
| copyTextureToTexture( srcTexture, dstTexture, srcRegion = null, dstPosition = null, level = 0, dstLevel = 0 ) { |
There was a problem hiding this comment.
Can you please rename level to srcLevel?
There was a problem hiding this comment.
Fixed, I've set dstLevel = null which is the same as the original WebGLRenderer as well.
zonkypop
force-pushed
the
WebGLBackend-copyTextureToTexture-3D
branch
from
2d401d7 to
008362f
Compare
| // gather the necessary dimensions to copy | ||
| let width, height, depth, minX, minY, minZ; | ||
| let dstX, dstY, dstZ; | ||
| const image = srcTexture.isCompressedTexture ? srcTexture.mipmaps[ dstLevel ] : srcTexture.image; |
There was a problem hiding this comment.
This bit isn't correct. The default value of dstLevel must be 0 otherwise you end up with srcTexture.mipmaps[ null ].
Please change the default value of dstLevel to 0 in all signatures as well in #30618.
There was a problem hiding this comment.
Regarding your comment in #30584 (comment): The signature in WebGLRenderer can't be used as a template since the value is null for backwards compatibility reason, AFAICS. Besides, it is set to 0 in the first if statement.
There was a problem hiding this comment.
Ah yes that makes sense, I have updated them to use dstLevel = 0 by default now.
zonkypop
force-pushed
the
WebGLBackend-copyTextureToTexture-3D
branch
from
93a1072 to
38be18a
Compare
Fix interface method, see #30584.
2 participants
Issue
#30619
Description
In the WebGLBackend, copyTextureToTexture has only implemented copying for 2D textures.
This PR uses parts of the code from the original GL renderer to add functionality for copying between 2D and 3D textures in new WebGL fallback renderer.