Extended Color Cleanup: Make alpha premultiplication code more consistent and clear...
authorweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 20:16:20 +0000 (20:16 +0000)
committerweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 20:16:20 +0000 (20:16 +0000)
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&).

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

Source/WebCore/ChangeLog
Source/WebCore/css/CSSGradientValue.cpp
Source/WebCore/platform/graphics/Color.cpp
Source/WebCore/platform/graphics/Color.h
Source/WebCore/platform/graphics/ColorUtilities.cpp
Source/WebCore/platform/graphics/ColorUtilities.h
Source/WebCore/platform/graphics/cairo/ImageBufferCairoImageSurfaceBackend.cpp
Source/WebCore/platform/graphics/cairo/NativeImageCairo.cpp
Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp

index b605374..a48ee26 100644 (file)
@@ -1,3 +1,58 @@
+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>
 
         [Apple Pay] Add new ApplePayInstallmentConfiguration members
index 8f981ed..4b99d9b 100644 (file)
@@ -483,7 +483,7 @@ Gradient::ColorStopVector CSSGradientValue::computeStops(GradientAdapter& gradie
             float relativeOffset = (*newStops[y].offset - offset1) / (offset2 - offset1);
             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);
         }
 
         stops.remove(x);
index fda96d0..3d07cc6 100644 (file)
@@ -55,11 +55,25 @@ RGBA32 makePremultipliedRGBA(int r, int g, int b, int a, bool ceiling)
     return makeRGBA(premultipliedChannel(r, a, ceiling), premultipliedChannel(g, a, ceiling), premultipliedChannel(b, a, ceiling), a);
 }
 
+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;
+}
+
 static int colorFloatToRGBAByte(float f)
 {
     // We use lroundf and 255 instead of nextafterf(256, 0) to match CG's rounding
@@ -496,6 +510,15 @@ std::pair<ColorSpace, FloatComponents> Color::colorSpaceAndComponents() const
     return { ColorSpace::SRGB, FloatComponents { red() / 255.0f, green() / 255.0f, blue() / 255.0f,  alpha() / 255.0f } };
 }
 
+SimpleColor Color::toSRGBASimpleColorLossy() const
+{
+    if (!isExtended())
+        return rgb();
+
+    auto [r, g, b, a] = toSRGBAComponentsLossy();
+    return makeRGBA32FromFloats(r, g, b, a);
+}
+
 FloatComponents Color::toSRGBAComponentsLossy() const
 {
     auto [colorSpace, components] = colorSpaceAndComponents();
@@ -511,27 +534,6 @@ FloatComponents Color::toSRGBAComponentsLossy() const
     return { };
 }
 
-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)
 {
     if (a.isExtended() && b.isExtended())
@@ -541,30 +543,43 @@ bool extendedColorsEqual(const Color& a, const Color& b)
     return false;
 }
 
-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.
     // We need to preserve the state of the valid flag at the end of the animation
     if (progress == 1 && !to.isValid())
         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;
+    // 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;
 
-        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));
+    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 Color(colorFromPremultipliedARGB(premultBlended.rgb()));
-    }
+    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 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));
+    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)
+    };
 }
 
 void Color::tagAsValid()
index ddb7c2e..ce5b91f 100644 (file)
@@ -90,7 +90,9 @@ constexpr RGBA32 makeRGB(int r, int g, int b);
 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 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);
 WEBCORE_EXPORT RGBA32 makeRGBAFromHSLA(float h, float s, float l, float a);
@@ -207,6 +209,9 @@ public:
     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.
     WEBCORE_EXPORT FloatComponents toSRGBAComponentsLossy() const;
 
     Color light() const;
@@ -317,12 +322,11 @@ private:
 bool operator==(const Color&, const Color&);
 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&);
 
index 97c41ce..c63273a 100644 (file)
@@ -283,6 +283,17 @@ FloatComponents hslToSRGB(const FloatComponents& hslColor)
     };
 }
 
+FloatComponents premultiplied(const FloatComponents& sRGBComponents)
+{
+    auto [r, g, b, a] = sRGBComponents;
+    return {
+        r * a,
+        g * a,
+        b * a,
+        a
+    };
+}
+
 ColorMatrix::ColorMatrix()
 {
     makeIdentity();
index a49b755..b9a1f06 100644 (file)
@@ -191,6 +191,8 @@ float lightness(const FloatComponents& sRGBCompontents);
 float luminance(const FloatComponents& sRGBCompontents);
 float contrastRatio(const FloatComponents&, const FloatComponents&);
 
+FloatComponents premultiplied(const FloatComponents& sRGBCompontents);
+
 class ColorMatrix {
 public:
     static ColorMatrix grayscaleMatrix(float);
index 40bf734..3e68849 100644 (file)
@@ -80,9 +80,9 @@ void ImageBufferCairoImageSurfaceBackend::platformTransformColorSpace(const std:
         unsigned* row = reinterpret_cast_ptr<unsigned*>(dataSrc + stride * y);
         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();
         }
     }
     cairo_surface_mark_dirty_rectangle(m_surface.get(), 0, 0, m_logicalSize.width(), m_logicalSize.height());
index de304f3..d89810a 100644 (file)
@@ -54,7 +54,7 @@ Color nativeImageSinglePixelSolidColor(const NativeImagePtr& image)
         return Color();
 
     RGBA32* pixel = reinterpret_cast_ptr<RGBA32*>(cairo_image_surface_get_data(image.get()));
-    return colorFromPremultipliedARGB(*pixel);
+    return makeUnPremultipliedRGBA(*pixel);
 }
 
 void drawNativeImage(const NativeImagePtr& image, GraphicsContext& context, const FloatRect& destRect, const FloatRect& srcRect, const IntSize& imageSize, const ImagePaintingOptions& options)
index e6699bf..212cff6 100644 (file)
@@ -253,8 +253,7 @@ void TextureMapperGL::drawBorder(const Color& color, float width, const FloatRec
     Ref<TextureMapperShaderProgram> program = data().getShaderProgram(TextureMapperShaderProgram::SolidColor);
     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);
 
@@ -415,8 +414,7 @@ static void prepareFilterProgram(TextureMapperShaderProgram& program, const Filt
             break;
         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()));
             glUniform2f(program.shadowOffsetLocation(), 0, 0);
@@ -678,8 +676,7 @@ void TextureMapperGL::drawSolidColor(const FloatRect& rect, const Transformation
     Ref<TextureMapperShaderProgram> program = data().getShaderProgram(options);
     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)
         flags |= ShouldBlend;