Diff
Modified: trunk/Source/WebCore/ChangeLog (262069 => 262070)
--- trunk/Source/WebCore/ChangeLog 2020-05-22 20:10:01 UTC (rev 262069)
+++ trunk/Source/WebCore/ChangeLog 2020-05-22 20:16:20 UTC (rev 262070)
@@ -1,3 +1,58 @@
+2020-05-22 Sam Weinig <[email protected]>
+
+ 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 <[email protected]>
[Apple Pay] Add new ApplePayInstallmentConfiguration members
Modified: trunk/Source/WebCore/css/CSSGradientValue.cpp (262069 => 262070)
--- trunk/Source/WebCore/css/CSSGradientValue.cpp 2020-05-22 20:10:01 UTC (rev 262069)
+++ trunk/Source/WebCore/css/CSSGradientValue.cpp 2020-05-22 20:16:20 UTC (rev 262070)
@@ -483,7 +483,7 @@
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);
Modified: trunk/Source/WebCore/platform/graphics/Color.cpp (262069 => 262070)
--- trunk/Source/WebCore/platform/graphics/Color.cpp 2020-05-22 20:10:01 UTC (rev 262069)
+++ trunk/Source/WebCore/platform/graphics/Color.cpp 2020-05-22 20:16:20 UTC (rev 262070)
@@ -55,11 +55,25 @@
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 @@
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 @@
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,7 +543,7 @@
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
@@ -548,23 +550,36 @@
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);
+}
- 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));
+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)
+ };
}
void Color::tagAsValid()
Modified: trunk/Source/WebCore/platform/graphics/Color.h (262069 => 262070)
--- trunk/Source/WebCore/platform/graphics/Color.h 2020-05-22 20:10:01 UTC (rev 262069)
+++ trunk/Source/WebCore/platform/graphics/Color.h 2020-05-22 20:16:20 UTC (rev 262070)
@@ -90,7 +90,9 @@
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 @@
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 @@
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&);
Modified: trunk/Source/WebCore/platform/graphics/ColorUtilities.cpp (262069 => 262070)
--- trunk/Source/WebCore/platform/graphics/ColorUtilities.cpp 2020-05-22 20:10:01 UTC (rev 262069)
+++ trunk/Source/WebCore/platform/graphics/ColorUtilities.cpp 2020-05-22 20:16:20 UTC (rev 262070)
@@ -283,6 +283,17 @@
};
}
+FloatComponents premultiplied(const FloatComponents& sRGBComponents)
+{
+ auto [r, g, b, a] = sRGBComponents;
+ return {
+ r * a,
+ g * a,
+ b * a,
+ a
+ };
+}
+
ColorMatrix::ColorMatrix()
{
makeIdentity();
Modified: trunk/Source/WebCore/platform/graphics/ColorUtilities.h (262069 => 262070)
--- trunk/Source/WebCore/platform/graphics/ColorUtilities.h 2020-05-22 20:10:01 UTC (rev 262069)
+++ trunk/Source/WebCore/platform/graphics/ColorUtilities.h 2020-05-22 20:16:20 UTC (rev 262070)
@@ -191,6 +191,8 @@
float luminance(const FloatComponents& sRGBCompontents);
float contrastRatio(const FloatComponents&, const FloatComponents&);
+FloatComponents premultiplied(const FloatComponents& sRGBCompontents);
+
class ColorMatrix {
public:
static ColorMatrix grayscaleMatrix(float);
Modified: trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoImageSurfaceBackend.cpp (262069 => 262070)
--- trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoImageSurfaceBackend.cpp 2020-05-22 20:10:01 UTC (rev 262069)
+++ trunk/Source/WebCore/platform/graphics/cairo/ImageBufferCairoImageSurfaceBackend.cpp 2020-05-22 20:16:20 UTC (rev 262070)
@@ -80,9 +80,9 @@
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());
Modified: trunk/Source/WebCore/platform/graphics/cairo/NativeImageCairo.cpp (262069 => 262070)
--- trunk/Source/WebCore/platform/graphics/cairo/NativeImageCairo.cpp 2020-05-22 20:10:01 UTC (rev 262069)
+++ trunk/Source/WebCore/platform/graphics/cairo/NativeImageCairo.cpp 2020-05-22 20:16:20 UTC (rev 262070)
@@ -54,7 +54,7 @@
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)
Modified: trunk/Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp (262069 => 262070)
--- trunk/Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp 2020-05-22 20:10:01 UTC (rev 262069)
+++ trunk/Source/WebCore/platform/graphics/texmap/TextureMapperGL.cpp 2020-05-22 20:16:20 UTC (rev 262070)
@@ -253,8 +253,7 @@
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 @@
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 @@
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;
