Force flattening for elements with non-initial value of opacity

Force flattening for elements with non-initial value of opacity

As specified in latest editor's draft of [css-transforms], non-initial value
of opacity should force the used value transform-style to become flat.

Approved Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/eBIp90_il1o

BUG=617798
Committed: https://crrev.com/696f27afa62eb426545c29cefe0b0b43feeafe94
Cr-Commit-Position: refs/heads/master@{#401197}

[trchen, 2016-06-14 21:16:20]

Description was changed from

==========
Force flattening for elements with non-initial value of opacity

As specified in latest editor's draft of [css-transforms], non-initial value
of opacity should force the used value transform-style to become flat.

Approved Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/eBIp90_il1o

BUG=617798
==========

to

==========
Force flattening for elements with non-initial value of opacity

As specified in latest editor's draft of [css-transforms], non-initial value
of opacity should force the used value transform-style to become flat.

Approved Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/eBIp90_il1o

BUG=617798
==========

[trchen, 2016-06-14 21:16:21]

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

[trchen, 2016-06-14 21:16:25]

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

[commit-bot: I haz the power, 2016-06-14 21:17:08]

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

[commit-bot: I haz the power, 2016-06-14 22:39:14]

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

[commit-bot: I haz the power, 2016-06-14 22:39:15]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[chrishtr, 2016-06-15 16:06:41]

https://codereview.chromium.org/2065233002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2065233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1265: if
(transformStyle3D() == TransformStyle3DFlat)
Does this really matter for performance?

https://codereview.chromium.org/2065233002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.h (right):

https://codereview.chromium.org/2065233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.h:462: bool
hasNonInitialOpacity() const { return opacity() != 1.0f ||
hasWillChangeOpacityHint() || hasCurrentOpacityAnimation(); }
hasOpacity() is the way to check for inequality with 1.0.

Also, do hasWillChangeOpacityHint() or hasCurrentOpacityAnimation() cause
stacking contexts in the code right now?

[trchen, 2016-06-18 00:27:09]

https://codereview.chromium.org/2065233002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2065233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1265: if
(transformStyle3D() == TransformStyle3DFlat)
On 2016/06/15 16:06:41, chrishtr wrote:
> Does this really matter for performance?

I don't have numbers, but checking will-change list can be potentially
expensive.

https://codereview.chromium.org/2065233002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.h (right):

https://codereview.chromium.org/2065233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.h:462: bool
hasNonInitialOpacity() const { return opacity() != 1.0f ||
hasWillChangeOpacityHint() || hasCurrentOpacityAnimation(); }
On 2016/06/15 16:06:41, chrishtr wrote:
> hasOpacity() is the way to check for inequality with 1.0.

Done.

> Also, do hasWillChangeOpacityHint() or hasCurrentOpacityAnimation() cause
> stacking contexts in the code right now?

Yes for will-change:opacity.

No for opacity animation (on the frame that opacity happens to be 1.0). I think
it is a bug but isn't really important.

[chrishtr, 2016-06-18 08:22:53]

lgtm

https://codereview.chromium.org/2065233002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/style/ComputedStyle.cpp (right):

https://codereview.chromium.org/2065233002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/style/ComputedStyle.cpp:1265: if
(transformStyle3D() == TransformStyle3DFlat)
On 2016/06/18 at 00:27:09, trchen wrote:
> On 2016/06/15 16:06:41, chrishtr wrote:
> > Does this really matter for performance?
> 
> I don't have numbers, but checking will-change list can be potentially
expensive.

All this saves is a call to hasGroupingProperty. I don't think it's worth it.

[trchen, 2016-06-20 22:22:05]

The CQ bit was checked by trchen@chromium.org

[trchen, 2016-06-20 22:22:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/2065233002/#ps40001
(title: "get rid of early out")

[commit-bot: I haz the power, 2016-06-20 22:22:22]

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

[commit-bot: I haz the power, 2016-06-20 23:17:57]

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

[commit-bot: I haz the power, 2016-06-20 23:17:59]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[trchen, 2016-06-21 21:37:24]

The CQ bit was checked by trchen@chromium.org

[commit-bot: I haz the power, 2016-06-21 21:38:27]

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

[commit-bot: I haz the power, 2016-06-21 23:03:42]

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

[commit-bot: I haz the power, 2016-06-21 23:03:43]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[trchen, 2016-06-22 02:12:38]

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

[commit-bot: I haz the power, 2016-06-22 02:13:33]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2065233002/60001

[commit-bot: I haz the power, 2016-06-22 03:33:43]

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

[commit-bot: I haz the power, 2016-06-22 03:33:44]

Dry run: This issue passed the CQ dry run.

[trchen, 2016-06-22 03:36:06]

The CQ bit was checked by trchen@chromium.org

[trchen, 2016-06-22 03:36:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/2065233002/#ps60001
(title: "fix assert. (overflow needs to be adjusted prior to querying used
transform-style)")

[commit-bot: I haz the power, 2016-06-22 03:36:21]

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

[commit-bot: I haz the power, 2016-06-22 03:40:43]

Description was changed from

==========
Force flattening for elements with non-initial value of opacity

As specified in latest editor's draft of [css-transforms], non-initial value
of opacity should force the used value transform-style to become flat.

Approved Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/eBIp90_il1o

BUG=617798
==========

to

==========
Force flattening for elements with non-initial value of opacity

As specified in latest editor's draft of [css-transforms], non-initial value
of opacity should force the used value transform-style to become flat.

Approved Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/eBIp90_il1o

BUG=617798
==========

[commit-bot: I haz the power, 2016-06-22 03:40:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-22 03:42:04]

Description was changed from

==========
Force flattening for elements with non-initial value of opacity

As specified in latest editor's draft of [css-transforms], non-initial value
of opacity should force the used value transform-style to become flat.

Approved Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/eBIp90_il1o

BUG=617798
==========

to

==========
Force flattening for elements with non-initial value of opacity

As specified in latest editor's draft of [css-transforms], non-initial value
of opacity should force the used value transform-style to become flat.

Approved Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/eBIp90_il1o

BUG=617798

Committed: https://crrev.com/696f27afa62eb426545c29cefe0b0b43feeafe94
Cr-Commit-Position: refs/heads/master@{#401197}
==========

[commit-bot: I haz the power, 2016-06-22 03:42:05]

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