1067748 - Solid-color display items which cover everything else in their layer should induce a ColorLayer

Status: NEW

(Core :: Layout, defect)

Description

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

This happens for fullscreen video elements. We're not getting a ColorLayer when we should.

Comment 1

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

Attachment: fix

Comment 2

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8489811 [details] [diff] [review]
fix

Nice.

Comment 3

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

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

Comment 4

[Carsten Book [:Tomcat]]

sorry had to back this out for test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=48162273&tree=Mozilla-Inbound

Comment 5

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

Attachment: fix v2

Comment 6

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8490424 [details] [diff] [review]
fix v2

>diff --git a/gfx/layers/composite/ContainerLayerComposite.cpp b/gfx/layers/composite/ContainerLayerComposite.cpp
>- * Returns a rectangle of content painted opaquely by aLayer. Very consertative;
>+ * Returns a rectangle of content painted opaquely by aLayer. Very conservative;

I don't see what this changes, DOS line ending?

>+// Return true if aItem is a uniform color over the intersection of its
>+// visible rect, bounds rect, and clip rect.
>+static bool
>+IsDisplayItemUniform(nsDisplayListBuilder* aBuilder,

I think something like "Return tue if aItem is a uniform color over the intersection of its visible rect and bounds rect, taking into account the clip rect aClip" would be more clear.

Comment 7

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

(In reply to Timothy Nikkel (:tn) from comment #6)
> Comment on attachment 8490424 [details] [diff] [review]
> fix v2
> 
> >diff --git a/gfx/layers/composite/ContainerLayerComposite.cpp b/gfx/layers/composite/ContainerLayerComposite.cpp
> >- * Returns a rectangle of content painted opaquely by aLayer. Very consertative;
> >+ * Returns a rectangle of content painted opaquely by aLayer. Very conservative;
> 
> I don't see what this changes, DOS line ending?

Typo in "consertative"

> >+// Return true if aItem is a uniform color over the intersection of its
> >+// visible rect, bounds rect, and clip rect.
> >+static bool
> >+IsDisplayItemUniform(nsDisplayListBuilder* aBuilder,
> 
> I think something like "Return tue if aItem is a uniform color over the
> intersection of its visible rect and bounds rect, taking into account the
> clip rect aClip" would be more clear.

Really? I think that's less clear :-)

Comment 8

[Timothy Nikkel (:tnikkel)]

Okay, I see what you are saying now. The comment is fine.

Comment 9

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/820188e039a0

Comment 10

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/6976243a25d0 for Android reftest-2 failures looking sort of racy (between 2 and 6 per run on opt but between 0 and 3 per run on debug), in a variety of different tests, mostly failing around the edges of borders but sometimes around the edges of text. Thanks to retriggers, plenty of examples in https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-inbound&revision=820188e039a0&searchQuery=plain-reftest-2

Comment 11

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

370629-1-ref.html is just:
<span style="display: inline-block; width: 720px; height:720px; background: red"></span>
It renders a column of not-quite-white pixels to the right of the red span --- but only for y>=512, which suggests a tiling-related graphics issue. Then again, it seems to me a ColorLayer shouldn't even be generated here so the code I changed should not be triggered, so I need to dig into that when I get my Android phone back.

Comment 12

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

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=92810410589f

Comment 13

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

Still got the bug:
http://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/[e-mail]-92810410589f/try-android-api-11/try_panda_android_test-plain-reftest-2-bm89-tests1-panda-build3580.txt.gz&only_show_unexpected=1

If you look at the test and reference images for layout/reftests/bugs/243519-9e.html, for example, you can see that the pixel values on the inside of the bottom left corner are not pure white for one row/column of pixels, only in the rectangle 0,512 to 256,768. So there's definitely some kind of issue with tile rendering on Android that's not obviously related to my patch, but somehow triggered by it.

Comment 14

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

These tests pass on my Android phone :-(.

Comment 15

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

With some logging:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=47db82f1d3aa

Comment 16

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=898c2a851a0b

Comment 17

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe58ed6cf00f