Consolidate NSColor to WebCore::Color conversion and fix system colors.
authortimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Mar 2018 02:39:35 +0000 (02:39 +0000)
committertimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 29 Mar 2018 02:39:35 +0000 (02:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184096
rdar://problem/38918925

Reviewed by Tim Horton.

Source/WebCore:

* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(CreateCGColorIfDifferent): Use CGColor property on NSColor, don't manually create new CGColor.
* platform/graphics/mac/ColorMac.h:
* platform/graphics/mac/ColorMac.mm:
(WebCore::makeRGBAFromNSColor): Move pattern code from RenderThemeMac's convertNSColorToColor.
Also use nextafter for proper RGBA float conversion.
* platform/mac/PlatformPasteboardMac.mm:
(WebCore::PlatformPasteboard::color): Use colorFromNSColor.
* rendering/RenderThemeMac.mm:
(WebCore::RenderThemeMac::platformActiveSelectionBackgroundColor const): Use colorFromNSColor.
(WebCore::RenderThemeMac::platformInactiveSelectionBackgroundColor const): Ditto.
(WebCore::RenderThemeMac::platformActiveListBoxSelectionBackgroundColor const): Ditto.
(WebCore::RenderThemeMac::systemColor const): Ditto.
(WebCore::paintAttachmentTitleBackground): Ditto.
(WebCore::convertNSColorToColor): Deleted.

LayoutTests:

* fast/css/apple-system-control-colors-expected.txt: Updated with rgba() colors.
* platform/mac/accessibility/content-editable-as-textarea-expected.txt: Updated with color space.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@230064 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/css/apple-system-control-colors-expected.txt
LayoutTests/platform/mac/accessibility/content-editable-as-textarea-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
Source/WebCore/platform/graphics/mac/ColorMac.h
Source/WebCore/platform/graphics/mac/ColorMac.mm
Source/WebCore/platform/mac/PlatformPasteboardMac.mm
Source/WebCore/rendering/RenderThemeMac.mm

index 23ec1c4..d051557 100644 (file)
@@ -1,3 +1,15 @@
+2018-03-28  Timothy Hatcher  <timothy@apple.com>
+
+        Consolidate NSColor to WebCore::Color conversion and fix system colors.
+
+        https://bugs.webkit.org/show_bug.cgi?id=184096
+        rdar://problem/38918925
+
+        Reviewed by Tim Horton.
+
+        * fast/css/apple-system-control-colors-expected.txt: Updated with rgba() colors.
+        * platform/mac/accessibility/content-editable-as-textarea-expected.txt: Updated with color space.
+
 2018-03-28  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         [iOS] Multiple select appearance doesn't update when selecting or deselecting rows in the picker view
index 492e8a5..6062db0 100644 (file)
@@ -1,9 +1,9 @@
--apple-system-header-text : rgb(0, 0, 0)
+-apple-system-header-text : rgba(0, 0, 0, 0.85098)
 -apple-system-text-background : rgb(255, 255, 255)
 -apple-system-alternate-selected-text : rgb(255, 255, 255)
--apple-system-label : rgb(0, 0, 0)
--apple-system-secondary-label : rgb(0, 0, 0)
--apple-system-tertiary-label : rgb(0, 0, 0)
--apple-system-quaternary-label : rgb(0, 0, 0)
+-apple-system-label : rgba(0, 0, 0, 0.85098)
+-apple-system-secondary-label : rgba(0, 0, 0, 0.498039)
+-apple-system-tertiary-label : rgba(0, 0, 0, 0.247059)
+-apple-system-quaternary-label : rgba(0, 0, 0, 0.0980392)
 -apple-system-grid : rgb(204, 204, 204)
-current-color with inherited -apple-system-label : rgb(0, 0, 0)
+current-color with inherited -apple-system-label : rgba(0, 0, 0, 0.85098)
index f1481ef..0ac70be 100644 (file)
@@ -12,23 +12,23 @@ String with range: ello
 worl
 Attributed string with range: ello
 {
-    AXBackgroundColor = " [ (kCGColorSpaceDeviceRGB)] ( 0 0 0 0 )";
+    AXBackgroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 0 )";
     AXFont =     {
         AXFontFamily = Times;
         AXFontName = "Times-Roman";
         AXFontSize = 16;
         AXVisibleName = "Times Roman";
     };
-    AXForegroundColor = " [ (kCGColorSpaceDeviceRGB)] ( 0 0 0 1 )";
+    AXForegroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 1 )";
 }worl{
-    AXBackgroundColor = " [ (kCGColorSpaceDeviceRGB)] ( 0 0 0 0 )";
+    AXBackgroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 0 )";
     AXFont =     {
         AXFontFamily = Times;
         AXFontName = "Times-Bold";
         AXFontSize = 16;
         AXVisibleName = "Times Bold";
     };
