759036 - [Azure] Enabling Azure-Thebes breaks mask layers

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Nick Cameron [:nrc]]

Attachment: test case

Bug 715768 (or possibly bug 757329) breaks masking of rounded rects for some cases. See the test case, with Accelerated layers enabled, DX10, when you mouse over the div, it renders correctly (the active path), but when the layer is inactive, it renders incorrectly, the mask is incorrectly translated by some amount.

I bisected m-c to identify those two changesets as the culprits.

As well as fixing the bug, we need some more tests for rounded rect clipping, but I get kind of stuck as to how to do them because of the subtle active/inactive layers thing.

Comment 1

[Nick Cameron [:nrc]]

Attachment: patch

Comment 2

[Nick Cameron [:nrc]]

Attachment: test

Sorry these took so long, I didn't know direct2D didn't work on Try so thought my test wasn't working. I have a passing Try run from earlier, but lost it, but it doesn't matter because it wasn't using Direct2D anyway :-(

Comment 3

[Nick Cameron [:nrc]]

Attachment: another test case

The patch does not fix this new test case. Here there is a pattern, rather than a source surface in gfxContext, and the pattern transform is different to the draw target transform, the effect is that the image is painted into the wrong place, see the distortion on the rhs of the image. It needs both a scale and translate on the image, it looks like the scale and translate are applied in the wrong order, but I suspect that is not the problem, more likely the source and target transforms are being confused again. Other than that, I can't figure out what is causing this. It is fixed by turning off content Azure.

Comment 4

[Bas Schouten (:bas.schouten)]

(In reply to Nick Cameron [:nrc] from comment #3)
> Created attachment 630098 [details]
> another test case
> 
> The patch does not fix this new test case. Here there is a pattern, rather
> than a source surface in gfxContext, and the pattern transform is different
> to the draw target transform, the effect is that the image is painted into
> the wrong place, see the distortion on the rhs of the image. It needs both a
> scale and translate on the image, it looks like the scale and translate are
> applied in the wrong order, but I suspect that is not the problem, more
> likely the source and target transforms are being confused again. Other than
> that, I can't figure out what is causing this. It is fixed by turning off
> content Azure.

Try fiddling with the order anyway, I think that actually might be the cause!

Comment 5

[Nick Cameron [:nrc]]

Attachment: patch

Comment 6

[Nick Cameron [:nrc]]

Attachment: tests

Comment 7

[Bas Schouten (:bas.schouten)]

Comment on attachment 631119 [details] [diff] [review]
patch

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

This patch is against the patch I gave you, not against m-c. That's why you need to comment out several of the inverts :)

Comment 8

[Bas Schouten (:bas.schouten)]

Comment on attachment 631121 [details] [diff] [review]
tests

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

::: layout/reftests/bugs/759036-2-ref.html
@@ +2,5 @@
> +<html>
> +<head>
> +<style>
> +.clipround {
> +	width:250px;

No tabs please! :)

::: layout/reftests/bugs/759036-2.html
@@ +2,5 @@
> +<html>
> +<head>
> +<style>
> +.clipround {
> +	width:250px;

Here too

Comment 9

[Nick Cameron [:nrc]]

Attachment: patch

Comment 10

[Nick Cameron [:nrc]]

Attachment: tests

removed tabs (how did they get there?), carrying r=Bas

Comment 11

[Nick Cameron [:nrc]]

Try push: https://tbpl.mozilla.org/?tree=Try&rev=ec337e39072d

Comment 12

[Nick Cameron [:nrc]]

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=88c6ee3a9aef

Comment 13

[Matt Brubeck (:mbrubeck)]

Sorry, I had to back this out on inbound because of a reftest.list syntax error and a leak in debug reftests:
https://hg.mozilla.org/integration/mozilla-inbound/rev/253d5996845e

Comment 14

[Nick Cameron [:nrc]]

Attachment: tests

increased a fuzz slightly so it passes for non-Windows platforms. Carrying r=Bas

Comment 15

[Nick Cameron [:nrc]]

Trying again:

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=537dc98ee4b4

Comment 16

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/32586e976b50
https://hg.mozilla.org/mozilla-central/rev/537dc98ee4b4

Comment 17

[Nick Cameron [:nrc]]

Comment on attachment 631179 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 715768 (or possibly bug 757329)
User impact if declined: incorrect rendering of content with D2D and rounded corners
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): no obvious risk
String or UUID changes made by this patch: none

Comment 18

[Nick Cameron [:nrc]]

Comment on attachment 632916 [details] [diff] [review]
tests

[Approval Request Comment]
see comment on patch approval request, these are the corresponding tests

Comment 19

[Alex Keybl [:akeybl]]

Comment on attachment 631179 [details] [diff] [review]
patch

[Triage Comment]
Definitely early enough in the cycle to take a regression fix like this. Approved for Aurora 15.

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/10d1a127d9b8
https://hg.mozilla.org/releases/mozilla-aurora/rev/ce812b5b6ab1

Comment 21

[Ed Morley [:emorley]]

Backed out the reftest part from aurora, for failures:
https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=ce812b5b6ab1

https://hg.mozilla.org/releases/mozilla-aurora/rev/42817f2f2c3e

(Is only the test part, but going to set back to affected, so this doesn't fall off the radar)

Comment 22

[Nick Cameron [:nrc]]

https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=771d99dd1dd8