New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 684842 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , All
Pri: 2
Type: Bug

Blocking:
issue 674317



Sign in to add a comment

No layer created in SPv2 for simple opacity animation.

Project Member Reported by wkorman@chromium.org, Jan 24 2017

Issue description

Filing bug to track investigation. The attached trivial opacity animation still fails in SPv2 at ToT with all recent patches submitted. It appears we're never painting the animated div at all, thus it has no chunk, thus we never create a separate layer for it.

Without a layer, we never call LayerTree::RegisterElement() and so even though we create the animation, it's considered composite-able, and we copy it to the impl thread, we never see true for ElementAnimations::has_element_in_active_list and so we never tick any animations, similar to my notes in  http://crbug.com/674258#c3 .

We do create an effect node for the DIV with a compositing reason, see PaintPropertyTreeBuilder::updateEffect() ref |compositingReasons|.

Layers in SPv2 (note there is none for DIV):

 {
  "layers": [
    {
      "name": "LayoutView #document",
      "bounds": [800, 600],
      "contentsOpaque": true,
      "drawsContent": true,
      "paintChunkContents": [
        [
          {
            "index": 2,
            "type": "DrawingDocumentBackground",
            "clientDebugName": "clientDebugName: \"LayoutView #document\"",
            "visualRect": "[0,0 800x600]"
          }
        ]
      ]
    }
  ]
}

Display item list in SPv2:

  {
    "index": 0,
    "properties": "client: \"0x16803604018 LayoutView #document\", type: \"ClipFrameToVisibleContentRect\", clipRect: [0,0,800,600]",
    "clientDebugName": "clientDebugName: \"LayoutView #document\"",
    "visualRect": "[0,0 800x600]"
  },
  {
    "index": 1,
    "properties": "client: \"0x16803614010 LayoutView #document\", type: \"Subsequence\"",
    "clientDebugName": "clientDebugName: \"LayoutView #document\"",
    "visualRect": "[0,0 800x600]"
  },
  {
    "index": 2,
    "properties": "client: \"0x16803604018 LayoutView #document\", type: \"DrawingDocumentBackground\", rect: [0.000000,0.000000 800.000000x600.000000]",
    "clientDebugName": "clientDebugName: \"LayoutView #document\"",
    "visualRect": "[0,0 800x600]"
  },
  {
    "index": 3,
    "properties": "client: \"0x168036140f0 LayoutBlockFlow HTML\", type: \"Subsequence\"",
    "clientDebugName": "clientDebugName: \"LayoutBlockFlow HTML\"",
    "visualRect": "[0,0 800x116]"
  },
  {
    "index": 4,
    "properties": "client: \"0x168036140f0 LayoutBlockFlow HTML\", type: \"EndSubsequence\"",
    "clientDebugName": "clientDebugName: \"LayoutBlockFlow HTML\"",
    "visualRect": "[0,0 800x116]"
  },
  {
    "index": 5,
    "properties": "client: \"0x16803614010 LayoutView #document\", type: \"EndSubsequence\"",
    "clientDebugName": "clientDebugName: \"LayoutView #document\"",
    "visualRect": "[0,0 800x600]"
  },
  {
    "index": 6,
    "properties": "client: \"0x16803604018 LayoutView #document\", type: \"EndClipFrameToVisibleContentRect\"",
    "clientDebugName": "clientDebugName: \"LayoutView #document\"",
    "visualRect": "[0,0 800x600]"
  }

Layers in SPv1 (the first three are boilerplate, note the last one for DIV):

  "layers": [
    {
      "this": "0x2e851f151450",
      "name": "Frame Clipping Layer",
      "bounds": [800, 600],
      "client": "0x2e851f090520",
      "compositingReasons": [],
      "squashingDisallowedReasons": []
    },
    {
      "this": "0x2e851f151210",
      "name": "Frame Scrolling Layer",
      "client": "0x2e851f090520",
      "compositingReasons": [],
      "squashingDisallowedReasons": []
    },
    {
      "this": "0x2e851f150b50",
      "name": "Content Root Layer",
      "bounds": [800, 600],
      "client": "0x2e851f090520",
      "compositingReasons": [],
      "squashingDisallowedReasons": []
    },
    {
      "this": "0x2e851f152890",
      "name": "LayoutView #document",
      "bounds": [800, 600],
      "contentsOpaque": true,
      "drawsContent": true,
      "client": "0x2e851f164250",
      "compositingReasons": [
        "Is the root layer"
      ],
      "squashingDisallowedReasons": []
    },
    {
      "this": "0x2e851f152ad0",
      "name": "LayoutBlockFlow DIV",
      "position": [8, 8],
      "bounds": [100, 100],
      "opacity": 0,
      "contentsOpaque": true,
      "drawsContent": true,
      "client": "0x2e851f164370",
      "backgroundColor": "#FF0000",
      "compositingReasons": [
        "Has an active accelerated animation or transition"
      ],
      "squashingDisallowedReasons": []
    }
  ]
}