-    AXForegroundColor = " [ (kCGColorSpaceDeviceRGB)] ( 0 0 0 1 )";
+    AXForegroundColor = " [ (kCGColorSpaceICCBased; kCGColorSpaceModelRGB; sRGB IEC61966-2.1)] ( 0 0 0 1 )";
     AXMarkedMisspelled = 1;
     AXMisspelled = 1;
 }
index edbe088..2794dd6 100644 (file)
@@ -1,3 +1,28 @@
+2018-03-28  Timothy Hatcher  <timothy@apple.com>
+
+        Consolidate NSColor to WebCore::Color conversion and fix system colors.
+
+        https://bugs.webkit.org/show_bug.cgi?id=184096
+        rdar://problem/38918925
+
+        Reviewed by Tim Horton.
+
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (CreateCGColorIfDifferent): Use CGColor property on NSColor, don't manually create new CGColor.
+        * platform/graphics/mac/ColorMac.h:
+        * platform/graphics/mac/ColorMac.mm:
+        (WebCore::makeRGBAFromNSColor): Move pattern code from RenderThemeMac's convertNSColorToColor.
+        Also use nextafter for proper RGBA float conversion.
+        * platform/mac/PlatformPasteboardMac.mm:
+        (WebCore::PlatformPasteboard::color): Use colorFromNSColor.
+        * rendering/RenderThemeMac.mm:
+        (WebCore::RenderThemeMac::platformActiveSelectionBackgroundColor const): Use colorFromNSColor.
+        (WebCore::RenderThemeMac::platformInactiveSelectionBackgroundColor const): Ditto.
+        (WebCore::RenderThemeMac::platformActiveListBoxSelectionBackgroundColor const): Ditto.
+        (WebCore::RenderThemeMac::systemColor const): Ditto.
+        (WebCore::paintAttachmentTitleBackground): Ditto.
+        (WebCore::convertNSColorToColor): Deleted.
+
 2018-03-28  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         The SVGAnimatedProperty wrappers have to be detached from the referenced values before the SVGAnimatedType is deleted
index 580d982..8ad2ed9 100644 (file)
@@ -793,44 +793,16 @@ static void AXAttributeStringSetFont(NSMutableAttributedString *attrString, NSSt
     
 }
 
-static CGColorRef CreateCGColorIfDifferent(NSColor *nsColor, CGColorRef existingColor)
-{
-    // get color information assuming NSDeviceRGBColorSpace
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-    NSColor *rgbColor = [nsColor colorUsingColorSpaceName:NSDeviceRGBColorSpace];
-#pragma clang diagnostic pop
-    if (rgbColor == nil)
-        rgbColor = [NSColor blackColor];
-    CGFloat components[4];
-    [rgbColor getRed:&components[0] green:&components[1] blue:&components[2] alpha:&components[3]];
-    
-    // create a new CGColorRef to return
-    CGColorSpaceRef cgColorSpace = CGColorSpaceCreateDeviceRGB();
-    CGColorRef cgColor = CGColorCreate(cgColorSpace, components);
-    CGColorSpaceRelease(cgColorSpace);
-    
-    // check for match with existing color
-    if (existingColor && CGColorEqualToColor(cgColor, existingColor)) {
-        CGColorRelease(cgColor);
-        cgColor = nullptr;
-    }
-    
-    return cgColor;
-}
-
 static void AXAttributeStringSetColor(NSMutableAttributedString* attrString, NSString* attribute, NSColor* color, NSRange range)
 {
     if (!AXAttributedStringRangeIsValid(attrString, range))
         return;
-    
+
     if (color) {
-        CGColorRef existingColor = (CGColorRef) [attrString attribute:attribute atIndex:range.location effectiveRange:nil];
-        CGColorRef cgColor = CreateCGColorIfDifferent(color, existingColor);
-        if (cgColor) {
+        CGColorRef cgColor = color.CGColor;
+        CGColorRef existingColor = (CGColorRef)[attrString attribute:attribute atIndex:range.location effectiveRange:nil];
+        if (!existingColor || !CGColorEqualToColor(existingColor, cgColor))
             [attrString addAttribute:attribute value:(id)cgColor range:range];
-            CGColorRelease(cgColor);
-        }
     } else
         [attrString removeAttribute:attribute range:range];
 }
