[Azure] Enabling Azure-Thebes breaks mask layers

RESOLVED FIXED in mozilla16

Status

()

Core
Graphics: Layers
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nrc, Assigned: nrc)

Tracking

15 Branch
mozilla16
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 5 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 627620 [details]
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.
(Assignee)

Comment 1

5 years ago
Created attachment 630068 [details] [diff] [review]
patch
Attachment #630068 - Flags: review?(bas.schouten)
(Assignee)

Comment 2

5 years ago
Created attachment 630069 [details] [diff] [review]
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 :-(
Attachment #630069 - Flags: review?(bas.schouten)
(Assignee)

Comment 3

5 years ago
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.
(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!
(Assignee)

Comment 5

5 years ago
Created attachment 631119 [details] [diff] [review]
patch
Assignee: nobody → ncameron
Attachment #630068 - Attachment is obsolete: true
Attachment #630068 - Flags: review?(bas.schouten)
Attachment #631119 - Flags: review?(bas.schouten)
(Assignee)

Comment 6

5 years ago
Created attachment 631121 [details] [diff] [review]
tests
Attachment #630069 - Attachment is obsolete: true
Attachment #630069 - Flags: review?(bas.schouten)
Attachment #631121 - Flags: review?(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 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
Attachment #631121 - Flags: review?(bas.schouten) → review+
(Assignee)

Comment 9

5 years ago
Created attachment 631179 [details] [diff] [review]
patch
Attachment #631119 - Attachment is obsolete: true
Attachment #631119 - Flags: review?(bas.schouten)
Attachment #631179 - Flags: review?(bas.schouten)
(Assignee)

Comment 10

5 years ago
Created attachment 631182 [details] [diff] [review]
tests

removed tabs (how did they get there?), carrying r=Bas
Attachment #631121 - Attachment is obsolete: true
Attachment #631182 - Flags: review+
Attachment #631179 - Flags: review?(bas.schouten) → review+
(Assignee)

Comment 11

5 years ago
Try push: https://tbpl.mozilla.org/?tree=Try&rev=ec337e39072d
(Assignee)

Comment 12

5 years ago
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=88c6ee3a9aef
Target Milestone: --- → mozilla16
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
Target Milestone: mozilla16 → ---
(Assignee)

Comment 14

5 years ago
Created attachment 632916 [details] [diff] [review]
tests

increased a fuzz slightly so it passes for non-Windows platforms. Carrying r=Bas
Attachment #631182 - Attachment is obsolete: true
Attachment #632916 - Flags: review+
(Assignee)

Comment 15

5 years ago
Trying again:

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=537dc98ee4b4
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/32586e976b50
https://hg.mozilla.org/mozilla-central/rev/537dc98ee4b4
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 17

5 years ago
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
Attachment #631179 - Flags: approval-mozilla-aurora?
(Assignee)

Comment 18

5 years ago
Comment on attachment 632916 [details] [diff] [review]
tests

[Approval Request Comment]
see comment on patch approval request, these are the corresponding tests
Attachment #632916 - Flags: approval-mozilla-aurora?
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.
Attachment #631179 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Updated

5 years ago
Attachment #632916 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/10d1a127d9b8
https://hg.mozilla.org/releases/mozilla-aurora/rev/ce812b5b6ab1
status-firefox15: --- → fixed
tracking-firefox15: --- → +
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)
status-firefox15: fixed → affected
(Assignee)

Comment 22

5 years ago
https://tbpl.mozilla.org/?tree=Mozilla-Aurora&rev=771d99dd1dd8
status-firefox15: affected → ---
tracking-firefox15: + → ---
You need to log in before you can comment on or make changes to this bug.