Title bar visible while transitioning to overview mode. |
||||
Issue descriptionWe'd want the transition to look like [1] eventually. But for now it would be sufficient to hide the title bar before transitioning to overview, instead of after. [1] https://mccanny.users.x20web.corp.google.com/www/wm-motion/index.html#overview-freeform
,
May 17 2018
There's no way to make this reliable smooth if we rely on raster for those animations. We should design these animations so we're not relying on raster if we want them to be smooth. Also, it looks like these animations use mask layers. That's has a significant cost and will limit how often these animations can be smooth.
,
May 18 2018
wutao@ - I did the some comparison on my worstation: Without deferred paint: Enter - max 100 - avg 43 Exit - max 64 - avg 44 With deferred paint: Enter - max 100 - avg 52 Exit - max 74 - avg 54 reveman@ - Are you referring to the new animations in the spec? UX is aware of the problems but for now we'd just like to remove the title bar so it doesn't appear during animation like in the screenshot.
,
May 18 2018
Thanks sammiequon@, it does have ~20% improvement at workstation. IIRU, on real device, the improvement may be not so big. Another possibility is that, we set hide the title bar and wait one frame, and then start animation with deferred paint in the next frame. There are several compositor callback, you can look into this as an example how to use it: https://chromium-review.googlesource.com/c/chromium/src/+/671688 Basically, you call hide title bar first, and then add "this" to the compositor observer, at the the OnCompositingDidCommit(), you start animations. Will this be too complicated?
,
Jun 13 2018
wutao@ please triage by assigning to appropriate owner?
,
Jun 13 2018
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a663e7fd51488006a133e4115cf306029ea7ee23 commit a663e7fd51488006a133e4115cf306029ea7ee23 Author: Sammie Quon <sammiequon@google.com> Date: Tue Jul 10 00:28:32 2018 overview: Hide title bars before animating into overview mode. The title bars should not visible while animating overview mode. This cl hides the title bar if needed and waits until the next paint before animating the windows. This will hide the title bar as expected while keeping the benefits of DeferPaint. Test: manual Bug: 843851 Change-Id: I99cf7ca39fd1ae14314ac406abd0fec999e2c024 Reviewed-on: https://chromium-review.googlesource.com/1112774 Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Tao Wu <wutao@chromium.org> Commit-Queue: Sammie Quon <sammiequon@chromium.org> Cr-Commit-Position: refs/heads/master@{#573552} [modify] https://crrev.com/a663e7fd51488006a133e4115cf306029ea7ee23/ash/system/overview/overview_button_tray_unittest.cc [modify] https://crrev.com/a663e7fd51488006a133e4115cf306029ea7ee23/ash/test/ash_test_helper.cc [modify] https://crrev.com/a663e7fd51488006a133e4115cf306029ea7ee23/ash/wm/overview/scoped_transform_overview_window.cc [modify] https://crrev.com/a663e7fd51488006a133e4115cf306029ea7ee23/ash/wm/overview/scoped_transform_overview_window.h [modify] https://crrev.com/a663e7fd51488006a133e4115cf306029ea7ee23/ash/wm/overview/window_selector_item.cc [modify] https://crrev.com/a663e7fd51488006a133e4115cf306029ea7ee23/ash/wm/overview/window_selector_item.h
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/90e3ae20e0e37d8dfc0284f2e4974a7a3861b409 commit 90e3ae20e0e37d8dfc0284f2e4974a7a3861b409 Author: Joshua Pawlicki <waffles@chromium.org> Date: Tue Jul 10 16:10:55 2018 Revert "overview: Hide title bars before animating into overview mode." This reverts commit a663e7fd51488006a133e4115cf306029ea7ee23. Reason for revert: Causes flaky thread-checker failure in debug builds; see crbug.com/862165 Original change's description: > overview: Hide title bars before animating into overview mode. > > The title bars should not visible while animating overview mode. This cl > hides the title bar if needed and waits until the next paint before > animating the windows. This will hide the title bar as expected while > keeping the benefits of DeferPaint. > > Test: manual > Bug: 843851 > Change-Id: I99cf7ca39fd1ae14314ac406abd0fec999e2c024 > Reviewed-on: https://chromium-review.googlesource.com/1112774 > Reviewed-by: Mitsuru Oshima <oshima@chromium.org> > Reviewed-by: Tao Wu <wutao@chromium.org> > Commit-Queue: Sammie Quon <sammiequon@chromium.org> > Cr-Commit-Position: refs/heads/master@{#573552} TBR=oshima@chromium.org,sammiequon@chromium.org,wutao@chromium.org Change-Id: I0878300bd4d9a5935d00776442f68b85a46981f2 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 843851 Reviewed-on: https://chromium-review.googlesource.com/1131714 Reviewed-by: Joshua Pawlicki <waffles@chromium.org> Commit-Queue: Joshua Pawlicki <waffles@chromium.org> Cr-Commit-Position: refs/heads/master@{#573759} [modify] https://crrev.com/90e3ae20e0e37d8dfc0284f2e4974a7a3861b409/ash/system/overview/overview_button_tray_unittest.cc [modify] https://crrev.com/90e3ae20e0e37d8dfc0284f2e4974a7a3861b409/ash/test/ash_test_helper.cc [modify] https://crrev.com/90e3ae20e0e37d8dfc0284f2e4974a7a3861b409/ash/wm/overview/scoped_transform_overview_window.cc [modify] https://crrev.com/90e3ae20e0e37d8dfc0284f2e4974a7a3861b409/ash/wm/overview/scoped_transform_overview_window.h [modify] https://crrev.com/90e3ae20e0e37d8dfc0284f2e4974a7a3861b409/ash/wm/overview/window_selector_item.cc [modify] https://crrev.com/90e3ae20e0e37d8dfc0284f2e4974a7a3861b409/ash/wm/overview/window_selector_item.h
,
Jul 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f00acd85de088eb905005c5bc0c144361caaa814 commit f00acd85de088eb905005c5bc0c144361caaa814 Author: Sammie Quon <sammiequon@google.com> Date: Wed Jul 11 17:21:12 2018 Reland "overview: Hide title bars before animating into overview mode." This is a reland of a663e7fd51488006a133e4115cf306029ea7ee23 The original cl causes some flaky tests with stack trace: [16202:16202:0709/211202.910243:FATAL:single_thread_proxy.cc(237)] Check failed: task_runner_provider_->IsMainThread(). #0 0x7f9b36f8672d base::debug::StackTrace::StackTrace() #1 0x7f9b36c9d3fc base::debug::StackTrace::StackTrace() #2 0x7f9b36d06dfd logging::LogMessage::~LogMessage() #3 0x7f9b2b5692e8 cc::SingleThreadProxy::SetNeedsCommit() #4 0x7f9b2b45ecef cc::LayerTreeHost::SetNeedsCommit() #5 0x7f9b2b467426 cc::LayerTreeHost::SetMutatorsNeedCommit() #6 0x7f9b0fd29981 cc::AnimationHost::SetNeedsCommit() #7 0x7f9b0fd1962d cc::Animation::SetNeedsCommit() #8 0x7f9b0fd41151 cc::KeyframeEffect::KeyframeModelAdded() #9 0x7f9b0fd44e6c cc::KeyframeEffect::AddKeyframeModel() #10 0x7f9b0fd17fbc cc::Animation::AddKeyframeModelForKeyframeEffect() #11 0x7f9b0fd71a6d cc::SingleKeyframeEffectAnimation::AddKeyframeModel() #12 0x7f9b28c83a95 ui::LayerAnimator::AddThreadedAnimation() #13 0x7f9b28c7498e ui::(anonymous namespace)::ThreadedLayerAnimationElement::RequestEffectiveStart() #14 0x7f9b28c71113 ui::LayerAnimationElement::Start() #15 0x7f9b28c7cbe3 ui::LayerAnimationSequence::Start() #16 0x7f9b28c84287 ui::LayerAnimator::StartSequenceImmediately() #17 0x7f9b28c81251 ui::LayerAnimator::StartAnimation() #18 0x7f9b28c80eb6 ui::LayerAnimator::SetTransform() #19 0x7f9b28c6122e ui::Layer::SetTransform() #20 0x7f9b28ded831 aura::Window::SetTransform() #21 0x7f9b22b732df ash::ScopedTransformOverviewWindow::SetTransform() #22 0x7f9b22b73f69 ash::ScopedTransformOverviewWindow::OnCompositingStarted() #23 0x7f9b28c4d734 ui::Compositor::DidSubmitCompositorFrame() #24 0x7f9b2b56cd05 cc::SingleThreadProxy::DoComposite() #25 0x7f9b2b56e835 cc::SingleThreadProxy::ScheduledActionDrawIfPossible() #26 0x7f9b2b3736fd cc::Scheduler::DrawIfPossible() #27 0x7f9b2b36dbdc cc::Scheduler::ProcessScheduledActions() #28 0x7f9b2b36d5fd cc::Scheduler::OnBeginImplFrameDeadline() This cl makes that test skip waiting for composit when entering overview, like the tests in ash. Original change's description: > overview: Hide title bars before animating into overview mode. > > The title bars should not visible while animating overview mode. This cl > hides the title bar if needed and waits until the next paint before > animating the windows. This will hide the title bar as expected while > keeping the benefits of DeferPaint. > > Test: manual > Bug: 843851 > Change-Id: I99cf7ca39fd1ae14314ac406abd0fec999e2c024 > Reviewed-on: https://chromium-review.googlesource.com/1112774 > Reviewed-by: Mitsuru Oshima <oshima@chromium.org> > Reviewed-by: Tao Wu <wutao@chromium.org> > Commit-Queue: Sammie Quon <sammiequon@chromium.org> > Cr-Commit-Position: refs/heads/master@{#573552} TBR=oshima@chromium.org Bug: 843851 Change-Id: I3a538250d5f6d42d6c73e218445080af533b54dc Reviewed-on: https://chromium-review.googlesource.com/1131874 Commit-Queue: Sammie Quon <sammiequon@chromium.org> Reviewed-by: Sammie Quon <sammiequon@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Cr-Commit-Position: refs/heads/master@{#574209} [modify] https://crrev.com/f00acd85de088eb905005c5bc0c144361caaa814/ash/system/overview/overview_button_tray_unittest.cc [modify] https://crrev.com/f00acd85de088eb905005c5bc0c144361caaa814/ash/test/ash_test_helper.cc [modify] https://crrev.com/f00acd85de088eb905005c5bc0c144361caaa814/ash/wm/overview/scoped_transform_overview_window.cc [modify] https://crrev.com/f00acd85de088eb905005c5bc0c144361caaa814/ash/wm/overview/scoped_transform_overview_window.h [modify] https://crrev.com/f00acd85de088eb905005c5bc0c144361caaa814/ash/wm/overview/window_selector_item.cc [modify] https://crrev.com/f00acd85de088eb905005c5bc0c144361caaa814/ash/wm/overview/window_selector_item.h [modify] https://crrev.com/f00acd85de088eb905005c5bc0c144361caaa814/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc
,
Jul 16
It seems this cl does not work with OnCompositingStarted anymore (issue 863795). Uploaded cl to fix the crash in the meantime, but this will need investigation why it started failing.
,
Jul 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b87381369ce222524f52343c2bd4f0387574191e commit b87381369ce222524f52343c2bd4f0387574191e Author: Sammie Quon <sammiequon@google.com> Date: Mon Jul 16 22:28:03 2018 overview: Fix crash with hiding title bars on entry. Using OnCompositorStarted does not work for overview anymore. See stack trace in bug. Test: manual Bug: 863795, 843851 Change-Id: I3dc8965cbece03ac36f1dc404c9b19b7dcac5b5e Reviewed-on: https://chromium-review.googlesource.com/1138737 Commit-Queue: Sammie Quon <sammiequon@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/master@{#575447} [modify] https://crrev.com/b87381369ce222524f52343c2bd4f0387574191e/ash/wm/overview/scoped_transform_overview_window.cc [modify] https://crrev.com/b87381369ce222524f52343c2bd4f0387574191e/ash/wm/overview/scoped_transform_overview_window.h
,
Jul 17
Chrome now crashes (hits a dcheck) when entering overview. This code snippet is not valid because the cast breaks for browser windows (among other window types):
CustomFrameViewAsh* frame =
static_cast<CustomFrameViewAsh*>(widget->non_client_view()->frame_view());
if (!frame)
return false;
,
Jul 18
Hi Sammie, are you aware of the above issue and/or working on a fix? It causes a crash in debug and undefined behavior in release, so I think we should fix it very soon or revert the offending change.
,
Jul 18
Hi estade - sorry i meant to reply yesterday but got sidetracked. There is a cl associated with issue 863795 submitted that should fix the crash on ToT. The cause was something else other than the invalid cast (which I will investigate). That should work for now in release ill investigate the debug crash more thoroughly.
,
Jul 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c89a63613279a3ebe0a0397c31f48949b55a4a6f commit c89a63613279a3ebe0a0397c31f48949b55a4a6f Author: Sammie Quon <sammiequon@chromium.org> Date: Wed Jul 18 18:06:19 2018 Revert "overview: Fix crash with hiding title bars on entry." This reverts commit b87381369ce222524f52343c2bd4f0387574191e. Reason for revert: crbug.com/863795 Original change's description: > overview: Fix crash with hiding title bars on entry. > > Using OnCompositorStarted does not work for overview anymore. See stack > trace in bug. > > Test: manual > Bug: 863795, 843851 > Change-Id: I3dc8965cbece03ac36f1dc404c9b19b7dcac5b5e > Reviewed-on: https://chromium-review.googlesource.com/1138737 > Commit-Queue: Sammie Quon <sammiequon@chromium.org> > Reviewed-by: Mitsuru Oshima <oshima@chromium.org> > Cr-Commit-Position: refs/heads/master@{#575447} TBR=oshima@chromium.org,sammiequon@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 863795, 843851 Change-Id: Ibc5965bc51a62e08b44d034a81f6954b8e799cad Reviewed-on: https://chromium-review.googlesource.com/1142166 Reviewed-by: Sammie Quon <sammiequon@chromium.org> Commit-Queue: Sammie Quon <sammiequon@chromium.org> Cr-Commit-Position: refs/heads/master@{#576135} [modify] https://crrev.com/c89a63613279a3ebe0a0397c31f48949b55a4a6f/ash/wm/overview/scoped_transform_overview_window.cc [modify] https://crrev.com/c89a63613279a3ebe0a0397c31f48949b55a4a6f/ash/wm/overview/scoped_transform_overview_window.h
,
Jul 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe17281dcd0cdcbd10524383b0eca16bd1d468c8 commit fe17281dcd0cdcbd10524383b0eca16bd1d468c8 Author: Sammie Quon <sammiequon@chromium.org> Date: Wed Jul 18 23:25:38 2018 Revert "Reland "overview: Hide title bars before animating into overview mode."" This reverts commit f00acd85de088eb905005c5bc0c144361caaa814. Reason for revert: crbug.com/863795 Original change's description: > Reland "overview: Hide title bars before animating into overview mode." > > This is a reland of a663e7fd51488006a133e4115cf306029ea7ee23 > > The original cl causes some flaky tests with stack trace: > > [16202:16202:0709/211202.910243:FATAL:single_thread_proxy.cc(237)] Check failed: task_runner_provider_->IsMainThread(). > #0 0x7f9b36f8672d base::debug::StackTrace::StackTrace() > #1 0x7f9b36c9d3fc base::debug::StackTrace::StackTrace() > #2 0x7f9b36d06dfd logging::LogMessage::~LogMessage() > #3 0x7f9b2b5692e8 cc::SingleThreadProxy::SetNeedsCommit() > #4 0x7f9b2b45ecef cc::LayerTreeHost::SetNeedsCommit() > #5 0x7f9b2b467426 cc::LayerTreeHost::SetMutatorsNeedCommit() > #6 0x7f9b0fd29981 cc::AnimationHost::SetNeedsCommit() > #7 0x7f9b0fd1962d cc::Animation::SetNeedsCommit() > #8 0x7f9b0fd41151 cc::KeyframeEffect::KeyframeModelAdded() > #9 0x7f9b0fd44e6c cc::KeyframeEffect::AddKeyframeModel() > #10 0x7f9b0fd17fbc cc::Animation::AddKeyframeModelForKeyframeEffect() > #11 0x7f9b0fd71a6d cc::SingleKeyframeEffectAnimation::AddKeyframeModel() > #12 0x7f9b28c83a95 ui::LayerAnimator::AddThreadedAnimation() > #13 0x7f9b28c7498e ui::(anonymous namespace)::ThreadedLayerAnimationElement::RequestEffectiveStart() > #14 0x7f9b28c71113 ui::LayerAnimationElement::Start() > #15 0x7f9b28c7cbe3 ui::LayerAnimationSequence::Start() > #16 0x7f9b28c84287 ui::LayerAnimator::StartSequenceImmediately() > #17 0x7f9b28c81251 ui::LayerAnimator::StartAnimation() > #18 0x7f9b28c80eb6 ui::LayerAnimator::SetTransform() > #19 0x7f9b28c6122e ui::Layer::SetTransform() > #20 0x7f9b28ded831 aura::Window::SetTransform() > #21 0x7f9b22b732df ash::ScopedTransformOverviewWindow::SetTransform() > #22 0x7f9b22b73f69 ash::ScopedTransformOverviewWindow::OnCompositingStarted() > #23 0x7f9b28c4d734 ui::Compositor::DidSubmitCompositorFrame() > #24 0x7f9b2b56cd05 cc::SingleThreadProxy::DoComposite() > #25 0x7f9b2b56e835 cc::SingleThreadProxy::ScheduledActionDrawIfPossible() > #26 0x7f9b2b3736fd cc::Scheduler::DrawIfPossible() > #27 0x7f9b2b36dbdc cc::Scheduler::ProcessScheduledActions() > #28 0x7f9b2b36d5fd cc::Scheduler::OnBeginImplFrameDeadline() > > This cl makes that test skip waiting for composit when entering overview, like the tests > in ash. > > Original change's description: > > overview: Hide title bars before animating into overview mode. > > > > The title bars should not visible while animating overview mode. This cl > > hides the title bar if needed and waits until the next paint before > > animating the windows. This will hide the title bar as expected while > > keeping the benefits of DeferPaint. > > > > Test: manual > > Bug: 843851 > > Change-Id: I99cf7ca39fd1ae14314ac406abd0fec999e2c024 > > Reviewed-on: https://chromium-review.googlesource.com/1112774 > > Reviewed-by: Mitsuru Oshima <oshima@chromium.org> > > Reviewed-by: Tao Wu <wutao@chromium.org> > > Commit-Queue: Sammie Quon <sammiequon@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#573552} > > TBR=oshima@chromium.org > > Bug: 843851 > Change-Id: I3a538250d5f6d42d6c73e218445080af533b54dc > Reviewed-on: https://chromium-review.googlesource.com/1131874 > Commit-Queue: Sammie Quon <sammiequon@chromium.org> > Reviewed-by: Sammie Quon <sammiequon@chromium.org> > Reviewed-by: Michael Wasserman <msw@chromium.org> > Cr-Commit-Position: refs/heads/master@{#574209} TBR=msw@chromium.org,oshima@chromium.org,sammiequon@chromium.org,wutao@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 843851 Change-Id: Ib2744c7f677be40004b70a714cebe0a337f7c60b Reviewed-on: https://chromium-review.googlesource.com/1142304 Reviewed-by: Sammie Quon <sammiequon@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Michael Wasserman <msw@chromium.org> Commit-Queue: Sammie Quon <sammiequon@chromium.org> Cr-Commit-Position: refs/heads/master@{#576250} [modify] https://crrev.com/fe17281dcd0cdcbd10524383b0eca16bd1d468c8/ash/system/overview/overview_button_tray_unittest.cc [modify] https://crrev.com/fe17281dcd0cdcbd10524383b0eca16bd1d468c8/ash/test/ash_test_helper.cc [modify] https://crrev.com/fe17281dcd0cdcbd10524383b0eca16bd1d468c8/ash/wm/overview/scoped_transform_overview_window.cc [modify] https://crrev.com/fe17281dcd0cdcbd10524383b0eca16bd1d468c8/ash/wm/overview/scoped_transform_overview_window.h [modify] https://crrev.com/fe17281dcd0cdcbd10524383b0eca16bd1d468c8/ash/wm/overview/window_selector_item.cc [modify] https://crrev.com/fe17281dcd0cdcbd10524383b0eca16bd1d468c8/ash/wm/overview/window_selector_item.h [modify] https://crrev.com/fe17281dcd0cdcbd10524383b0eca16bd1d468c8/chrome/browser/ui/views/frame/browser_non_client_frame_view_ash_browsertest.cc |
||||
►
Sign in to add a comment |
||||
Comment 1 by wutao@chromium.org
, May 17 2018