New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 766013 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 672962



Sign in to add a comment

Surface synchronization: Design and Implement Guttering strategy

Project Member Reported by fsamuel@google.com, Sep 17 2017

Issue description

Guttering is a bit subtle with surface synchronization: it's usually determined by the child except on the first frame, in which case it's determined by the parent.

For the first frame generated, the parent has a primary SurfaceInfo for the child client, but doesn't have a fallback SurfaceInfo. In that case, it's possible that at aggregation time, a CompositorFrame isn't ready from the child and we need a default behavior. That default behavior is to show a gutter the size of the primary SurfaceDrawQuad instead. The color of that quad should be determined by the parent, defaulting to white, likely.

Once a fallback is available, the gutter color will come from the fallback. Two SolidColorDrawQuads will be generated at aggregation time equal to the size of the difference between the primary and fallback surfaces: One for the bottom gutter and one for the right. This is similar to what's done in DelegatedFrameHost today.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/926ed774a89ac0aa5d4def934c8e430f89757ec7

commit 926ed774a89ac0aa5d4def934c8e430f89757ec7
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Sep 19 00:51:55 2017

Surface synchronization: Use default background color if no fallback specified.

When a parent first embeds a child, it picks a LocalSurfaceId for the child
and embeds it.

During that bootstrapping process, there is no fallback surface to provide
because the child hasn't yet submitted a CompositorFrame to viz.

Prior to this CL, if a primary surface isn't available and there is no fallback
then SurfaceAggregator would report a warning, and the surface would simply
be dropped.

This CL addresses this bootstrapping problem. It introduces the notion of a
"default background color" specified by the parent when the primary Surface
is unavailable and there is no fallback yet. In that case, SurfaceAggregator
will use the "default background color" to create a SolidColorDrawQuad in
place of the primary Surface.

This CL implements this color and plumbs it all the way from SurfaceLayer,
to SurfaceLayerImpl, SurfaceDrawQuad, serialization/deserialization to
SurfaceAggregator, ultimately where it decides whether or not to create
a SolidColorDrawQuad in lieu of a surface or not. All unit tests along this
plumbing have been updated accordingly.

Bug:  766013 
TBR: boliu@chromium.org
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I367f7eab7b0189d438926dea407076ef6007f0a4
Reviewed-on: https://chromium-review.googlesource.com/669444
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502736}
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/android_webview/browser/surfaces_instance.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/cc/ipc/cc_param_traits_macros.h
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/cc/ipc/cc_param_traits_unittest.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/cc/layers/surface_layer.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/cc/layers/surface_layer.h
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/cc/layers/surface_layer_impl.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/cc/layers/surface_layer_impl.h
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/cc/layers/surface_layer_impl_unittest.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/cc/layers/surface_layer_unittest.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/components/viz/common/quads/draw_quad_unittest.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/components/viz/common/quads/surface_draw_quad.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/components/viz/common/quads/surface_draw_quad.h
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/components/viz/service/display/surface_aggregator.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/components/viz/service/display/surface_aggregator_perftest.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/components/viz/service/display/surface_aggregator_pixeltest.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/components/viz/service/display/surface_aggregator_unittest.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/components/viz/test/surface_hittest_test_helpers.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/content/renderer/android/synchronous_layer_tree_frame_sink.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/services/viz/public/cpp/compositing/quads_struct_traits.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/services/viz/public/cpp/compositing/quads_struct_traits.h
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/services/viz/public/cpp/compositing/struct_traits_unittest.cc
[modify] https://crrev.com/926ed774a89ac0aa5d4def934c8e430f89757ec7/services/viz/public/interfaces/compositing/quads.mojom

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/720af21468343b19123f8f883d68c01a8097fafa

commit 720af21468343b19123f8f883d68c01a8097fafa
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Sep 19 08:19:25 2017

SurfaceAggregator: Place a SolidColorDrawQuad if surface missing

This is a generalization to the 669444 CL. If a fallback SurfaceDrawQuad is
provided but the Surface is unavailable at aggregation time then put a
SolidColorDrawQuad in its place. For release builds without DCHECK, use
the default background color provided by the parent. For DCHECK builds,
use Magneta to highlight that this is a bug.

