1258168 - Fonts under certain condition render as white, regression after 46b2

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[fireattack [:fireattack]]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160316065941

Steps to reproduce:

Repro: http://output.jsbin.com/tasikupiwu

Set "gfx.content.use-native-pushlayer" to false can produce expected result.

Comment 2

[Bas Schouten (:bas.schouten)]

Attachment: MozReview Request: Bug 1258168: Push ClearType compatible clipping layers when the last pushed layer was marked as opaque. r=jrmuizel

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

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8732638 [details]
MozReview Request: Bug 1258168: Push ClearType compatible clipping layers when the last pushed layer was marked as opaque. r=jrmuizel

https://reviewboard.mozilla.org/r/41265/#review37945

Any chance for a test?

Comment 4

[Pulsebot]

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

Comment 5

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

Backed out for R(R) failures on Windows 8 x64:

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/b21390b218bf

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=f251bbc37cb0
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=24335315&repo=mozilla-inbound
08:04:25     INFO -  REFTEST TEST-START | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/bugs/817019-1.html
08:04:25     INFO -  REFTEST TEST-LOAD | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/bugs/817019-1.html | 4561 / 13473 (33%)
08:04:25     INFO -  REFTEST TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/bugs/817019-1.html | image comparison (==), max difference: 255, number of differing pixels: 20038

Comment 6

[Bas Schouten (:bas.schouten)]

Ugh I'm sorry. I forget to include win64 on https://treeherder.mozilla.org/#/jobs?repo=try&revision=9aeac1634c4b. :(

Comment 7

[Bas Schouten (:bas.schouten)]

Attachment: Push ClearType compatible clipping layers when the last pushed layer was marked as opaque

This was due to us pushing cleartype clipping layers for temporary targets we were using for complex operators or unsupported patterns. I've removed clipping of the temporary surface as some investigation has lead me to conclude we'll clip the final composition anyway. This should improve performance as well.

Comment 8

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

Tracking, since this is a recent regression.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/06a0c49f1206

Comment 10

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

Backed out for failing Windows 8 x64 opt css-blending reftests:

Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/a9fbf9394340

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=24648249&repo=mozilla-inbound


REFTEST TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/canvas/1151821-1.html | image comparison (==), max difference: 255, number of differing pixels: 5000
REFTEST TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/css-blending/blend-difference-stacking.html | image comparison (==), max difference: 255, number of differing pixels: 12000
REFTEST TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/css-blending/background-blending-alpha.html | image comparison (==), max difference: 255, number of differing pixels: 40000
REFTEST TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/css-blending/background-blending-gradient-color.html | image comparison (==), max difference: 255, number of differing pixels: 40000 
and more

Comment 11

[Bas Schouten (:bas.schouten)]

Yep, these failures are actually on my try run too. My mistake. Will keep working on this.

Comment 12

[Bas Schouten (:bas.schouten)]

Attachment: Push ClearType compatible clipping layers when the last pushed layer was marked as opaque v2

This patch passes all tests and should guarantee we always do the optimal thing.

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 8736719 [details] [diff] [review]
Push ClearType compatible clipping layers when the last pushed layer was marked as opaque v2

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

::: gfx/2d/DrawTargetD2D1.cpp
@@ +1196,5 @@
>    mUsedCommandListsSincePurge++;
>  
> +  D2D1_RECT_F rect;
> +  bool isAligned;
> +  RefPtr<ID2D1Image> tmpImage;

tmpImage looks unused.

@@ +1200,5 @@
> +  RefPtr<ID2D1Image> tmpImage;
> +  bool clipIsComplex = CurrentLayer().mPushedClips.size() && !GetDeviceSpaceClipRect(rect, isAligned);
> +
> +  if (patternSupported && !CurrentLayer().mIsOpaque && D2DSupportsCompositeMode(aOp) &&
> +      IsOperatorBoundByMask(aOp) && clipIsComplex) {

Can we move this condition into a helper function? It's complicated duplicated and as a reader I don't know what it does without reading closely.

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/538d248fa252

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/538d248fa252

Comment 16

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

Can you request uplift ? I'm not sure how common this is, but since this first appeared in 46 it seems good to fix it in 46.   We can still get it into beta 10 if you think that's a good idea.

Comment 17

[Bas Schouten (:bas.schouten)]

Comment on attachment 8736719 [details] [diff] [review]
Push ClearType compatible clipping layers when the last pushed layer was marked as opaque v2

Approval Request Comment
[Feature/regressing bug #]: Native pushlayer/poplayer
[User impact if declined]: Some incorrect text rendering
[Describe test coverage new/current, TreeHerder]: Nightly usage
[Risks and why]: Fairly low, small risk of artifacts should the patch contain a mistake but it's reasonably contained
[String/UUID change made/needed]: None

Comment 18

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

Comment on attachment 8736719 [details] [diff] [review]
Push ClearType compatible clipping layers when the last pushed layer was marked as opaque v2

Fix for recent regression, let's take this for aurora and for beta 11

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2ff41ae73a11

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/803cfc0649bf8601f68f42343bbe961622bc4017