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

Issue 699834 link

Starred by 3 users

Issue metadata

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

Blocking:
issue 471333



Sign in to add a comment

Implement rational clipping and effects in SPv2

Project Member Reported by chrishtr@chromium.org, Mar 9 2017

Issue description

We should modify the clip and effect trees in Blink in order to enforce

1. The property trees sent from Blink to cc will not have any clips that escape effects. ("fx-containment" from Jeremy's doc)
2. All rounded rect clips will be represented in the effect tree. ("cc-square-corners")
3. All effects can be drawn atomically in cc.

Reference doc (describes the above as well as the needed changes) https://docs.google.com/document/d/1H_SfEixMCUUG5m3D0K1tc58xSVwNT3Sm1ak4V87qug0/edit#

 
Cc: chrishtr@chromium.org pdr@chromium.org weiliangc@chromium.org
 Issue 597158  has been merged into this issue.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 25 2017

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

commit 930a3adf239219b1606b8cbca691f9130fb466ca
Author: chrishtr <chrishtr@chromium.org>
Date: Tue Apr 25 22:14:51 2017

[SPv2] Apply effects before clips for when the effect does not move pixels

In such cases, the clip and effect commute, so we have the option to apply
them in either order.

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

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

[modify] https://crrev.com/930a3adf239219b1606b8cbca691f9130fb466ca/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeState.cpp
[modify] https://crrev.com/930a3adf239219b1606b8cbca691f9130fb466ca/third_party/WebKit/Source/platform/graphics/paint/PropertyTreeStateTest.cpp

Owner: trchen@chromium.org
Current sketch of a prototype:

https://codereview.chromium.org/2859673004
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 10 2017

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

commit 74cabe344138045a60157eecf6f6514de0230c0c
Author: Tien-Ren Chen <trchen@chromium.org>
Date: Thu Aug 10 05:14:13 2017

[SPv2] Implement rounded clip as synthetic effect nodes

This CL implements composited rounded clip for SPv2. Currently the compositor only
supports applying clips that are rects orthogonal to target render surface. Therefore
we implement rounded clips as mask layers. As mask layers need to be applied as effect
nodes, these rounded-clip-induced effects are called "synthetic effect node" as oppose
to real effect nodes that represents a grouping effect.

This CL changes PropertyTreeManager so that the converted cc effect tree have synthetic
effect nodes inserted in the tree while maintaining proper nesting for layers.

For example with the following clip and effect tree and pending layers:
E0 <-- E1
C0 <-- C1(rounded)
P0(E1,C0), P1(E1,C1), P2(E0, C1)
In effect stack diagram:
P0(C0) P1(C1)
[    E1     ] P2(C1)
[        E0        ]

The following cc property trees and layers will be generated:
E0 <+- E1 <-- E_C1_1 <-- E_C1_1M
    +- E_C1_2 <-- E_C1_2M
C0 <-- C1
L0(E1,C0), L1(E_C1_1, C1), L_C1_1(E_C1_1M, C1), L2(E0, C1), L_C1_2(E_C1_2M, C1)
In effect stack diagram:
                L_C1_1
       L1(C1) [ E_C1_1M ]          L_C2_2
L0(C0) [     E_C1_1     ] L2(C1) [ E_C1_2M ]
[          E1           ][     E_C1_2      ]
[                    E0                    ]

Two mask layers L1_C1_1 and L2_C1_2 are generated for clip C1 because it can't apply
to P1 and P2 as a group due to nesting. Two synthetic effects E_C1_1 and E_C1_2 are
generated to isolate the masked contents, and another two synthetic effects E_C1_1M
and E_C1_2M are generated to apply the mask layer.

BUG= 699834 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I0b8ddf2cc9f81966b570bb085d8cb57a06a0b125
Reviewed-on: https://chromium-review.googlesource.com/597126
Commit-Queue: Tien-Ren Chen <trchen@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493248}
[modify] https://crrev.com/74cabe344138045a60157eecf6f6514de0230c0c/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/74cabe344138045a60157eecf6f6514de0230c0c/third_party/WebKit/Source/platform/graphics/CompositorElementId.cpp
[modify] https://crrev.com/74cabe344138045a60157eecf6f6514de0230c0c/third_party/WebKit/Source/platform/graphics/CompositorElementId.h
[modify] https://crrev.com/74cabe344138045a60157eecf6f6514de0230c0c/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/74cabe344138045a60157eecf6f6514de0230c0c/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h
[modify] https://crrev.com/74cabe344138045a60157eecf6f6514de0230c0c/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/74cabe344138045a60157eecf6f6514de0230c0c/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
[modify] https://crrev.com/74cabe344138045a60157eecf6f6514de0230c0c/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.h
[modify] https://crrev.com/74cabe344138045a60157eecf6f6514de0230c0c/third_party/WebKit/Source/platform/graphics/paint/PaintPropertyNode.h

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 10 2017

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

