Changeset 262070 in webkit

Timestamp:

May 22, 2020 1:16:20 PM (4 years ago)

Author:

weinig@apple.com

Message:

Extended Color Cleanup: Make alpha premultiplication code more consistent and clear regarding what works with extended colors
​https://bugs.webkit.org/show_bug.cgi?id=212265

Reviewed by Simon Fraser.

• Adds premultiplied(const FloatComponents&) to do premutiplication directly on FloatComponents rather than doing it on the ints and losing precision.
• Makes non-FloatComponent alpha premultiplication all take place only for SimpleColors as that is what callers need. The existing premulitplication for ExtendedColors in blend() was incorrect as it never did a conversion to sRGB.
• Adds new toSRGBASimpleColorLossy() (to complement toSRGBAComponentsLossy()). Will make it easy to find all the conversions in the future.
• Broke non-premultiplying blend() out of blend() (removing parameter) and made a new blendWithoutPremultiply() function for it (no callers needed to make this decision dynamically).

• css/CSSGradientValue.cpp:

(WebCore::CSSGradientValue::computeStops):
Use blendWithoutPremultiply() explicitly.

• platform/graphics/Color.h:
• platform/graphics/Color.cpp:

(WebCore::makePremultipliedRGBA): Renamed from premultipliedARGBFromColor and now only operates on SimpleColors.
(WebCore::makeUnPremultipliedRGBA): Renamed from colorFromPremultipliedARGB and now only operates on SimpleColors.
(WebCore::colorFromPremultipliedARGB): Deleted.
(WebCore::premultipliedARGBFromColor): Deleted.

(WebCore::Color::toSRGBASimpleColorLossy const):
Added. Useful for finding all non-colorspace preserving users of the color channels.

(WebCore::blend):
(WebCore::blendWithoutPremultiply):
Split these out from each other. Made blend() use toSRGBASimpleColorLossy() and do all
operations on SimpleColors directly. The old code that preported to work with extended
colors was nonsense as it didn't actually take the colorspaces into account, just grabbed
the channels regardless of space.

• platform/graphics/cairo/ImageBufferCairoImageSurfaceBackend.cpp:

(WebCore::ImageBufferCairoImageSurfaceBackend::platformTransformColorSpace):
Adopt update premulitiplication names and stay in SimpleColor for entire conversion.

• platform/graphics/cairo/NativeImageCairo.cpp:

(WebCore::nativeImageSinglePixelSolidColor):
Adopt update premulitiplication names.

• platform/graphics/ColorUtilities.cpp:

(WebCore::premultiplied):

• platform/graphics/ColorUtilities.h:
• platform/graphics/texmap/TextureMapperGL.cpp:

