Last Comment Bug 759036 - [Azure] Enabling Azure-Thebes breaks mask layers
: [Azure] Enabling Azure-Thebes breaks mask layers
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics: Layers (show other bugs)
: 15 Branch
: x86_64 Windows 7
: -- normal (vote)
: mozilla16
Assigned To: Nick Cameron [:nrc]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-27 23:06 PDT by Nick Cameron [:nrc]
Modified: 2012-06-28 18:50 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test case (637 bytes, text/html)
2012-05-27 23:06 PDT, Nick Cameron [:nrc]
no flags Details
patch (1.16 KB, patch)
2012-06-04 22:04 PDT, Nick Cameron [:nrc]
no flags Details | Diff | Review
test (1.84 KB, patch)
2012-06-04 22:06 PDT, Nick Cameron [:nrc]
no flags Details | Diff | Review
another test case (301 bytes, text/html)
2012-06-05 00:29 PDT, Nick Cameron [:nrc]
no flags Details
patch (4.61 KB, patch)
2012-06-07 13:57 PDT, Nick Cameron [:nrc]
no flags Details | Diff | Review
tests (8.13 KB, patch)
2012-06-07 13:58 PDT, Nick Cameron [:nrc]
bas: review+
Details | Diff | Review
patch (4.58 KB, patch)
2012-06-07 15:28 PDT, Nick Cameron [:nrc]
bas: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review
tests (8.14 KB, patch)
2012-06-07 15:31 PDT, Nick Cameron [:nrc]
ncameron: review+
Details | Diff | Review
tests (8.24 KB, patch)
2012-06-13 15:40 PDT, Nick Cameron [:nrc]
ncameron: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Review

Description Nick Cameron [:nrc] 2012-05-27 23:06:04 PDT
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.
Comment 1 Nick Cameron [:nrc] 2012-06-04 22:04:16 PDT
Created attachment 630068 [details] [diff] [review]
patch
Comment 2 Nick Cameron [:nrc] 2012-06-04 22:06:57 PDT
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 :-(
Comment 3 Nick Cameron [:nrc] 2012-06-05 00:29:23 PDT
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.
Comment 4 Bas Schouten (:bas.schouten) 2012-06-05 08:35:16 PDT
(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] 2012-06-07 13:57:39 PDT
Created attachment 631119 [details] [diff] [review]
patch
Comment 6 Nick Cameron [:nrc] 2012-06-07 13:58:15 PDT
Created attachment 631121 [details] [diff] [review]
tests
Comment 7 Bas Schouten (:bas.schouten) 2012-06-07 14:13:28 PDT
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) 2012-06-07 14:14:40 PDT
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] 2012-06-07 15:28:11 PDT
Created attachment 631179 [details] [diff] [review]
patch
Comment 10 Nick Cameron [:nrc] 2012-06-07 15:31:56 PDT
Created attachment 631182 [details] [diff] [review]
tests

removed tabs (how did they get there?), carrying r=Bas
Comment 11 Nick Cameron [:nrc] 2012-06-11 14:01:39 PDT
Try push: https://tbpl.mozilla.org/?tree=Try&rev=ec337e39072d
Comment 13 Matt Brubeck (:mbrubeck) 2012-06-11 23:29:26 PDT
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] 2012-06-13 15:40:02 PDT
Created attachment 632916 [details] [diff] [review]
tests

increased a fuzz slightly so it passes for non-Windows platforms. Carrying r=Bas
Comment 15 Nick Cameron [:nrc] 2012-06-13 15:42:50 PDT
Trying again:

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=537dc98ee4b4
Comment 17 Nick Cameron [:nrc] 2012-06-24 20:33:41 PDT
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] 2012-06-24 20:34:25 PDT
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] 2012-06-26 10:00:00 PDT
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) (Exited; email my personal email if necessary) 2012-06-26 20:38:25 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/10d1a127d9b8
https://hg.mozilla.org/releases/mozilla-aurora/rev/ce812b5b6ab1
Comment 21 Ed Morley [:emorley] 2012-06-27 02:23:32 PDT
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)

Note You need to log in before you can comment on or make changes to this bug.