KhronosGroup / glTF

glTF – Runtime 3D Asset Delivery
Other
7.14k stars 1.14k forks source link

Texture transform should be referenced by the material texture #1422

Open UX3D-nopper opened 6 years ago

UX3D-nopper commented 6 years ago

see https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_texture_transform

Having the current specification, this implies, that for each texture we could potentially have a unique texture transformation matrix. This could result in - at this point of time - 5 uv coordinates.

What I do not like about this is, that a transformation matrix is calculated for each texture, even if they are using the same resulting transformation matrix. Beside the calculation overhead, obsolete uv sets need to be created. Looking ahead - having more textures in the future - this does bloat up the final shader code.

Finally, if the usage in Unity and Three.js are carefully read, they also do not support "inifinite" texture transforms: https://threejs.org/docs/#api/textures/Texture.offset https://docs.unity3d.com/ScriptReference/Material-mainTextureOffset.html

donmccurdy commented 6 years ago

Could you explain your suggestion of "referenced by the material texture" further? Does that mean only a single texture transform per material, which applies to all textures?

I don't think per-texture transforms require additional UV sets, just uniforms to store the transforms, which can be applied to UVs in the shader. You're correct that three.js supports only a single texture transform per material (with some nuances), although we're working on fixing this.

andrewvarga commented 6 years ago

I think, if the UV transformation is applied in the fragment shader, indeed just +1 uniform is required for each texture. But if the UV transformation is applied in the vertex shader, which is I guess more common (?), you indeed need to have as many varyings as required.

For example if you have 3 textures, each having its own texture transformation, texture1 and texture2 using uv channel1 and texture3 using uv channel2, then you need 2 varyings for texture1 and texture2 even though they use the same vertex attribute (for uv channel1): (varying) v_uv1_texture1 = texture1Transform a_uv1; (varying) v_uv1_texture2 = texture2Transform a_uv1;

So you need a new set of varyings, even if texture1Transform and texture2Transform is the same. Maybe that's what @UX3D-nopper means?

UX3D-nopper commented 6 years ago

I was just writing a long comment, deleteid it and to make it short: I dislike the possibility of all the permutations. Probably there is no way around, but we should probably discuss this during the 3D formats meeting.

UX3D-nopper commented 2 years ago

Need to be solved in the future.

emackey commented 2 years ago

I'm wondering if we eventually need a KHR_texture_transform2 (or whatever name) extension that completely breaks away from the existing transform extension. Things we could consider:

donmccurdy commented 2 years ago

Each individual texture could then supply a (small) index into this root list, so it could use one of the calculated transform matricies.

Just to be sure it's explicit – it's not the total number of transforms in the scene that is a problem (for three.js anyway, and presumably Unity) but the number of transforms per material. three.js supports only 2 texcoord attributes per mesh primitive, and all textures used by a particular material must share a texture transform if they share a texcoord attribute. So that's a maximum of two transforms, with further restrictions based on texture slot and texture coordinates.

We have some long-term ideas of how to improve that, but I expect it to be a few years before that's generally available, and even then it may have performance tradeoffs.

It would be awesome to allow animation of the transforms at the root level.

Agreed that it would make sense to allow animation somehow if we were re-doing KHR_texture_transform. I'm not sure about implications of the "at the root level" part.

emackey commented 2 years ago

number of transforms per material

Interesting. If we were to specify the list of texture transforms at the material level (instead of glTF root), and then allow each texture reference within a material to index into that list, would ThreeJS fully support 2 transforms per material? (Meaning, not just the AO map, but all textures? Or perhaps I've mixed up my TEXCOORD indices with transform indices.)

donmccurdy commented 2 years ago

The "root level" vs "material level" question is unimportant to three.js – maybe saves a bit of JSON if you have a lot of materials, but having lots of materials brings unavoidable performance problems quickly so that's probably a wash. I don't think that's what @UX3D-nopper is referring to but I could be wrong.

And yeah, unfortunately the tl;dr for three.js is: