Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Images used by textures with different color spaces? #2308

Open
donmccurdy opened this issue Aug 12, 2023 · 6 comments
Open

Images used by textures with different color spaces? #2308

donmccurdy opened this issue Aug 12, 2023 · 6 comments

Comments

@donmccurdy
Copy link
Contributor

The spec discusses image color spaces under Images, Section 3.8.3:

Effective transfer function (encoding) is defined by a glTF object that refers to the image (in most cases it’s a texture that is used by a material).

I believe this implies that the same texture object cannot be associated with multiple color spaces, and so cannot be used both as a baseColorTexture and occlusionTexture. Would it be valid for the same image object to be used by two textures in those material slots?

I've run into a question in glTF Transform, where it merges two identical textures, resulting in a color space difference in three.js (which expects 1 color space per glTF texture), and I'm not sure which is correct.

@Samsy
Copy link

Samsy commented Aug 14, 2023

+1

@bghgary
Copy link
Contributor

bghgary commented Aug 14, 2023

Would it be valid for the same image object to be used by two textures in those material slots?

Doing this is probably unusual, but I think it is valid. @lexaknyazev What do you think? For reference, if you load the two models in the referenced glTF-Transform issue in Babylon.js sandbox, they look the same before and after.

image

@julienduroure
Copy link
Contributor

Hello,
I got this case in a report last year:
KhronosGroup/glTF-Blender-IO#1800

@Samsy
Copy link

Samsy commented Aug 15, 2023

Would it be valid for the same image object to be used by two textures in those material slots?

Doing this is probably unusual, but I think it is valid. @lexaknyazev What do you think? For reference, if you load the two models in the referenced glTF-Transform issue in Babylon.js sandbox, they look the same before and after.

image

I just checked Babylon viewer and indeed they look the same,
I inspected with spector, babylon viewer isn't using sRGB color spacing which was the problem originally, this may be why they both look the same on babylon

@bghgary
Copy link
Contributor

bghgary commented Aug 15, 2023

babylon viewer isn't using sRGB color spacing which was the problem originally

I think you mean the sandbox doesn't use EXT_sRGB formats. If so, yes, by default, we have this turned off because of a Chromium issue. If you use https://3dcommerce.babylonjs.com, it will use EXT_sRGB formats. The result is the same though.

@donmccurdy
Copy link
Contributor Author

donmccurdy commented Aug 31, 2023

FWIW, three.js has moved to use texStorage2D which avoids the Chromium performance issues. We do still see similar performance issues on video textures, and use inline sRGB decoding in the shader to handle that instead.

While investigating support for wide gamut colorspaces in WebGL and three.js, I've encountered an issue that might be relevant here. Images are unpacked by WebGL into a target color space identified by gl.unpackColorSpace. The web browser infers the source color space of an image from its ICC profile, and uses that to make the required conversion into the unpack color space. While the glTF Spec advises setting the "UNPACK_COLORSPACE_CONVERSION_WEBGL flag to NONE," we can no longer do that in scenes whose textures use more than one set of color primaries, or where image primaries do not match the primaries of the working/rendering color space, without re-implementing the entire image decoding pipeline ourselves (and we aren't going to do that).

As a result — I think the glTF specification's current advice about ignoring ICC Profiles is still reasonable, however, I do not believe we should be encouraging situations where the embedded ICC Profile cannot possibly be accurate, as would be the case when the image contains mixed data from different color spaces.

EDIT: Perhaps this is something the validator should try to warn about?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants