885020 - Make Mask() use MaskSurface when drawing with EXTEND_NONE

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: Make Mask() use MaskSurface when drawing with EXTEND_NONE

This code becomes a little ugly but it does seem to work.

Comment 1

[Bas Schouten (:bas.schouten)]

Comment on attachment 764989 [details] [diff] [review]
Make Mask() use MaskSurface when drawing with EXTEND_NONE

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

I agree with the idea, suggesting a restructuring of the code to make it cleaner.

::: gfx/thebes/gfxContext.cpp
@@ +1447,1 @@
>        }

I see almost no difference between the Moz2D and non-Moz2D versions. I suggest just initializing offset in the scope of if(EXTEND_NONE) and setting it inside !pattern->IsAzure() && pattern->GetType() == gfxPattern::PATTERN_SURFACE.

Then over here I'd do an if (pattern->IsAzure() || pattern->GetType()) and do your generic masking code, that should reduce a lot of the code duplication and clean it up.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Attachment: Make Mask() use MaskSurface when drawing with EXTEND_NONE v2

A lot cleaner and also more correct. This uses ChangeTransform to make sure that source pattern gets it matrix appropriately adjusted.

Comment 3

[Bas Schouten (:bas.schouten)]

Comment on attachment 765753 [details] [diff] [review]
Make Mask() use MaskSurface when drawing with EXTEND_NONE v2

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

::: gfx/thebes/gfxContext.cpp
@@ +1419,3 @@
>  
> +        mask = pattern->GetAzureSurface();
> +        offset = Point(0.f, 0.f);

You don't need this, as Point is initialized to 0.f, 0.f.

@@ +1425,5 @@
> +
> +	// this lets GetAzureSurface work
> +	pattern->GetPattern(mDT);
> +
> +	mask = pattern->GetAzureSurface();

This line looks a suspicious amount like the one at L1420 :)

@@ +1430,1 @@
>        }

In other words I think the if (pattern->IsAzure()) block can go away completely!

Comment 4

[Jeff Muizelaar [:jrmuizel]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/872cb1c92a73

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/872cb1c92a73