1031713 - Stop wastefully using IsOperatorBoundByMask() in DrawTargetCairo::DrawSurface()

Status: NEW

(Core :: Graphics, defect)

Description

[Jonathan Watt [:jwatt]]

We are checking IsOperatorBoundByMask() in DrawTargetCairo::DrawSurface() and appear to be sometimes needlessly creating groups that need to be composited as a result. Checking IsOperatorBoundByMask() would make sense if we were calling cairo_mask or cairo_mask_preserve, but we are not [any more].

Comment 1

[Matt Woodrow (:mattwoodrow)]

The 'mask' for cairo_fill() is the current path, rather than an explicit mask surface. Cairo implements OPERATOR_SOURCE as being bound by the mask, but Moz2D wants it not to be. That's why we push a group.

I'm less sure about the other operators that return false for IsOperatorBoundByMask, since I think we match cairo's behaviour for those.

Comment 2

[Jonathan Watt [:jwatt]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #1)
> The 'mask' for cairo_fill() is the current path,

So presumably for cairo_stroke() the implicit mask is the stroke area?

> rather than an explicit
> mask surface. Cairo implements OPERATOR_SOURCE as being bound by the mask,
> but Moz2D wants it not to be. That's why we push a group.

It wants it _not_ to be?? I'm coming at this from the perspective of looking at:

  drawTarget->FillRect(rect, solidColorWithAlphaPattern,
                       DrawOptions(1.f, CompositionOp::OP_SOURCE));

At least for a DrawTargetCairo on Win8 this is filling the rect as expected, but contrary to my expectations it is also blowing away all the pixel data outside the rect (turning it transparent black). Is this the expected behavior?

Comment 3

[Bas Schouten (:bas.schouten)]

(In reply to Jonathan Watt [:jwatt] from comment #2)
> (In reply to Matt Woodrow (:mattwoodrow) from comment #1)
> > The 'mask' for cairo_fill() is the current path,
> 
> So presumably for cairo_stroke() the implicit mask is the stroke area?
> 
> > rather than an explicit
> > mask surface. Cairo implements OPERATOR_SOURCE as being bound by the mask,
> > but Moz2D wants it not to be. That's why we push a group.
> 
> It wants it _not_ to be?? I'm coming at this from the perspective of looking
> at:
> 
>   drawTarget->FillRect(rect, solidColorWithAlphaPattern,
>                        DrawOptions(1.f, CompositionOp::OP_SOURCE));
> 
> At least for a DrawTargetCairo on Win8 this is filling the rect as expected,
> but contrary to my expectations it is also blowing away all the pixel data
> outside the rect (turning it transparent black). Is this the expected
> behavior?

Yes, the effect is only limited to the clip. That's the Canvas spec so that's how we've defined the Moz2D behavior.

Comment 4

[Jonathan Watt [:jwatt]]

Having fill operations touch pixels outside the fill area, or stroke operations touch pixels outside the stroke area, seems gross to me. Do any of the Moz2D backends behave in this way?

(In reply to Bas Schouten (:bas.schouten) from comment #3)
> Yes, the effect is only limited to the clip. That's the Canvas spec so
> that's how we've defined the Moz2D behavior.

It would be really easy for the canvas code to do the push-group, so I'm not sure this is necessarily a good justification for the behavior. If it's common for platform graphics libraries to do so I'd feel that is a stronger argument.

Comment 5

[Bas Schouten (:bas.schouten)]

(In reply to Jonathan Watt [:jwatt] from comment #4)
> Having fill operations touch pixels outside the fill area, or stroke
> operations touch pixels outside the stroke area, seems gross to me. Do any
> of the Moz2D backends behave in this way?

Yes.

> (In reply to Bas Schouten (:bas.schouten) from comment #3)
> > Yes, the effect is only limited to the clip. That's the Canvas spec so
> > that's how we've defined the Moz2D behavior.
> 
> It would be really easy for the canvas code to do the push-group, so I'm not
> sure this is necessarily a good justification for the behavior. If it's
> common for platform graphics libraries to do so I'd feel that is a stronger
> argument.

Platform libraries differ in their behavior. I personally believe since there's a wide variety doing the thing canvas does makes sense. And things are slightly more flexible this way.