(WebCore::TextureMapperGL::drawBorder):
(WebCore::prepareFilterProgram):
(WebCore::TextureMapperGL::drawSolidColor):
Add and adopt premultiplied(const FloatComponents&).

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• css/CSSGradientValue.cpp (modified) (1 diff)
• platform/graphics/Color.cpp (modified) (5 diffs)
• platform/graphics/Color.h (modified) (3 diffs)
• platform/graphics/ColorUtilities.cpp (modified) (1 diff)
• platform/graphics/ColorUtilities.h (modified) (1 diff)
• platform/graphics/cairo/ImageBufferCairoImageSurfaceBackend.cpp (modified) (1 diff)
• platform/graphics/cairo/NativeImageCairo.cpp (modified) (1 diff)
• platform/graphics/texmap/TextureMapperGL.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-05-22  Sam Weinig  <weinig@apple.com>
+
+        Extended Color Cleanup: Make alpha premultiplication code more consistent and clear regarding what works with extended colors
+        https://bugs.webkit.org/show_bug.cgi?id=212265
+
+        Reviewed by Simon Fraser.
+
+        - Adds premultiplied(const FloatComponents&) to do premutiplication directly on FloatComponents
+          rather than doing it on the ints and losing precision.
+        - Makes non-FloatComponent alpha premultiplication all take place only for SimpleColors as that
+          is what callers need. The existing premulitplication for ExtendedColors in blend() was incorrect
+          as it never did a conversion to sRGB.
+        - Adds new toSRGBASimpleColorLossy() (to complement toSRGBAComponentsLossy()). Will make it easy
+          to find all the conversions in the future.
+        - Broke non-premultiplying blend() out of blend() (removing parameter) and made a new blendWithoutPremultiply()
+          function for it (no callers needed to make this decision dynamically).
+
+        * css/CSSGradientValue.cpp:
+        (WebCore::CSSGradientValue::computeStops):
+        Use blendWithoutPremultiply() explicitly.
+
+        * platform/graphics/Color.h:
+        * platform/graphics/Color.cpp:
+        (WebCore::makePremultipliedRGBA): Renamed from premultipliedARGBFromColor and now only operates on SimpleColors.
+        (WebCore::makeUnPremultipliedRGBA): Renamed from colorFromPremultipliedARGB and now only operates on SimpleColors.
+        (WebCore::colorFromPremultipliedARGB): Deleted.
+        (WebCore::premultipliedARGBFromColor): Deleted.
+
+        (WebCore::Color::toSRGBASimpleColorLossy const):
+        Added. Useful for finding all non-colorspace preserving users of the color channels. 
+
+        (WebCore::blend):
+        (WebCore::blendWithoutPremultiply):
+        Split these out from each other. Made blend() use toSRGBASimpleColorLossy() and do all
+        operations on SimpleColors directly. The old code that preported to work with extended
+        colors was nonsense as it didn't actually take the colorspaces into account, just grabbed
+        the channels regardless of space.  
+        
+        * platform/graphics/cairo/ImageBufferCairoImageSurfaceBackend.cpp:
+        (WebCore::ImageBufferCairoImageSurfaceBackend::platformTransformColorSpace):
+        Adopt update premulitiplication names and stay in SimpleColor for entire conversion.
+
+        * platform/graphics/cairo/NativeImageCairo.cpp:
+        (WebCore::nativeImageSinglePixelSolidColor):
+        Adopt update premulitiplication names.
+
+        * platform/graphics/ColorUtilities.cpp:
+        (WebCore::premultiplied):
+        * platform/graphics/ColorUtilities.h:
+        * platform/graphics/texmap/TextureMapperGL.cpp:
+        (WebCore::TextureMapperGL::drawBorder):
+        (WebCore::prepareFilterProgram):
+        (WebCore::TextureMapperGL::drawSolidColor):
+        Add and adopt premultiplied(const FloatComponents&).
+
 2020-05-22  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebCore/css/CSSGradientValue.cpp

@@ -484 +484 @@
             float multiplier = std::pow(relativeOffset, std::log(.5f) / std::log(midpoint));
             // FIXME: Why not premultiply here?
-            newStops[y].color = blend(color1, color2, multiplier, false /* do not premultiply */);
+            newStops[y].color = blendWithoutPremultiply(color1, color2, multiplier);
         }
 

trunk/Source/WebCore/platform/graphics/Color.cpp

@@ -56 +56 @@
 }
 
+RGBA32 makePremultipliedRGBA(RGBA32 pixelColor)
+{
+    if (pixelColor.isOpaque())
+        return pixelColor;
+    return makePremultipliedRGBA(pixelColor.redComponent(), pixelColor.greenComponent(), pixelColor.blueComponent(), pixelColor.alphaComponent());
+}
+
 RGBA32 makeUnPremultipliedRGBA(int r, int g, int b, int a)
 {
     return makeRGBA(unpremultipliedChannel(r, a), unpremultipliedChannel(g, a), unpremultipliedChannel(b, a), a);
+}
+
+RGBA32 makeUnPremultipliedRGBA(RGBA32 pixelColor)
+{
+    if (pixelColor.isVisible() && !pixelColor.isOpaque())
+        return makeUnPremultipliedRGBA(pixelColor.redComponent(), pixelColor.greenComponent(), pixelColor.blueComponent(), pixelColor.alphaComponent());
+    return pixelColor;
 }
 
@@ -497 +511 @@
 }
 
+SimpleColor Color::toSRGBASimpleColorLossy() const
+{
+    if (!isExtended())
+        return rgb();
+
+    auto [r, g, b, a] = toSRGBAComponentsLossy();
+    return makeRGBA32FromFloats(r, g, b, a);
+}
+
 FloatComponents Color::toSRGBAComponentsLossy() const
 {
@@ -512 +535 @@
 }
 
