KhronosGroup/glTF-Sample-Models

BoxTextured and BoxTexturedNonPowerOfTwo have different texture brightness

hybridherbst opened this issue · 8 comments

hybridherbst commented

The screenshots of both models suggest that they are supposed to be identical, and also the textures seem to match:
https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/BoxTexturedNonPowerOfTwo/glTF/CesiumLogoFlat.png
and
https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/BoxTextured/glTF/CesiumLogoFlat.png.

  • Viewing the two files in any viewer shows that they have different brightness.
  • When I view both files, the textures displayed in the texture view gltf.report seem to match.
  • However, when viewing the individual textures in e.g. Squoosh, they differ.

All viewers I tested display these models with separate colors:

image
BoxTextured | BoxTexturedNonPowerOfTwo. Note the right is brighter.

My assumption is that at some point during image resizing, some software messed with the colors... now, from #137 it looks like the non-power-of-two version actually came first.

On further checking, it turns out that while the non-power-of-two PNG doesn't contain a color profile, the power-of-two version contains one. I'm not sure but I don't think any viewer respects color profiles in PNG? That would explain why the textures look identical in gltf.report's texture viewer but not the viewport - the texture viewer uses HTML img which respects the profiles...

Tested to look different in model-viewer, babylon, glTFast, UnityGLTF, Gestaltor, glTF Sample Viewer, Squoosh (textures only)

cc @donmccurdy for a number of reasons (color profiles, gltf.report)

javagl commented

Just as a pointer, https://www.khronos.org/registry/glTF/specs/2.0/glTF-2.0.html#file-extensions-and-media-types says

PNG images SHOULD NOT contain animations, non-square pixel ratios, or embedded ICC profiles. Such features, if present, MUST be ignored by client implementations.

So iff (if and only if) the profile is the only difference, then this MUST be ignored by the client, and thus, any visual difference could be considered as a bug in the viewer.

hybridherbst commented

Thanks for the pointer! I think it's the other way around though: the textures do have a visual difference when only the raw bytes are considered and the ICC profile is not applied.

The good thing: that means all viewers do the right thing (besides maybe the peculiarity that gltf.report shows the textures separately with ICC profile applied).

It also means that the bug here is that https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/BoxTextured/glTF/CesiumLogoFlat.png has an ICC profile which is neither explained in the description nor I think intentional; there could be a separate file that explicitly contains an ICC profile in the PNG to test this.

donmccurdy commented

Correct, gltf.report uses HTML <img/> tags when displaying images in the texture browser tab.

At one point I added a ktxfix command (donmccurdy/glTF-Transform#222) to gltf-transform to correct color profiles in KTX2 images, I believe this was @lexaknyazev's suggestion. Early implementations had been writing some color profiles incorrectly at the time. I wonder if something similar would be useful for PNG/JPEG/WEBP, and whether it's easy to correct that metadata without external dependencies?

hybridherbst commented

With "correct the metadata", do you just mean "remove the color profile" (which would be incorrect I believe) or "recalculate the correct pixel values and remove the color profile"?

donmccurdy commented

Ah yeah, not rewriting any pixels. Maybe infer the correct color profile from the usage in the asset (e.g. baseColorTexture vs normalTexure) and update the color profile accordingly. gltf-transform can do that for color primaries in KTX2 currently.

hybridherbst commented

Understood. I think that makes sense if the spec says ICC profiles should not be included, but it needs a big fat warning somewhere that textures will look different than expected. I haven't looked into toktx yet and if that respectes ICC profiles when turning PNG into KTX2.

For example, "fixing" the profiles in the BoxTextured.glb by just removing them will result in an incorrect (too bright) result still.

lexaknyazev commented

In any case, glTF viewers must ignore any embedded colorspace metadata. The TextureEncodingTest model covers that.

The image used for the BoxTexturedNonPowerOfTwo model has no color metadata and is treated as Rec.709 / sRGB by default.

The image used for the BoxTextured model is problematic. I'll follow up with more details soon.

  • 👍2
lexaknyazev commented

The image from the BoxTextured model contains a custom ICC profile with a gamma value of 1.8. Additionally, it contains unnecessary XMP metadata with the following content:

<x:xmpmeta xmlns:x="adobe:ns:meta/" x:xmptk="XMP Core 5.4.0">
   <rdf:RDF xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
      <rdf:Description rdf:about=""
            xmlns:tiff="http://ns.adobe.com/tiff/1.0/">
         <tiff:Orientation>1</tiff:Orientation>
      </rdf:Description>
   </rdf:RDF>
</x:xmpmeta>