Display item list in SPv1:

(for the document layer)

  {
    "index": 0,
    "properties": "client: \"0x3f2b7c204018 LayoutView #document\", type: \"DrawingDocumentBackground\", rect: [0.000000,0.000000 800.000000x600.000000]",
    "clientDebugName": "clientDebugName: \"LayoutView #document\"",
    "visualRect": "[0,0 800x600]"
  },
  {
    "index": 1,
    "properties": "client: \"0x3f2b7c2140f0 LayoutBlockFlow HTML\", type: \"Subsequence\"",
    "clientDebugName": "clientDebugName: \"LayoutBlockFlow HTML\"",
    "visualRect": "[0,0 800x116]"
  },
  {
    "index": 2,
    "properties": "client: \"0x3f2b7c2140f0 LayoutBlockFlow HTML\", type: \"EndSubsequence\"",
    "clientDebugName": "clientDebugName: \"LayoutBlockFlow HTML\"",
    "visualRect": "[0,0 800x116]"
  }

(for the div layer)

  {
    "index": 0,
    "properties": "client: \"0x3f2b7c21c268 LayoutBlockFlow DIV\", type: \"DrawingBoxDecorationBackground\", rect: [0.000000,0.000000 100.000000x100.000000]",
    "clientDebugName": "clientDebugName: \"LayoutBlockFlow DIV\"",
    "visualRect": "[0,0 100x100]"
  }

 
opacity.html
272 bytes View Download
Notes aiming to figure out why we never paint the div background in SPv2. Presuming it's some difference with invalidation, so tracing that logic.

In SPv1 when we invalidate paint for the div the paint invalidation container is the box itself:

From BoxPaintInvalidator::invalidatePaintIfNeeded -> ObjectPaintInvalidator::invalidatePaintUsingContainer :

(gdb) p paintInvalidationContainer.showLayoutTreeForThis()
LayoutView 0x11c674804010               #document
  LayoutBlockFlow 0x11c67481c010        HTML
    LayoutBlockFlow 0x11c67481c138      BODY
*     LayoutBlockFlow 0x11c67481c260    DIV

thus we execute setBackingNeedsPaintInvalidationInRect(...) with the new dirty rect, reason = blink::PaintInvalidationLayoutObjectInsertion, dirtyRect = LayoutPoint(0px, 0px), m_size = LayoutSize(100px, 100px) which ends up doing:

    layer.compositedLayerMapping()->setContentsNeedDisplayInRect(rect, reason,
                                                                 m_object);

which wouldn't be feasible for SPv2 anyway due to CLM dependence.

Code ref at https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp?q=objectpaintinvalidator.cpp&sq=package:chromium&dr&l=330

In SPv2 the paint invalidation container is the LayoutView #document for some reason, which means we skip the above and instead execute invalidatePaintRectangleOnWindow(). Code ref at https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp?q=objectpaintinvalidator.cpp&sq=package:chromium&dr&l=374

This seems not what's intended since that block is noted as specifically for unrooted things such as SVG, though maybe a single element that's become composited is considered one of these unrooted cases? Seems unlikely.
In SPv2 the div is not considered a paint invalidation container because it fails this test:

PaintInvalidator::invalidatePaintIfNeeded ->
  PaintInvalidator::updateContext ->
    object.isPaintInvalidationContainer() ->
      LayoutObject::isPaintInvalidationContainer ->
         hasLayer() &&
         toLayoutBoxModelObject(this)->layer()->isPaintInvalidationContainer();

The div has a PaintLayer but PaintLayer::isPaintInvalidationContainer is based on:

         compositingState() == PaintsIntoOwnBacking ||
         compositingState() == PaintsIntoGroupedBacking

And in SPv2 this compositing state is not set/relevant.

For comparison, in SPv1 the div is considered a paint invalidation container as its layer does have compositing state == PaintsIntoOwnBacking. The div is set as the paint invalidation container in the context in LayoutBoxModelObject::invalidateTreeIfNeeded before we recurse down into invalidatePaintIfNeeded().

