Open Bug 1067748 Opened 10 years ago Updated 2 years ago

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

Categories

(Core :: Layout, defect)

x86_64
Linux
defect

Tracking

()

People

(Reporter: roc, Unassigned)

Details

Attachments

(2 files)

This happens for fullscreen video elements. We're not getting a ColorLayer when we should.
Attached patch fixSplinter Review
Attachment #8489811 - Flags: review?(tnikkel)
Comment on attachment 8489811 [details] [diff] [review]
fix

Nice.
Attachment #8489811 - Flags: review?(tnikkel) → review+
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.
Attachment #8490424 - Flags: review?(tnikkel) → review+
(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 :-)
Okay, I see what you are saying now. The comment is fine.
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
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.
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/rocallahan@mozilla.com-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.
These tests pass on my Android phone :-(.
Assignee: roc → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.