1251431 - Demo doesn't work on Win10

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Jeff Griffiths (:canuckistani) (:⚡︎)]

STR:

1. open this demo on Win10 / Nightly http://codepen.io/thebabydino/details/yOByme#comment-id-130071

ER: https://twitter.com/anatudor/status/702622449716822020

AR: blank 

This demo seems to work correctly on Win7 and OS X just fine.

Comment 1

[Jeff Griffiths (:canuckistani) (:⚡︎)]

This is probably gpu or resolution dependent. bwinton tested on a Win10 VM and it worked fine, and in vmware webgl rendering and other acceleration are done in software. On any win10 / intel integrated machine we tested, Win10 fails.

Comment 2

[Mike Taylor [:miketaylr]]

I don't have any "real" Windows machines to test on sorry. :(

Comment 3

[Patrick Brosset <:pbro>]

Just tested on Win10 with FF46: it's blank for me.
Tested in FF43: works fine!

Comment 4

[Alice0775 White]

I can reproduce the problem on Windows7 HWA on.

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=1e5f3d1151d60a1edd6424a35a2e38b5ab17adad&tochange=af5c9cf6898b0e2931a809774387cad5953363c3

Via local build,
Last Good: 11ce065429a5
First Bad: 5515fd47eb5a

Triggered by: 5515fd47eb5a	Bas Schouten — Bug 1210560 - Part 3: Convert more complex SVG usecases to PushGroupForBlendBack. r=jwatt r=jrmuizel

Comment 5

[Alice0775 White]

Attachment: Bug1251431.html

Comment 6

[Bas Schouten (:bas.schouten)]

This is apparently a regression from my work. I will have a look at this.

Comment 7

[Bas Schouten (:bas.schouten)]

Attachment: Heavily reduced test case

Comment 8

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1251431 - Part 1: Allow usage of an A8 source pattern to MaskSurface for D2D 1.1 Moz2D backend. r=jwatt

Review commit: https://reviewboard.mozilla.org/r/39093/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39093/

Comment 9

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1251431 - Part 2: Do not apply the device transform when drawing to an already intermediate surface. r=jwatt

Review commit: https://reviewboard.mozilla.org/r/39095/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39095/

Comment 10

[Bas Schouten (:bas.schouten)]

Reftest upcoming.

Comment 11

[Bas Schouten (:bas.schouten)]

Comment on attachment 8728757 [details]
MozReview Request: Bug 1251431 - Part 1: Allow usage of an A8 source pattern to MaskSurface for D2D 1.1 Moz2D backend. r=jwatt

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39093/diff/1-2/

Comment 12

[Bas Schouten (:bas.schouten)]

Comment on attachment 8728758 [details]
MozReview Request: Bug 1251431 - Part 2: Do not apply the device transform when drawing to an already intermediate surface. r=jwatt

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/39095/diff/1-2/

Comment 13

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1251431 - Part 3: Add a reftest for masked clip paths. r=jwatt

Review commit: https://reviewboard.mozilla.org/r/39097/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/39097/

Comment 14

[Jonathan Watt [:jwatt]]

Comment on attachment 8728757 [details]
MozReview Request: Bug 1251431 - Part 1: Allow usage of an A8 source pattern to MaskSurface for D2D 1.1 Moz2D backend. r=jwatt

https://reviewboard.mozilla.org/r/39093/#review35893

::: gfx/2d/DrawTargetD2D1.cpp:1634
(Diff revision 2)
> +      // See bug 1251431, at least FillOpacityMask does not appear to allow a source bitmapbrush
> +      // with source format A8. This creates a BGRA surface with the same alpha values that
> +      // the A8 surface has.

This comment is a bit confusing because it refers to FillOpacityMask which is called directly below it, but you're refering to other FillOpacityMask calls elsewhere that will consume the pattern. I'd suggest rewording this comment to something like:

      // Some consumers of the brush that we return will not work if we return
      // a source bitmapbrush with source format A8. Specifically,
      // FillOpacityMask does not appear to support this - see bug 1251431.
      // This code works around that by creating a BGRA surface with the same
      // alpha values that the A8 surface has.

Comment 15

[Jonathan Watt [:jwatt]]

Comment on attachment 8728758 [details]
MozReview Request: Bug 1251431 - Part 2: Do not apply the device transform when drawing to an already intermediate surface. r=jwatt

