Add support for KHR_texture_transform#11904
Add support for KHR_texture_transform#11904alice-i-cecile merged 18 commits intobevyengine:mainfrom janhohenheim:khr_texture_transform
Conversation
Co-authored-by: Al McElrath <hello@yrns.org>
|
@mockersf you wrote the following on the original PR:
Could you explain to me why? Unless I understood something wrong, we'll need the |
|
Note that the current code recalculates the uv in both pbr_prepass_functions.wgsl and pbr_types.wgsl. Could someone with more knowledge of the shaders tell me how you usually cache these sorts of values? |
|
Not sure what I'm supposed to do about the duplicated dependencies check, could someone help me out there as well? The feature I added shouldn't pull in any additional transient dependencies according to |
Since these happen in separate passes and the result vary depending on the input vertex I don't think you can share the transformed uvs easily? Besides I don't think that's a major perf cost.
I think the current CI failure for that isn't caused by your changes, it is present in other PRs and isn't a blocker for merging. |
|
Alright, thanks! Then this PR is ready for merging from my point of view :) |
Kanabenki
left a comment
There was a problem hiding this comment.
A couple of suggestions 🙂
|
Good improvement suggestions, thanks! I'll implement them later today :) |
|
@Kanabenki ready for a re-review and I've got a question for you in one of the comments. |
Co-authored-by: Kanabenki <lucien.menassol@gmail.com>
|
@Kanabenki I think I addressed all your comments now :) Thanks for the review! |
|
@janhohenheim - the renaming of |
|
It looks like your PR is a breaking change, but you didn't provide a migration guide. Could you add some context on what users should update when this change get released in a new version of Bevy? |
|
this PR broke WebGL2 rendering: #12081 |
|
@mockersf ah heck! I'll look into it later today. Sorry for not testing that! |
Adopted bevyengine#8266, so copy-pasting the description from there: # Objective Support the KHR_texture_transform extension for the glTF loader. - Fixes bevyengine#6335 - Fixes bevyengine#11869 - Implements part of bevyengine#11350 - Implements the GLTF part of bevyengine#399 ## Solution As is, this only supports a single transform. Looking at Godot's source, they support one transform with an optional second one for detail, AO, and emission. glTF specifies one per texture. The public domain materials I looked at seem to share the same transform. So maybe having just one is acceptable for now. I tried to include a warning if multiple different transforms exist for the same material. Note the gltf crate doesn't expose the texture transform for the normal and occlusion textures, which it should, so I just ignored those for now. (note by @janhohenheim: this is still the case) Via `cargo run --release --example scene_viewer ~/src/clone/glTF-Sample-Models/2.0/TextureTransformTest/glTF/TextureTransformTest.gltf`:  ## Changelog Support for the [KHR_texture_transform](https://github.com/KhronosGroup/glTF/tree/main/extensions/2.0/Khronos/KHR_texture_transform) extension added. Texture UVs that were scaled, rotated, or offset in a GLTF are now properly handled. --------- Co-authored-by: Al McElrath <hello@yrns.org> Co-authored-by: Kanabenki <lucien.menassol@gmail.com>
Adopted bevyengine#8266, so copy-pasting the description from there: # Objective Support the KHR_texture_transform extension for the glTF loader. - Fixes bevyengine#6335 - Fixes bevyengine#11869 - Implements part of bevyengine#11350 - Implements the GLTF part of bevyengine#399 ## Solution As is, this only supports a single transform. Looking at Godot's source, they support one transform with an optional second one for detail, AO, and emission. glTF specifies one per texture. The public domain materials I looked at seem to share the same transform. So maybe having just one is acceptable for now. I tried to include a warning if multiple different transforms exist for the same material. Note the gltf crate doesn't expose the texture transform for the normal and occlusion textures, which it should, so I just ignored those for now. (note by @janhohenheim: this is still the case) Via `cargo run --release --example scene_viewer ~/src/clone/glTF-Sample-Models/2.0/TextureTransformTest/glTF/TextureTransformTest.gltf`:  ## Changelog Support for the [KHR_texture_transform](https://github.com/KhronosGroup/glTF/tree/main/extensions/2.0/Khronos/KHR_texture_transform) extension added. Texture UVs that were scaled, rotated, or offset in a GLTF are now properly handled. --------- Co-authored-by: Al McElrath <hello@yrns.org> Co-authored-by: Kanabenki <lucien.menassol@gmail.com>
|
Thank you to everyone involved with the authoring or reviewing of this PR! This work is relatively important and needs release notes! Head over to bevyengine/bevy-website#1287 if you'd like to help out. |
Adopted #8266, so copy-pasting the description from there:
Objective
Support the KHR_texture_transform extension for the glTF loader.
Solution
As is, this only supports a single transform. Looking at Godot's source, they support one transform with an optional second one for detail, AO, and emission. glTF specifies one per texture. The public domain materials I looked at seem to share the same transform. So maybe having just one is acceptable for now. I tried to include a warning if multiple different transforms exist for the same material.
Note the gltf crate doesn't expose the texture transform for the normal and occlusion textures, which it should, so I just ignored those for now. (note by @janhohenheim: this is still the case)
Via
cargo run --release --example scene_viewer ~/src/clone/glTF-Sample-Models/2.0/TextureTransformTest/glTF/TextureTransformTest.gltf:Changelog
affine2_to_squareto the WGSL API. An Affine2 can be efficiently packed for the GPU as amat3x2. This new methods unpacks that into amat3x3that can be used to transform coordinates.affine2_to_square, we renamedaffine_to_squaretoaffine3_to_squareMigration guide
Rename
affine_to_squaretoaffine3_to_square