Title: [275981] trunk/Source/WebCore
Revision
275981
Author
wei...@apple.com
Date
2021-04-14 17:15:02 -0700 (Wed, 14 Apr 2021)

Log Message

Use range-for loop in target contrast selection to simplify/improve code readability
https://bugs.webkit.org/show_bug.cgi?id=224512

Reviewed by Darin Adler.

Address some additional post landing feedback from Darin.

- Use range-for loop in target contrast selection.
- Use double to represent luminance, lightness and contrast everywhere.
  This avoid unnecessary conversions to and from float.
- Rename Color::contrastRatio(Color, Color) to just contrastRatio(Color, Color)
  and move it ColorLuminance.h/cpp with the other contrastRatio overloads.

* Sources.txt:
* WebCore.xcodeproj/project.pbxproj:
Add ColorLuminance.cpp

* css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::selectFirstColorThatMeetsOrExceedsTargetContrast):
Convert colorsToCompareAgainst to an r-value since vector is never used after
calling this.

(WebCore::CSSPropertyParserHelpers::selectFirstColorWithHighestContrast):
Convert colorsToCompareAgainst to an r-value since vector is never used after
calling this. Also, switch to a range-for loop to aid readability.

(WebCore::CSSPropertyParserHelpers::parseColorContrastFunctionParameters):
Move colorsToCompareAgainst to the select functions.

* platform/graphics/Color.h:
* platform/graphics/Color.cpp:
(WebCore::Color::lightness const):
(WebCore::Color::luminance const):
Use double to represent lightness and luminance.

(WebCore::Color::contrastRatio): Deleted.
Moved to ColorLuminance.h/cpp

* platform/graphics/ColorLuminance.cpp: Added.
(WebCore::contrastRatio):
* platform/graphics/ColorLuminance.h:
(WebCore::relativeLuminance):
(WebCore::contrastRatio):
* rendering/RenderTheme.cpp:
(WebCore::RenderTheme::disabledTextColor const):
* rendering/TextPaintStyle.cpp:
(WebCore::textColorIsLegibleAgainstBackgroundColor):
Use double to represent luminance and constrast everywhere.

Modified Paths

Added Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (275980 => 275981)


--- trunk/Source/WebCore/ChangeLog	2021-04-15 00:11:33 UTC (rev 275980)
+++ trunk/Source/WebCore/ChangeLog	2021-04-15 00:15:02 UTC (rev 275981)
@@ -1,3 +1,54 @@
+2021-04-14  Sam Weinig  <wei...@apple.com>
+
+        Use range-for loop in target contrast selection to simplify/improve code readability
+        https://bugs.webkit.org/show_bug.cgi?id=224512
+
+        Reviewed by Darin Adler.
+
+        Address some additional post landing feedback from Darin.
+
+        - Use range-for loop in target contrast selection.
+        - Use double to represent luminance, lightness and contrast everywhere.
+          This avoid unnecessary conversions to and from float.
+        - Rename Color::contrastRatio(Color, Color) to just contrastRatio(Color, Color)
+          and move it ColorLuminance.h/cpp with the other contrastRatio overloads.
+
+        * Sources.txt:
+        * WebCore.xcodeproj/project.pbxproj:
+        Add ColorLuminance.cpp
+
+        * css/parser/CSSPropertyParserHelpers.cpp:
+        (WebCore::CSSPropertyParserHelpers::selectFirstColorThatMeetsOrExceedsTargetContrast):
+        Convert colorsToCompareAgainst to an r-value since vector is never used after
+        calling this.
+
+        (WebCore::CSSPropertyParserHelpers::selectFirstColorWithHighestContrast):
+        Convert colorsToCompareAgainst to an r-value since vector is never used after
+        calling this. Also, switch to a range-for loop to aid readability.
+
+        (WebCore::CSSPropertyParserHelpers::parseColorContrastFunctionParameters):
+        Move colorsToCompareAgainst to the select functions.
+
+        * platform/graphics/Color.h:
+        * platform/graphics/Color.cpp:
+        (WebCore::Color::lightness const):
+        (WebCore::Color::luminance const):
+        Use double to represent lightness and luminance.
+
+        (WebCore::Color::contrastRatio): Deleted.
+        Moved to ColorLuminance.h/cpp
+
+        * platform/graphics/ColorLuminance.cpp: Added.
+        (WebCore::contrastRatio):
+        * platform/graphics/ColorLuminance.h:
+        (WebCore::relativeLuminance):
+        (WebCore::contrastRatio):
+        * rendering/RenderTheme.cpp:
+        (WebCore::RenderTheme::disabledTextColor const):
+        * rendering/TextPaintStyle.cpp:
+        (WebCore::textColorIsLegibleAgainstBackgroundColor):
+        Use double to represent luminance and constrast everywhere.
+
 2021-04-14  Antti Koivisto  <an...@apple.com>
 
         RunIterator::traverseNext/PreviousOnLineIgnoringLineBreak should skip over WBRs

