Markus Stange [:mstange] Assignee
	Description • 9 years ago

If a scrollable page has multiple layers that scroll along with it, and any part of the page checkerboards, we apply the checkerboarding background color to all individual layers, even the transparent ones.

You can see this on http://www.w3.org/conf/2013sf/ , for example.

	Markus Stange [:mstange] Assignee
	Comment 1 • 9 years ago

We use the background color so that we don't show garbage, right? In that case, we can just exclude transparent layers from this behavior, since their background is expected to be shown anyway.

	Botond Ballo [:botond]
	Comment 2 • 9 years ago

Comment on attachment 8585128 [details] [diff] [review]
only apply the background color to opaque layers

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

I would expect that for transparent layers, the background color has alpha=1. Are you not seeing this, or is this an optimization to avoid calling DrawQuad() in this case?

	Markus Stange [:mstange] Assignee
	Comment 3 • 9 years ago

Botond and I talked in person; we came to the following conclusions:
 - Checkerboarding background colors should probably only be painted for the lowest layer belonging to a certain scroll frame.
 - We only really need the checkerboarding background color in the case that FrameLayerBuilder assumes our layer is opaque and does occlusion culling.
 - The content root scrollable is the only one where we know with certainty that we can always get an opaque checkerboarding background color.
 - When the checkerboarding background color is partially transparent, the output will look wrong - the color will be applied once in the checkerboarding part of the layer (correct), but twice in the parts of the layer that already have content (and that have that partially transparent background color painted as part of their content).
 - It would be nice if we could only do FrameLayerBuilder (i.e. layout-side) occlusion culling for the root content scrollable and not for subframes, and then only draw a checkerboarding background color for the bottommost layer of that root content scrollable, because that's the only one we have a guaranteed opaque background color for.

	Botond Ballo [:botond]
	Comment 4 • 9 years ago

(In reply to Markus Stange [:mstange] from comment #3)
>  - Checkerboarding background colors should probably only be painted for the
> lowest layer belonging to a certain scroll frame.

For completeness, this only applies for the root content scroll frame, because for subframes, we can get into situations where their layers are beside each other rather than on top of each other, so the "lowest" one wouldn't cover the frame's whole area.

>  - When the checkerboarding background color is partially transparent, the
> output will look wrong - the color will be applied once in the
> checkerboarding part of the layer (correct), but twice in the parts of the
> layer that already have content (and that have that partially transparent
> background color painted as part of their content).

This was the subject of bug 1037624. If necessary, we could solve it by only drawing the background color over the checkerboarding area.

	Markus Stange [:mstange] Assignee
	Comment 5 • 9 years ago

One more thing I forgot to note in comment 3:
 - We don't understand how the "layer bounds" can be the right area to fill with the background color. The layer bounds are the bounds of clipped display item bounds of the display items that are painted to the layer, so they shouldn't include stuff outside the display port.

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 6 • 9 years ago

(In reply to Markus Stange [:mstange] from comment #5)
> One more thing I forgot to note in comment 3:
>  - We don't understand how the "layer bounds" can be the right area to fill
> with the background color. The layer bounds are the bounds of clipped
> display item bounds of the display items that are painted to the layer, so
> they shouldn't include stuff outside the display port.

Are you sure the display items bounds are also clipped here? The assumption was that the layer bounds would include the bounds of the background display item if there was one (unclipped) and so would still include stuff outside the display port. See bug

	Kartikaya Gupta (email:kats@mozilla.staktrace.com)
	Comment 7 • 9 years ago

... bug 1013385 comment 9 and 10.

	Markus Stange [:mstange] Assignee
	Comment 8 • 9 years ago

(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #6)
> (In reply to Markus Stange [:mstange] from comment #5)
> > One more thing I forgot to note in comment 3:
> >  - We don't understand how the "layer bounds" can be the right area to fill
> > with the background color. The layer bounds are the bounds of clipped
> > display item bounds of the display items that are painted to the layer, so
> > they shouldn't include stuff outside the display port.
> 
> Are you sure the display items bounds are also clipped here? The assumption
> was that the layer bounds would include the bounds of the background display
> item if there was one (unclipped) and so would still include stuff outside
> the display port.

Indeed, you're right! They're not clipped.

	Markus Stange [:mstange] Assignee
	Comment 9 • 9 years ago

Comment on attachment 8585128 [details] [diff] [review]
only apply the background color to opaque layers

I've read through the notes again, and I think this gets us close enough for a first pass. In the cases where this doesn't match what we proposed, the difference mostly doesn't matter. For example, with this patch we'll still paint the checkerboarding background color for opaque layers that are not the bottommost layer of their scroll frame.

	Botond Ballo [:botond]
	Comment 10 • 9 years ago

(In reply to Botond Ballo [:botond] from comment #2)

To be clear, the reason my objection in comment 2:

> I would expect that for transparent layers, the background color has
> alpha=1. Are you not seeing this, or is this an optimization to avoid
> calling DrawQuad() in this case?

no longer applies in light of the subsequent discussion, is that we pick a single background color per scroll frame, and apply it to all layers associated with that scroll frame, so we can get into a situation where the chosen background color is opaque, but some of the layers are transparent.

	Markus Stange [:mstange] Assignee
	Comment 11 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/507818e44ac8

	Ryan VanderMeulen [:RyanVM]
	Comment 12 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/507818e44ac8