87437 – Add tests for JPEG and PNG images with a non-generic RGB color profile

RESOLVED FIXED 87437

Add tests for JPEG and PNG images with a non-generic RGB color profile

https://bugs.webkit.org/show_bug.cgi?id=87437

Summary Add tests for JPEG and PNG images with a non-generic RGB color profile

Tony Payne

Reported 2012-05-24 16:27:24 PDT

Adds Layout Tests for JPEG and PNG with ICC color profile

Attachments
Patch (907.24 KB, patch) 2012-05-24 16:29 PDT, Tony Payne	no flags	Details Formatted Diff Diff
Patch (907.19 KB, patch) 2012-05-24 16:47 PDT, Tony Payne	no flags	Details Formatted Diff Diff
Patch (502.18 KB, patch) 2012-05-24 20:56 PDT, Tony Payne	no flags	Details Formatted Diff Diff
Patch (501.89 KB, patch) 2012-05-24 21:32 PDT, Tony Payne	no flags	Details Formatted Diff Diff
Patch (740.43 KB, patch) 2012-05-24 23:16 PDT, noel gordon	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Tony Payne

Comment 1 2012-05-24 16:29:47 PDT

Created attachment 143920 [details] Patch

WebKit Review Bot

Comment 2 2012-05-24 16:34:16 PDT

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.

Tony Payne

Comment 3 2012-05-24 16:47:02 PDT

Created attachment 143925 [details] Patch

WebKit Review Bot

Comment 4 2012-05-24 16:49:23 PDT

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.

noel gordon

Comment 5 2012-05-24 18:33:46 PDT

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 :(

Tony Payne

Comment 6 2012-05-24 20:56:29 PDT

Created attachment 143961 [details] Patch

WebKit Review Bot

Comment 7 2012-05-24 20:58:39 PDT

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.

Tony Payne

Comment 8 2012-05-24 21:00:19 PDT

(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 :(

Tony Payne

Comment 9 2012-05-24 21:32:23 PDT

Created attachment 143964 [details] Patch

noel gordon

Comment 10 2012-05-24 23:10:41 PDT

Thanks Tony. A discussed, I'll generate the mac port test result, and update ChangeLog, etc.

noel gordon

Comment 11 2012-05-24 23:16:23 PDT

Created attachment 143984 [details] Patch

noel gordon

Comment 12 2012-05-24 23:25:31 PDT

Comment on attachment 143984 [details] Patch tamura-san: Tony's patch looks fine to me, rubber stamp please?

Kent Tamura

Comment 13 2012-05-24 23:29:25 PDT

Comment on attachment 143984 [details] Patch ok

WebKit Review Bot

Comment 14 2012-05-25 00:26:50 PDT

Comment on attachment 143984 [details] Patch Clearing flags on attachment: 143984 Committed r118487: <http://trac.webkit.org/changeset/118487>

WebKit Review Bot

Comment 15 2012-05-25 00:26:55 PDT

All reviewed patches have been landed. Closing bug.

Tony Payne

Comment 16 2012-05-25 15:41:04 PDT

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

noel gordon

Comment 17 2012-05-25 22:30:09 PDT

This bug is closed. File new bugs if you have concerns. Note that webkit layout tests snapshot 800 x 600 px regardless of platform.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component New Bugs

Assignee

Tony Payne

Reported

2012-05-24 16:27 PDT

Modified

2012-05-25 22:30 PDT History

CC List

3 users Show

URL

Keywords

Depends on

Blocks

87434

Dependencies

tree graph