Adds Layout Tests for JPEG and PNG with ICC color profile
Created attachment 143920 [details] Patch
Attachment 143920 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 WARNING: skia test_expectations overrides file '/mnt/git/webkit-style-queue/Source/WebKit/chromium/skia/skia_test_expectations.txt' does not exist WARNING: chromium test_expectations overrides file '/mnt/git/webkit-style-queue/Source/WebKit/chromium/webkit/tools/layout_tests/test_expectations.txt' does not exist LayoutTests/platform/chromium/test_expectations.txt:3699: Path does not exist. http/tests/media/media-source/webm/video-media-source-play.html [test/expectations] [5] LayoutTests/platform/chromium/test_expectations.txt:3700: Path does not exist. http/tests/media/media-source/webm/video-media-source-seek.html [test/expectations] [5] LayoutTests/platform/chromium/test_expectations.txt:3701: Path does not exist. http/tests/media/media-source/webm/video-media-source-state-changes.html [test/expectations] [5] LayoutTests/platform/chromium/test_expectations.txt:3702: Path does not exist. http/tests/media/media-source/webm/video-media-source-state-changes.html [test/expectations] [5] Total errors found: 4 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 143925 [details] Patch
Attachment 143925 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 WARNING: skia test_expectations overrides file '/mnt/git/webkit-style-queue/Source/WebKit/chromium/skia/skia_test_expectations.txt' does not exist WARNING: chromium test_expectations overrides file '/mnt/git/webkit-style-queue/Source/WebKit/chromium/webkit/tools/layout_tests/test_expectations.txt' does not exist LayoutTests/platform/chromium/test_expectations.txt:3699: Path does not exist. http/tests/media/media-source/webm/video-media-source-play.html [test/expectations] [5] LayoutTests/platform/chromium/test_expectations.txt:3700: Path does not exist. http/tests/media/media-source/webm/video-media-source-seek.html [test/expectations] [5] LayoutTests/platform/chromium/test_expectations.txt:3701: Path does not exist. http/tests/media/media-source/webm/video-media-source-state-changes.html [test/expectations] [5] LayoutTests/platform/chromium/test_expectations.txt:3702: Path does not exist. http/tests/media/media-source/webm/video-media-source-state-changes.html [test/expectations] [5] Total errors found: 4 in 7 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 143925 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143925&action=review > LayoutTests/ChangeLog:10 > + for a non0identity transform. "non0identity"? > LayoutTests/ChangeLog:12 > + * fast/images/jpeg-with-icc-profile-expected.txt: Added. Sorry for this, could you change "icc-profile" -> "color-profile" in all these file names? For example, red-at-12-oclock-with-color-profile.jpg, etc. > LayoutTests/fast/images/jpeg-with-icc-profile.html:1 > +<!DOCTYPE html> Add the following line here: The red quadrant of the image should be in the 12 o'clock position.<p> > LayoutTests/fast/images/jpeg-with-icc-profile.html:2 > +<img src="resources/red-at-12-oclock-with-icc-profile.jpg" width="800px" height="800px"> DRT has an 800 x 600 screen size during layout tests. Would be nice to make the test result fit in that screen without scroll bars. Try the following: <img src="resources/red-at-12-oclock-with-icc-profile.jpg" width="50%"> > LayoutTests/fast/images/png-with-icc-profile.html:1 > +<!DOCTYPE html> Add the following line here: The red quadrant of the image should be in the 12 o'clock position.<p> > LayoutTests/fast/images/png-with-icc-profile.html:2 > +<img src="resources/red-at-12-oclock-with-icc-profile.png" width="800px" height="800px"> Avoid the scroll bars, use: <img src="resources/red-at-12-oclock-with-icc-profile.png" width="50%"> > LayoutTests/platform/chromium/test_expectations.txt:1164 > +// Implement color profile support. The linux EWS will complain here and won't submit your change due to the test regression I believe (and yes it sucks). Tell the EWS via test_expectations that we'll rebaseline this result in future. Here's the current way BUGCR143 : fast/images/{test-name} = MISSING And the expected result is LayoutTests/platform/chromium-linux/fast/images/png-with-icc-profile-expected.png How will the other chromium ports use this as their expected result? Answer: they won't and their bots will go red on submit :(
Created attachment 143961 [details] Patch
Attachment 143961 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 WARNING: skia test_expectations overrides file '/mnt/git/webkit-style-queue/Source/WebKit/chromium/skia/skia_test_expectations.txt' does not exist WARNING: chromium test_expectations overrides file '/mnt/git/webkit-style-queue/Source/WebKit/chromium/webkit/tools/layout_tests/test_expectations.txt' does not exist LayoutTests/platform/chromium/test_expectations.txt:1165: Path does not exist. fast/images/jpeg-with-icc-profile.html [test/expectations] [5] LayoutTests/platform/chromium/test_expectations.txt:1166: Path does not exist. fast/images/png-with-icc-profile.html [test/expectations] [5] Total errors found: 2 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
(In reply to comment #5) > (From update of attachment 143925 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=143925&action=review > > > LayoutTests/ChangeLog:10 > > + for a non0identity transform. > > "non0identity"? Fixed > > > LayoutTests/ChangeLog:12 > > + * fast/images/jpeg-with-icc-profile-expected.txt: Added. > > Sorry for this, could you change "icc-profile" -> "color-profile" in all these file names? For example, red-at-12-oclock-with-color-profile.jpg, etc. > Done > > LayoutTests/fast/images/jpeg-with-icc-profile.html:1 > > +<!DOCTYPE html> > > Add the following line here: > > The red quadrant of the image should be in the 12 o'clock position.<p> > Done > > LayoutTests/fast/images/jpeg-with-icc-profile.html:2 > > +<img src="resources/red-at-12-oclock-with-icc-profile.jpg" width="800px" height="800px"> > > DRT has an 800 x 600 screen size during layout tests. Would be nice to make the test result fit in that screen without scroll bars. Try the following: > > <img src="resources/red-at-12-oclock-with-icc-profile.jpg" width="50%"> Went with 400px, 400px > > LayoutTests/fast/images/png-with-icc-profile.html:1 > > +<!DOCTYPE html> > > Add the following line here: > > The red quadrant of the image should be in the 12 o'clock position.<p> > done > > LayoutTests/fast/images/png-with-icc-profile.html:2 > > +<img src="resources/red-at-12-oclock-with-icc-profile.png" width="800px" height="800px"> > > Avoid the scroll bars, use: > > <img src="resources/red-at-12-oclock-with-icc-profile.png" width="50%"> > Fixed > > LayoutTests/platform/chromium/test_expectations.txt:1164 > > +// Implement color profile support. > > The linux EWS will complain here and won't submit your change due to the test regression I believe (and yes it sucks). Tell the EWS via test_expectations that we'll rebaseline this result in future. Here's the current way > > BUGCR143 : fast/images/{test-name} = MISSING > Done > And the expected result is LayoutTests/platform/chromium-linux/fast/images/png-with-icc-profile-expected.png > Removed > How will the other chromium ports use this as their expected result? Answer: they won't and their bots will go red on submit :(
Created attachment 143964 [details] Patch
Thanks Tony. A discussed, I'll generate the mac port test result, and update ChangeLog, etc.
Created attachment 143984 [details] Patch
Comment on attachment 143984 [details] Patch tamura-san: Tony's patch looks fine to me, rubber stamp please?
Comment on attachment 143984 [details] Patch ok
Comment on attachment 143984 [details] Patch Clearing flags on attachment: 143984 Committed r118487: <http://trac.webkit.org/changeset/118487>
All reviewed patches have been landed. Closing bug.
Comment on attachment 143984 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143984&action=review > LayoutTests/fast/images/jpeg-with-color-profile.html:2 > +<img src="resources/red-at-12-oclock-with-color-profile.jpg" width="50%"> You changed the test file to use width="50%" which is really not what you want. That's 50%, not of the image size, but of the page size. Page size changes from platform to platform depending on the window manager's chrome. I highly recommend putting that back to "400px" which will make the image size be stable across platforms > LayoutTests/fast/images/png-with-color-profile.html:2 > +<img src="resources/red-at-12-oclock-with-color-profile.png" width="50%"> You changed the test file to use width="50%" which is really not what you want. That's 50%, not of the image size, but of the page size. Page size changes from platform to platform depending on the window manager's chrome. I highly recommend putting that back to "400px" which will make the image size be stable across platforms
This bug is closed. File new bugs if you have concerns. Note that webkit layout tests snapshot 800 x 600 px regardless of platform.