index 8f70138..f4e83e0 100644 (file)
@@ -36,7 +36,6 @@ OBJC_CLASS NSColor;
 
 namespace WebCore {
 
-// These functions assume NSColors are in DeviceRGB colorspace
 WEBCORE_EXPORT Color colorFromNSColor(NSColor *);
 WEBCORE_EXPORT NSColor *nsColor(const Color&);
 
index 78c9e0a..b6df6f4 100644 (file)
@@ -28,6 +28,7 @@
 
 #if USE(APPKIT)
 
+#import "LocalCurrentGraphicsContext.h"
 #import <wtf/BlockObjCExceptions.h>
 #import <wtf/NeverDestroyed.h>
 #import <wtf/RetainPtr.h>
@@ -52,27 +53,46 @@ bool usesTestModeFocusRingColor()
     return useOldAquaFocusRingColor;
 }
 
-static RGBA32 makeRGBAFromNSColor(NSColor *c)
+static RGBA32 makeRGBAFromNSColor(NSColor *color)
 {
+    ASSERT_ARG(color, color);
+
     CGFloat redComponent;
     CGFloat greenComponent;
     CGFloat blueComponent;
     CGFloat alpha;
 
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
-    NSColor *rgbColor = [c colorUsingColorSpace:[NSColorSpace sRGBColorSpace]];
-    if (!rgbColor)
-        return makeRGBA(0, 0, 0, 0);
+    NSColor *rgbColor = [color colorUsingColorSpace:NSColorSpace.deviceRGBColorSpace];
+    if (!rgbColor) {
+        // The color space conversion above can fail if the NSColor is in the NSPatternColorSpace.
+        // These colors are actually a repeating pattern, not just a solid color. To workaround
+        // this we simply draw a one pixel image of the color and use that pixel's color.
+        // FIXME: It might be better to use an average of the colors in the pattern instead.
+        RetainPtr<NSBitmapImageRep> offscreenRep = adoptNS([[NSBitmapImageRep alloc] initWithBitmapDataPlanes:nil pixelsWide:1 pixelsHigh:1
+            bitsPerSample:8 samplesPerPixel:4 hasAlpha:YES isPlanar:NO colorSpaceName:NSDeviceRGBColorSpace bytesPerRow:4 bitsPerPixel:32]);
+
+        GraphicsContext bitmapContext([NSGraphicsContext graphicsContextWithBitmapImageRep:offscreenRep.get()].CGContext);
+        LocalCurrentGraphicsContext localContext(bitmapContext);
+
+        [color drawSwatchInRect:NSMakeRect(0, 0, 1, 1)];
+
+        NSUInteger pixel[4];
+        [offscreenRep getPixel:pixel atX:0 y:0];
+
+        return makeRGBA(pixel[0], pixel[1], pixel[2], pixel[3]);
+    }
 
     [rgbColor getRed:&redComponent green:&greenComponent blue:&blueComponent alpha:&alpha];
     END_BLOCK_OBJC_EXCEPTIONS;
 
-    return makeRGBA(255 * redComponent, 255 * greenComponent, 255 * blueComponent, 255 * alpha);
+    static const double scaleFactor = nextafter(256.0, 0.0);
+    return makeRGBA(scaleFactor * redComponent, scaleFactor * greenComponent, scaleFactor * blueComponent, scaleFactor * alpha);
 }
 
-Color colorFromNSColor(NSColor *c)
+Color colorFromNSColor(NSColor *color)
 {
-    return Color(makeRGBAFromNSColor(c));
+    return Color(makeRGBAFromNSColor(color));
 }
 
 NSColor *nsColor(const Color& color)
index 8421131..6c7fbe6 100644 (file)
@@ -29,6 +29,7 @@
 #if PLATFORM(MAC)
 
 #import "Color.h"
+#import "ColorMac.h"
 #import "LegacyNSPasteboardTypes.h"
 #import "Pasteboard.h"
 #import "URL.h"
