Skip to content

Commit

Permalink
Bug 1618345 - Enforce proper color management by splitting gfx::Color…
Browse files Browse the repository at this point in the history
… into sRGBColor and DeviceColor types. r=jrmuizel

gfx::Color is currently misused in many places. The DrawTargets expect
the color space to be in device space, e.g. what we are actually going
to draw using. Everything sitting above generally deals with sRGB, as
specified in CSS. Sometimes we missed the conversion from sRGB to device
space when issuing draw calls, and similarly sometimes we converted the
color to device space twice.

This patch splits the type in two. sRGBColor and DeviceColor now
represent sRGB and device color spaces respectively. DrawTarget only
accepts DeviceColor, and one can get a DeviceColor from an sRGBColor via
the ToDeviceColor helper API. The reftests now pass with color
management enabled for everything (e.g. CSS) instead of just tagged
raster images.

There will be a follow up patch to enable color management everywhere by
default on all supported platforms.

Differential Revision: https://phabricator.services.mozilla.com/D64771
  • Loading branch information
aosmond committed Mar 9, 2020
1 parent d00ed55 commit 65665d4
Show file tree
Hide file tree
Showing 172 changed files with 929 additions and 736 deletions.
8 changes: 4 additions & 4 deletions dom/canvas/CanvasRenderingContext2D.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ bool CanvasRenderingContext2D::PatternIsOpaque(
// TODO: for gradient patterns we could check that all stops are opaque
// colors.
// it's a color pattern.
opaque = Color::FromABGR(state.colorStyles[aStyle]).a >= 1.0;
opaque = sRGBColor::FromABGR(state.colorStyles[aStyle]).a >= 1.0;
color = true;
}
}
Expand Down Expand Up @@ -524,7 +524,7 @@ class AdjustedTargetForShadow {

mFinalTarget->DrawSurfaceWithShadow(
snapshot, mTempRect.TopLeft(),
Color::FromABGR(mCtx->CurrentState().shadowColor),
ToDeviceColor(mCtx->CurrentState().shadowColor),
mCtx->CurrentState().shadowOffset, mSigma, mCompositionOp);
}

