Enable compositing of scrolling content on low-DPI when LCD text will be maintained
The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.
R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
Committed: https://crrev.com/ef06887f125941e96ab19fba834d9027d36d4c88
Cr-Commit-Position: refs/heads/master@{#420955}
Description was changed from ========== Enable compositing of scrolling content on low-DPI when LCD text ...
4 years, 3 months ago
(2016-09-21 19:38:57 UTC)
#2
Description was changed from
==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained
The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.
I've been running layout tests ocally with the change and no failures. Trybots
will check browser tests.
R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========
to
==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained
The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.
I've been running layout tests locally with the change and no failures. Trybots
will check browser tests.
R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2358203002/1
4 years, 3 months ago
(2016-09-21 19:39:09 UTC)
#3
Based off a similar change in https://codereview.chromium.org/1988423002/. The goal is to turn on this feature ...
4 years, 3 months ago
(2016-09-21 19:40:08 UTC)
#4
Based off a similar change in https://codereview.chromium.org/1988423002/. The
goal is to turn on this feature in official builds but have an opt-out mechanism
that we can access for finch experiments.
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
4 years, 3 months ago
(2016-09-21 19:48:29 UTC)
#5
Dry run: Try jobs failed on following builders: cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_android/builds/132322) linux_chromium_chromeos_ozone_rel_ng on ...
4 years, 3 months ago
(2016-09-21 19:48:30 UTC)
#6
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_rel_ng/builds/283726)
4 years, 3 months ago
(2016-09-21 21:06:39 UTC)
#10
On 2016/09/21 19:40:08, Stephen Chennney wrote: > Based off a similar change in https://codereview.chromium.org/1988423002/. The ...
4 years, 2 months ago
(2016-09-22 14:59:14 UTC)
#11
On 2016/09/21 19:40:08, Stephen Chennney wrote:
> Based off a similar change in https://codereview.chromium.org/1988423002/. The
> goal is to turn on this feature in official builds but have an opt-out
mechanism
> that we can access for finch experiments.
Ouch. Apparently tests have changed. Investigating.
Stephen Chennney
On 2016/09/22 14:59:14, Stephen Chennney wrote: > On 2016/09/21 19:40:08, Stephen Chennney wrote: > > ...
4 years, 2 months ago
(2016-09-23 14:49:21 UTC)
#12
On 2016/09/22 14:59:14, Stephen Chennney wrote:
> On 2016/09/21 19:40:08, Stephen Chennney wrote:
> > Based off a similar change in https://codereview.chromium.org/1988423002/.
The
> > goal is to turn on this feature in official builds but have an opt-out
> mechanism
> > that we can access for finch experiments.
>
> Ouch. Apparently tests have changed. Investigating.
Fixed the asserts so there should not be crashers. Some tests have changed so
getting new baselines. There are 3 classes of test result changes:
1) Minor pixel differences on scrollbar painting due to now compositing the
scroller
2) Changes to layer trees now that we composite the scroller. Have manually
verified the test result is otherwise correct.
3) Some invalidations have grown larger, specifically the scrollbar
invalidation. The corresponding content invalidation is the same, but more
scrollbar is invalidated. I will seek an explanation for this.
chrishtr
lgtm I see crashes still in the latest CQ dry run: ASSERTION FAILED: !m_zOrderListsDirty Is ...
4 years, 2 months ago
(2016-09-23 15:05:24 UTC)
#13
lgtm
I see crashes still in the latest CQ dry run:
ASSERTION FAILED: !m_zOrderListsDirty
Is this fixed yet?
chrishtr
I did not mean to LGTM just yet.
4 years, 2 months ago
(2016-09-23 15:10:05 UTC)
#14
I did not mean to LGTM just yet.
Stephen Chennney
On 2016/09/23 15:05:24, chrishtr wrote: > lgtm > > I see crashes still in the ...
4 years, 2 months ago
(2016-09-23 15:22:13 UTC)
#15
On 2016/09/23 15:05:24, chrishtr wrote:
> lgtm
>
> I see crashes still in the latest CQ dry run:
>
> ASSERTION FAILED: !m_zOrderListsDirty
>
> Is this fixed yet?
I understand the inadvertent lgtm.
The dirty z order lists assert was fixed by
https://codereview.chromium.org/2365673002. It's confusing, but only the *_rel
trybots are with that patch, and there are no crashes so far.
I'll put up new baselines once I manage to get try runs on all platforms. A
couple of people need to sign off on the changes to test results.
Stephen Chennney
Description was changed from ========== Enable compositing of scrolling content on low-DPI when LCD text ...
4 years, 2 months ago
(2016-09-23 15:44:29 UTC)
#16
Description was changed from
==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained
The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.
I've been running layout tests locally with the change and no failures. Trybots
will check browser tests.
R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========
to
==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained
The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.
R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========
Stephen Chennney
New baselines. For paint/invalidation, wangxianxhu could you look at those? For layers, flackr could you ...
4 years, 2 months ago
(2016-09-23 21:21:05 UTC)
#17
New baselines.
For paint/invalidation, wangxianxhu could you look at those?
For layers, flackr could you comment?
The cases where we get float overlay wrong is a known issue that we decided to
live with.
Other tests are minor differences due to compositing.
Xianzhu
On 2016/09/23 21:21:05, Stephen Chennney wrote: > New baselines. > > For paint/invalidation, wangxianxhu could ...
4 years, 2 months ago
(2016-09-23 21:32:26 UTC)
#18
On 2016/09/23 21:21:05, Stephen Chennney wrote:
> New baselines.
>
> For paint/invalidation, wangxianxhu could you look at those?
>
> For layers, flackr could you comment?
>
> The cases where we get float overlay wrong is a known issue that we decided to
> live with.
>
> Other tests are minor differences due to compositing.
Given the layer tree structure changes are all as expected, the new paint
invalidations look legit. Only one minor thing: in
paint/invalidation/flexbox/scrollbars-changed.html, there seem too many
invalidations of the vertical scrollbar layers.
flackr
On 2016/09/23 at 21:32:26, wangxianzhu wrote: > On 2016/09/23 21:21:05, Stephen Chennney wrote: > > ...
4 years, 2 months ago
(2016-09-23 22:01:54 UTC)
#19
On 2016/09/23 at 21:32:26, wangxianzhu wrote:
> On 2016/09/23 21:21:05, Stephen Chennney wrote:
> > New baselines.
> >
> > For paint/invalidation, wangxianxhu could you look at those?
> >
> > For layers, flackr could you comment?
> >
> > The cases where we get float overlay wrong is a known issue that we decided
to
> > live with.
> >
> > Other tests are minor differences due to compositing.
>
> Given the layer tree structure changes are all as expected, the new paint
invalidations look legit. Only one minor thing: in
paint/invalidation/flexbox/scrollbars-changed.html, there seem too many
invalidations of the vertical scrollbar layers.
The layer tree structures look correct.
Looks like there is a problem with
third_party/WebKit/LayoutTests/platform/linux/fast/overflow/overflow-stacking-expected.png
Stephen Chennney
On 2016/09/23 22:01:54, flackr wrote: > On 2016/09/23 at 21:32:26, wangxianzhu wrote: > > On ...
4 years, 2 months ago
(2016-09-24 00:01:34 UTC)
#20
On 2016/09/23 22:01:54, flackr wrote:
> On 2016/09/23 at 21:32:26, wangxianzhu wrote:
> > On 2016/09/23 21:21:05, Stephen Chennney wrote:
> > > New baselines.
> > >
> > > For paint/invalidation, wangxianxhu could you look at those?
> > >
> > > For layers, flackr could you comment?
> > >
> > > The cases where we get float overlay wrong is a known issue that we
decided
> to
> > > live with.
> > >
> > > Other tests are minor differences due to compositing.
> >
> > Given the layer tree structure changes are all as expected, the new paint
> invalidations look legit. Only one minor thing: in
> paint/invalidation/flexbox/scrollbars-changed.html, there seem too many
> invalidations of the vertical scrollbar layers.
I propose we file a bug on this one. I agree it's odd but my guess is that it
reveals an existing problem.
> The layer tree structures look correct.
>
> Looks like there is a problem with
>
third_party/WebKit/LayoutTests/platform/linux/fast/overflow/overflow-stacking-expected.png
This is a known issue. Vollick, chrishtr and myself discussed back when we
originally landed the behind-a-flag version and we decided to accept the failure
because it's the fundamental compositing bug and
--enable-prefer-compositing-to-lcd-text users (Mac, ChromeOS) have always seen
it. fast/overflow/overflow-float-stacking-expected.png also demonstrates the
problem.
kinuko
lgtm for content/ and platform changes once all breakages are fixed & other reviewers are ...
4 years, 2 months ago
(2016-09-25 14:49:11 UTC)
#21
lgtm for content/ and platform changes once all breakages are fixed & other
reviewers are happy
kinuko
https://codereview.chromium.org/2358203002/diff/40001/content/public/common/content_features.cc File content/public/common/content_features.cc (right): https://codereview.chromium.org/2358203002/diff/40001/content/public/common/content_features.cc#newcode25 content/public/common/content_features.cc:25: base::FEATURE_ENABLED_BY_DEFAULT}; The indent looks off
4 years, 2 months ago
(2016-09-25 14:49:21 UTC)
#22
Description was changed from ========== Enable compositing of scrolling content on low-DPI when LCD text ...
4 years, 2 months ago
(2016-09-26 19:16:26 UTC)
#31
Message was sent while issue was closed.
Description was changed from
==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained
The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.
R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========
to
==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained
The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.
R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years, 2 months ago
(2016-09-26 19:16:33 UTC)
#32
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
commit-bot: I haz the power
Description was changed from ========== Enable compositing of scrolling content on low-DPI when LCD text ...
4 years, 2 months ago
(2016-09-26 19:21:41 UTC)
#33
Message was sent while issue was closed.
Description was changed from
==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained
The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.
R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
==========
to
==========
Enable compositing of scrolling content on low-DPI when LCD text will be
maintained
The RuntimeEnabledFeature is set to on for stable, with switches added to
enable a finch reverse experiment that disables the feature. We should thus
be able to determine the performance impact of this change.
R=chrishtr@chromium.org,kinuko@chromium.org
BUG=649051
Committed: https://crrev.com/ef06887f125941e96ab19fba834d9027d36d4c88
Cr-Commit-Position: refs/heads/master@{#420955}
==========
commit-bot: I haz the power
Patchset 4 (id:??) landed as https://crrev.com/ef06887f125941e96ab19fba834d9027d36d4c88 Cr-Commit-Position: refs/heads/master@{#420955}
4 years, 2 months ago
(2016-09-26 19:21:43 UTC)
#34
Issue 2358203002: Enable compositing of scrolling content on low-DPI when LCD text will be maintained
(Closed)
Created 4 years, 3 months ago by Stephen Chennney
Modified 4 years, 2 months ago
Reviewers: chrishtr, kinuko
Base URL:
Comments: 2