@@ -201,22 +202,7 @@ String PlatformPasteboard::uniqueName()
 
 Color PlatformPasteboard::color()
 {
-    NSColor *color = [NSColor colorFromPasteboard:m_pasteboard.get()];
-
-    // FIXME: If it's OK to use sRGB instead of what we do here, then change this to use colorFromNSColor.
-
-    // The color may not be in an RGB colorspace.
-    // This commonly occurs when a color is dragged from the NSColorPanel grayscale picker.
-    // FIXME: What are the pros and cons of converting to sRGB if the color is in another RGB color space?
-    // FIXME: Shouldn't we be converting to sRGB instead of to calibrated RGB?
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-    if ([[color colorSpace] colorSpaceModel] != NSRGBColorSpaceModel)
-        color = [color colorUsingColorSpaceName:NSCalibratedRGBColorSpace];
-#pragma clang diagnostic pop
-
-    return makeRGBA((int)([color redComponent] * 255.0 + 0.5), (int)([color greenComponent] * 255.0 + 0.5),
-        (int)([color blueComponent] * 255.0 + 0.5), (int)([color alphaComponent] * 255.0 + 0.5));
+    return colorFromNSColor([NSColor colorFromPasteboard:m_pasteboard.get()]);
 }
 
 URL PlatformPasteboard::url()
index d0aa053..66215a5 100644 (file)
@@ -292,29 +292,17 @@ String RenderThemeMac::imageControlsStyleSheet() const
 
 Color RenderThemeMac::platformActiveSelectionBackgroundColor() const
 {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-    NSColor* color = [[NSColor selectedTextBackgroundColor] colorUsingColorSpaceName:NSDeviceRGBColorSpace];
-#pragma clang diagnostic pop
-    return Color(static_cast<int>(255.0 * [color redComponent]), static_cast<int>(255.0 * [color greenComponent]), static_cast<int>(255.0 * [color blueComponent]));
+    return colorFromNSColor([NSColor selectedTextBackgroundColor]);
 }
 
 Color RenderThemeMac::platformInactiveSelectionBackgroundColor() const
 {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-    NSColor* color = [[NSColor secondarySelectedControlColor] colorUsingColorSpaceName:NSDeviceRGBColorSpace];
-#pragma clang diagnostic pop
-    return Color(static_cast<int>(255.0 * [color redComponent]), static_cast<int>(255.0 * [color greenComponent]), static_cast<int>(255.0 * [color blueComponent]));
+    return colorFromNSColor([NSColor secondarySelectedControlColor]);
 }
 
 Color RenderThemeMac::platformActiveListBoxSelectionBackgroundColor() const
 {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-    NSColor* color = [[NSColor alternateSelectedControlColor] colorUsingColorSpaceName:NSDeviceRGBColorSpace];
-#pragma clang diagnostic pop
-    return Color(static_cast<int>(255.0 * [color redComponent]), static_cast<int>(255.0 * [color greenComponent]), static_cast<int>(255.0 * [color blueComponent]));
+    return colorFromNSColor([NSColor alternateSelectedControlColor]);
 }
 
 Color RenderThemeMac::platformActiveListBoxSelectionForegroundColor() const
@@ -414,78 +402,24 @@ void RenderThemeMac::updateCachedSystemFontDescription(CSSValueID cssValueId, Fo
     fontDescription.setIsItalic([fontManager traitsOfFont:font] & NSItalicFontMask);
 }
 