-Color colorFromPremultipliedARGB(RGBA32 pixelColor)
-{
-    if (pixelColor.isVisible() && !pixelColor.isOpaque())
-        return makeUnPremultipliedRGBA(pixelColor.redComponent(), pixelColor.greenComponent(), pixelColor.blueComponent(), pixelColor.alphaComponent());
-    return pixelColor;
-}
-
-RGBA32 premultipliedARGBFromColor(const Color& color)
-{
-    if (color.isOpaque()) {
-        if (color.isExtended())
-            return makeRGB(color.asExtended().red() * 255, color.asExtended().green() * 255, color.asExtended().blue() * 255);
-        return color.rgb();
-    }
-
-    if (color.isExtended())
-        return makePremultipliedRGBA(color.asExtended().red() * 255, color.asExtended().green() * 255, color.asExtended().blue() * 255, color.asExtended().alpha() * 255);
-
-    return makePremultipliedRGBA(color.red(), color.green(), color.blue(), color.alpha());
-}
-
 bool extendedColorsEqual(const Color& a, const Color& b)
 {
@@ -542 +544 @@
 }
 
-Color blend(const Color& from, const Color& to, double progress, bool blendPremultiplied)
+Color blend(const Color& from, const Color& to, double progress)
 {
     // FIXME: ExtendedColor - needs to handle color spaces.
@@ -549 +551 @@
         return Color();
 
-    if (blendPremultiplied) {
-        // Since premultipliedARGBFromColor() bails on zero alpha, special-case that.
-        Color premultFrom = from.alpha() ? premultipliedARGBFromColor(from) : Color::transparent;
-        Color premultTo = to.alpha() ? premultipliedARGBFromColor(to) : Color::transparent;
-
-        Color premultBlended(blend(premultFrom.red(), premultTo.red(), progress),
-            blend(premultFrom.green(), premultTo.green(), progress),
-            blend(premultFrom.blue(), premultTo.blue(), progress),
-            blend(premultFrom.alpha(), premultTo.alpha(), progress));
-
-        return Color(colorFromPremultipliedARGB(premultBlended.rgb()));
-    }
-
-    return Color(blend(from.red(), to.red(), progress),
-        blend(from.green(), to.green(), progress),
-        blend(from.blue(), to.blue(), progress),
-        blend(from.alpha(), to.alpha(), progress));
+    // Since makePremultipliedRGBA() bails on zero alpha, special-case that.
+    auto premultFrom = from.alpha() ? makePremultipliedRGBA(from.toSRGBASimpleColorLossy()) : Color::transparent;
+    auto premultTo = to.alpha() ? makePremultipliedRGBA(to.toSRGBASimpleColorLossy()) : Color::transparent;
+
+    RGBA32 premultBlended = makeRGBA(
+        WebCore::blend(premultFrom.redComponent(), premultTo.redComponent(), progress),
+        WebCore::blend(premultFrom.greenComponent(), premultTo.greenComponent(), progress),
+        WebCore::blend(premultFrom.blueComponent(), premultTo.blueComponent(), progress),
+        WebCore::blend(premultFrom.alphaComponent(), premultTo.alphaComponent(), progress)
+    );
+
+    return makeUnPremultipliedRGBA(premultBlended);
+}
+
+Color blendWithoutPremultiply(const Color& from, const Color& to, double progress)
+{
+    // FIXME: ExtendedColor - needs to handle color spaces.
+    // We need to preserve the state of the valid flag at the end of the animation
+    if (progress == 1 && !to.isValid())
+        return { };
+
+    auto fromSRGB = from.toSRGBASimpleColorLossy();
+    auto toSRGB = from.toSRGBASimpleColorLossy();
+
+    return {
+        WebCore::blend(fromSRGB.redComponent(), toSRGB.redComponent(), progress),
+        WebCore::blend(fromSRGB.greenComponent(), toSRGB.greenComponent(), progress),
+        WebCore::blend(fromSRGB.blueComponent(), toSRGB.blueComponent(), progress),
+        WebCore::blend(fromSRGB.alphaComponent(), toSRGB.alphaComponent(), progress)
+    };
 }
 

trunk/Source/WebCore/platform/graphics/Color.h

@@ -91 +91 @@
 
 RGBA32 makePremultipliedRGBA(int r, int g, int b, int a, bool ceiling = true);
+RGBA32 makePremultipliedRGBA(RGBA32);
 RGBA32 makeUnPremultipliedRGBA(int r, int g, int b, int a);
+RGBA32 makeUnPremultipliedRGBA(RGBA32);
 
 WEBCORE_EXPORT RGBA32 makeRGBA32FromFloats(float r, float g, float b, float a);
@@ -206 +208 @@
 
     WEBCORE_EXPORT std::pair<ColorSpace, FloatComponents> colorSpaceAndComponents() const;