commit 6e6241cd595997c6d0404be9578022d74a761230
Author: Olga Sharonova <olka@chromium.org>
Date: Thu Aug 10 10:51:01 2017

Revert "[SPv2] Implement rounded clip as synthetic effect nodes"

This reverts commit 74cabe344138045a60157eecf6f6514de0230c0c.

Reason for revert: crbug/754201
PaintArtifactCompositorTestWithPropertyTrees.SynthesizedClip*: failing on chromium.webkit/WebKit Linux Trusty MSAN

Original change's description:
> [SPv2] Implement rounded clip as synthetic effect nodes
> 
> This CL implements composited rounded clip for SPv2. Currently the compositor only
> supports applying clips that are rects orthogonal to target render surface. Therefore
> we implement rounded clips as mask layers. As mask layers need to be applied as effect
> nodes, these rounded-clip-induced effects are called "synthetic effect node" as oppose
> to real effect nodes that represents a grouping effect.
> 
> This CL changes PropertyTreeManager so that the converted cc effect tree have synthetic
> effect nodes inserted in the tree while maintaining proper nesting for layers.
> 
> For example with the following clip and effect tree and pending layers:
> E0 <-- E1
> C0 <-- C1(rounded)
> P0(E1,C0), P1(E1,C1), P2(E0, C1)
> In effect stack diagram:
> P0(C0) P1(C1)
> [    E1     ] P2(C1)
> [        E0        ]
> 
> The following cc property trees and layers will be generated:
> E0 <+- E1 <-- E_C1_1 <-- E_C1_1M
>     +- E_C1_2 <-- E_C1_2M
> C0 <-- C1
> L0(E1,C0), L1(E_C1_1, C1), L_C1_1(E_C1_1M, C1), L2(E0, C1), L_C1_2(E_C1_2M, C1)
> In effect stack diagram:
>                 L_C1_1
>        L1(C1) [ E_C1_1M ]          L_C2_2
> L0(C0) [     E_C1_1     ] L2(C1) [ E_C1_2M ]
> [          E1           ][     E_C1_2      ]
> [                    E0                    ]
> 
> Two mask layers L1_C1_1 and L2_C1_2 are generated for clip C1 because it can't apply
> to P1 and P2 as a group due to nesting. Two synthetic effects E_C1_1 and E_C1_2 are
> generated to isolate the masked contents, and another two synthetic effects E_C1_1M
> and E_C1_2M are generated to apply the mask layer.
> 
> BUG= 699834 
> 
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
> Change-Id: I0b8ddf2cc9f81966b570bb085d8cb57a06a0b125
> Reviewed-on: https://chromium-review.googlesource.com/597126
> Commit-Queue: Tien-Ren Chen <trchen@chromium.org>
> Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#493248}

TBR=trchen@chromium.org,pdr@chromium.org,chrishtr@chromium.org

