Use counters for opacity with transform-style:preserve-3d

Use counters for opacity with transform-style:preserve-3d

This CL add a use counter for elements that have opacity, preserve-3d,
and 3D-transformed descendants at the same time.

This is done as a part of CSS transform spec research to collect statistics
about compatibility quirks. Currently all vendors implement this corner case
as applying opacity to each descendant planes separately, but latest W3C
editor's draft recommends forcing transform-style to flat.

BUG=612913, 598917
Committed: https://crrev.com/9d8ea9509008c684392a42d688dfd3ababdf9f66
Cr-Commit-Position: refs/heads/master@{#394920}

[trchen, 2016-05-18 22:06:27]

The CQ bit was checked by trchen@chromium.org to run a CQ dry run

[trchen, 2016-05-18 22:06:36]

trchen@chromium.org changed reviewers:
+ chrishtr@chromium.org

[commit-bot: I haz the power, 2016-05-18 22:06:57]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1987283003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1987283003/1

[commit-bot: I haz the power, 2016-05-18 22:09:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-18 22:09:46]

Dry run: Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[trchen, 2016-05-18 22:23:03]

The CQ bit was checked by trchen@chromium.org to run a CQ dry run

[trchen, 2016-05-18 22:23:33]

trchen@chromium.org changed reviewers:
+ holte@google.com

[trchen, 2016-05-18 22:23:33]

+holte for histogram

[commit-bot: I haz the power, 2016-05-18 22:23:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1987283003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1987283003/20001

[chrishtr, 2016-05-18 23:16:03]

https://codereview.chromium.org/1987283003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/GraphicsLayerUpdater.cpp
(right):

https://codereview.chromium.org/1987283003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/GraphicsLayerUpdater.cpp:108:
if (style.preserves3D() && style.hasOpacity() &&
layer.has3DTransformedDescendant())
Does this account for whether the child flattens itself, as opposed to being
inside the same preserves-3d
space as this element?

[chrishtr, 2016-05-18 23:22:10]

https://codereview.chromium.org/1987283003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/GraphicsLayerUpdater.cpp
(right):

https://codereview.chromium.org/1987283003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/GraphicsLayerUpdater.cpp:108:
if (style.preserves3D() && style.hasOpacity() &&
layer.has3DTransformedDescendant())
On 2016/05/18 at 23:16:03, chrishtr wrote:
> Does this account for whether the child flattens itself, as opposed to being
inside the same preserves-3d
> space as this element?

Also, I was expecting this usecounter to be in
CLM::updateGraphicsLayerConfiguration.

[Steven Holte, 2016-05-18 23:53:44]

holte@chromium.org changed reviewers:
+ holte@chromium.org

[Steven Holte, 2016-05-18 23:53:45]

lgtm

[chrishtr, 2016-05-19 00:01:58]

lgtm

[trchen, 2016-05-19 00:31:07]

The CQ bit was checked by trchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-19 00:32:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1987283003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1987283003/40001

[trchen, 2016-05-19 00:33:29]

The 3rd patchset fixes the assertion failure by invoking
update3DTransformedDescendantStatus() from CLM. I don't think it will create any
regression, but let me know if it sounds wrong to you.

https://codereview.chromium.org/1987283003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/compositing/GraphicsLayerUpdater.cpp
(right):

https://codereview.chromium.org/1987283003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/GraphicsLayerUpdater.cpp:108:
if (style.preserves3D() && style.hasOpacity() &&
layer.has3DTransformedDescendant())
On 2016/05/18 23:22:10, chrishtr wrote:
> On 2016/05/18 at 23:16:03, chrishtr wrote:
> > Does this account for whether the child flattens itself, as opposed to being
> inside the same preserves-3d
> > space as this element?
> 
> Also, I was expecting this usecounter to be in
> CLM::updateGraphicsLayerConfiguration.

Done.

[chrishtr, 2016-05-19 00:51:35]

https://codereview.chromium.org/1987283003/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
(right):

https://codereview.chromium.org/1987283003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:559:
m_owningLayer.update3DTransformedDescendantStatus();
This is a linear-time operation in the depth of the PaintLayer tree, which leads
to an n^2 overall.
You'll need to integrate this computation into CompositingInputsUpdater to avoid
a regression unfortunately.

[trchen, 2016-05-19 01:22:06]

On 2016/05/19 00:51:35, chrishtr wrote:
>
https://codereview.chromium.org/1987283003/diff/40001/third_party/WebKit/Sour...
> File
> third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
> (right):
> 
>
https://codereview.chromium.org/1987283003/diff/40001/third_party/WebKit/Sour...
>
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:559:
> m_owningLayer.update3DTransformedDescendantStatus();
> This is a linear-time operation in the depth of the PaintLayer tree, which
leads
> to an n^2 overall.
> You'll need to integrate this computation into CompositingInputsUpdater to
avoid
> a regression unfortunately.

I think it is O(n) where n is number of layers because the value is cached and
reused, i.e. dynamic programming.

[chrishtr, 2016-05-19 01:59:58]

On 2016/05/19 at 01:22:06, trchen wrote:
> On 2016/05/19 00:51:35, chrishtr wrote:
> >
https://codereview.chromium.org/1987283003/diff/40001/third_party/WebKit/Sour...
> > File
> > third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
> > (right):
> > 
> >
https://codereview.chromium.org/1987283003/diff/40001/third_party/WebKit/Sour...
> >
third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp:559:
> > m_owningLayer.update3DTransformedDescendantStatus();
> > This is a linear-time operation in the depth of the PaintLayer tree, which
leads
> > to an n^2 overall.
> > You'll need to integrate this computation into CompositingInputsUpdater to
avoid
> > a regression unfortunately.
> 
> I think it is O(n) where n is number of layers because the value is cached and
reused, i.e. dynamic programming.

Right, ok.

Let's double-check the UMAs for compositing don't get worse after this patch
though.

[chrishtr, 2016-05-19 02:00:02]

lgtm

[chrishtr, 2016-05-19 02:00:07]

The CQ bit was unchecked by chrishtr@chromium.org

[chrishtr, 2016-05-19 02:00:11]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-05-19 02:00:12]

The patchset sent to the CQ was uploaded after l-g-t-m from holte@chromium.org
Link to the patchset: https://codereview.chromium.org/1987283003/#ps40001
(title: "revised")

[commit-bot: I haz the power, 2016-05-19 02:00:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1987283003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1987283003/40001

[commit-bot: I haz the power, 2016-05-19 03:56:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-19 03:56:53]

Failed to apply patch for third_party/WebKit/Source/core/frame/UseCounter.h:
While running git apply --index -3 -p1;
  error: patch failed: third_party/WebKit/Source/core/frame/UseCounter.h:1188
  Falling back to three-way merge...
  Applied patch to 'third_party/WebKit/Source/core/frame/UseCounter.h' with
conflicts.
  U third_party/WebKit/Source/core/frame/UseCounter.h

Patch:       third_party/WebKit/Source/core/frame/UseCounter.h
Index: third_party/WebKit/Source/core/frame/UseCounter.h
diff --git a/third_party/WebKit/Source/core/frame/UseCounter.h
b/third_party/WebKit/Source/core/frame/UseCounter.h
index
aabdfaf7fa3850bb6da7e8716b9c86d6d6feadd5..855c3469ff3dcab107660a83194a00e21ee781a8
100644
--- a/third_party/WebKit/Source/core/frame/UseCounter.h
+++ b/third_party/WebKit/Source/core/frame/UseCounter.h
@@ -1188,6 +1188,7 @@ public:
         UntrustedEventDefaultHandled = 1372,
         FixedRasterScaleBlurryContent = 1373,
         FixedRasterScalePotentialPerformanceRegression = 1374,
+        OpacityWithPreserve3DQuirk = 1375,
 
         // Add new features immediately above this line. Don't change assigned
         // numbers of any item, and don't reuse removed slots.

[trchen, 2016-05-19 21:18:34]

The CQ bit was checked by trchen@chromium.org

[trchen, 2016-05-19 21:18:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org, holte@chromium.org
Link to the patchset: https://codereview.chromium.org/1987283003/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-19 21:19:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1987283003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1987283003/60001

[commit-bot: I haz the power, 2016-05-20 00:16:01]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-20 00:17:10]

Description was changed from

==========
Use counters for opacity with transform-style:preserve-3d

This CL add a use counter for elements that have opacity, preserve-3d,
and 3D-transformed descendants at the same time.

This is done as a part of CSS transform spec research to collect statistics
about compatibility quirks. Currently all vendors implement this corner case
as applying opacity to each descendant planes separately, but latest W3C
editor's draft recommends forcing transform-style to flat.

BUG=612913,598917
==========

to

==========
Use counters for opacity with transform-style:preserve-3d

This CL add a use counter for elements that have opacity, preserve-3d,
and 3D-transformed descendants at the same time.

This is done as a part of CSS transform spec research to collect statistics
about compatibility quirks. Currently all vendors implement this corner case
as applying opacity to each descendant planes separately, but latest W3C
editor's draft recommends forcing transform-style to flat.

BUG=612913,598917

Committed: https://crrev.com/9d8ea9509008c684392a42d688dfd3ababdf9f66
Cr-Commit-Position: refs/heads/master@{#394920}
==========

[commit-bot: I haz the power, 2016-05-20 00:17:11]

Patchset 4 (id:??) landed as
https://crrev.com/9d8ea9509008c684392a42d688dfd3ababdf9f66
Cr-Commit-Position: refs/heads/master@{#394920}