Modified: trunk/Source/WebCore/Sources.txt (275980 => 275981)


--- trunk/Source/WebCore/Sources.txt	2021-04-15 00:11:33 UTC (rev 275980)
+++ trunk/Source/WebCore/Sources.txt	2021-04-15 00:15:02 UTC (rev 275981)
@@ -1935,6 +1935,7 @@
 platform/graphics/Color.cpp
 platform/graphics/ColorBlending.cpp
 platform/graphics/ColorConversion.cpp
+platform/graphics/ColorLuminance.cpp
 platform/graphics/ColorSerialization.cpp
 platform/graphics/ColorSpace.cpp
 platform/graphics/ColorUtilities.cpp

Modified: trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj (275980 => 275981)


--- trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj	2021-04-15 00:11:33 UTC (rev 275980)
+++ trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj	2021-04-15 00:15:02 UTC (rev 275981)
@@ -14227,6 +14227,7 @@
 		BC6D44EA0C07F2ED0072D2C9 /* JSHTMLEmbedElement.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = JSHTMLEmbedElement.cpp; sourceTree = "<group>"; };
 		BC6D44EB0C07F2ED0072D2C9 /* JSHTMLEmbedElement.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = JSHTMLEmbedElement.h; sourceTree = "<group>"; };
 		BC6D6E2509AF943500F59759 /* ScrollView.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = ScrollView.h; sourceTree = "<group>"; };
+		BC6EB84526266B61003225A7 /* ColorLuminance.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = ColorLuminance.cpp; sourceTree = "<group>"; };
 		BC772B360C4EA91E0083285F /* CSSHelper.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = CSSHelper.h; sourceTree = "<group>"; };
 		BC772C440C4EB2C60083285F /* XMLHttpRequest.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = XMLHttpRequest.cpp; sourceTree = "<group>"; };
 		BC772C450C4EB2C60083285F /* XMLHttpRequest.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = XMLHttpRequest.h; sourceTree = "<group>"; };
@@ -26798,6 +26799,7 @@
 				7C514E0224AF80580050710F /* ColorConversion.cpp */,
 				7C514E0024AF80580050710F /* ColorConversion.h */,
 				3103B7DE1DB01556008BB890 /* ColorHash.h */,
+				BC6EB84526266B61003225A7 /* ColorLuminance.cpp */,
 				BC4A23EB25EC160200AAC630 /* ColorLuminance.h */,
 				7CAC6AE8247F082000E61D59 /* ColorMatrix.h */,
 				BC10137B25C3624B00DC773C /* ColorModels.h */,

Modified: trunk/Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp (275980 => 275981)


--- trunk/Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp	2021-04-15 00:11:33 UTC (rev 275980)
+++ trunk/Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp	2021-04-15 00:15:02 UTC (rev 275981)
@@ -1509,38 +1509,37 @@
     return color;
 }
 