+
+    // This will convert non-sRGB colorspace colors into sRGB.
+    WEBCORE_EXPORT SimpleColor toSRGBASimpleColorLossy() const;
 
     // This will convert non-sRGB colorspace colors into sRGB.
@@ -318 +323 @@
 bool operator!=(const Color&, const Color&);
 
-Color colorFromPremultipliedARGB(RGBA32);
-RGBA32 premultipliedARGBFromColor(const Color&);
 // One or both must be extended colors.
 WEBCORE_EXPORT bool extendedColorsEqual(const Color&, const Color&);
 
-Color blend(const Color& from, const Color& to, double progress, bool blendPremultiplied = true);
+Color blend(const Color& from, const Color& to, double progress);
+Color blendWithoutPremultiply(const Color& from, const Color& to, double progress);
 
 int differenceSquared(const Color&, const Color&);

trunk/Source/WebCore/platform/graphics/ColorUtilities.cpp

@@ -284 +284 @@
 }
 
+FloatComponents premultiplied(const FloatComponents& sRGBComponents)
+{
+    auto [r, g, b, a] = sRGBComponents;
+    return {
+        r * a,
+        g * a,
+        b * a,
+        a
+    };
+}
+
 ColorMatrix::ColorMatrix()
 {

trunk/Source/WebCore/platform/graphics/ColorUtilities.h

@@ -192 +192 @@
 float contrastRatio(const FloatComponents&, const FloatComponents&);
 
+FloatComponents premultiplied(const FloatComponents& sRGBCompontents);
+
 class ColorMatrix {
 public:

trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoImageSurfaceBackend.cpp

@@ -81 +81 @@
         for (int x = 0; x < m_logicalSize.width(); x++) {
             unsigned* pixel = row + x;
-            Color pixelColor = colorFromPremultipliedARGB(*pixel);
-            pixelColor = Color(lookUpTable[pixelColor.red()], lookUpTable[pixelColor.green()], lookUpTable[pixelColor.blue()], pixelColor.alpha());
-            *pixel = premultipliedARGBFromColor(pixelColor).value();
+            auto pixelColor = makeUnPremultipliedRGBA(*pixel);
+            pixelColor = makeRGBA(lookUpTable[pixelColor.redComponent()], lookUpTable[pixelColor.greenComponent()], lookUpTable[pixelColor.blueComponent()], pixelColor.alphaComponent());
+            *pixel = makePremultipliedRGBA(pixelColor).value();
         }
     }

trunk/Source/WebCore/platform/graphics/cairo/NativeImageCairo.cpp

@@ -55 +55 @@
 
     RGBA32* pixel = reinterpret_cast_ptr<RGBA32*>(cairo_image_surface_get_data(image.get()));
-    return colorFromPremultipliedARGB(*pixel);
+    return makeUnPremultipliedRGBA(*pixel);
 }
 

trunk/Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp

@@ -254 +254 @@
     glUseProgram(program->programID());
 
-    // FIXME: Do the premultiply on FloatComponents directly.
-    auto [r, g, b, a] = Color(premultipliedARGBFromColor(color)).toSRGBAComponentsLossy();
+    auto [r, g, b, a] = premultiplied(color.toSRGBAComponentsLossy());
     glUniform4f(program->colorLocation(), r, g, b, a);
     glLineWidth(width);
@@ -416 +415 @@
         case 1:
             // Second pass: we need the shadow color and the content texture for compositing.
-            // FIXME: Do the premultiply on FloatComponents directly.
-            auto [r, g, b, a] = Color(premultipliedARGBFromColor(shadow.color())).toSRGBAComponentsLossy();
+            auto [r, g, b, a] = premultiplied(shadow.color().toSRGBAComponentsLossy());
             glUniform4f(program.colorLocation(), r, g, b, a);
             glUniform2f(program.blurRadiusLocation(), 0, shadow.stdDeviation() / float(size.height()));
@@ -679 +677 @@
     glUseProgram(program->programID());
 
-    // FIXME: Do the premultiply on FloatComponents directly.
-    auto [r, g, b, a] = Color(premultipliedARGBFromColor(color)).toSRGBAComponentsLossy();
+    auto [r, g, b, a] = premultiplied(color.toSRGBAComponentsLossy());
     glUniform4f(program->colorLocation(), r, g, b, a);
     if (a < 1 && isBlendingAllowed)