1142211 - Allow reftests to test whether an element is part of an opaque layer, and whether two elements share the same layer

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Markus Stange [:mstange]]

I'd like to make changes to the way we split and merge layers in the face of different animated geometry roots that interleave the display items in z-order, and to the way we decide to pull up opaque background colors when there are differing animated geometry roots.
However, at the moment I can't really test the things I'd like to guarantee. There are three objectives to optimize for:
 - Splitting layers has the goal of avoiding invalidations during animations (e.g. scrolling), regardless of whether those animations are async or driven from the main thread.
 - Merging layers has the goal of reducing memory consumption and overdraw.
 - Pulling opaque background colors into layers has the goal of making layers opaque, which makes them more efficient to draw and means they don't need component alpha.

At the moment, only the first goal is testable by reftests. For example, nothing will fail if we start putting every display item into its own layer (except for fuzzable differences in blending, maybe).

Comment 1

[Markus Stange [:mstange]]

Attachment: MozReview Request: bz://1142211/mstange

/r/5203 - Bug 1142211 - Add layerization testing mechanisms to reftest. r=roc

Pull down this commit:

hg pull review -r 6e8413934bdcd7680566b2cfedf6606022e352c4

Comment 2

[Markus Stange [:mstange]]

This patch adds support to the reftest framework for testing the other two goals: You can set class="reftest-opaque-layer" on elements that should be part of an opaque PaintedLayer, and you can set reftest-assigned-layer="page-background" on two elements that should end up in the same PaintedLayer.
The patch also adds support for testing whether elements are assigned to different PaintedLayers, if those elements have different reftest-assigned-layer values, but only because it's a natural extension of the same-layer test method. I don't think we should make much use of that capability - the point of splitting stuff into different layers is to avoid invalidation, so we should be using class="reftest-no-paint" to test for those invalidations instead.

Comment 3

[Markus Stange [:mstange]]

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

Comment 4

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

Comment on attachment 8576188 [details]
MozReview Request: bz://1142211/mstange

https://reviewboard.mozilla.org/r/5201/#review4237

Great stuff!

::: layout/base/FrameLayerBuilder.cpp
(Diff revision 1)
> +/* static */ Layer*
> +FrameLayerBuilder::GetDebugSingleOldLayerForFrame(nsIFrame* aFrame)
> +{
> +  nsTArray<DisplayItemData*> *array =
> +    reinterpret_cast<nsTArray<DisplayItemData*>*>(aFrame->Properties().Get(LayerManagerDataProperty()));
> +
> +  if (!array) {
> +    return nullptr;
> +  }
> +
> +  Layer* layer = nullptr;
> +  for (DisplayItemData *data : *array) {
> +    if (layer && layer != data->mLayer) {
> +      // More than one layer assigned, bail.
> +      return nullptr;
> +    }
> +    layer = data->mLayer;
> +  }
> +  return layer;
> +}
> +

Great stuff!

::: layout/base/FrameLayerBuilder.cpp
(Diff revision 1)
> +  nsTArray<DisplayItemData*> *array =

nsTArray<DisplayItemData\*>\* array

::: layout/base/FrameLayerBuilder.cpp
(Diff revision 1)
> +  for (DisplayItemData *data : *array) {

DisplayItemData\* data : \*array

Comment 5

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

Comment on attachment 8576188 [details]
MozReview Request: bz://1142211/mstange

https://reviewboard.mozilla.org/r/5201/#review4239

Ship It!

Comment 6

[Markus Stange [:mstange]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9b17a03fe1b5

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9b17a03fe1b5

Comment 9

[Markus Stange [:mstange]]

Attachment: MozReview Request: Bug 1142211 - Add layerization testing mechanisms to reftest. r=roc