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

glTF fails texture encoding test #144

Closed
andreasplesch opened this issue Aug 18, 2023 · 6 comments
Closed

glTF fails texture encoding test #144

andreasplesch opened this issue Aug 18, 2023 · 6 comments
Labels
confirmed The issue can be reproduced enhancement New feature or request fixed The issue is already fixed

Comments

@andreasplesch
Copy link
Contributor

https://create3000.github.io/x_ite/laboratory/gltf-sample-viewer renders the Texture Encoding Test as:

image

but the reference image at https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/TextureEncodingTest is:

image

x3dom has the same issue x3dom/x3dom#1258.

@create3000
Copy link
Owner

I was a bit wrong, because Safari always does gl.UNPACK_COLORSPACE_CONVERSION_WEBGL with gl.NONE, which is what has to be done to get the right rendering behavior.

@create3000
Copy link
Owner

create3000 commented Aug 18, 2023

glTF ignores ICC color profiles and custom gamma in images, but not X3D.

images SHOULD NOT contain ..., or embedded ICC profiles. Such features, if present, MUST be ignored by client implementations.

Here is a example image with custom ICC color profile:

https://create3000.github.io/x_ite/playground/?url=https://rawgit.com/create3000/Library/main/Tests/Components/Texturing/ColorManagement.x3d.

https://andreasplesch.github.io/Library/Viewer/index.html?url=https://rawgit.com/create3000/Library/main/Tests/Components/Texturing/ColorManagement.x3d.

Either we should always do gl.UNPACK_COLORSPACE_CONVERSION_WEBGL with gl.NONE (what Safari always does), or ImageTexture should have a new field colorspaceConversion (think this is the right capitalization).

@create3000
Copy link
Owner

create3000 commented Aug 18, 2023

@andreasplesch
Copy link
Contributor Author

Thank you for the investigation. Probably time to ask on x3d-public for guidance. I can do that.

@andreasplesch
Copy link
Contributor Author

Let's see if there is a response on x3d-public.

I remembered why I did not focus on that at the time. It is clear that the test glTF asset in the issue and the image used in the comments have unrealistic, extreme ICC profiles which is not a fair representation. For most textures the problem would be subtle. So this is not an urgent problem. Also, authors can always decide to use images without embedded profiles.

But there is a problem if pixel accurate glTF rendering with exact colors is a goal.

@create3000 create3000 added enhancement New feature or request confirmed The issue can be reproduced labels Aug 19, 2023
@andreasplesch
Copy link
Contributor Author

Castle renders the test correctly:

image

It converted the gltf to this x3d:
textureencodingtest.zip

The x3d has the textures converted to giant PixelTextures without the ICC applied.

So that could be an option but it is not practical for larger images.

@create3000 create3000 added the fixed The issue is already fixed label Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed The issue can be reproduced enhancement New feature or request fixed The issue is already fixed
Projects
None yet
Development

No branches or pull requests

2 participants