Expand Down Expand Up @@ -730,7 +730,7 @@ void CanvasGradient::AddColorStop(float aOffset, const nsACString& aColorstr,
GradientStop newStop;

newStop.offset = aOffset;
newStop.color = Color::FromABGR(color);
newStop.color = ToDeviceColor(color);

mRawStops.AppendElement(newStop);
}
Expand Down Expand Up @@ -3641,7 +3641,7 @@ struct MOZ_STACK_CLASS CanvasBidiProcessor
if (state->StyleIsColor(style)) { // Color
nscolor fontColor = state->colorStyles[style];
if (style == Style::FILL) {
params.context->SetColor(Color::FromABGR(fontColor));
params.context->SetColor(sRGBColor::FromABGR(fontColor));
} else {
params.textStrokeColor = fontColor;
}
Expand Down
14 changes: 7 additions & 7 deletions dom/plugins/ipc/PluginInstanceChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3228,7 +3228,7 @@ void PluginInstanceChild::PaintRectToPlatformSurface(const nsIntRect& aRect,

void PluginInstanceChild::PaintRectToSurface(const nsIntRect& aRect,
gfxASurface* aSurface,
const Color& aColor) {
const DeviceColor& aColor) {
// Render using temporary X surface, with copy to image surface
nsIntRect plPaintRect(aRect);
RefPtr<gfxASurface> renderSurface = aSurface;
Expand Down Expand Up @@ -3335,7 +3335,7 @@ void PluginInstanceChild::PaintRectWithAlphaExtraction(const nsIntRect& aRect,

// Paint the plugin directly onto the target, with a white
// background and copy the result
PaintRectToSurface(rect, aSurface, Color(1.f, 1.f, 1.f));
PaintRectToSurface(rect, aSurface, DeviceColor::MaskOpaqueWhite());
{
RefPtr<DrawTarget> dt = CreateDrawTargetForSurface(whiteImage);
RefPtr<SourceSurface> surface =
Expand All @@ -3345,7 +3345,7 @@ void PluginInstanceChild::PaintRectWithAlphaExtraction(const nsIntRect& aRect,

// Paint the plugin directly onto the target, with a black
// background
PaintRectToSurface(rect, aSurface, Color(0.f, 0.f, 0.f));
PaintRectToSurface(rect, aSurface, DeviceColor::MaskOpaqueBlack());

// Don't copy the result, just extract a subimage so that we can
// recover alpha directly into the target
Expand All @@ -3356,7 +3356,7 @@ void PluginInstanceChild::PaintRectWithAlphaExtraction(const nsIntRect& aRect,
gfxPoint deviceOffset = -targetRect.TopLeft();
// Paint onto white background
whiteImage->SetDeviceOffset(deviceOffset);
PaintRectToSurface(rect, whiteImage, Color(1.f, 1.f, 1.f));
PaintRectToSurface(rect, whiteImage, DeviceColor::MaskOpaqueWhite());

if (useSurfaceSubimageForBlack) {
gfxImageSurface* surface = static_cast<gfxImageSurface*>(aSurface);
Expand All @@ -3368,7 +3368,7 @@ void PluginInstanceChild::PaintRectWithAlphaExtraction(const nsIntRect& aRect,

// Paint onto black background
blackImage->SetDeviceOffset(deviceOffset);
PaintRectToSurface(rect, blackImage, Color(0.f, 0.f, 0.f));
PaintRectToSurface(rect, blackImage, DeviceColor::MaskOpaqueBlack());
#endif

MOZ_ASSERT(whiteImage && blackImage, "Didn't paint enough!");
Expand Down Expand Up @@ -3528,7 +3528,7 @@ bool PluginInstanceChild::ShowPluginFrame() {
}
// ... and hand off to the plugin
// BEWARE: mBackground may die during this call
PaintRectToSurface(rect, mCurrentSurface, Color());
PaintRectToSurface(rect, mCurrentSurface, DeviceColor());
} else if (!temporarilyMakeVisible && mDoAlphaExtraction) {
// We don't want to pay the expense of alpha extraction for
// phony paints.
Expand All @@ -3545,7 +3545,7 @@ bool PluginInstanceChild::ShowPluginFrame() {
? mHelperSurface
: mCurrentSurface;

PaintRectToSurface(rect, target, Color());
PaintRectToSurface(rect, target, DeviceColor());
}
mHasPainted = true;

Expand Down
2 changes: 1 addition & 1 deletion dom/plugins/ipc/PluginInstanceChild.h
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ class PluginInstanceChild : public PPluginInstanceChild {

// Paint plugin content rectangle to surface with bg color filling
void PaintRectToSurface(const nsIntRect& aRect, gfxASurface* aSurface,
const gfx::Color& aColor);
const gfx::DeviceColor& aColor);

// Render plugin content to surface using
// white/black image alpha extraction algorithm
Expand Down
5 changes: 3 additions & 2 deletions dom/svg/SVGFEDropShadowElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,12 @@ FilterPrimitiveDescription SVGFEDropShadowElement::GetPrimitiveDescription(
nsIFrame* frame = GetPrimaryFrame();
if (frame) {
const nsStyleSVGReset* styleSVGReset = frame->Style()->StyleSVGReset();
Color color(Color::FromABGR(styleSVGReset->mFloodColor.CalcColor(frame)));
sRGBColor color(
sRGBColor::FromABGR(styleSVGReset->mFloodColor.CalcColor(frame)));
color.a *= styleSVGReset->mFloodOpacity;
atts.mColor = color;
} else {
atts.mColor = Color();
atts.mColor = sRGBColor();
}
return FilterPrimitiveDescription(AsVariant(std::move(atts)));
}
Expand Down
5 changes: 3 additions & 2 deletions dom/svg/SVGFEFloodElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ FilterPrimitiveDescription SVGFEFloodElement::GetPrimitiveDescription(
nsIFrame* frame = GetPrimaryFrame();
if (frame) {
const nsStyleSVGReset* styleSVGReset = frame->Style()->StyleSVGReset();
Color color(Color::FromABGR(styleSVGReset->mFloodColor.CalcColor(frame)));
sRGBColor color(
sRGBColor::FromABGR(styleSVGReset->mFloodColor.CalcColor(frame)));
color.a *= styleSVGReset->mFloodOpacity;
atts.mColor = color;
} else {
atts.mColor = Color();
atts.mColor = sRGBColor();
}
return FilterPrimitiveDescription(AsVariant(std::move(atts)));
}
Expand Down
2 changes: 1 addition & 1 deletion dom/svg/SVGFETurbulenceElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ FilterPrimitiveDescription SVGFETurbulenceElement::GetPrimitiveDescription(
return FilterPrimitiveDescription();
}
FloodAttributes atts;
atts.mColor = Color(0.5, 0.5, 0.5, 0.5);
atts.mColor = sRGBColor(0.5, 0.5, 0.5, 0.5);
return FilterPrimitiveDescription(AsVariant(std::move(atts)));
}

Expand Down
3 changes: 2 additions & 1 deletion dom/svg/SVGFilters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,8 @@ bool SVGFELightingElement::AddLightingAttributes(
}

const nsStyleSVGReset* styleSVGReset = frame->Style()->StyleSVGReset();
Color color(Color::FromABGR(styleSVGReset->mLightingColor.CalcColor(frame)));
sRGBColor color(
sRGBColor::FromABGR(styleSVGReset->mLightingColor.CalcColor(frame)));
color.a = 1.f;
float surfaceScale = mNumberAttributes[SURFACE_SCALE].GetAnimValue();
Size kernelUnitLength = GetKernelUnitLength(
Expand Down
11 changes: 6 additions & 5 deletions gfx/2d/2D.h
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,11 @@ class ColorPattern : public Pattern {
public:
// Explicit because consumers should generally use ToDeviceColor when
// creating a ColorPattern.
explicit ColorPattern(const Color& aColor) : mColor(aColor) {}
explicit ColorPattern(const DeviceColor& aColor) : mColor(aColor) {}

PatternType GetType() const override { return PatternType::COLOR; }

Color mColor;
DeviceColor mColor;
};

/**
Expand Down Expand Up @@ -1175,7 +1175,8 @@ class DrawTarget : public external::AtomicRefCounted<DrawTarget> {
* @param aOperator Composition operator used
*/
virtual void DrawSurfaceWithShadow(SourceSurface* aSurface,
const Point& aDest, const Color& aColor,
const Point& aDest,
const DeviceColor& aColor,
const Point& aOffset, Float aSigma,
CompositionOp aOperator) = 0;

Expand Down Expand Up @@ -1800,8 +1801,8 @@ class GFX2D_API Factory {
#ifdef XP_DARWIN
static already_AddRefed<ScaledFont> CreateScaledFontForMacFont(
CGFontRef aCGFont, const RefPtr<UnscaledFont>& aUnscaledFont, Float aSize,
const Color& aFontSmoothingBackgroundColor, bool aUseFontSmoothing = true,
bool aApplySyntheticBold = false);
const DeviceColor& aFontSmoothingBackgroundColor,
bool aUseFontSmoothing = true, bool aApplySyntheticBold = false);
#endif

#ifdef MOZ_WIDGET_GTK
Expand Down
4 changes: 2 additions & 2 deletions gfx/2d/CGTextDrawing.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ GetCGContextSetFontSmoothingBackgroundColorFunc() {
return func;
}

static CGColorRef ColorToCGColor(CGColorSpaceRef aColorSpace, const Color& aColor) {
static CGColorRef ColorToCGColor(CGColorSpaceRef aColorSpace, const DeviceColor& aColor) {
CGFloat components[4] = {aColor.r, aColor.g, aColor.b, aColor.a};
return CGColorCreate(aColorSpace, components);
}

static bool SetFontSmoothingBackgroundColor(CGContextRef aCGContext, CGColorSpaceRef aColorSpace,
const Color& aFontSmoothingBackgroundColor) {
const DeviceColor& aFontSmoothingBackgroundColor) {
if (aFontSmoothingBackgroundColor.a > 0) {
CGContextSetFontSmoothingBackgroundColorFunc setFontSmoothingBGColorFunc =
GetCGContextSetFontSmoothingBackgroundColorFunc();
Expand Down
4 changes: 2 additions & 2 deletions gfx/2d/DrawCommands.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ class DrawSurfaceCommand : public DrawingCommand {
class DrawSurfaceWithShadowCommand : public DrawingCommand {
public:
DrawSurfaceWithShadowCommand(SourceSurface* aSurface, const Point& aDest,
const Color& aColor, const Point& aOffset,
const DeviceColor& aColor, const Point& aOffset,
Float aSigma, CompositionOp aOperator)
: mSurface(aSurface),
mDest(aDest),
Expand Down Expand Up @@ -192,7 +192,7 @@ class DrawSurfaceWithShadowCommand : public DrawingCommand {
private:
RefPtr<SourceSurface> mSurface;
Point mDest;
Color mColor;
DeviceColor mColor;
Point mOffset;
Float mSigma;
CompositionOp mOperator;
Expand Down
4 changes: 2 additions & 2 deletions gfx/2d/DrawTargetCairo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ static cairo_pattern_t* GfxPatternToCairoPattern(const Pattern& aPattern,

switch (aPattern.GetType()) {
case PatternType::COLOR: {
Color color = static_cast<const ColorPattern&>(aPattern).mColor;
DeviceColor color = static_cast<const ColorPattern&>(aPattern).mColor;
pat = cairo_pattern_create_rgba(color.r, color.g, color.b,
color.a * aAlpha);
break;
Expand Down Expand Up @@ -849,7 +849,7 @@ void DrawTargetCairo::DrawFilter(FilterNode* aNode, const Rect& aSourceRect,

void DrawTargetCairo::DrawSurfaceWithShadow(SourceSurface* aSurface,
const Point& aDest,
const Color& aColor,
const DeviceColor& aColor,
const Point& aOffset, Float aSigma,
CompositionOp aOperator) {
if (aSurface->GetType() != SurfaceType::CAIRO) {
Expand Down
3 changes: 2 additions & 1 deletion gfx/2d/DrawTargetCairo.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@ class DrawTargetCairo final : public DrawTarget {
const Point& aDestPoint,
const DrawOptions& aOptions = DrawOptions()) override;
virtual void DrawSurfaceWithShadow(SourceSurface* aSurface,
const Point& aDest, const Color& aColor,
const Point& aDest,
const DeviceColor& aColor,
const Point& aOffset, Float aSigma,
CompositionOp aOperator) override;

Expand Down
2 changes: 1 addition & 1 deletion gfx/2d/DrawTargetCapture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ void DrawTargetCaptureImpl::DrawSurface(SourceSurface* aSurface,
}

void DrawTargetCaptureImpl::DrawSurfaceWithShadow(
SourceSurface* aSurface, const Point& aDest, const Color& aColor,
SourceSurface* aSurface, const Point& aDest, const DeviceColor& aColor,
const Point& aOffset, Float aSigma, CompositionOp aOperator) {
aSurface->GuaranteePersistance();
AppendCommand(DrawSurfaceWithShadowCommand)(aSurface, aDest, aColor, aOffset,
Expand Down
2 changes: 1 addition & 1 deletion gfx/2d/DrawTargetCapture.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class DrawTargetCaptureImpl final : public DrawTargetCapture {
const Point& aDestPoint,
const DrawOptions& aOptions = DrawOptions()) override;
void DrawSurfaceWithShadow(SourceSurface* aSurface, const Point& aDest,
const Color& aColor, const Point& aOffset,
const DeviceColor& aColor, const Point& aOffset,
Float aSigma, CompositionOp aOperator) override;

void ClearRect(const Rect& aRect) override;
Expand Down
12 changes: 6 additions & 6 deletions gfx/2d/DrawTargetD2D1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ void DrawTargetD2D1::DrawSurface(SourceSurface* aSurface, const Rect& aDest,
const Rect& aSource,
const DrawSurfaceOptions& aSurfOptions,
const DrawOptions& aOptions) {
PrepareForDrawing(aOptions.mCompositionOp, ColorPattern(Color()));
PrepareForDrawing(aOptions.mCompositionOp, ColorPattern(DeviceColor()));

D2D1_RECT_F samplingBounds;

Expand Down Expand Up @@ -251,7 +251,7 @@ void DrawTargetD2D1::DrawSurface(SourceSurface* aSurface, const Rect& aDest,
mDC->FillRectangle(D2DRect(aDest), brush);
}

FinalizeDrawing(aOptions.mCompositionOp, ColorPattern(Color()));
FinalizeDrawing(aOptions.mCompositionOp, ColorPattern(DeviceColor()));
}

void DrawTargetD2D1::DrawFilter(FilterNode* aNode, const Rect& aSourceRect,
Expand All @@ -262,7 +262,7 @@ void DrawTargetD2D1::DrawFilter(FilterNode* aNode, const Rect& aSourceRect,
return;
}

PrepareForDrawing(aOptions.mCompositionOp, ColorPattern(Color()));
PrepareForDrawing(aOptions.mCompositionOp, ColorPattern(DeviceColor()));

mDC->SetAntialiasMode(D2DAAMode(aOptions.mAntialiasMode));

Expand All @@ -289,12 +289,12 @@ void DrawTargetD2D1::DrawFilter(FilterNode* aNode, const Rect& aSourceRect,
imageBrush);
}

FinalizeDrawing(aOptions.mCompositionOp, ColorPattern(Color()));
FinalizeDrawing(aOptions.mCompositionOp, ColorPattern(DeviceColor()));
}

void DrawTargetD2D1::DrawSurfaceWithShadow(SourceSurface* aSurface,
const Point& aDest,
const Color& aColor,
const DeviceColor& aColor,
const Point& aOffset, Float aSigma,
CompositionOp aOperator) {
if (!EnsureInitialized()) {
Expand Down Expand Up @@ -1954,7 +1954,7 @@ already_AddRefed<ID2D1Brush> DrawTargetD2D1::CreateBrushForPattern(
}

if (aPattern.GetType() == PatternType::COLOR) {
Color color = static_cast<const ColorPattern*>(&aPattern)->mColor;
DeviceColor color = static_cast<const ColorPattern*>(&aPattern)->mColor;
return GetSolidColorBrush(
D2D1::ColorF(color.r, color.g, color.b, color.a * aAlpha));
}
Expand Down
3 changes: 2 additions & 1 deletion gfx/2d/DrawTargetD2D1.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class DrawTargetD2D1 : public DrawTarget {
const Point& aDestPoint,
const DrawOptions& aOptions = DrawOptions()) override;
virtual void DrawSurfaceWithShadow(SourceSurface* aSurface,
const Point& aDest, const Color& aColor,
const Point& aDest,
const DeviceColor& aColor,
const Point& aOffset, Float aSigma,
CompositionOp aOperator) override;
virtual void ClearRect(const Rect& aRect) override;
Expand Down
6 changes: 3 additions & 3 deletions gfx/2d/DrawTargetDual.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ void DrawTargetDual::DrawSurface(SourceSurface* aSurface, const Rect& aDest,

void DrawTargetDual::DrawSurfaceWithShadow(SourceSurface* aSurface,
const Point& aDest,
const Color& aColor,
const DeviceColor& aColor,
const Point& aOffset, Float aSigma,
CompositionOp aOp) {
DualSurface surface(aSurface);
Expand All @@ -114,8 +114,8 @@ void DrawTargetDual::MaskSurface(const Pattern& aSource, SourceSurface* aMask,
}

void DrawTargetDual::ClearRect(const Rect& aRect) {
mA->FillRect(aRect, ColorPattern(Color(0.0, 0.0, 0.0, 1.0)));
mB->FillRect(aRect, ColorPattern(Color(1.0, 1.0, 1.0, 1.0)));
mA->FillRect(aRect, ColorPattern(DeviceColor::MaskOpaqueBlack()));
mB->FillRect(aRect, ColorPattern(DeviceColor::MaskOpaqueWhite()));
}

void DrawTargetDual::CopySurface(SourceSurface* aSurface,
Expand Down
3 changes: 2 additions & 1 deletion gfx/2d/DrawTargetDual.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ class DrawTargetDual : public DrawTarget {
const DrawOptions& aOptions = DrawOptions()) override;

virtual void DrawSurfaceWithShadow(SourceSurface* aSurface,
const Point& aDest, const Color& aColor,
const Point& aDest,
const DeviceColor& aColor,
const Point& aOffset, Float aSigma,
CompositionOp aOp) override;

Expand Down
2 changes: 1 addition & 1 deletion gfx/2d/DrawTargetOffset.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class DrawTargetOffset : public DrawTarget {
const Point& aDestPoint,
const DrawOptions& aOptions = DrawOptions()) override;
virtual void DrawSurfaceWithShadow(
SourceSurface* aSurface, const Point& aDest, const Color& aColor,
SourceSurface* aSurface, const Point& aDest, const DeviceColor& aColor,
const Point& aOffset, Float aSigma,
CompositionOp aOperator) override { /* Not implemented */
MOZ_CRASH("GFX: DrawSurfaceWithShadow");
Expand Down
4 changes: 2 additions & 2 deletions gfx/2d/DrawTargetRecording.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ class FilterNodeRecording : public FilterNode {
FORWARD_SET_ATTRIBUTE(const Matrix&, MATRIX);
FORWARD_SET_ATTRIBUTE(const Matrix5x4&, MATRIX5X4);
FORWARD_SET_ATTRIBUTE(const Point3D&, POINT3D);
FORWARD_SET_ATTRIBUTE(const Color&, COLOR);
FORWARD_SET_ATTRIBUTE(const DeviceColor&, COLOR);

#undef FORWARD_SET_ATTRIBUTE

Expand Down Expand Up @@ -372,7 +372,7 @@ void DrawTargetRecording::DrawDependentSurface(
}

void DrawTargetRecording::DrawSurfaceWithShadow(
SourceSurface* aSurface, const Point& aDest, const Color& aColor,
SourceSurface* aSurface, const Point& aDest, const DeviceColor& aColor,
const Point& aOffset, Float aSigma, CompositionOp aOp) {
EnsureSurfaceStoredRecording(mRecorder, aSurface, "DrawSurfaceWithShadow");

Expand Down
3 changes: 2 additions & 1 deletion gfx/2d/DrawTargetRecording.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ class DrawTargetRecording : public DrawTarget {
* aOperator Composition operator used
*/
virtual void DrawSurfaceWithShadow(SourceSurface* aSurface,
const Point& aDest, const Color& aColor,
const Point& aDest,
const DeviceColor& aColor,
const Point& aOffset, Float aSigma,
CompositionOp aOperator) override;

Expand Down
Loading

0 comments on commit 65665d4

Please sign in to comment.