Extended Color Cleanup: Remove trivial uses of Color::rgb()
authorweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 May 2020 23:33:16 +0000 (23:33 +0000)
committerweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 May 2020 23:33:16 +0000 (23:33 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212231

Source/WebCore:

Reviewed by Darin Adler

Replaces a few unnecessary uses of Color::rgb():
- Uses of an idiom where code round-tripped a Color via Color(myColor.rgb()). This is
  not compatible with extended colors and seems to be unnecessary.
- Uses of colorWithOverrideAlpha(). This function requires a SimpleColor, so required
  using color.rgb(). We can't transition to Color::colorWithAlpha due to a slightly
  different rounding of the alpha, so a new function Color::colorWithAlphaUsingAlternativeRounding
  was added to which implements the alternative rounding. A later change can reconcile
  the two versions.
- Creation of D2D1::ColorF. D2D1::ColorF has a constructor that takes a four floats that
  is used instead.
- Comparing two colors using rgb() for each to avoid comparing the semantic bit. equalIgnoringSemanticColor
  exists for just this use.

* editing/cocoa/HTMLConverter.mm:
(HTMLConverterCaches::colorPropertyValueForNode):
* html/HTMLElement.cpp:
(WebCore::HTMLElement::addHTMLColorToStyle):
* html/canvas/CanvasRenderingContext2DBase.cpp:
(WebCore::CanvasRenderingContext2DBase::setStrokeStyle):
(WebCore::CanvasRenderingContext2DBase::setFillStyle):
(WebCore::CanvasRenderingContext2DBase::setShadow):
* html/canvas/CanvasStyle.cpp:
(WebCore::CanvasStyle::createFromStringWithOverrideAlpha):
* html/track/InbandGenericTextTrack.cpp:
(WebCore::InbandGenericTextTrack::updateCueFromCueData):
* platform/graphics/Color.cpp:
(WebCore::Color::colorWithAlphaMultipliedByUsingAlternativeRounding const):
(WebCore::Color::colorWithAlpha const):
(WebCore::Color::colorWithAlphaUsingAlternativeRounding const):
(WebCore::colorWithOverrideAlpha): Deleted.
* platform/graphics/Color.h:
(WebCore::colorWithOverrideAlpha): Deleted.
* platform/graphics/cairo/CairoOperations.cpp:
(WebCore::Cairo::prepareCairoContextSource):
* platform/graphics/filters/FEFlood.cpp:
(WebCore::FEFlood::platformApplySoftware):
* platform/graphics/win/ColorDirect2D.cpp:
(WebCore::Color::operator D2D1_COLOR_F const):
(WebCore::Color::operator D2D1_VECTOR_4F const):
* platform/graphics/win/GraphicsContextDirect2D.cpp:
(WebCore::GraphicsContext::colorWithGlobalAlpha const):
* platform/mac/ThemeMac.mm:
(WebCore::drawCellFocusRingWithFrameAtTime):
* rendering/RenderThemeIOS.mm:
(WebCore::RenderThemeIOS::paintFileUploadIconDecorations):
* rendering/RenderThemeMac.mm:
(WebCore::RenderThemeMac::platformFocusRingColor const):
* rendering/RenderTreeAsText.cpp:
(WebCore::RenderTreeAsText::writeRenderObject):
* svg/SVGStopElement.cpp:
(WebCore::SVGStopElement::stopColorIncludingOpacity const):

Source/WebKit:

Reviewed by Darin Adler.

* UIProcess/API/ios/WKWebViewIOS.mm:
(scrollViewBackgroundColor):
Replace colorWithOverrideAlpha() with Color::colorWithAlphaUsingAlternativeRounding() to avoid
unnecessary use of Color::rgb()

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

20 files changed:
Source/WebCore/ChangeLog
Source/WebCore/editing/cocoa/HTMLConverter.mm
Source/WebCore/html/HTMLElement.cpp
Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp
Source/WebCore/html/canvas/CanvasStyle.cpp
Source/WebCore/html/canvas/CanvasStyle.h
Source/WebCore/html/track/InbandGenericTextTrack.cpp
Source/WebCore/platform/graphics/Color.cpp
Source/WebCore/platform/graphics/Color.h
Source/WebCore/platform/graphics/cairo/CairoOperations.cpp
Source/WebCore/platform/graphics/filters/FEFlood.cpp
Source/WebCore/platform/graphics/win/ColorDirect2D.cpp
Source/WebCore/platform/graphics/win/GraphicsContextDirect2D.cpp
Source/WebCore/platform/mac/ThemeMac.mm
Source/WebCore/rendering/RenderThemeIOS.mm
Source/WebCore/rendering/RenderThemeMac.mm
Source/WebCore/rendering/RenderTreeAsText.cpp
Source/WebCore/svg/SVGStopElement.cpp
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm

index 1603b0b..fba569e 100644 (file)
@@ -1,3 +1,62 @@
+2020-05-21  Sam Weinig  <weinig@apple.com>
+
+        Extended Color Cleanup: Remove trivial uses of Color::rgb()
+        https://bugs.webkit.org/show_bug.cgi?id=212231
+
+        Reviewed by Darin Adler
+
+        Replaces a few unnecessary uses of Color::rgb():
+        - Uses of an idiom where code round-tripped a Color via Color(myColor.rgb()). This is
+          not compatible with extended colors and seems to be unnecessary.
+        - Uses of colorWithOverrideAlpha(). This function requires a SimpleColor, so required
+          using color.rgb(). We can't transition to Color::colorWithAlpha due to a slightly 
+          different rounding of the alpha, so a new function Color::colorWithAlphaUsingAlternativeRounding
+          was added to which implements the alternative rounding. A later change can reconcile
+          the two versions.
+        - Creation of D2D1::ColorF. D2D1::ColorF has a constructor that takes a four floats that
+          is used instead.
+        - Comparing two colors using rgb() for each to avoid comparing the semantic bit. equalIgnoringSemanticColor
+          exists for just this use.
+
+        * editing/cocoa/HTMLConverter.mm:
+        (HTMLConverterCaches::colorPropertyValueForNode):
+        * html/HTMLElement.cpp:
+        (WebCore::HTMLElement::addHTMLColorToStyle):
+        * html/canvas/CanvasRenderingContext2DBase.cpp:
+        (WebCore::CanvasRenderingContext2DBase::setStrokeStyle):
+        (WebCore::CanvasRenderingContext2DBase::setFillStyle):
+        (WebCore::CanvasRenderingContext2DBase::setShadow):
+        * html/canvas/CanvasStyle.cpp:
+        (WebCore::CanvasStyle::createFromStringWithOverrideAlpha):
+        * html/track/InbandGenericTextTrack.cpp:
+        (WebCore::InbandGenericTextTrack::updateCueFromCueData):
+        * platform/graphics/Color.cpp:
+        (WebCore::Color::colorWithAlphaMultipliedByUsingAlternativeRounding const):
+        (WebCore::Color::colorWithAlpha const):
+        (WebCore::Color::colorWithAlphaUsingAlternativeRounding const):
+        (WebCore::colorWithOverrideAlpha): Deleted.
+        * platform/graphics/Color.h:
+        (WebCore::colorWithOverrideAlpha): Deleted.
+        * platform/graphics/cairo/CairoOperations.cpp:
+        (WebCore::Cairo::prepareCairoContextSource):
+        * platform/graphics/filters/FEFlood.cpp:
+        (WebCore::FEFlood::platformApplySoftware):
+        * platform/graphics/win/ColorDirect2D.cpp:
+        (WebCore::Color::operator D2D1_COLOR_F const):
+        (WebCore::Color::operator D2D1_VECTOR_4F const):
+        * platform/graphics/win/GraphicsContextDirect2D.cpp:
+        (WebCore::GraphicsContext::colorWithGlobalAlpha const):
+        * platform/mac/ThemeMac.mm:
+        (WebCore::drawCellFocusRingWithFrameAtTime):
+        * rendering/RenderThemeIOS.mm:
+        (WebCore::RenderThemeIOS::paintFileUploadIconDecorations):
+        * rendering/RenderThemeMac.mm:
+        (WebCore::RenderThemeMac::platformFocusRingColor const):
+        * rendering/RenderTreeAsText.cpp:
+        (WebCore::RenderTreeAsText::writeRenderObject):
+        * svg/SVGStopElement.cpp:
+        (WebCore::SVGStopElement::stopColorIncludingOpacity const):
+
 2020-05-21  Oriol Brufau  <obrufau@igalia.com>
 
         [css-grid] Don't create renderers for whitespace nodes
index f5e3cbf..b4a1ac5 100644 (file)
@@ -882,13 +882,13 @@ Color HTMLConverterCaches::colorPropertyValueForNode(Node& node, CSSPropertyID p
     Element& element = downcast<Element>(node);
     if (RefPtr<CSSValue> value = computedStylePropertyForElement(element, propertyId)) {
         if (is<CSSPrimitiveValue>(*value) && downcast<CSSPrimitiveValue>(*value).isRGBColor())
-            return normalizedColor(Color(downcast<CSSPrimitiveValue>(*value).color().rgb()), ignoreDefaultColor, element);
+            return normalizedColor(downcast<CSSPrimitiveValue>(*value).color(), ignoreDefaultColor, element);
     }
 
     bool inherit = false;
     if (RefPtr<CSSValue> value = inlineStylePropertyForElement(element, propertyId)) {
         if (is<CSSPrimitiveValue>(*value) && downcast<CSSPrimitiveValue>(*value).isRGBColor())
-            return normalizedColor(Color(downcast<CSSPrimitiveValue>(*value).color().rgb()), ignoreDefaultColor, element);
+            return normalizedColor(downcast<CSSPrimitiveValue>(*value).color(), ignoreDefaultColor, element);
         if (value->isInheritedValue())
             inherit = true;
     }
index 5b7f9af..d652e09 100644 (file)
@@ -1050,7 +1050,7 @@ void HTMLElement::addHTMLColorToStyle(MutableStyleProperties& style, CSSProperty
     if (!color.isValid())
         color = Color(parseColorStringWithCrazyLegacyRules(colorString));
 
-    style.setProperty(propertyID, CSSValuePool::singleton().createColorValue(color.rgb()));
+    style.setProperty(propertyID, CSSValuePool::singleton().createColorValue(color));
 }
 
 bool HTMLElement::willRespondToMouseMoveEvents()
index 2077eef..573e576 100644 (file)
@@ -374,13 +374,9 @@ void CanvasRenderingContext2DBase::setStrokeStyle(CanvasStyle style)
     if (state().strokeStyle.isValid() && state().strokeStyle.isEquivalentColor(style))
         return;
 
-    if (style.isCurrentColor()) {
-        if (style.hasOverrideAlpha()) {
-            // FIXME: Should not use RGBA32 here.
-            style = CanvasStyle(colorWithOverrideAlpha(currentColor(canvasBase()).rgb(), style.overrideAlpha()));
-        } else
-            style = CanvasStyle(currentColor(canvasBase()));
-    } else
+    if (style.isCurrentColor())
+        style = CanvasStyle(currentColor(canvasBase()).colorWithAlphaUsingAlternativeRounding(style.overrideAlpha()));
+    else
         checkOrigin(style.canvasPattern().get());
 
     realizeSaves();
@@ -401,13 +397,9 @@ void CanvasRenderingContext2DBase::setFillStyle(CanvasStyle style)
     if (state().fillStyle.isValid() && state().fillStyle.isEquivalentColor(style))
         return;
 
-    if (style.isCurrentColor()) {
-        if (style.hasOverrideAlpha()) {
-            // FIXME: Should not use RGBA32 here.
-            style = CanvasStyle(colorWithOverrideAlpha(currentColor(canvasBase()).rgb(), style.overrideAlpha()));
-        } else
-            style = CanvasStyle(currentColor(canvasBase()));
-    } else
+    if (style.isCurrentColor())
+        style = CanvasStyle(currentColor(canvasBase()).colorWithAlphaUsingAlternativeRounding(style.overrideAlpha()));
+    else
         checkOrigin(style.canvasPattern().get());
 
     realizeSaves();
@@ -1310,8 +1302,7 @@ void CanvasRenderingContext2DBase::setShadow(float width, float height, float bl
         if (!color.isValid())
             return;
     }
-    // FIXME: Should not use RGBA32 here.
-    setShadow(FloatSize(width, height), blur, colorWithOverrideAlpha(color.rgb(), alpha));
+    setShadow(FloatSize(width, height), blur, color.colorWithAlphaUsingAlternativeRounding(alpha));
 }
 
 void CanvasRenderingContext2DBase::setShadow(float width, float height, float blur, float grayLevel, float alpha)
index 1129e74..7f275d0 100644 (file)
@@ -145,7 +145,7 @@ CanvasStyle CanvasStyle::createFromStringWithOverrideAlpha(const String& colorSt
     if (!color.isValid())
         return { };
 
-    return Color { colorWithOverrideAlpha(color.rgb(), alpha) };
+    return color.colorWithAlphaUsingAlternativeRounding(alpha);
 }
 
 bool CanvasStyle::isEquivalentColor(const CanvasStyle& other) const
index debeb30..0b52712 100644 (file)
@@ -52,8 +52,8 @@ public:
 
     bool isValid() const { return !WTF::holds_alternative<Invalid>(m_style); }
     bool isCurrentColor() const { return WTF::holds_alternative<CurrentColor>(m_style); }
-    bool hasOverrideAlpha() const { return isCurrentColor() && WTF::get<CurrentColor>(m_style).overrideAlpha; }
-    float overrideAlpha() const { return WTF::get<CurrentColor>(m_style).overrideAlpha.value(); }
+    Optional<float> overrideAlpha() const { return WTF::get<CurrentColor>(m_style).overrideAlpha; }
+    
 
     String color() const;
     RefPtr<CanvasGradient> canvasGradient() const;
index ff82d88..14f42b6 100644 (file)
@@ -97,11 +97,11 @@ void InbandGenericTextTrack::updateCueFromCueData(TextTrackCueGeneric& cue, Inba
     if (inbandCue.size() > 0)
         cue.setSize(std::round(inbandCue.size()));
     if (inbandCue.backgroundColor().isValid())
-        cue.setBackgroundColor(inbandCue.backgroundColor().rgb());
+        cue.setBackgroundColor(inbandCue.backgroundColor());
     if (inbandCue.foregroundColor().isValid())
-        cue.setForegroundColor(inbandCue.foregroundColor().rgb());
+        cue.setForegroundColor(inbandCue.foregroundColor());
     if (inbandCue.highlightColor().isValid())
-        cue.setHighlightColor(inbandCue.highlightColor().rgb());
+        cue.setHighlightColor(inbandCue.highlightColor());
 
     if (inbandCue.align() == GenericCueData::Alignment::Start)
         cue.setAlign("start"_s);
index a9b1dfb..fda96d0 100644 (file)
@@ -71,11 +71,6 @@ RGBA32 makeRGBA32FromFloats(float r, float g, float b, float a)
     return makeRGBA(colorFloatToRGBAByte(r), colorFloatToRGBAByte(g), colorFloatToRGBAByte(b), colorFloatToRGBAByte(a));
 }
 
-RGBA32 colorWithOverrideAlpha(RGBA32 color, float overrideAlpha)
-{
-    return { (color.value() & 0x00FFFFFF) | colorFloatToRGBAByte(overrideAlpha) << 24 };
-}
-
 RGBA32 makeRGBAFromHSLA(float hue, float saturation, float lightness, float alpha)
 {
     const float scaleFactor = 255.0;
@@ -460,12 +455,19 @@ Color Color::colorWithAlphaMultipliedBy(float amount) const
     return colorWithAlpha(newAlpha);
 }
 
+Color Color::colorWithAlphaMultipliedByUsingAlternativeRounding(float amount) const
+{
+    float newAlpha = amount * (isExtended() ? m_colorData.extendedColor->alpha() : static_cast<float>(alpha()) / 255);
+    return colorWithAlphaUsingAlternativeRounding(newAlpha);
+}
+
 Color Color::colorWithAlpha(float alpha) const
 {
     if (isExtended())
         return Color { m_colorData.extendedColor->red(), m_colorData.extendedColor->green(), m_colorData.extendedColor->blue(), alpha, m_colorData.extendedColor->colorSpace() };
 
-    int newAlpha = alpha * 255; // Why doesn't this use colorFloatToRGBAByte() like colorWithOverrideAlpha()?
+    // FIXME: This is where this function differs from colorWithAlphaUsingAlternativeRounding.
+    int newAlpha = alpha * 255;
 
     Color result = { red(), green(), blue(), newAlpha };
     if (isSemantic())
@@ -473,6 +475,17 @@ Color Color::colorWithAlpha(float alpha) const
     return result;
 }
 
+Color Color::colorWithAlphaUsingAlternativeRounding(float alpha) const
+{
+    if (isExtended())
+        return Color { m_colorData.extendedColor->red(), m_colorData.extendedColor->green(), m_colorData.extendedColor->blue(), alpha, m_colorData.extendedColor->colorSpace() };
+
+    Color result = SimpleColor { (rgb().value() & 0x00FFFFFF) | colorFloatToRGBAByte(alpha) << 24 };
+    if (isSemantic())
+        result.setIsSemantic();
+    return result;
+}
+
 std::pair<ColorSpace, FloatComponents> Color::colorSpaceAndComponents() const
 {
     if (isExtended()) {
index 43168d1..b13a477 100644 (file)
@@ -92,9 +92,6 @@ constexpr RGBA32 makeRGBA(int r, int g, int b, int a);
 RGBA32 makePremultipliedRGBA(int r, int g, int b, int a, bool ceiling = true);
 RGBA32 makeUnPremultipliedRGBA(int r, int g, int b, int a);
 
-WEBCORE_EXPORT RGBA32 colorWithOverrideAlpha(RGBA32 color, float overrideAlpha);
-RGBA32 colorWithOverrideAlpha(RGBA32 color, Optional<float> overrideAlpha);
-
 WEBCORE_EXPORT RGBA32 makeRGBA32FromFloats(float r, float g, float b, float a);
 WEBCORE_EXPORT RGBA32 makeRGBAFromHSLA(float h, float s, float l, float a);
 RGBA32 makeRGBAFromCMYKA(float c, float m, float y, float k, float a);
@@ -224,9 +221,14 @@ public:
     Color blendWithWhite() const;
 
     Color colorWithAlphaMultipliedBy(float) const;
-
-    // Returns a color that has the same RGB values, but with the given A.
     Color colorWithAlpha(float) const;
+
+    // FIXME: Remove the need for AlternativeRounding variants by settling on a rounding behavior.
+    Color colorWithAlphaMultipliedByUsingAlternativeRounding(Optional<float>) const;
+    Color colorWithAlphaMultipliedByUsingAlternativeRounding(float) const;
+    Color colorWithAlphaUsingAlternativeRounding(Optional<float>) const;
+    WEBCORE_EXPORT Color colorWithAlphaUsingAlternativeRounding(float) const;
+
     Color opaqueColor() const { return colorWithAlpha(1.0f); }
 
     // True if the color originated from a CSS semantic color name.
@@ -393,9 +395,14 @@ inline uint16_t fastDivideBy255(uint16_t value)
     return approximation + (remainder >> 8);
 }
 
-inline RGBA32 colorWithOverrideAlpha(RGBA32 color, Optional<float> overrideAlpha)
+inline Color Color::colorWithAlphaMultipliedByUsingAlternativeRounding(Optional<float> alpha) const
+{
+    return alpha ? colorWithAlphaMultipliedByUsingAlternativeRounding(alpha.value()) : *this;
+}
+
+inline Color Color::colorWithAlphaUsingAlternativeRounding(Optional<float> alpha) const
 {
-    return overrideAlpha ? colorWithOverrideAlpha(color, overrideAlpha.value()) : color;
+    return alpha ? colorWithAlphaUsingAlternativeRounding(alpha.value()) : *this;
 }
 
 inline RGBA32 Color::rgb() const
index 2929c98..d16b8b4 100644 (file)
@@ -78,7 +78,7 @@ static void prepareCairoContextSource(cairo_t* cr, cairo_pattern_t* pattern, cai
     } else {
         // Solid color source
         if (globalAlpha < 1)
-            setSourceRGBAFromColor(cr, colorWithOverrideAlpha(color.rgb(), color.alpha() / 255.f * globalAlpha));
+            setSourceRGBAFromColor(cr, color.colorWithAlphaUsingAlternativeRounding(color.alpha() / 255.f * globalAlpha));
         else
             setSourceRGBAFromColor(cr, color);
     }
index 60476eb..897dcb8 100644 (file)
@@ -63,9 +63,7 @@ void FEFlood::platformApplySoftware()
     if (!resultImage)
         return;
 
-    // FIXME: This should use colorWithAlphaMultipliedBy() but that has different rounding of the alpha component that breaks some tests.
-    float colorAlpha = floodColor().alpha() / 255.0;
-    auto color = colorWithOverrideAlpha(floodColor().rgb(), colorAlpha * floodOpacity());
+    auto color = floodColor().colorWithAlphaMultipliedByUsingAlternativeRounding(floodOpacity());
     resultImage->context().fillRect(FloatRect(FloatPoint(), absolutePaintRect().size()), color);
 }
 
index 2382559..e328b17 100644 (file)
@@ -41,19 +41,14 @@ Color::Color(D2D1_COLOR_F color)
 
 Color::operator D2D1_COLOR_F() const
 {
-    if (isExtended()) {
-        auto asRGBA = toSRGBAComponentsLossy();
-        return D2D1::ColorF(asRGBA.components[0], asRGBA.components[1], asRGBA.components[2], asRGBA.components[3]);
-    }
-
-    float colorAlpha = alpha() / 255.0f;
-    return D2D1::ColorF(rgb().value(), colorAlpha);
+    auto [r, g, b, a] = toSRGBAComponentsLossy();
+    return D2D1::ColorF(r, g, b, a);
 }
 
 Color::operator D2D1_VECTOR_4F() const
 {
-    auto asRGBA = toSRGBAComponentsLossy();
-    return D2D1::Vector4F(asRGBA.components[0], asRGBA.components[1], asRGBA.components[2], asRGBA.components[3]);
+    auto [r, g, b, a] = toSRGBAComponentsLossy();
+    return D2D1::Vector4F(r, g, b, a);
 }
 
 }