Change-Id: I731812ca251d3641dbd3ed083177efaf048011ac
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  699834 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Reviewed-on: https://chromium-review.googlesource.com/609017
Reviewed-by: Olga Sharonova <olka@chromium.org>
Commit-Queue: Olga Sharonova <olka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493353}
[modify] https://crrev.com/6e6241cd595997c6d0404be9578022d74a761230/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/6e6241cd595997c6d0404be9578022d74a761230/third_party/WebKit/Source/platform/graphics/CompositorElementId.cpp
[modify] https://crrev.com/6e6241cd595997c6d0404be9578022d74a761230/third_party/WebKit/Source/platform/graphics/CompositorElementId.h
[modify] https://crrev.com/6e6241cd595997c6d0404be9578022d74a761230/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/6e6241cd595997c6d0404be9578022d74a761230/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h
[modify] https://crrev.com/6e6241cd595997c6d0404be9578022d74a761230/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/6e6241cd595997c6d0404be9578022d74a761230/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
[modify] https://crrev.com/6e6241cd595997c6d0404be9578022d74a761230/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.h
[modify] https://crrev.com/6e6241cd595997c6d0404be9578022d74a761230/third_party/WebKit/Source/platform/graphics/paint/PaintPropertyNode.h

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 11 2017

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

commit f799ac5d51f242122fd171d5376111e2dbc30e55
Author: Tien-Ren Chen <trchen@chromium.org>
Date: Fri Aug 11 01:04:13 2017

Reland "[SPv2] Implement rounded clip as synthetic effect nodes"

The original CL forgot to initialize a SkRRect, now it's zero-initialized.

This CL implements composited rounded clip for SPv2. Currently the compositor only
supports applying clips that are rects orthogonal to target render surface. Therefore
we implement rounded clips as mask layers. As mask layers need to be applied as effect
nodes, these rounded-clip-induced effects are called "synthetic effect node" as oppose
to real effect nodes that represents a grouping effect.

This CL changes PropertyTreeManager so that the converted cc effect tree have synthetic
effect nodes inserted in the tree while maintaining proper nesting for layers.

For example with the following clip and effect tree and pending layers:
E0 <-- E1
C0 <-- C1(rounded)
P0(E1,C0), P1(E1,C1), P2(E0, C1)
In effect stack diagram:
P0(C0) P1(C1)
[    E1     ] P2(C1)
[        E0        ]

The following cc property trees and layers will be generated:
E0 <+- E1 <-- E_C1_1 <-- E_C1_1M
    +- E_C1_2 <-- E_C1_2M
C0 <-- C1
L0(E1,C0), L1(E_C1_1, C1), L_C1_1(E_C1_1M, C1), L2(E0, C1), L_C1_2(E_C1_2M, C1)
In effect stack diagram:
                L_C1_1
       L1(C1) [ E_C1_1M ]          L_C2_2
L0(C0) [     E_C1_1     ] L2(C1) [ E_C1_2M ]
[          E1           ][     E_C1_2      ]
[                    E0                    ]

Two mask layers L1_C1_1 and L2_C1_2 are generated for clip C1 because it can't apply
to P1 and P2 as a group due to nesting. Two synthetic effects E_C1_1 and E_C1_2 are
generated to isolate the masked contents, and another two synthetic effects E_C1_1M
and E_C1_2M are generated to apply the mask layer.

BUG= 699834 

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I5f64c47c9b58938106926d1abe1220b024e00929
Reviewed-on: https://chromium-review.googlesource.com/610487
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Tien-Ren Chen <trchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493627}
[modify] https://crrev.com/f799ac5d51f242122fd171d5376111e2dbc30e55/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/f799ac5d51f242122fd171d5376111e2dbc30e55/third_party/WebKit/Source/platform/graphics/CompositorElementId.cpp
[modify] https://crrev.com/f799ac5d51f242122fd171d5376111e2dbc30e55/third_party/WebKit/Source/platform/graphics/CompositorElementId.h
[modify] https://crrev.com/f799ac5d51f242122fd171d5376111e2dbc30e55/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp
[modify] https://crrev.com/f799ac5d51f242122fd171d5376111e2dbc30e55/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.h
[modify] https://crrev.com/f799ac5d51f242122fd171d5376111e2dbc30e55/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp
[modify] https://crrev.com/f799ac5d51f242122fd171d5376111e2dbc30e55/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp
[modify] https://crrev.com/f799ac5d51f242122fd171d5376111e2dbc30e55/third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.h
[modify] https://crrev.com/f799ac5d51f242122fd171d5376111e2dbc30e55/third_party/WebKit/Source/platform/graphics/paint/PaintPropertyNode.h

Is this bug now fixed?
Status: Fixed (was: Assigned)

Sign in to add a comment