1201272 - Canvas composite mode 'destination-out' is broken if the context has a shadow

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

[ryanackley]

Attachment: Firefox

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150826023504

Steps to reproduce:

See this jsfiddle for a demonstration.

http://jsfiddle.net/ojLv9abr/1/

1. Use canvas to fill a circle. 
2. Set the global compositing operation to 'destination-out'. 
3. Give the context a visible shadow setting. 
4. Fill a smaller circle on top of the existing canvas.


Actual results:

The original filled circle is on the canvas unmodified


Expected results:

The canvas should show a donut because the compositing should cause the smaller circle to remove the overlapping parts of the destination when it was drawn.

Comment 1

[ryanackley]

Attachment: Screen Shot 2015-09-02 at 5.04.11 PM.png

Chrome

Comment 2

[Loic]

WFM with FF40 on Win 7, i see the donut.

Comment 3

[ryanackley]

Yes, confirmed that it seems to be OS X specific. Works for me on Windows 7 but still broken on OS X

This used to work. A recent change must have introduced this bug.

Comment 4

[Loic]

If it used to work with previous versions and you have some time, you can find a regression range wy using the tool Mozregression.
See http://mozilla.github.io/mozregression/ for details about install and usage.
Running the command "mozregression --good-release 38" (or 39) should be enough to get the regression range (pushlog).

Comment 5

[Benoit Girard (:BenWa)]

It's specific to skia canvas then. We should fix this.

Comment 6

[Lee Salzman [:lsalzman]]

(In reply to Benoit Girard (:BenWa) from comment #5)
> It's specific to skia canvas then. We should fix this.

I can confirm on Linux as well, this reproduces with Skia canvas, but it works fine with Cairo canvas.

Comment 7

[Alice0775 White]

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=45003a7ef682&tochange=7aa032d90c91

Triggered by:
947d7ce90e26	George Wright — Bug 976184 - Apply a drop shadow image filter to our SkPaint when drawing a surface with a shadow r=mattwoodrow

Comment 8

[Lee Salzman [:lsalzman]]

Attachment: part 1 - use a SkBlurImageFilter for Skia canvas shadows so we can better control composite operations

The bug results from an interaction between drawBitmap and SkDropShadowImageFilter.

When given an image filter, drawBitmap does an implicit saveLayer, does the actual draw with the composite op into the temporary layer, runs the image filter to generate a new temporary layer, then on implicit restore composites the layer as src-over.

The SkDropShadowImageFilter generates the shadow image and then uses a hardcoded src-over op as well to composite both the shadow and original image into a temporary layer. This then gets composited as described above.

However, this order disagrees with what the spec says we should be doing:  https://html.spec.whatwg.org/multipage/scripting.html#drawing-model
We should be generating the shadow image and then using the composite op to composite it into the result, and then use the composite op to composite the original image over it.

This patch uses a combination of SkBlurImageFilter (what SkDropShadowImageFilter was using internally) and drawSprite (which avoids the implicit saveLayer/restore) to do just that. It should also be faster as a consequence because it avoids several intermediate bitmap copies.

Comment 9

[Lee Salzman [:lsalzman]]

Attachment: part 2 - add reftest for canvas shadows with destination-out op

This makes the test case into a reftest. It verifies that the hole still gets gets cut out of the circle even when shadow blue is used.

As a slight modification, I draw a stroke over the actual shadow just so it can be compared with the non-shadowed ref.

Comment 10

[Lee Salzman [:lsalzman]]

Attachment: part 1 - use a SkBlurImageFilter for Skia canvas shadows so we can better control composite operations

Fix a bug with image filters using the current transform matrix even though drawSprite does not.

Comment 11

[Lee Salzman [:lsalzman]]

Attachment: part 2 - add reftest for canvas shadows with destination-out op

Make final stroke a bit wider to better hide the shadow on all platforms.

Comment 12

[Lee Salzman [:lsalzman]]

Try run for patch and new reftest: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ce3ee87be7b

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b73f3910cdd
https://hg.mozilla.org/integration/mozilla-inbound/rev/4683e2a8d998

Comment 14

[Lee Salzman [:lsalzman]]

Attachment: part 3 - remove failure meta-data from WPT shadow tests

Some WPT tests are showing up as failures because they, in fact, now actually pass, but the meta-data said they should be failing. These particular shadow tests used a global composite op of 'destination-atop', which wouldn't have worked with the old shadow blur code, but now works since these patches have been incorporated.

Comment 15

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/af0a7430ffae

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5b73f3910cdd
https://hg.mozilla.org/mozilla-central/rev/4683e2a8d998
https://hg.mozilla.org/mozilla-central/rev/af0a7430ffae