-
Notifications
You must be signed in to change notification settings - Fork 685
Test JPEG with linear gamma in image_bitmap_from_blob tests. #3100
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
Test JPEG with linear gamma in image_bitmap_from_blob tests. #3100
Conversation
Restructure the tests to use the Fetch API and Promises to make this addition easier to read. Make tolerance configurable, and use a large value for compatibility with Firefox Nightly on macOS. The new JPEG resource was exported from the included Photoshop document which was constructed from scratch for this test case, based on the original report from Figma. Regression test for http://crbug.com/1058160 .
|
All CC'd - please review. Thanks. This change has been tested locally on macOS with Safari Technology Preview, Firefox, and Chromium with the needed bug fix.
|
shrekshao
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for the change
|
I don't think a JPEG file with linear gamma is valid. I hope browser vendors aren't trying to encourage this. |
Can you provide more details on why this is invalid? A JPEG with an embedded color profile can have the gamma adjusted to 1.0. Figma receives files like this on an ongoing basis. Not trying to encourage invalid behavior, but a regression test for this is absolutely needed. |
|
Which file format are we talking about? I was thinking JFIF in which the color space is required to be BT.601 which has a non-linear transfer function. If you are talking about EXIF, although the default color space is sRGB, complete color space information can be provided including a transfer function which is specified as a table. I got permission denied when I tried to look at the bug this PR is a regression test for. |
|
@MarkCallow |
|
The Chromium bug was automatically marked restricted view because it's a crash report. It's not a security bug though. You can provoke the same crash by patching this PR into your checkout of the WebGL conformance suite and running any of the conformance[2]/textures/image_bitmap_from_blob/ tests in Chromium. I don't know the details of the JPEG file which I exported from Photoshop - please feel free to examine the one in this PR. To create it, I defined a custom color profile, set the gamma to 1.0, and exported the file as a JPEG with an embedded color profile. The original PSD file is included. |
|
I found how the profile is being embedded. Profiles are indeed not part of JFIF. However the ICC spec. in Annex 4, Section B.4 describes how to use JPEG Interchange Format (note the missing "F") APP2 markers to store an ICC profile. .jpg files with profiles are valid JPEG Interchange Format files and valid in some subset of the family of JPEG Interchange Format files, not including the JFIF format. |
sdk/tests/js/tests/tex-image-and-sub-image-2d-with-image-bitmap-from-blob.js
Outdated
Show resolved
Hide resolved
sdk/tests/js/tests/tex-image-and-sub-image-with-image-bitmap-utils.js
Outdated
Show resolved
Hide resolved
|
Thanks for your reviews. Merging now. |
Restructure the tests to use the Fetch API and Promises to make this
addition easier to read.
Make tolerance configurable, and use a large value for compatibility
with Firefox Nightly on macOS.
The new JPEG resource was exported from the included Photoshop
document which was constructed from scratch for this test case, based
on the original report from Figma.
Regression test for http://crbug.com/1058160 .