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

web_platform: copyExternalImageToTexture with more formats, extended-srgb and other color spaces #913

Closed
kainino0x opened this issue Jan 13, 2022 · 2 comments
Assignees

Comments

@kainino0x
Copy link
Collaborator

kainino0x commented Jan 13, 2022

https://crbug.com/dawn/1138 adds new formats to copyExternalImageToTexture.

Some of the new formats are float, which means they go outside [0,1]. Color conversion behavior is now specified (colorspaces like srgb are extended). Find a way to test that no clamping occurs for float formats, e.g. upload a Display P3 source image - if possible.


Here's the single minimal test we would need in order to make sure we're implementing this correctly. It's quite simple. This is what we need to start implementing with some confidence.

  • Create a Display P3 (1,0,0,1) image source. This is a red that's outside the srgb range.
    It probably has to be a PNG because it's going to have the best browser compatibility and least room for extra bugs.
  • Use copyExternalImageToTexture to put it in an r(gba)16float or r(gba)32float texture with the default options.
  • Test that the red channel of that texture is > 1.

Here are some test images:
https://bugs.chromium.org/p/dawn/issues/detail?id=1139#c8
I took the WebKit test image and cropped it to 1x1 in GIMP.
Source: https://webkit.org/blog-files/color-gamut/

Verified they display as expected on my MacBook (displayp3 is brighter than the other two).

We can test all three of these. The displayp3 red should be > 1, the others should be == 1.

shaoboyan added a commit to shaoboyan/cts that referenced this issue Mar 7, 2022
This PR add cts to test copyExternalImageToTexture() color space
conversion ability.
It creates canvas with color space attr and copy to dst texture in
user defined color space through CopyExternalImageToTexture().

Issue:gpuweb#913
kainino0x pushed a commit that referenced this issue Mar 15, 2022
* CTS: Color space conversion operation test for CopyToTexture

This PR add cts to test copyExternalImageToTexture() color space
conversion ability.
It creates canvas with color space attr and copy to dst texture in
user defined color space through CopyExternalImageToTexture().

Issue:#913

* Update

* Address comments

* Apply suggestions from code review

* Fix F16 format slow running issue
@shaoboyan
Copy link
Contributor

This issue is fixed by above PRs

@kainino0x
Copy link
Collaborator Author

Thanks for the work on this! When closing an issue, if verifying it's complete please change the status in the project tracker to "Complete" (you can do this in the right-hand sidebar on the github issue).

Though, I noticed we never actually added any test image files to the CTS, so I guess we are only testing 2d canvas sources. I think we still need to expand these to more sources (image files, and ImageData with colorSpace), so I filed #1476 for that.

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

No branches or pull requests

2 participants