https://reviewboard.mozilla.org/r/39095/#review35897

Comment 16

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/da29c45e89db
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bbc2f4a3065

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/da29c45e89db
https://hg.mozilla.org/mozilla-central/rev/1bbc2f4a3065

Comment 18

[Pulsebot]

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

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/bc7572388c5a

Comment 20

[Jeff Muizelaar [:jrmuizel]]

Should part1 be uplifted?

Comment 21

[Bas Schouten (:bas.schouten)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #20)
> Should part1 be uplifted?

Maybe, I seem to recall more stuff was landed on top of this now..

Comment 22

[Bas Schouten (:bas.schouten)]

(In reply to Bas Schouten (:bas.schouten) from comment #21)
> (In reply to Jeff Muizelaar [:jrmuizel] from comment #20)
> > Should part1 be uplifted?
> 
> Maybe, I seem to recall more stuff was landed on top of this now..

Like, the work you did...

Comment 23

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8728757 [details]
MozReview Request: Bug 1251431 - Part 1: Allow usage of an A8 source pattern to MaskSurface for D2D 1.1 Moz2D backend. r=jwatt

Approval Request Comment
[Feature/regressing bug #]: This is needed to fix bug 1258650
[User impact if declined]: bug 1258650 will remain unfixed for D2D users
[Describe test coverage new/current, TreeHerder]: Has been in 48 for a while
[Risks and why]:

Comment 24

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8728757 [details]
MozReview Request: Bug 1251431 - Part 1: Allow usage of an A8 source pattern to MaskSurface for D2D 1.1 Moz2D backend. r=jwatt

I am sorry but I cannot take the risk for release...

Comment 25

[Milan Sreckovic [:milan] (needinfo for best results)]

I believe bug 1258650 is on 45 ESR - we would need this one on 45 ESR then as well, right?

Comment 26

[Jeff Muizelaar [:jrmuizel]]

That is my understanding.

Comment 27

[Bas Schouten (:bas.schouten)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #26)
> That is my understanding.

Only part 1 if I'm not mistaking, Jeff's patch overrides part 2.

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8728757 [details]
MozReview Request: Bug 1251431 - Part 1: Allow usage of an A8 source pattern to MaskSurface for D2D 1.1 Moz2D backend. r=jwatt

Part 1 should uplift to aurora and beta. This should make it into beta 10.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/108c0f85a245

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/da19d18b7fb3

Comment 31

[Cornel Ionce [:noni] [Hubs QA]]

Reproduced the initial issue on Windows 10 x64 with the attached testcases using Firefox 45.0.2.

There is a small inconsistency between 46.0b10 and latest Nightly/Aurora builds, but only with the reduced test case:
*46 beta 10: http://i.imgur.com/9VzNYd5.png
*Latest Aurora/Nightly: http://i.imgur.com/ZPnetel.png
*Also mentioning that Chrome displays a square instead of a circle.
The other testcase works correctly on the mentioned versions.

Testing was performed using:
*Latest 48.0a1 Nightly, build ID:20160411030231,
*Latest 47.0a2 Aurora, build ID: 20160411004020,
*Firefox 46 beta 10, build ID: 20160411042519.

Assuming it's safe to mark this bug as verified, please let me know if we should file a separate issue for the above statement.

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b46b071d48cf
part 3 - Reftest for mask and clipPath applied to the same element. r=Bas

Comment 33

[Jonathan Watt [:jwatt]]

Comment on attachment 8728760 [details]
MozReview Request: Bug 1251431 - Part 3: Add a reftest for masked clip paths. r=jwatt

https://reviewboard.mozilla.org/r/39097/#review65256

As discussed on IRC some time ago, I wrote a new regression test that actually fails pre-patch, and that doesn't contain the unnecessary HTML junk.

Comment 34

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out part 3 for failing the new reftest:

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b08ae92f655

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b46b071d48cf7d26edcd077970e1c3445248a171
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=32970470&repo=mozilla-inbound

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/svg/mask-and-clipPath-2.html == file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/svg/pass.svg | (LOAD ONLY), max difference: 255, number of differing pixels: 755000

Comment 35

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/34f662c8a932
part 3 - Reftest for mask and clipPath applied to the same element. r=Bas

Comment 36

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/34f662c8a932