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

Add THREE.CompressedArrayTexture #24745

Merged
merged 21 commits into from
Oct 18, 2022

Conversation

RenaudRohlinger
Copy link
Collaborator

WIP

WebGLTextures part should work, I'm just very confused with the KTXLoader part and on how to create a proper dataset with toktx.

Right now I am using the following command toktx --layers 2 --t2 textures.ktx2 hair_m_baseColor.png hair_m_normal.png in the examples/models/gltf/sougen/ folder.

I suppose that the support of this should work:
https://developer.mozilla.org/en-US/docs/Web/API/WebGL_API/Compressed_texture_formats

https://github.com/KhronosGroup/KTX-Software/pull/468/files

This might interest 😄 @donmccurdy

Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

Thanks @RenaudRohlinger, great to see this! 👍

I don't think I know enough to comment on the renderer internals, perhaps @Mugen87 can say more.

Right now I am using the following command toktx --layers 2 --t2 textures.ktx2 hair_m_baseColor.png hair_m_normal.png in the examples/models/gltf/sougen/ folder.

Careful on this – these texture types have different encodings, so one or the other is likely to be incorrectly-encoded or to have lost precision. Probably want to keep color and non-color textures in separate arrays, and use --assign_oetf accordingly.

examples/jsm/loaders/KTX2Loader.js Outdated Show resolved Hide resolved
examples/jsm/loaders/KTX2Loader.js Outdated Show resolved Hide resolved
examples/jsm/loaders/KTX2Loader.js Outdated Show resolved Hide resolved
examples/jsm/loaders/KTX2Loader.js Outdated Show resolved Hide resolved
examples/jsm/loaders/KTX2Loader.js Outdated Show resolved Hide resolved
@RenaudRohlinger
Copy link
Collaborator Author

RenaudRohlinger commented Oct 5, 2022

Good news @donmccurdy! I made some progress on this feature and I can make the example work with some hard-coded data. In the example, the right part of the hair reflects layer 0, and the left part is layer 1.

I managed to isolate the issues and I think the remaining part is inside the ktx transcoded. The array buffer data of the transcoded image will only output the first layer.

I made a very basic compressed array texture of 4x4 px red on layer 0 and 4x4 px blue on layer 1 to make these tests easier.

There is a bunch of comments inside the KTX2Loader file with my catch so far but my knowledge is very limited in this area so help would be more than welcome 😄

image

@donmccurdy
Copy link
Collaborator

Related:

The Basis transcoder API requires layerIndex arguments for the transcodeImage and getImageLevelInfo methods, which we will need. Currently those are unused by the implementation. Once we have data for each layer, I am not sure what to do with it... do we just concatenate the data for all layers at the same mipmap level? I haven't found any examples yet.

I think we are going to need a THREE.CompressedArrayTexture class for this implementation. Adding THREE.Compressed3DTexture might also be nice but is not required here. The main difference is in the mipmap layouts.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Oct 5, 2022

do we just concatenate the data for all layers at the same mipmap level?

I guess that's right — pushed 32c1a67.

Screen Shot 2022-10-05 at 11 34 48 AM

Result appears correct with mipmaps too, although I do get a warning in the console for that case:

GL_INVALID_OPERATION: Level of detail outside of range.

Generated with:

toktx --genmipmap --assign_oetf srgb --target_type RGBA --layers 2 --encode etc1s \ 
  examples/models/gltf/Sougen/tex.ktx2 \
  examples/models/gltf/Sougen/red.png \
  examples/models/gltf/Sougen/blue.png

@RenaudRohlinger
Copy link
Collaborator Author

Added CompressedArrayTexture, updated the example, and cleaned up some code.

Should we try to support Compressed3DTexture at the same time?

Also maybe it's worth mentioning that compressed array texture generates a bunch of warnings on firefox that should be ignored.
https://stackoverflow.com/questions/57734645/error-tex-image-texture-2d-level-0-is-incurring-lazy-initialization-on-webgl/57734917#57734917


So in other words, getting Firefox to shut up about its warning actually makes your code much slower and use much more memory then if firefox just silently cleared the texture on its own.

image

Copy link
Collaborator

@donmccurdy donmccurdy left a comment

Choose a reason for hiding this comment

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

Should we try to support Compressed3DTexture at the same time?

Let's consider this in a future PR. Updating KTX2Loader, WebGLRenderer, adding CompressedArrayTexture, and adding an example is already a fair bit for one PR!

The changes in KTX2Loader to support compressed array textures look good to me. I am not sure I can review the changes to WebGLState and WebGLTextures, @Mugen87 do you mind reviewing this part?

src/textures/CompressedArrayTexture.js Outdated Show resolved Hide resolved
src/textures/CompressedArrayTexture.js Outdated Show resolved Hide resolved
@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 6, 2022

I am not sure I can review the changes to WebGLState and WebGLTextures, @Mugen87 do you mind reviewing this part?

The changes to WebGLState look good, just need more time for WebGLTextures. I'll wait for that until the PR is "ready for review".

@RenaudRohlinger
Copy link
Collaborator Author

Thanks for all the feedback!

I updated CompressedArrayTexture, the code in WebGLTextures accordingly, updated copyTextureToTexture3D in the renderer too, and added the documentation.

Should be ready for review 👍

@RenaudRohlinger RenaudRohlinger marked this pull request as ready for review October 7, 2022 03:25
@donmccurdy donmccurdy changed the title Texture2darray compressed Add THREE.CompressedArrayTexture Oct 7, 2022
@Mugen87 Mugen87 added this to the r146 milestone Oct 7, 2022
@RenaudRohlinger
Copy link
Collaborator Author

I replaced the example with something a lot more basic. I'm using 6 frames from a Ghibli movie, I have absolutely no idea what's the boundaries about licenses here, I suppose it could be problematic? In that case, any cc0 loop would be welcome as I can't really find anything looking decent enough for the example section.

image

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 17, 2022

I suppose it could be problematic?

I'm not a lawyer but I guess it's the same like when the clip would appear in a movie review video or something similar. It should fall under the fair use area so it's safe to use it like in the example.

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.

4 participants