Bug:  766013 
Change-Id: Ide73d60cd4f47b8e905f5011f67561811cf70aaa
Reviewed-on: https://chromium-review.googlesource.com/671158
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502799}
[modify] https://crrev.com/720af21468343b19123f8f883d68c01a8097fafa/components/viz/service/display/surface_aggregator.cc
[modify] https://crrev.com/720af21468343b19123f8f883d68c01a8097fafa/components/viz/service/display/surface_aggregator_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e494fc874f455309c9b025727b40a837bf713971

commit e494fc874f455309c9b025727b40a837bf713971
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Sep 19 20:17:42 2017

Simplify transparent backgrounds

Prior to this CL, there were two ways to specify transparent backgrounds:

1. LayerTreeHost::SetHasTransparentBackground(true)
2. LayerTreeHost::SetBackgroundColor(SK_ColorTRANSPARENT)

The former avoided filling the background of the LayerTreeHost with
SolidColorDrawQuads, while the latter would cause transparent gutter
in DelegatedFrameHost, and fill the background with *transparent*
SolidColorDrawQuads.

This CL unifies this into 2. Setting SK_ColorTRANSPARENT as the background
color will now avoid generating extra SolidColorDrawQuads.

After this CL, guttering can be implemented in Viz based on the background
color specified in CompositorFrameMetadata if that color is not
Sk_ColorTRANSPARENT. If the color is transparent, like in top level
windows in Mus+Ash, the embedder (Ash in this case) will perform the
guttering (in order to avoid obscuring the window decorations).

Bug:  766013 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Iaca9a230ba42cbd8cf70b8fb833af5e8b5e145ea
Reviewed-on: https://chromium-review.googlesource.com/669646
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Alexandre Elias <aelias@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502936}
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/cc/trees/layer_tree_host.h
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/chrome/browser/android/compositor/compositor_view.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/chromecast/graphics/cast_window_manager_aura.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/content/browser/android/content_view_render_view.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/content/browser/renderer_host/browser_compositor_view_mac.h
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/content/public/browser/android/compositor.h
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/third_party/WebKit/Source/platform/testing/WebLayerTreeViewImplForTesting.cpp
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/ui/aura/mus/window_tree_host_mus.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/ui/compositor/compositor.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/ui/compositor/compositor.h
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc
[modify] https://crrev.com/e494fc874f455309c9b025727b40a837bf713971/ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f994250960da54843339d16c790feb9693c31083

commit f994250960da54843339d16c790feb9693c31083
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Sep 21 13:43:03 2017

Surface synchronization: Implement gutter in SurfaceAggregator

This CL implements gutter in SurfaceAggregator. If the primary
surface is unavailable at aggregation time, then SurfaceAggregator
will compute the difference in size between the primary and fallback
surfaces, and produce two SolidColorDrawQuads (right gutter and bottom
gutter) to fill in the space. SurfaceAggregator uses the root_background_color
in CompositorFrameMetadata as the fill color for the gutter SolidColorDrawQuads.

With this change, we only need to inject gutter in the embedder for top level
windows in Mus+Ash.

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ie2f7c4f1fc610d992f5483f8f5c0fdcea7b2b402
Bug:  766013 
Reviewed-on: https://chromium-review.googlesource.com/673274
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503433}
[modify] https://crrev.com/f994250960da54843339d16c790feb9693c31083/components/viz/service/display/surface_aggregator.cc
[modify] https://crrev.com/f994250960da54843339d16c790feb9693c31083/components/viz/service/display/surface_aggregator.h
[modify] https://crrev.com/f994250960da54843339d16c790feb9693c31083/components/viz/service/display/surface_aggregator_unittest.cc
[modify] https://crrev.com/f994250960da54843339d16c790feb9693c31083/ui/aura/mus/client_surface_embedder.cc
[modify] https://crrev.com/f994250960da54843339d16c790feb9693c31083/ui/aura/mus/client_surface_embedder.h
[modify] https://crrev.com/f994250960da54843339d16c790feb9693c31083/ui/aura/mus/window_port_mus.cc
[modify] https://crrev.com/f994250960da54843339d16c790feb9693c31083/ui/aura/mus/window_tree_client_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment