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

AnimationPointerUVs #115

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

AnimationPointerUVs #115

wants to merge 2 commits into from

Conversation

echadwick-artist
Copy link
Contributor

@echadwick-artist echadwick-artist commented Mar 26, 2024

This model tests UV transform animations with KHR_animation_pointer for position, scale, and rotation.

screenshot_Large

This model tests UV transform animations with `KHR_animation_pointer` for position, scale, and rotation.
"rotate" changed to "rotation"
"target": {
"extensions": {
"KHR_animation_pointer": {
"pointer": "/materials/23/extensions/KHR_materials_transmission/transmissionTexture/extensions/KHR_texture_transform/rotation"
Copy link

@bghgary bghgary Mar 27, 2024

Choose a reason for hiding this comment

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

There are a few of these transmissionTexture pointers that point to properties that don't exist.

/animations/0/channels/38/extensions/KHR_animation_pointer/pointer: Invalid pointer (/materials/23/extensions/KHR_materials_transmission/transmissionTexture/extensions/KHR_texture_transform/rotation) skipped
/animations/0/channels/40/extensions/KHR_animation_pointer/pointer: Invalid pointer (/materials/22/extensions/KHR_materials_transmission/transmissionTexture/extensions/KHR_texture_transform/scale) skipped
/animations/0/channels/42/extensions/KHR_animation_pointer/pointer: Invalid pointer (/materials/21/extensions/KHR_materials_transmission/transmissionTexture/extensions/KHR_texture_transform/offset) skipped
/animations/0/channels/44/extensions/KHR_animation_pointer/pointer: Invalid pointer (/materials/20/extensions/KHR_materials_transmission/transmissionTexture/extensions/KHR_texture_transform/rotation) skipped
/animations/0/channels/46/extensions/KHR_animation_pointer/pointer: Invalid pointer (/materials/19/extensions/KHR_materials_transmission/transmissionTexture/extensions/KHR_texture_transform/scale) skipped
/animations/0/channels/48/extensions/KHR_animation_pointer/pointer: Invalid pointer (/materials/18/extensions/KHR_materials_transmission/transmissionTexture/extensions/KHR_texture_transform/offset) skipped

@javagl
Copy link
Contributor

javagl commented Mar 27, 2024

This looks fancy in Babylon.js. (Some look like they are not animated - probably not all properties are supported there yet?).

More out of curiosity: Is the labeling 'U Tiling / V Tiling' intentional, instead of 'U Scale / V Scale'?

@bghgary
Copy link

bghgary commented Mar 28, 2024

We are fixing the issues in Babylon.js. There are issues with the asset also. Discussions are happening here if you are curious.

Here is the latest.

@echadwick-artist
Copy link
Contributor Author

echadwick-artist commented Mar 28, 2024

The labels are for the UI labels in the 3ds Max source, to help the developers of the glTF exporter I am using.

When this asset is closer to completion, I will probably rename the labels to match glTF conventions.

Should I change the meshes into instances? In 3ds Max it’s possible to assign a separate material to each instance, but I’m not sure if this is possible in glTF?

@javagl
Copy link
Contributor

javagl commented Mar 28, 2024

Should I change the meshes into instances? In 3ds Max it’s possible to assign a separate material to each instance, but I’m not sure if this is possible in glTF?

The geometry data of the spheres is indeed duplicated. So one could probably squeeze out a few bytes with intancing or plainly sharing the indices/attributes accessors. But the spheres do not have sooo many triangles, so that's probably negligible compared to the textures.
(These, in turn, seem to be mostly JPG. I think that a recommendation is to not use JPG for things like normal textures, because the compression may cause very visible artifacts there, but ... in this case, the image one of "real tiles", which are a tiny bit "noisy" anyhow - and PNG would probably be twice as large, so JPG seems like a reasonable choice here...)

@echadwick-artist
Copy link
Contributor Author

I have a question about properties sharing UVs.

The baseColor texture and the alphaMode texture must use the same texcoord and the same uv transforms (KHR_texture_transform), because they are loaded using a single baseColorTexture Property per material (there can’t be two baseColorTextures in a single material).

Similarly, the metallic texture and the roughness texture must use the same texcoord and the same uv transforms, because they are loaded using a single metallicRoughnessTexture property per material (there can’t be two metallicRoughnessTextures in a single material).

However… occlusion uses a separate Property from metallicRoughness, and occlusion may use the same bitmap, or it may use a separate bitmap.

If the occlusionTexture uses the same index as the metallicRoughnessTexture, does this mean they must only use the same texcoord and the same uv transforms?

What about other textures with separate Properties but packed color channels? sheenColorTexture.rgb and
sheenRoughnessTexture.a for example.

Would the general rule be that if two or more Properties share the same Index, then they must specify the same texcoord and transforms?

@lexaknyazev
Copy link
Member

Would the general rule be that if two or more Properties share the same Index, then they must specify the same texcoord and transforms?

There's no rule here. Each texture slot may have its own UV set and transforms.

@echadwick-artist
Copy link
Contributor Author

There's no rule here. Each texture slot may have its own UV set and transforms.

Thanks Alexey. What do you mean by texture slot? The texture Index? or the texture Property?

@donmccurdy
Copy link

donmccurdy commented Apr 3, 2024

I would think of it as a material having the following texture "slots" —

  • baseColorTexture
  • normalTexture
  • metallicRoughnessTexture
  • occlusionTexture
  • sheenColorTexture
  • sheenRoughnessTexture
  • ...

These slots ("textureInfo" in spec language) might point to the same or different textures, identified by indices. Each slot may select its UV set and UV transforms independently of the others, even if two slots point to the same texture.

Because there is no glTF concept of an alphaTexture slot at all, it cannot be separated from baseColorTexture. Whereas sheenColorTexture and
sheenRoughnessTexture are separate slots (even if they could optionally point to the same texture) and so their UVs and UV transforms may be separated.

Some engines may be less flexible in supporting all of these permutations, of course. :)

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.

None yet

5 participants