Smooth window closing animation with external processes |
|||||
Issue descriptionWhen a window closes for an out of process client the window close animation is not as smooth as it is when in process. You can see this with the keyboard shortcut viewer (press control-alt-/ then close window). This is likely a combination of having ash draw the frame decorations, and the external app draw the content. The content from the external app is connected to ash by way of ClientSurfaceEmbedder [1]. ws2 makes use of ClientSurfaceEmbedder from ClientRoot [2]. I'm not sure what the exact issue is. It may be the client is detached *before* the close animation starts, which would mean ClientSurfaceEmbedder is torn down before the clone of the layers is created. This would likely result in the behavior we're seeing. Fady/Sadrul, do you guys have any other ideas? I will be OOO until the 9th. I don't think this should block KSV, but it would be good to solve soonish. If someone has the cycles, please grab it. [1] https://chromium.googlesource.com/chromium/src/+/master/ui/aura/mus/client_surface_embedder.cc [2] https://chromium.googlesource.com/chromium/src/+/master/services/ui/ws2/client_root.cc
,
Jun 24 2018
The same approach we take for renderers when animate-closing a window should work for this too, right? (in both cases, the content includes out-of-process content)
,
Jun 25 2018
This will be a visual regression vs. the old shortcut viewer. fsamuel, any chance this could get on the worklist for your next sprint?
,
Jun 25 2018
I'll take a look at it sometime this week.
,
Jul 9
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e52d8104a10a7357e49ab1ac21a9c2dfbca2ff4e commit e52d8104a10a7357e49ab1ac21a9c2dfbca2ff4e Author: Scott Violet <sky@chromium.org> Date: Tue Jul 10 00:37:32 2018 views: makes DesktopNativeWidgetAura::Close() not call content_window_->Hide() Prior to this patch DesktopNativeWidgetAura::Close() would call content_window_->Hide() and then hide the WindowTreeHost. The call to hide for the WindowTreeHost is what triggers animations in ash. The hide/close animation works by cloning the layers. Because the content window was hidden before the close animation starts, the animations would happen on a hidden layer (all black). This patch moves the calls to content_window->Hide() to the DesktopWindowTreeHost implementations, and changes DesktopWindowTreeHostMus to hide the WindowTreeHost first. This ensures animations in ash operate on the content. I tend to think hiding the content is entirely unnecessary, but I am not about to try to tackle that here. BUG= 855658 TEST=covered by test Change-Id: Ie07775beecc2a6627ac2d7c6e0c57ad563bbb8d1 Reviewed-on: https://chromium-review.googlesource.com/1130502 Reviewed-by: Michael Wasserman <msw@chromium.org> Commit-Queue: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#573555} [modify] https://crrev.com/e52d8104a10a7357e49ab1ac21a9c2dfbca2ff4e/ui/views/mus/desktop_window_tree_host_mus.cc [modify] https://crrev.com/e52d8104a10a7357e49ab1ac21a9c2dfbca2ff4e/ui/views/mus/desktop_window_tree_host_mus_unittest.cc [modify] https://crrev.com/e52d8104a10a7357e49ab1ac21a9c2dfbca2ff4e/ui/views/widget/desktop_aura/desktop_native_widget_aura.cc [modify] https://crrev.com/e52d8104a10a7357e49ab1ac21a9c2dfbca2ff4e/ui/views/widget/desktop_aura/desktop_window_tree_host_platform.cc [modify] https://crrev.com/e52d8104a10a7357e49ab1ac21a9c2dfbca2ff4e/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc [modify] https://crrev.com/e52d8104a10a7357e49ab1ac21a9c2dfbca2ff4e/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
,
Jul 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8f21903ed1fd103550952df63a74ddf1f2b89e56 commit 8f21903ed1fd103550952df63a74ddf1f2b89e56 Author: Scott Violet <sky@chromium.org> Date: Tue Jul 10 15:55:42 2018 chromeos: make ClientSurfaceEmbedder use LayerOwner The animation code may end up cloning layers. The cloning logic is only done if the Layer has a LayerOwner. This means without the LayerOwner the close/hide animation doesn't work right. This convers ClientSurfaceEmbedder to use a LayerOwner so that close/hide animation works correctly. BUG= 855658 TEST=none Change-Id: I92587100526c7f9c06603a1e29610c93b12e0f4d Reviewed-on: https://chromium-review.googlesource.com/1130234 Reviewed-by: Fady Samuel <fsamuel@chromium.org> Commit-Queue: Scott Violet <sky@chromium.org> Cr-Commit-Position: refs/heads/master@{#573751} [modify] https://crrev.com/8f21903ed1fd103550952df63a74ddf1f2b89e56/ui/aura/mus/client_surface_embedder.cc [modify] https://crrev.com/8f21903ed1fd103550952df63a74ddf1f2b89e56/ui/aura/mus/client_surface_embedder.h [modify] https://crrev.com/8f21903ed1fd103550952df63a74ddf1f2b89e56/ui/compositor/layer_owner.cc [modify] https://crrev.com/8f21903ed1fd103550952df63a74ddf1f2b89e56/ui/compositor/layer_owner.h
,
Jul 10
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by sky@chromium.org
, Jun 22 2018