-static Color selectFirstColorThatMeetsOrExceedsTargetContrast(const Color& originBackgroundColor, const Vector<Color>& colorsToCompareAgainst, double targetContrast)
+static Color selectFirstColorThatMeetsOrExceedsTargetContrast(const Color& originBackgroundColor, Vector<Color>&& colorsToCompareAgainst, double targetContrast)
 {
     auto originBackgroundColorLuminance = originBackgroundColor.luminance();
 
     for (auto& color : colorsToCompareAgainst) {
-        auto contrastRatio = WebCore::contrastRatio(originBackgroundColorLuminance, color.luminance());
-        if (contrastRatio >= targetContrast)
-            return color;
+        if (contrastRatio(originBackgroundColorLuminance, color.luminance()) >= targetContrast)
+            return WTFMove(color);
     }
-    
+
     // If there is a target contrast, and the end of the list is reached without meeting that target,
     // either white or black is returned, whichever has the higher contrast.
-    auto contrastRatioWithWhite = WebCore::contrastRatio(originBackgroundColorLuminance, 1.0f);
-    auto contrastRatioWithBlack = WebCore::contrastRatio(originBackgroundColorLuminance, 0.0f);
-    return contrastRatioWithWhite > contrastRatioWithBlack ? Color::white : Color::black;
+    auto contrastWithWhite = contrastRatio(originBackgroundColorLuminance, 1.0);
+    auto contrastWithBlack = contrastRatio(originBackgroundColorLuminance, 0.0);
+    return contrastWithWhite > contrastWithBlack ? Color::white : Color::black;
 }
 
-static Color selectFirstColorWithHighestContrast(const Color& originBackgroundColor, const Vector<Color>& colorsToCompareAgainst)
+static Color selectFirstColorWithHighestContrast(const Color& originBackgroundColor, Vector<Color>&& colorsToCompareAgainst)
 {
     auto originBackgroundColorLuminance = originBackgroundColor.luminance();
 
-    size_t indexOfColorWithHigestContrastRatio = 0;
-    float highestContrastRatioSoFar = 0;
-    for (size_t i = 0; i < colorsToCompareAgainst.size(); ++i) {
-        auto contrastRatio = WebCore::contrastRatio(originBackgroundColorLuminance, colorsToCompareAgainst[i].luminance());
-        if (contrastRatio > highestContrastRatioSoFar) {
-            highestContrastRatioSoFar = contrastRatio;
-            indexOfColorWithHigestContrastRatio = i;
+    auto* colorWithGreatestContrast = &colorsToCompareAgainst[0];
+    double greatestContrastSoFar = 0;
+    for (auto& color : colorsToCompareAgainst) {
+        auto contrast = contrastRatio(originBackgroundColorLuminance, color.luminance());
+        if (contrast > greatestContrastSoFar) {
+            greatestContrastSoFar = contrast;
+            colorWithGreatestContrast = &color;
         }
     }
 
-    return colorsToCompareAgainst[indexOfColorWithHigestContrastRatio];
+    return WTFMove(*colorWithGreatestContrast);
 }
 
 static Color parseColorContrastFunctionParameters(CSSParserTokenRange& range, const CSSParserContext& context)
@@ -1590,11 +1589,11 @@
             return { };
         
         // When a target constast is specified, we select "the first color color to meet or exceed the target contrast."
-        return selectFirstColorThatMeetsOrExceedsTargetContrast(originBackgroundColor, colorsToCompareAgainst, *targetContrast);
+        return selectFirstColorThatMeetsOrExceedsTargetContrast(originBackgroundColor, WTFMove(colorsToCompareAgainst), *targetContrast);
     }
 
     // When a target constast is NOT specified, we select "the first color with the highest contrast to the single color."
-    return selectFirstColorWithHighestContrast(originBackgroundColor, colorsToCompareAgainst);
+    return selectFirstColorWithHighestContrast(originBackgroundColor, WTFMove(colorsToCompareAgainst));
 }
 
 enum class ColorMixColorSpace {

Modified: trunk/Source/WebCore/platform/graphics/Color.cpp (275980 => 275981)


--- trunk/Source/WebCore/platform/graphics/Color.cpp	2021-04-15 00:11:33 UTC (rev 275980)
+++ trunk/Source/WebCore/platform/graphics/Color.cpp	2021-04-15 00:15:02 UTC (rev 275981)
@@ -109,15 +109,15 @@
     return convertColor<SRGBA<uint8_t>>(SRGBA<float> { multiplier * r, multiplier * g, multiplier * b, a });
 }
 