index ef3540a..209bba9 100644 (file)
@@ -379,10 +379,8 @@ void GraphicsContextPlatformPrivate::rotate(float angle)
 
 D2D1_COLOR_F GraphicsContext::colorWithGlobalAlpha(const Color& color) const
 {
-    float colorAlpha = color.alphaAsFloat();
-    float globalAlpha = m_data->currentGlobalAlpha();
-
-    return D2D1::ColorF(color.rgb(), globalAlpha * colorAlpha);
+    auto [r, g, b, a] = color.toSRGBAComponentsLossy();
+    return D2D1::ColorF(r, g, b, a * m_data->currentGlobalAlpha());
 }
 
 ID2D1Brush* GraphicsContext::solidStrokeBrush() const
index 1d12264..a8cff60 100644 (file)
@@ -402,7 +402,7 @@ static bool drawCellFocusRingWithFrameAtTime(NSCell *cell, NSRect cellFrame, NSV
 
     // FIXME: This color should be shared with RenderThemeMac. For now just use the same NSColor color.
     // The color is expected to be opaque, since CoreGraphics will apply opacity when drawing (because opacity is normally animated).
-    auto color = colorWithOverrideAlpha(colorFromNSColor([NSColor keyboardFocusIndicatorColor]).rgb(), 1);
+    auto color = colorFromNSColor([NSColor keyboardFocusIndicatorColor]).opaqueColor();
     auto style = adoptCF(CGStyleCreateFocusRingWithColor(&focusRingStyle, cachedCGColor(color)));
     CGContextSetStyle(cgContext, style.get());
 
index aa9e733..2141a5f 100644 (file)
@@ -1093,7 +1093,7 @@ bool RenderThemeIOS::paintFileUploadIconDecorations(const RenderObject&, const R
         thumbnailRect.contract(kMultipleThumbnailShrinkSize, kMultipleThumbnailShrinkSize);
 
         // Background picture frame and simple background icon with a gradient matching the button.
-        Color backgroundImageColor = Color(buttonRenderer.style().visitedDependentColor(CSSPropertyBackgroundColor).rgb());
+        Color backgroundImageColor = buttonRenderer.style().visitedDependentColor(CSSPropertyBackgroundColor);
         paintInfo.context().fillRoundedRect(FloatRoundedRect(thumbnailPictureFrameRect, cornerSize, cornerSize, cornerSize, cornerSize), pictureFrameColor);
         paintInfo.context().fillRect(thumbnailRect, backgroundImageColor);
         {
index a260f9d..a808dd3 100644 (file)
@@ -469,7 +469,7 @@ Color RenderThemeMac::platformFocusRingColor(OptionSet<StyleColor::Options> opti
         return oldAquaFocusRingColor();
     LocalDefaultSystemAppearance localAppearance(options.contains(StyleColor::Options::UseDarkAppearance));
     // The color is expected to be opaque, since CoreGraphics will apply opacity when drawing (because opacity is normally animated).
-    return colorWithOverrideAlpha(colorFromNSColor([NSColor keyboardFocusIndicatorColor]).rgb(), 1);
+    return colorFromNSColor([NSColor keyboardFocusIndicatorColor]).opaqueColor();
 }
 
 Color RenderThemeMac::platformTextSearchHighlightColor(OptionSet<StyleColor::Options> options) const
index e54a832..5787584 100644 (file)
@@ -238,23 +238,23 @@ void RenderTreeAsText::writeRenderObject(TextStream& ts, const RenderObject& o,
 
         if (o.parent()) {
             Color color = o.style().visitedDependentColor(CSSPropertyColor);
-            if (o.parent()->style().visitedDependentColor(CSSPropertyColor).rgb() != color.rgb())
+            if (!equalIgnoringSemanticColor(o.parent()->style().visitedDependentColor(CSSPropertyColor), color))
                 ts << " [color=" << color.nameForRenderTreeAsText() << "]";
 
             // Do not dump invalid or transparent backgrounds, since that is the default.
             Color backgroundColor = o.style().visitedDependentColor(CSSPropertyBackgroundColor);
-            if (o.parent()->style().visitedDependentColor(CSSPropertyBackgroundColor).rgb() != backgroundColor.rgb()
-                && backgroundColor.rgb() != Color::transparent)
+            if (!equalIgnoringSemanticColor(o.parent()->style().visitedDependentColor(CSSPropertyBackgroundColor), backgroundColor)
+                && backgroundColor != Color::transparent)
                 ts << " [bgcolor=" << backgroundColor.nameForRenderTreeAsText() << "]";
             
             Color textFillColor = o.style().visitedDependentColor(CSSPropertyWebkitTextFillColor);
-            if (o.parent()->style().visitedDependentColor(CSSPropertyWebkitTextFillColor).rgb() != textFillColor.rgb()  
-                && textFillColor.rgb() != color.rgb() && textFillColor.rgb() != Color::transparent)
+            if (!equalIgnoringSemanticColor(o.parent()->style().visitedDependentColor(CSSPropertyWebkitTextFillColor), textFillColor)
+                && textFillColor != color && textFillColor != Color::transparent)
                 ts << " [textFillColor=" << textFillColor.nameForRenderTreeAsText() << "]";
 
             Color textStrokeColor = o.style().visitedDependentColor(CSSPropertyWebkitTextStrokeColor);
-            if (o.parent()->style().visitedDependentColor(CSSPropertyWebkitTextStrokeColor).rgb() != textStrokeColor.rgb()
-                && textStrokeColor.rgb() != color.rgb() && textStrokeColor.rgb() != Color::transparent)
+            if (!equalIgnoringSemanticColor(o.parent()->style().visitedDependentColor(CSSPropertyWebkitTextStrokeColor), textStrokeColor)
+                && textStrokeColor != color && textStrokeColor != Color::transparent)
                 ts << " [textStrokeColor=" << textStrokeColor.nameForRenderTreeAsText() << "]";
 
             if (o.parent()->style().textStrokeWidth() != o.style().textStrokeWidth() && o.style().textStrokeWidth() > 0)
index 186d27e..b575b76 100644 (file)
@@ -94,9 +94,7 @@ Color SVGStopElement::stopColorIncludingOpacity() const
     auto& svgStyle = style.svgStyle();
     auto stopColor = style.colorResolvingCurrentColor(svgStyle.stopColor());
 
-    float colorAlpha = stopColor.alpha() / 255.0;
-    // FIXME: This should use colorWithAlphaMultipliedBy() but that has different rounding of the alpha component.
-    return colorWithOverrideAlpha(stopColor.rgb(), colorAlpha * svgStyle.stopOpacity());
+    return stopColor.colorWithAlphaMultipliedByUsingAlternativeRounding(svgStyle.stopOpacity());
 }
 
 }
index 96ca797..8f41ed8 100644 (file)
@@ -1,3 +1,15 @@
+2020-05-21  Sam Weinig  <weinig@apple.com>
+
+        Extended Color Cleanup: Remove trivial uses of Color::rgb()
+        https://bugs.webkit.org/show_bug.cgi?id=212231
+
+        Reviewed by Darin Adler.
+
+        * UIProcess/API/ios/WKWebViewIOS.mm:
+        (scrollViewBackgroundColor):
+        Replace colorWithOverrideAlpha() with Color::colorWithAlphaUsingAlternativeRounding() to avoid
+        unnecessary use of Color::rgb()
+
 2020-05-21  Alex Christensen  <achristensen@webkit.org>
 
         Use an OptionSet instead of uint8_t for MessageFlags
index f820b20..b8a077a 100644 (file)
@@ -513,7 +513,7 @@ static WebCore::Color scrollViewBackgroundColor(WKWebView *webView)
     if (zoomScale < minimumZoomScale) {
         CGFloat slope = 12;
         CGFloat opacity = std::max<CGFloat>(1 - slope * (minimumZoomScale - zoomScale), 0);
-        color = WebCore::colorWithOverrideAlpha(color.rgb(), opacity);
+        color = color.colorWithAlphaUsingAlternativeRounding(opacity);
     }
 
     return color;