KhronosGroup / glTF-Sample-Assets

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

metallicRoughnessTexture encoding is wrong #84

Open helviett opened 6 months ago

helviett commented 6 months ago

The spec says:

The textures for metalness and roughness properties are packed together in a single texture called metallicRoughnessTexture. Its green channel contains roughness values and its blue channel contains metalness values. This texture MUST be encoded with linear transfer function and MAY use more than 8 bits per channel.

But metallic-roughness textures in Sponza folder are encoded with sRGB. Checked it with ImageMagick and PVRTexTool.

javagl commented 6 months ago

Depending on whether someone can definitely say whether this is duplicate of the issue around https://github.com/KhronosGroup/glTF-Sample-Models/issues/316#issuecomment-860333473 , it might be closed. (I think it is, but others should confirm...)

donmccurdy commented 6 months ago

tl;dr – the ICC profiles are often wrong, the data is likely not sRGB. Agreed it'd be nice if we can fix the ICC profiles, that's a bit of a messy area in photo-editing software historically IMHO. Implementations of glTF are required to ignore the profiles for that reason.

I'd be open to putting something in glTF-Transform to fix incorrect ICC profiles too though, it'd be ideal to have these samples be clean unless there're explicitly testing that requirement. The library I'm currently using (sharp.js) does not let me edit the ICC profile without re-encoding the image, unfortunately.

DRx3D commented 6 months ago

This repo is being ARCHIVED. Transferred to glTF-Sample-Assets.

donmccurdy commented 2 months ago

I've looked into this more, and haven't yet found a clear answer as to how one should tag a particular image as "non-color data". With KTX2 textures there's a clear way to do this. With ICC profiles and EXIF, I have no idea. See discussion in:

https://github.com/lovell/sharp/issues/4011

I imagine this is all part of the reason the spec requires implementations to ignore any ICC profile present. Anything else would require some external convention on how to represent non-color data in images, and that convention does not appear to exist today. 😕