New issue
Advanced search Search tips

Issue 905859 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Ancestor opacity seems to not be affecting rendering of descendant in BGPT / spv2

Project Member Reported by flackr@chromium.org, Nov 15

Issue description

In spv2, there seem to be cases where an ancestor filter node's opacity does not affect the rendering of the descendant, see

transitions/opacity-transform-transitions-inside-iframe.html

Notably this started failing after https://chromium-review.googlesource.com/c/chromium/src/+/1297305 where the opacity is applied in a distinct filter node.
 
Status: Assigned (was: Untriaged)
Here's the minimal testcase:
<!DOCTYPE html>
<style>
  body { background: black; }
  #outer {
    background: white;
    width: 100px;
    height: 100px;
    animation: 100s opacityanim;
  }
  #inner {
    background: red;
    width: 20px;
    height: 20px;
    will-change: opacity;
  }
  @keyframes opacityanim {
    from { opacity: 0.4; }
    to   { opacity: 0.4; }
  }
</style>
<div id="outer">
    <div id="inner"></div>
</div>


This should have a small red square in a grey square.
After https://chromium-review.googlesource.com/c/chromium/src/+/1297305, this is a small pink square in a grey square.
Cc: pdr@chromium.org
Labels: -Pri-3 Pri-2
Owner: wangxianzhu@chromium.org
It seems that we are rendering the layers in wrong hierarchy:

Expected:
 outer: opacity: 0.4
   inner

The inner's final color should be red*0.4 + black*0.6.

Actual:
 outer: opacity 0.4
 inner: opacity 0.4

The inner's final color is red*0.4 + (white*0.4 + black*0.6)*0.6

It seems that the render surface logic at https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/graphics/compositing/property_tree_manager.cc?rcl=89035f09a029139da8d81f18c93989dfdc9ac701&l=689 doesn't work as expected.

The bug disappears if I force render surface for all effect nodes.

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 5

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

commit d560b578390fb80b01229e8db64a878eb7e8dfb8
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Wed Dec 05 22:27:50 2018

[BGPT/CAP] Create render surface for opacity effect with child

We delay render surface decision for opacity-only effects. Previously we
would create a render surface for an opacity-only effect only when we
saw more than one layer or child for the effect. This caused missing
render surface in the following case:

    Opacity (0.5)
        |
  No-op effect (for compositing)
        | -- layer1
Another no-op effect (for compositing)
        | -- layer2

Though the opacity node has only one child, it affects 2 layers through
its descendants, so a render surface is needed.

Now move the optimization for opacity-only-effects out of
PropertyTreeManager into PaintArtifactCompositor::
UpdateRenderSurfaceForEffects() which is run after all layers are
updated and can handle the bug case.

Bug:  905859 
Change-Id: Id2e63b42ef539f7bd97357389d4b31a908cbe50a
Reviewed-on: https://chromium-review.googlesource.com/c/1357083
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614134}
[modify] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc
[modify] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.h
[modify] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor_test.cc
[modify] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/renderer/platform/graphics/compositing/property_tree_manager.cc
[modify] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/renderer/platform/graphics/compositing/property_tree_manager.h
[modify] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/web_tests/FlagExpectations/enable-blink-features=BlinkGenPropertyTrees
[modify] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/web_tests/FlagExpectations/enable-blink-features=CompositeAfterPaint
[add] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/web_tests/flag-specific/enable-blink-features=BlinkGenPropertyTrees/compositing/overflow/nested-render-surfaces-with-rotation-expected.png
[add] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/web_tests/flag-specific/enable-blink-features=BlinkGenPropertyTrees/virtual/prefer_compositing_to_lcd_text/compositing/overflow/nested-render-surfaces-with-rotation-expected.png
[modify] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/web_tests/flag-specific/enable-blink-features=CompositeAfterPaint/compositing/gestures/gesture-tapHighlight-pixel-rotated-div-expected.png
[modify] https://crrev.com/d560b578390fb80b01229e8db64a878eb7e8dfb8/third_party/blink/web_tests/flag-specific/enable-blink-features=CompositeAfterPaint/compositing/gestures/gesture-tapHighlight-pixel-rotated-link-expected.png

Status: Fixed (was: Assigned)

Sign in to add a comment