-static RGBA32 convertNSColorToColor(NSColor *color)
+static RGBA32 menuBackgroundColor()
 {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-    NSColor *colorInColorSpace = [color colorUsingColorSpaceName:NSDeviceRGBColorSpace];
-#pragma clang diagnostic pop
-    if (colorInColorSpace) {
-        static const double scaleFactor = nextafter(256.0, 0.0);
-        return makeRGB(static_cast<int>(scaleFactor * [colorInColorSpace redComponent]),
-            static_cast<int>(scaleFactor * [colorInColorSpace greenComponent]),
-            static_cast<int>(scaleFactor * [colorInColorSpace blueComponent]));
-    }
-
-    // This conversion above can fail if the NSColor in question is an NSPatternColor
-    // (as many system colors are). These colors are actually a repeating pattern
-    // not just a solid color. To work around this we simply draw a 1x1 image of
-    // the color and use that pixel's color. It might be better to use an average of
-    // the colors in the pattern instead.
-    NSBitmapImageRep *offscreenRep = [[NSBitmapImageRep alloc] initWithBitmapDataPlanes:nil
-                                                                             pixelsWide:1
-                                                                             pixelsHigh:1
-                                                                          bitsPerSample:8
-                                                                        samplesPerPixel:4
-                                                                               hasAlpha:YES
-                                                                               isPlanar:NO
-                                                                         colorSpaceName:NSDeviceRGBColorSpace
-                                                                            bytesPerRow:4
-                                                                           bitsPerPixel:32];
-
-    [NSGraphicsContext saveGraphicsState];
-    [NSGraphicsContext setCurrentContext:[NSGraphicsContext graphicsContextWithBitmapImageRep:offscreenRep]];
-    NSEraseRect(NSMakeRect(0, 0, 1, 1));
-    [color drawSwatchInRect:NSMakeRect(0, 0, 1, 1)];
-    [NSGraphicsContext restoreGraphicsState];
-
-    NSUInteger pixel[4];
-    [offscreenRep getPixel:pixel atX:0 y:0];
+    RetainPtr<NSBitmapImageRep> offscreenRep = adoptNS([[NSBitmapImageRep alloc] initWithBitmapDataPlanes:nil pixelsWide:1 pixelsHigh:1
+        bitsPerSample:8 samplesPerPixel:4 hasAlpha:YES isPlanar:NO colorSpaceName:NSDeviceRGBColorSpace bytesPerRow:4 bitsPerPixel:32]);
 
-    [offscreenRep release];
+    CGContextRef bitmapContext = [NSGraphicsContext graphicsContextWithBitmapImageRep:offscreenRep.get()].CGContext;
+    const CGRect rect = CGRectMake(0, 0, 1, 1);
 
-    return makeRGB(pixel[0], pixel[1], pixel[2]);
-}
-
-static RGBA32 menuBackgroundColor()
-{
-    NSBitmapImageRep *offscreenRep = [[NSBitmapImageRep alloc] initWithBitmapDataPlanes:nil
-                                                                             pixelsWide:1
-                                                                             pixelsHigh:1
-                                                                          bitsPerSample:8
-                                                                        samplesPerPixel:4
-                                                                               hasAlpha:YES
-                                                                               isPlanar:NO
-                                                                         colorSpaceName:NSDeviceRGBColorSpace
-                                                                            bytesPerRow:4
-                                                                           bitsPerPixel:32];
-
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-    CGContextRef context = static_cast<CGContextRef>([[NSGraphicsContext graphicsContextWithBitmapImageRep:offscreenRep] graphicsPort]);
-#pragma clang diagnostic pop
-    CGRect rect = CGRectMake(0, 0, 1, 1);
     HIThemeMenuDrawInfo drawInfo;
     drawInfo.version =  0;
     drawInfo.menuType = kThemeMenuTypePopUp;
-    HIThemeDrawMenuBackground(&rect, &drawInfo, context, kHIThemeOrientationInverted);
+
+    HIThemeDrawMenuBackground(&rect, &drawInfo, bitmapContext, kHIThemeOrientationInverted);
 
     NSUInteger pixel[4];
     [offscreenRep getPixel:pixel atX:0 y:0];
 
-    [offscreenRep release];
-
-    return makeRGB(pixel[0], pixel[1], pixel[2]);
+    return makeRGBA(pixel[0], pixel[1], pixel[2], pixel[3]);
 }
 
 void RenderThemeMac::platformColorsDidChange()
@@ -592,7 +526,7 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, bool useSystemAppearanc
         };
         if (auto selector = selectCocoaColor()) {
             if (auto color = wtfObjcMsgSend<NSColor *>([NSColor class], selector))
-                return convertNSColorToColor(color);
+                return colorFromNSColor(color);
         }
         switch (cssValueID) {
         case CSSValueActivebuttontext:
@@ -2468,7 +2402,7 @@ static void paintAttachmentTitleBackground(const RenderAttachment& attachment, G
 
     Color backgroundColor;
     if (attachment.frame().selection().isFocusedAndActive())
-        backgroundColor = convertNSColorToColor([NSColor alternateSelectedControlColor]);
+        backgroundColor = colorFromNSColor([NSColor alternateSelectedControlColor]);
     else
         backgroundColor = attachmentTitleInactiveBackgroundColor();