SPv1 paint layer tree:

  LayoutView 0x5eec7804010 at (0,0) size 800x600
 positive z-order list(1)
   layer 0x5eec78140f0 at (0,0) size 800x116
    LayoutBlockFlow 0x5eec781c010 {HTML} at (0,0) size 800x116
      LayoutBlockFlow 0x5eec781c138 {BODY} at (8,8) size 784x100
   positive z-order list(1)
*    layer 0x5eec78141d0 at (8,8) size 100x100 transparent (composited, bounds=at (0,0) size 100x100, drawsContent=1)
      LayoutBlockFlow 0x5eec781c260 {DIV} at (0,0) size 100x100 [bgcolor=#FF0000]

SPv2 paint layer tree:

  LayoutView 0x21ab9a404010 at (0,0) size 800x600
 positive z-order list(1)
   layer 0x21ab9a4140f0 at (0,0) size 800x116
    LayoutBlockFlow 0x21ab9a41c010 {HTML} at (0,0) size 800x116
      LayoutBlockFlow 0x21ab9a41c138 {BODY} at (8,8) size 784x100
   positive z-order list(1)
*    layer 0x21ab9a4141d0 at (8,8) size 100x100 transparent
      LayoutBlockFlow 0x21ab9a41c260 {DIV} at (0,0) size 100x100 [bgcolor=#FF0000]

Possible that the above may not turn out to be useful. The fact that invalidation is performed differently may not relate to our failure to subsequently paint. We are, in the end, invalidating the div, though we're doing so against a different container. I'll trace paint in more detail next.
The div is marked as having a self-painting layer, thus BlockPainter::paintChild skips painting it when painting its parent BODY.

The div is considered to need/have a self-painting layer due to PaintLayer::isSelfPaintingLayerForIntrinsicOrScrollingReasons() returning true due to layoutObject()->layerTypeRequired() == NormalPaintLayer.

The layer type is NormalPaintLayer (vs. NoPaintLayer) due to LayoutBox::layerTypeRequired() checking style()->shouldCompositeForCurrentAnimations() which will return true for hasCurrentOpacityAnimation().

So, like, the div is all configured as having an animation and needing to paint into its own layer, but for SPv2 layering is decided layer/separately and we need to paint the thing regardless.

I would think we need to change something for SPv2 so that we don't skip painting children with a self-painting layer.  Will discuss further.
Cc: loyso@chromium.org
PaintLayerPainter::paintedOutputInvisible() skips painting the self-painting layer for the opacity animation as its opacity is 0.0. We could hack this by adding logic to not skip if the layer has an animation animating from opacity of 0.0.

We expect further such hacks could turn out to be necessary involving for example an offscreen element animating from 0.1 - 0.5 in a loop, and then coming on-screen through application of a 3d transform.

The root problem for such cases in SPv2 is that, if/when we paint nothing for an animated object, we have no paint chunk, and thus we end up creating no layer, and the Animation subsystem is predicated on a Layer existing with an ElementId in order to trigger LayerTree::RegisterElement.

An alternate hack might be, for any case in which we've a self-painting layer due to a composited animation, always paint a dummy display item such that we ensure a paint chunk and thus a layer. This left us with an unsavory taste in our mouths, so to speak.

We'd prefer to avoid these hacks. Discussion with loyso@ as next step. If the animation subsystem were revised to be driven solely by property tree nodes, and remove dependency on layers, that would resolve this issue. As I understand it this is already a medium/long term goal for cc.

Comment 5 by ajuma@chromium.org, Jan 26 2017

Removing the dependency on layers fixes part of this, but if we paint nothing for the animated object what happens after the animation starts (e.g. say the next frame of the animation has opacity 0.1, and we haven't yet gone back to the main frame for another commit).
I was presuming that if we remove the layer dependency, the animation will be fully wired up and ticking, and when the opacity changes it will enact a style update -> invalidate -> paint and at that point it won't hit the PaintLayerPainter::paintedOutputInvisible() checks as opacity != 0.0.

But I may be missing something you know re: how animations / frame / commit / paint interact?

Comment 7 by ajuma@chromium.org, Jan 26 2017

Animation updates in cc don't trigger any main thread work on their own (so no style update or invalidate or paint). Instead, the opacity value in the effect tree will change and then the compositor will draw another frame using the existing layers.
http://i.imgur.com/ForPOJQ.gif

Then, somehow, we need to force painting the drawings for self-painting layers with a composited animation.

Doing this in a performant manner for all situations where today we have optimizations around things that are offscreen or otherwise culled/paint-skipped may be tricky. We should consider and discuss thoughts.
I wanted to understand how the animated div's background is painted initially for SPv1, given what we found for SPv2 where it bails on initial frame paint due to opacity = 0.0, and c#5 above which must mean that we have the full painted output for SPv1 somehow.

The answer is that in SPv1 we never call PaintLayerPainter::paint() for the animated div's paint layer. Rather, since it's composited and has its own graphics layer, and in SPv1 FrameView::paintTree() iterates through GraphicsLayers painting them all, we see a call stack like below which goes straight from CompositedLayerMapping::doPaintTask() to PaintLayerPainter::paintLayerContents(). This code path never calls PaintLayerPainter::paintedOutputInvisible() (whose only callsite is from PaintLayerPainter::paint()).

#1 0x7f4a0ef170b1 blink::BoxPainter::paintBoxDecorationBackgroundWithRect()
#2 0x7f4a0ef16d5a blink::BoxPainter::paintBoxDecorationBackground()
#3 0x7f4a0eb91285 blink::LayoutBox::paintBoxDecorationBackground()
#4 0x7f4a0ef019ed blink::BlockPainter::paintObject()
#5 0x7f4a0eb41be5 blink::LayoutBlock::paintObject()
#6 0x7f4a0ef00fa3 blink::BlockPainter::paint()
#7 0x7f4a0eb41b65 blink::LayoutBlock::paint()
#8 0x7f4a0ef79259 blink::PaintLayerPainter::paintFragmentWithPhase()
#9 0x7f4a0ef7787b blink::PaintLayerPainter::paintBackgroundForFragments()
#10 0x7f4a0ef76bfd blink::PaintLayerPainter::paintLayerContents()
#11 0x7f4a0ed0bebc blink::CompositedLayerMapping::doPaintTask()
#12 0x7f4a0ed0d33f blink::CompositedLayerMapping::paintContents()
#13 0x7f4a115bfc0c blink::GraphicsLayer::paintWithoutCommit()
#14 0x7f4a115bf7af blink::GraphicsLayer::paint()
#15 0x7f4a0e61796d blink::FrameView::paintGraphicsLayerRecursively()
#16 0x7f4a0e617a2b blink::FrameView::paintGraphicsLayerRecursively()
#17 0x7f4a0e617a2b blink::FrameView::paintGraphicsLayerRecursively()
#18 0x7f4a0e617a2b blink::FrameView::paintGraphicsLayerRecursively()
#19 0x7f4a0e617a2b blink::FrameView::paintGraphicsLayerRecursively()
#20 0x7f4a0e617a2b blink::FrameView::paintGraphicsLayerRecursively()
#21 0x7f4a0e616aa1 blink::FrameView::paintTree()

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ce0839c605303284391c7efacccdcc00dc38437

commit 9ce0839c605303284391c7efacccdcc00dc38437
Author: wkorman <wkorman@chromium.org>
Date: Fri Jan 27 05:48:04 2017

Paint invisible layer content in presence of composited animations.

For SPv2 we must paint layer content even if the content is invisible
w.r.t. opacity.

This is required (a) so that we have a paint chunk, which will produce a layer,
which is required by the animation subsystem, and (b) so that cc has the paint
commands needed in order to actually enact the animation compositor-side through
mutating the property tree state and re-rastering as needed.

In SPv1 this code path (PaintLayerPainter::paintedOutputInvisible) is skipped
when painting a layer for composited animated content. Instead of calling
PaintLayerPainter::paint(), we end up painting the GraphicsLayer for the
composited animation via CompositedLayerMapping::doPaintTask ->
PaintLayerPainter::paintLayerContents.

BUG= 684842 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2652273004
Cr-Commit-Position: refs/heads/master@{#446605}

[modify] https://crrev.com/9ce0839c605303284391c7efacccdcc00dc38437/third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp
[modify] https://crrev.com/9ce0839c605303284391c7efacccdcc00dc38437/third_party/WebKit/Source/core/paint/PaintLayerPainter.h
[modify] https://crrev.com/9ce0839c605303284391c7efacccdcc00dc38437/third_party/WebKit/Source/core/paint/PaintLayerPainterTest.cpp
[modify] https://crrev.com/9ce0839c605303284391c7efacccdcc00dc38437/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilder.cpp
[modify] https://crrev.com/9ce0839c605303284391c7efacccdcc00dc38437/third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
[modify] https://crrev.com/9ce0839c605303284391c7efacccdcc00dc38437/third_party/WebKit/Source/platform/graphics/paint/EffectPaintPropertyNode.h
[modify] https://crrev.com/9ce0839c605303284391c7efacccdcc00dc38437/third_party/WebKit/Source/platform/graphics/paint/TransformPaintPropertyNode.h

Status: Fixed (was: Started)

Sign in to add a comment