New issue
Advanced search Search tips

Issue 890919 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 3
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 836886



Sign in to add a comment

[BGPT] Rotated clips

Project Member Reported by wangxianzhu@chromium.org, Oct 1

Issue description

This bug tracks the remaining issues of found during investigation of  bug 879173 .

The basic case is:
<!DOCTYPE html>
<div id="container" style="margin: 100px; transform: rotate(45deg); overflow: hidden;
            width: 200px; height: 200px; border: 20px solid green">
  <div id="content" style="will-change: transform; width: 400px; height: 400px; background: red">
  </div>
</div>

The following cases are based on the basic case, which may affect the implementation of the fix:
1. The rotated clip can be a non-stacking-context:
<div id="container" style="margin: 100px; transform: rotate(45deg); width: 200px">
  <div id="clip" style="overflow: hidden; width: 200px; height: 200px; border: 20px solid green">
    <div id="content" style="will-change: transform; width: 400px; height: 400px; background: red">
    </div>
  </div>
</div>

1a. A more complicated text case: the clip of the non-stacking-context needs to apply on two paint sub-trees:
<div id="rotate" style="margin: 100px; width: 300px; transform: rotate(45deg)">
  <div id="yellow" style="position: absolute; width: 200px; height: 22px; background: yellow; z-index: 1; will-change: transform"></div>
  <div id="clip" style="width: 100px; height: 100px; overflow: hidden">
    <div id="green" style="position: relative; width: 200px; height: 100px; background: green; will-change: transform"></div>
    <div id="red" style="position: relative; top: -100px; z-index: 2; width: 50px; height: 200px; background: red; will-change: transform"></div>
  </div>
</div>

2. The rotated clip interleaves with an effect. Test case TBD.

These cases are rare, so this bug should not block experimental launch of BGPT.
 
A WIP fix is https://chromium-review.googlesource.com/c/chromium/src/+/1252522. It has some problems:
1. It creates too many extra effect nodes;
2. It doesn't fix case 1. Or we need to create effect node on non-stacking-context, but the effect will apply on two discrete subtrees which might be expected by PaintChunksToCcLayer and/or PropertyTreeManager.

Another approach is not to create blink effect node for rotated clips, but create them in PropertyTreeManager, in a way similar to what we do for interleaving effect and clip.
Correction to #c1: s/which might be expected/which is unexpected/.

I've verified that PropertyTreeManager doesn't expect effect on non-stacking-context: https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/graphics/compositing/property_tree_manager.cc?rcl=c77be1ddec7d5b2a31c4ddb1fb844f50ad89eff4&l=622
Cc: vmp...@chromium.org
Just to re-state the issue: cc clip nodes have a restriction that requires an effect node to ensure clipping works properly. This restriction has to do with render surface decisions that are not made until later in cc. The issue is: where should these nodes be created: PaintArtifactCompositor (PAC) or PropertyTreeManager (PTM)?

To write down some reasons why the PaintArtifactCompositor approach is good:
1) Just one place to create nodes, rather than two (PAC & PTM)
2) Easy to detect some cases because the context is readily available

And some reasons why the PropertyTreeManager approach is good:
1) Can be more optimal about only creating nodes that are needed
2) Keeps cc knowledge closer to cc

It sounds like property tree manager is the only viable approach for now, so SGTM to give that a shot.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 3

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

commit 491d10976dc99e58b57857916ea4ecaadeecee37
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Wed Oct 03 18:53:27 2018

[BGPT] Generate effect node for rotated clip and force render surface

Cc requires that a rectangular clip is 2d-axis-aligned with the render
surface to correctly apply the clip. When we find that a rectangular
clip is not 2d-axis-aligned with the render surface, we should create
an effect node and let it create a render surface.

Bug:  890919 
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I6f7b462d5506949d1571c60231e066d27b9a238d
Reviewed-on: https://chromium-review.googlesource.com/1244104
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596304}
[modify] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-clip-expected.png
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-clip-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-clip.html
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-then-clip-effect-interleave-expected.png
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-then-clip-effect-interleave-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-then-clip-effect-interleave.html
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-then-clip-expected.png
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-then-clip-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-then-clip-z-order-interleave-expected.png
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-then-clip-z-order-interleave-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-then-clip-z-order-interleave.html
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/compositing/overflow/rotate-then-clip.html
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-blink-gen-property-trees/compositing/overflow/rotate-clip-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-blink-gen-property-trees/compositing/overflow/rotate-then-clip-effect-interleave-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-blink-gen-property-trees/compositing/overflow/rotate-then-clip-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-blink-gen-property-trees/compositing/overflow/rotate-then-clip-z-order-interleave-expected.png
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-blink-gen-property-trees/compositing/overflow/rotate-then-clip-z-order-interleave-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/compositing/overflow/rotate-clip-expected.png
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/compositing/overflow/rotate-clip-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/compositing/overflow/rotate-then-clip-effect-interleave-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/compositing/overflow/rotate-then-clip-expected.png
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/compositing/overflow/rotate-then-clip-expected.txt
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/compositing/overflow/rotate-then-clip-z-order-interleave-expected.png
[add] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/compositing/overflow/rotate-then-clip-z-order-interleave-expected.txt
[modify] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/blink/renderer/platform/graphics/compositing/property_tree_manager.cc
[modify] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/blink/renderer/platform/graphics/compositing/property_tree_manager.h
[modify] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/blink/renderer/platform/transforms/transformation_matrix.cc
[modify] https://crrev.com/491d10976dc99e58b57857916ea4ecaadeecee37/third_party/blink/renderer/platform/transforms/transformation_matrix.h

Status: Fixed (was: Assigned)

Sign in to add a comment