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

Issue 857575 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 10
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Sign in to add a comment

Decouple fallback SurfaceIds and SurfaceReferences

Project Member Reported by samans@chromium.org, Jun 28 2018

Issue description

Currently referenced_surfaces in CompositorFrameMetadata (which contains the list of fallback SurfaceIds of the children) is used for adding references from the parent to the child surfaces. However, this is not ideal. It takes time (and cycles) for the parent to learn about a new active child surface and update its fallback and submit a CompositorFrame with the new fallback. If Viz can update the reference without involving the parent, we can avoid this delay and the notion of fallback SurfaceId can become optional.
 

Comment 1 by samans@chromium.org, Jun 28 2018

Blockedon: 857571
Cc: kylec...@chromium.org
Blockedon: 860306
Blocking: 836194
Cc: akaba@google.com piman@chromium.org jonr...@chromium.org
 Issue 836194  has been merged into this issue.
Blockedon: 861761
Blocking: -836194
Blockedon: 861764
Blockedon: 861769
Blockedon: 861834
Blockedon: 866113
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 23

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

commit 1490393e0cf17a21aaa5d9ee29216448cbd97761
Author: akaba <akaba@google.com>
Date: Mon Jul 23 21:14:50 2018

Allow SurfaceId to be checked against a SurfaceRange

This CL add a method to SurfaceId that allows for checking if the surfaceId is
Inside some SurfaceRange.

Bug:  857575 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I7b083c82f84e47dc08b9dd2f2ddf8179889ba26e
Reviewed-on: https://chromium-review.googlesource.com/1145628
Commit-Queue: Andre Kaba <akaba@google.com>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577259}
[modify] https://crrev.com/1490393e0cf17a21aaa5d9ee29216448cbd97761/components/viz/common/BUILD.gn
[modify] https://crrev.com/1490393e0cf17a21aaa5d9ee29216448cbd97761/components/viz/common/surfaces/local_surface_id.cc
[modify] https://crrev.com/1490393e0cf17a21aaa5d9ee29216448cbd97761/components/viz/common/surfaces/surface_id.cc
[modify] https://crrev.com/1490393e0cf17a21aaa5d9ee29216448cbd97761/components/viz/common/surfaces/surface_id.h
[modify] https://crrev.com/1490393e0cf17a21aaa5d9ee29216448cbd97761/components/viz/common/surfaces/surface_range.cc
[modify] https://crrev.com/1490393e0cf17a21aaa5d9ee29216448cbd97761/components/viz/common/surfaces/surface_range.h
[add] https://crrev.com/1490393e0cf17a21aaa5d9ee29216448cbd97761/components/viz/common/surfaces/surface_range_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 26

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

commit 8dc96162b2ebe924cf7e6233bd1a6eb05e2befce
Author: akaba <akaba@google.com>
Date: Thu Jul 26 20:41:30 2018

Referencing SurfaceManager::GetLatestInFlightSurface in surface activation instead of fallback surface.

Use SurfaceManager::LatestInFlightSurface in Surface::ActivateFrame to use the latest in-flight
surface as the active reference instead of fallback. This allows for better garbage collection
of surfaces and optimize the notion of fallback surface.

Bug:  857575 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I4f597e431866b3a9886c04b982101199fd4011d7
Reviewed-on: https://chromium-review.googlesource.com/1149092
Commit-Queue: Andre Kaba <akaba@google.com>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578429}
[modify] https://crrev.com/8dc96162b2ebe924cf7e6233bd1a6eb05e2befce/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/8dc96162b2ebe924cf7e6233bd1a6eb05e2befce/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/8dc96162b2ebe924cf7e6233bd1a6eb05e2befce/components/viz/service/surfaces/surface.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 26

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

commit e94a326ff1ed856750b006b092f24444d3841687
Author: Fady Samuel <fsamuel@chromium.org>
Date: Thu Jul 26 22:31:39 2018

Revert "Referencing SurfaceManager::GetLatestInFlightSurface in surface activation instead of fallback surface."

This reverts commit 8dc96162b2ebe924cf7e6233bd1a6eb05e2befce.

Reason for revert: This causes jitter during resize.

Original change's description:
> Referencing SurfaceManager::GetLatestInFlightSurface in surface activation instead of fallback surface.
> 
> Use SurfaceManager::LatestInFlightSurface in Surface::ActivateFrame to use the latest in-flight
> surface as the active reference instead of fallback. This allows for better garbage collection
> of surfaces and optimize the notion of fallback surface.
> 
> Bug:  857575 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
> Change-Id: I4f597e431866b3a9886c04b982101199fd4011d7
> Reviewed-on: https://chromium-review.googlesource.com/1149092
> Commit-Queue: Andre Kaba <akaba@google.com>
> Reviewed-by: Fady Samuel <fsamuel@chromium.org>
> Reviewed-by: Saman Sami <samans@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578429}

TBR=fsamuel@chromium.org,samans@chromium.org,akaba@google.com

