Closed Bug 885020 Opened 6 years ago Closed 6 years ago

Make Mask() use MaskSurface when drawing with EXTEND_NONE

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: jrmuizel, Assigned: jrmuizel)

References

Details

Attachments

(1 file, 1 obsolete file)

This code becomes a little ugly but it does seem to work.
Attachment #764989 - Flags: review?(bas)
Depends on: 885016
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.
Attachment #764989 - Flags: review?(bas) → review+
Depends on: 885621
A lot cleaner and also more correct. This uses ChangeTransform to make sure that source pattern gets it matrix appropriately adjusted.
Attachment #764989 - Attachment is obsolete: true
Attachment #765753 - Flags: review?(bas)
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!
Attachment #765753 - Flags: review?(bas) → review+
https://hg.mozilla.org/mozilla-central/rev/872cb1c92a73
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Depends on: 891650
You need to log in before you can comment on or make changes to this bug.