1208283 - Remove gfxRGBA (part 2)

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Nicholas Nethercote [inactive]]

A follow-up to 1207944, which started removing gfxRGBA from the tree.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: (part 1) - Change gfxPattern's single-arg constructor to take a gfx::Color& instead of a gfxRGBA&

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Change ColorLayer::mColor, ColorLayerProperties::mColor, ReadbackLayer::mBackgroundColor from gfxRBGA to gfx::Color

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: (part 2) - Rename Color::{To,From}ARGB() so they aren't easily confused with {To,From}ABGR()

This is a little weird, arguably, but while doing this refactoring I kept
worrying I'd type the wrong name and I went back multiple times to check, and
then I got sick of doing that and decided to make it an impossible mistake to
make.

Comment 4

[Nicholas Nethercote [inactive]]

Attachment: (part 3) - Rename Color::{To,From}ARGB() so they aren't easily confused with {To,From}ABGR()

Fix the part number.

Comment 5

[Nicholas Nethercote [inactive]]

Attachment: (part 4) - Remove the ToDeviceColor() that takes a gfxRGBA

Comment 6

[Nicholas Nethercote [inactive]]

Attachment: (part 5) - Change FrameMetrics::mBackgroundColor from gfxRGBA to gfx::Color

Comment 7

[Nicholas Nethercote [inactive]]

Attachment: (part 6) - Pass a gfx::Color instead of a gfxRGBA to PaintRectToSurface()

Comment 8

[Jonathan Watt [:jwatt]]

I'm going to take these reviews as well, since Matt's timezone means he's now into his weekend and I'd like to keep this moving while you're churning out patches. Hope no one minds.

Comment 9

[Jonathan Watt [:jwatt]]

Comment on attachment 8665732 [details] [diff] [review]
(part 3) - Rename Color::{To,From}ARGB() so they aren't easily confused with {To,From}ABGR()

Redirecting to Bas, since this concerns Moz2D API.

Comment 10

[Jonathan Watt [:jwatt]]

Comment on attachment 8665722 [details] [diff] [review]
(part 1) - Change gfxPattern's single-arg constructor to take a gfx::Color& instead of a gfxRGBA&

Review of attachment 8665722 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxSVGGlyphs.cpp
@@ +39,5 @@
>  typedef mozilla::dom::Element Element;
>  
>  mozilla::gfx::UserDataKey gfxTextContextPaint::sUserDataKey;
>  
> +/* static */ const Color SimpleTextContextPaint::sZero = Color();

Boo for the implicit defaults. ;)

::: layout/svg/nsSVGGradientFrame.cpp
@@ +234,5 @@
>  
>    // SVG specification says that no stops should be treated like
>    // the corresponding fill or stroke had "none" specified.
>    if (nStops == 0) {
> +    nsRefPtr<gfxPattern> pattern = new gfxPattern(gfx::Color());

Layout code generally uses mozilla:gfx in .cpp files. Add another 'using' statement at the top of this file so you don't need the gfx prefix.

@@ +244,5 @@
>      // gradient step if there are more than one.
>      float stopOpacity = stopFrames[nStops-1]->StyleSVGReset()->mStopOpacity;
>      nscolor stopColor = stopFrames[nStops-1]->StyleSVGReset()->mStopColor;
>  
> +    gfx::Color stopColor2 = gfx::Color::FromABGR(stopColor);

Here too.

@@ +299,5 @@
>        offset = lastOffset;
>      else
>        lastOffset = offset;
>  
> +    gfx::Color stopColor2 = gfx::Color::FromABGR(stopColor);

And here.

Comment 11

[Nicholas Nethercote [inactive]]

> I'm going to take these reviews as well... Hope no one minds.

Nope! Thank you.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8fbfb98cba6b
https://hg.mozilla.org/integration/mozilla-inbound/rev/daf573ce289f
https://hg.mozilla.org/integration/mozilla-inbound/rev/1795954013f5
https://hg.mozilla.org/integration/mozilla-inbound/rev/715b6528b7aa
https://hg.mozilla.org/integration/mozilla-inbound/rev/02072bae0239

Comment 13

[Nicholas Nethercote [inactive]]

> (part 2) - Change ColorLayer::mColor, ColorLayerProperties::mColor,
> ReadbackLayer::mBackgroundColor from gfxRBGA to gfx::Color

I landed all the patches except this one, which I spun off into bug 1208945, because it's something of a tangent to the other patches.

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8fbfb98cba6b
https://hg.mozilla.org/mozilla-central/rev/daf573ce289f
https://hg.mozilla.org/mozilla-central/rev/1795954013f5
https://hg.mozilla.org/mozilla-central/rev/715b6528b7aa
https://hg.mozilla.org/mozilla-central/rev/02072bae0239