KhronosGroup / glTF

glTF – Runtime 3D Asset Delivery
Other
7.06k stars 1.13k forks source link

In KHR_texture_transform extension, uv should be "fract" before transforming? #2412

Open PrimaryFantasty opened 3 weeks ago

PrimaryFantasty commented 3 weeks ago

Current:

varying in vec2 Uv;

uniform vec2 Offset, Scale;
uniform float Rotation;

mat3 translation = mat3(1,0,0, 0,1,0, Offset.x, Offset.y, 1);
mat3 rotation = mat3(
    cos(Rotation), sin(Rotation), 0,
   -sin(Rotation), cos(Rotation), 0,
                0,             0, 1
);
mat3 scale = mat3(Scale.x,0,0, 0,Scale.y,0, 0,0,1);

mat3 matrix = translation * rotation * scale;
vec2 uvTransformed = ( matrix * vec3(Uv.xy, 1) ).xy;

But in CesiumJS 1.96:

  fragmentShader +=
    "vec2 computeTexCoord(vec2 texCoords, vec2 offset, float rotation, vec2 scale) \n" +
    "{\n" +
    "    rotation = -rotation; \n" +
    "    mat3 transform = mat3(\n" +
    "        cos(rotation) * scale.x, sin(rotation) * scale.x, 0.0, \n" +
    "       -sin(rotation) * scale.y, cos(rotation) * scale.y, 0.0, \n" +
    "        offset.x, offset.y, 1.0); \n" +
    "    vec2 transformedTexCoords = (transform * vec3(fract(texCoords), 1.0)).xy; \n" +
    "    return transformedTexCoords; \n" +
    "}\n\n";

The difference between them is that interpolated UVs has been "fract" before transforming,and they make different effect. So what kind of them should I take?

javagl commented 3 weeks ago

(Just a cross-link to the forum post: https://community.cesium.com/t/why-modified-the-behavior-of-khr-texture-transform-extension-since-cesiumjs-1-97/32984 )

scurest commented 3 weeks ago

The fract is incorrect.

javagl commented 3 weeks ago

Thanks @scurest , I also suspected that. I'm not entirely sure how to confirm that to an extent that would allow closing this issue. Using https://github.com/cx20/gltf-test to look at the TextureTransformTest, namey rendered in CesiumJS, it looks correct:

Cesium Texture Transform Test

With the small caveat that it's not clear whether the model contains texture coordinates where fract would actually make a difference. (I suspect such models are rare to begin with).

I'd vote to close this, saying that the fract does not seem to make sense (and is also not used in the sample GLSL code of the specification). So I assume that this was a bug in earlier CesiumJS version, but has been fixed with the "Model" rewrite that happened in 1.97.

PrimaryFantasty commented 3 weeks ago

The fract is incorrect.分形不正确。

Thanks for your reply!

PrimaryFantasty commented 3 weeks ago

Thanks @scurest , I also suspected that. I'm not entirely sure how to confirm that to an extent that would allow closing this issue. Using https://github.com/cx20/gltf-test to look at the TextureTransformTest, namey rendered in CesiumJS, it looks correct:

Cesium Texture Transform Test

With the small caveat that it's not clear whether the model contains texture coordinates where fract would actually make a difference. (I suspect such models are rare to begin with).

I'd vote to close this, saying that the fract does not seem to make sense (and is also not used in the sample GLSL code of the specification). So I assume that this was a bug in earlier CesiumJS version, but has been fixed with the "Model" rewrite that happened in 1.97.

Thank you! In fact, The first time I found it was that in Esri I3S uvRegion, which contains the concept of uvRegion. Certainly, It's different from"KHR_texture_transorm", I tried converting it into glTF by adding a custom glTF extesion in Cesium to adapt "uvRegion", but the same to you, I'm not sure the meaning of this detail, cause of CesiumJS 1.96, 1.97 and "uvRegion" in I3S.

PrimaryFantasty commented 3 weeks ago

image @javagl I made a typo, sry~