Summary: | WebKit should default to using sRGB with NSColor conversion instead of device color space | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Beth Dakin <bdakin> | ||||||||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | bdakin, bfulgham, buildbot, dino, rniwa, simon.fraser, thorton, zalan | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Beth Dakin
2017-05-05 13:16:04 PDT
Created attachment 309201 [details]
Patch
Comment on attachment 309201 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309201&action=review > Source/WebCore/platform/graphics/mac/ColorMac.mm:64 > + if (color.colorSpace == [NSColorSpace sRGBColorSpace]) Please use isEqual. Comment on attachment 309201 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309201&action=review > Source/WebCore/platform/graphics/mac/ColorMac.mm:64 > + if (color.colorSpace == [NSColorSpace sRGBColorSpace]) This should use isEqual(To?): right? > Source/WebCore/platform/graphics/mac/ColorMac.mm:67 > + rgbColor = [color colorUsingColorSpaceName:NSDeviceRGBColorSpace]; Is this equivalent to NSCalibratedRGBColorSpace and which is sRGB? (In reply to Simon Fraser (smfr) from comment #3) > Comment on attachment 309201 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=309201&action=review > > > Source/WebCore/platform/graphics/mac/ColorMac.mm:64 > > + if (color.colorSpace == [NSColorSpace sRGBColorSpace]) > > This should use isEqual(To?): right? > Yup, fixing. > > Source/WebCore/platform/graphics/mac/ColorMac.mm:67 > > + rgbColor = [color colorUsingColorSpaceName:NSDeviceRGBColorSpace]; > > Is this equivalent to NSCalibratedRGBColorSpace and which is sRGB? I'm not sure. This is what we have always done in this function before this patch. Comment on attachment 309201 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=309201&action=review >>> Source/WebCore/platform/graphics/mac/ColorMac.mm:67 >>> + rgbColor = [color colorUsingColorSpaceName:NSDeviceRGBColorSpace]; >> >> Is this equivalent to NSCalibratedRGBColorSpace and which is sRGB? > > I'm not sure. This is what we have always done in this function before this patch. Let's use colorUsingColorSpace:[NSColorSpace sRGBColorSpace] Created attachment 309206 [details]
Patch
Comment on attachment 309206 [details] Patch Attachment 309206 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3681199 New failing tests: editing/mac/attributed-string/comment-cdata-section.html editing/mac/attributed-string/font-style-variant-effect.html editing/mac/attributed-string/font-weight.html editing/mac/attributed-string/letter-spacing.html editing/mac/attributed-string/vertical-align.html editing/mac/attributed-string/text-decorations.html editing/mac/attributed-string/font-size.html editing/mac/attributed-string/anchor-element.html editing/mac/attributed-string/basic.html Created attachment 309221 [details]
Archive of layout-test-results from ews100 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
I think those can be safely rebaselined. Created attachment 309236 [details]
Patch with new test results
For the bots to try.
Comment on attachment 309236 [details] Patch with new test results Attachment 309236 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/3681987 New failing tests: editing/mac/attributed-string/font-style-variant-effect.html Created attachment 309246 [details]
Archive of layout-test-results from ews103 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-elcapitan Platform: Mac OS X 10.11.6
|