-float Color::lightness() const
+double Color::lightness() const
 {
     // FIXME: Replace remaining uses with luminance.
     auto [r, g, b, a] = toSRGBALossy<float>();
     auto [min, max] = std::minmax({ r, g, b });
-    return 0.5f * (max + min);
+    return 0.5 * (max + min);
 }
 
-float Color::luminance() const
+double Color::luminance() const
 {
     return callOnUnderlyingType([&] (const auto& underlyingColor) {
         return WebCore::relativeLuminance(underlyingColor);
@@ -124,15 +124,6 @@
     });
 }
 
-float Color::contrastRatio(const Color& colorA, const Color& colorB)
-{
-    return colorA.callOnUnderlyingType([&] (const auto& underlyingColorA) {
-        return colorB.callOnUnderlyingType([&] (const auto& underlyingColorB) {
-            return WebCore::contrastRatio(underlyingColorA, underlyingColorB);
-        });
-    });
-}
-
 Color Color::colorWithAlpha(float alpha) const
 {
     return callOnUnderlyingType([&] (const auto& underlyingColor) -> Color {

Modified: trunk/Source/WebCore/platform/graphics/Color.h (275980 => 275981)


--- trunk/Source/WebCore/platform/graphics/Color.h	2021-04-15 00:11:33 UTC (rev 275980)
+++ trunk/Source/WebCore/platform/graphics/Color.h	2021-04-15 00:15:02 UTC (rev 275981)
@@ -106,9 +106,8 @@
     uint8_t alphaByte() const { return isOutOfLine() ? convertFloatAlphaTo<uint8_t>(asOutOfLine().alpha()) : asInline().alpha; }
     float alphaAsFloat() const { return isOutOfLine() ? asOutOfLine().alpha() : convertByteAlphaTo<float>(asInline().alpha); }
 
-    WEBCORE_EXPORT float luminance() const;
-    WEBCORE_EXPORT float lightness() const; // FIXME: Replace remaining uses with luminance.
-    WEBCORE_EXPORT static float contrastRatio(const Color&, const Color&);
+    WEBCORE_EXPORT double luminance() const;
+    WEBCORE_EXPORT double lightness() const; // FIXME: Replace remaining uses with luminance.
 
     template<typename Functor> decltype(auto) callOnUnderlyingType(Functor&&) const;
 

Added: trunk/Source/WebCore/platform/graphics/ColorLuminance.cpp (0 => 275981)


--- trunk/Source/WebCore/platform/graphics/ColorLuminance.cpp	                        (rev 0)
+++ trunk/Source/WebCore/platform/graphics/ColorLuminance.cpp	2021-04-15 00:15:02 UTC (rev 275981)
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2021 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "ColorLuminance.h"
+
+#include "Color.h"
+
+namespace WebCore {
+
+double contrastRatio(const Color& colorA, const Color& colorB)
+{
+    return colorA.callOnUnderlyingType([&] (const auto& underlyingColorA) {
+        return colorB.callOnUnderlyingType([&] (const auto& underlyingColorB) {
+            return contrastRatio(underlyingColorA, underlyingColorB);
+        });
+    });
+}
+
+}

Modified: trunk/Source/WebCore/platform/graphics/ColorLuminance.h (275980 => 275981)


--- trunk/Source/WebCore/platform/graphics/ColorLuminance.h	2021-04-15 00:11:33 UTC (rev 275980)
+++ trunk/Source/WebCore/platform/graphics/ColorLuminance.h	2021-04-15 00:15:02 UTC (rev 275981)
@@ -29,7 +29,9 @@
 
 namespace WebCore {
 
-template<typename ColorType> inline float relativeLuminance(const ColorType& color)
+class Color;
+
+template<typename ColorType> inline double relativeLuminance(const ColorType& color)
 {
     // https://en.wikipedia.org/wiki/Relative_luminance
 
@@ -46,7 +48,7 @@
     return convertColor<XYZA<float, WhitePoint::D65>>(color).y;
 }
 
-inline float contrastRatio(float relativeLuminanceA, float relativeLuminanceB)
+inline double contrastRatio(double relativeLuminanceA, double relativeLuminanceB)
 {
     // Uses the WCAG 2.0 definition of contrast ratio.
     // https://www.w3.org/TR/WCAG20/#contrast-ratiodef
@@ -59,9 +61,11 @@
     return (lighterLuminance + 0.05) / (darkerLuminance + 0.05);
 }
 
-template<typename ColorTypeA, typename ColorTypeB> inline float contrastRatio(const ColorTypeA& colorA, const ColorTypeB& colorB)
+template<typename ColorTypeA, typename ColorTypeB> inline double contrastRatio(const ColorTypeA& colorA, const ColorTypeB& colorB)
 {
     return contrastRatio(relativeLuminance(colorA), relativeLuminance(colorB));
 }
 
+double contrastRatio(const Color&, const Color&);
+
 }

Modified: trunk/Source/WebCore/rendering/RenderTheme.cpp (275980 => 275981)


--- trunk/Source/WebCore/rendering/RenderTheme.cpp	2021-04-15 00:11:33 UTC (rev 275980)
+++ trunk/Source/WebCore/rendering/RenderTheme.cpp	2021-04-15 00:15:02 UTC (rev 275981)
@@ -23,6 +23,7 @@
 
 #include "CSSValueKeywords.h"
 #include "ColorBlending.h"
+#include "ColorLuminance.h"
 #include "ControlStates.h"
 #include "Document.h"
 #include "FileList.h"
@@ -1409,7 +1410,7 @@
 #endif
 
 // Value chosen by observation. This can be tweaked.
-constexpr float minColorContrastValue = 1.195f;
+constexpr double minColorContrastValue = 1.195;
 
 // For transparent or translucent background color, use lightening.
 constexpr float minDisabledColorAlphaValue = 0.5f;
@@ -1427,7 +1428,7 @@
     // If there's not very much contrast between the disabled color and the background color,
     // just leave the text color alone. We don't want to change a good contrast color scheme so that it has really bad contrast.
     // If the contrast was already poor, then it doesn't do any good to change it to a different poor contrast color scheme.
-    if (Color::contrastRatio(disabledColor, backgroundColor) < minColorContrastValue)
+    if (contrastRatio(disabledColor, backgroundColor) < minColorContrastValue)
         return textColor;
 
     return disabledColor;

Modified: trunk/Source/WebCore/rendering/TextPaintStyle.cpp (275980 => 275981)


--- trunk/Source/WebCore/rendering/TextPaintStyle.cpp	2021-04-15 00:11:33 UTC (rev 275980)
+++ trunk/Source/WebCore/rendering/TextPaintStyle.cpp	2021-04-15 00:15:02 UTC (rev 275981)
@@ -26,7 +26,7 @@
 #include "config.h"
 #include "TextPaintStyle.h"
 
-#include "ColorUtilities.h"
+#include "ColorLuminance.h"
 #include "FocusController.h"
 #include "Frame.h"
 #include "GraphicsContext.h"
@@ -63,7 +63,7 @@
 {
     // Uses the WCAG 2.0 definition of legibility: a contrast ratio of 4.5:1 or greater.
     // https://www.w3.org/TR/WCAG20/#visual-audio-contrast-contrast
-    return Color::contrastRatio(textColor, backgroundColor) > 4.5;
+    return contrastRatio(textColor, backgroundColor) >= 4.5;
 }
 
 static Color adjustColorForVisibilityOnBackground(const Color& textColor, const Color& backgroundColor)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to