Change-Id: I53bd4bf7b7ecc1db79e3ac2e4a535fc49269b858
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  857575 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1152309
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578465}
[modify] https://crrev.com/e94a326ff1ed856750b006b092f24444d3841687/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/e94a326ff1ed856750b006b092f24444d3841687/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/e94a326ff1ed856750b006b092f24444d3841687/components/viz/service/surfaces/surface.cc

Owner: akaba@chromium.org
Blockedon: 870452
Blocking: 870456
Project Member

Comment 17 by bugdroid1@chromium.org, Aug 14

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

commit 554b55f386428e5e78e4331c0b84e1353f957c25
Author: akaba <akaba@google.com>
Date: Tue Aug 14 00:59:09 2018

Dynamically update references when a child activates.

This CL implements an observer pattern for activated child surface,
where each surface will be able to observe activation events
happening in a subset of FrameSinkIds and update SurfaceReferences
accordingly. This is needed since we will no longer refer to the fallback
but instead will use GetLatestInFlightSurface to acquire references from
the SurfaceRanges specified in CompositorFrameMetaData.

Bug:  857575 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I8ee1a9633273e116c5e7b1ae840dcc8db7a0407a
Reviewed-on: https://chromium-review.googlesource.com/1157404
Commit-Queue: Andre Kaba <akaba@google.com>
Reviewed-by: Saman Sami <samans@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582773}
[modify] https://crrev.com/554b55f386428e5e78e4331c0b84e1353f957c25/cc/trees/layer_tree_host.h
[modify] https://crrev.com/554b55f386428e5e78e4331c0b84e1353f957c25/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/554b55f386428e5e78e4331c0b84e1353f957c25/components/viz/service/frame_sinks/surface_synchronization_unittest.cc
[modify] https://crrev.com/554b55f386428e5e78e4331c0b84e1353f957c25/components/viz/service/surfaces/surface.cc
[modify] https://crrev.com/554b55f386428e5e78e4331c0b84e1353f957c25/components/viz/service/surfaces/surface.h
[modify] https://crrev.com/554b55f386428e5e78e4331c0b84e1353f957c25/components/viz/service/surfaces/surface_manager.cc
[modify] https://crrev.com/554b55f386428e5e78e4331c0b84e1353f957c25/components/viz/service/surfaces/surface_manager.h

Status: Fixed (was: Assigned)
Owner: samans@chromium.org
Status: Assigned (was: Fixed)
Blockedon: 878372
Blockedon: 874983
Project Member

Comment 22 by bugdroid1@chromium.org, Sep 18

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

commit 3ee63ecf302814c98a2a2f7f07e0975b99a9459c
Author: Saman Sami <samans@chromium.org>
Date: Tue Sep 18 17:43:53 2018

Don't reset fallback in OnLostVizProcess

Resetting the fallback is no longer necessary and might even result in
surfaces outside of the desired range being shown.

Bug:  857575 
Change-Id: Ib66ad50d0297f2c93e3e9815a3ec2349fde4289b
Reviewed-on: https://chromium-review.googlesource.com/1231034
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592101}
[modify] https://crrev.com/3ee63ecf302814c98a2a2f7f07e0975b99a9459c/content/browser/renderer_host/delegated_frame_host.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Sep 19

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

commit e00935ccec3e83f3fc7c6b7ef895d533174245f6
Author: Saman Sami <samans@chromium.org>
Date: Wed Sep 19 21:58:41 2018

Don't reset fallback lowerbound on frame eviction

Resetting the fallback can result in surfaces outside of the desired
range being shown on next show.

Bug:  857575 
Change-Id: Ic9a80bb7f0b6e963adb32ffd0e234108e323d394
Reviewed-on: https://chromium-review.googlesource.com/1226020
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592564}
[modify] https://crrev.com/e00935ccec3e83f3fc7c6b7ef895d533174245f6/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/e00935ccec3e83f3fc7c6b7ef895d533174245f6/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc
[modify] https://crrev.com/e00935ccec3e83f3fc7c6b7ef895d533174245f6/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/e00935ccec3e83f3fc7c6b7ef895d533174245f6/ui/compositor/layer.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Sep 24

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

commit f03a9dbadf9f69ee774123ef50ca82bef8e8731d
Author: Saman Sami <samans@chromium.org>
Date: Mon Sep 24 18:05:07 2018

Use primary surface in TakeFallbackContentFrom when there is no fallback

Also when a fallback exists but has a different FrameSinkId or embed token
than the primary, don't use the fallback because the resulting range in
the new view will not cover any surface with the FrameSinkId or embed
token of the old view's primary.

Bug:  857575 , 870456 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ia4736f354160dfd510793bf35ae4292c276bf115
Reviewed-on: https://chromium-review.googlesource.com/1236456
Reviewed-by: Khushal <khushalsagar@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593598}
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/components/viz/common/surfaces/local_surface_id.cc
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/components/viz/common/surfaces/local_surface_id.h
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/components/viz/common/surfaces/surface_id.cc
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/components/viz/common/surfaces/surface_id.h
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/f03a9dbadf9f69ee774123ef50ca82bef8e8731d/ui/android/delegated_frame_host_android.h

Status: Fixed (was: Assigned)

Sign in to add a comment