search

KhronosGroup / glTF-Sample-Assets

To store all models and other assets related to glTF
256 stars 30 forks source link

Changes to Khronos-TextureTransformMultiTest? #41

Open bhouston opened 8 months ago

bhouston commented 8 months ago

I noticed that the render fidelity test framework produces different results for Khronos-textureTransformMultiTest when one uses the model in this repo as compared to the one if the previous glTF-SampleModels repo?

The issue is with clear coat normals.

On the left is Three.js render of glTF-Sample-Models, and the right is using it from glTF-SampleAssets:

Screenshot 2023-10-25 at 9 37 15 AM

I also noticed that the Filament engine has similar issues (glTF-Sample-Model on the left, Sample-Aseets on the right):

Screenshot 2023-10-25 at 9 43 41 AM

Other render engines (Babylon.js, gltf-model-viewer) do not have differences.

I figure it must be because of normal reference frame code?

Did someone change the Khronos-TextureTransformMultiTest model in the migration to this repo?

I first noticed the issue here: https://github.com/google/model-viewer/pull/4534#issuecomment-1779299328

emackey commented 7 months ago

This is my model, but I'm not the one who changed it between repos.

A quick binary compare of old-vs-new shows that the TestMap_Normal.png file has changed, and in particular the old one falsely claimed to be sRGB colorspace in the header, and the new one does not specify that. This is likely a correction to the model.

Note however, this should make no difference to your renders! The glTF spec requires that colorspace information from PNG files be ignored, in favor of taking the colorspace as directed by glTF spec itself (which would be linear for a non-color image like this one). WebGL 1.0 may not be able to abide by this rule, but newer rendering systems should be capable.

See for example: TextureLinearInterpolationTest

So, my guess is that ThreeJS and Filament are not respecting glTF's choice of colorspace, creating this diff, but BabylonJS and the Khronos Sample Viewer are handling it correctly (with no diff).

lexaknyazev commented 7 months ago

WebGL 1.0 may not be able to abide by this rule, but newer rendering systems should be capable.

Both WebGL versions support setting this state via gl.pixelStorei(gl.UNPACK_COLORSPACE_CONVERSION_WEBGL, gl. NONE) command.

lexaknyazev commented 7 months ago

The root cause here is that this repo uses an old version of this asset. The normal map has been updated in https://github.com/KhronosGroup/glTF-Sample-Models/pull/365 and https://github.com/KhronosGroup/glTF-Sample-Models/pull/367.