Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: njn, Assigned: njn)

Tracking

(Blocks 1 bug)

Trunk
mozilla44
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 fixed)

Details

Attachments

(5 attachments, 2 obsolete attachments)

A follow-up to 1207944, which started removing gfxRGBA from the tree.
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.
Attachment #8665728 - Flags: review?(matt.woodrow)
Attachment #8665728 - Attachment is obsolete: true
Attachment #8665728 - Flags: review?(matt.woodrow)
Blocks: 1208300
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 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.
Attachment #8665732 - Flags: review?(matt.woodrow) → review?(bas)
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.
Attachment #8665722 - Flags: review?(matt.woodrow) → review+
Attachment #8665726 - Flags: review?(matt.woodrow) → review+
Attachment #8665733 - Flags: review?(matt.woodrow) → review+
Attachment #8665735 - Flags: review?(matt.woodrow) → review+
Attachment #8665736 - Flags: review?(matt.woodrow) → review+
> I'm going to take these reviews as well... Hope no one minds.

Nope! Thank you.
Attachment #8665732 - Attachment is obsolete: true
Attachment #8665732 - Flags: review?(bas)
> (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.
You need to log in before you can comment on or make changes to this bug.