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

Issue 812059 link

Starred by 3 users
Status: Fixed
Owner:
samans@chromium.org
Closed: Mar 2018
Cc:
█ fsam...@chromium.org
m...@chromium.org
kylec...@chromium.org
mfo...@chromium.org
powerb@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 706246



Sign in to add a comment

CopyOutputRequest "screenshots" for Surfaces in VIZ

Project Member Reported by m...@chromium.org, Feb 14 2018 
In  bug 806375 , we got CopyOutputRequests working when VIZ is enabled. However, this was for Layers, and only when the layer tree is in a privileged process.

Some callpoints, such as RenderWidgetHostViewChildFrame::SubmitSurfaceCopyRequest(), don't have a way to get at the privileged Layer in the browser process. So, we still need a true "copy from surface" API and impl for VIZ.

Note that content::DelegatedFrameHost currently makes Layer snapshots instead of Surface snapshots when VIZ is enabled. So, once we have the "copy from surface" API for VIZ, DFH should be updated to call into that instead.

Current thinking is to add a FrameSinkManager::TakeSnapshot() API alongside the existing FrameSinkManager::CreateVideoCapturer().
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 14 2018 

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

commit 0f32a834c7a12c426d18c82e6e21be66d48a4500
Author: Yuri Wiitala <miu@chromium.org>
Date: Wed Feb 14 20:52:27 2018

Migrate RWHVChildFrame to use new CopyOutputRequest APIs.

This change brings RenderWidgetHostViewChildFrame::CopyFromSurface()
in-sync with DelegatedFrameHost. It switches over to using the new
CopyOutputRequest scaling APIs, freeing up one of the last dependencies
on the "GLHelper mass of code" in surface_utils.cc.

Bug:  759310 ,  812059 
Change-Id: I7fc5e997cf1e442dfd674bdc131e9de7f6d349eb
Reviewed-on: https://chromium-review.googlesource.com/918361
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536810}
[modify] https://crrev.com/0f32a834c7a12c426d18c82e6e21be66d48a4500/content/browser/renderer_host/render_widget_host_view_child_frame.cc

Comment 2 by m...@chromium.org, Feb 20 2018

Blocking: 706246
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 22 2018 

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

commit 419ed0fc82d4f2ddde0a3a54a721e0c623882eff
Author: Yuri Wiitala <miu@chromium.org>
Date: Thu Feb 22 23:21:38 2018

Migrate RWHVAndroid to use new CopyOutputRequest APIs.

This change brings RenderWidgetHostViewAndroid::CopyFromSurface() and
DelegatedFrameHostAndroid in-sync with the desktop platform versions of
the same, and also fixes a bug where the output_size argument was being
device-scaled. It switches over to using the new CopyOutputRequest
scaling APIs, and deletes all the special client-side use of GLHelper
(which was being used to perform scaling post-copy).

Also, two important fixes were made:

1. A weirdness in the existing switcher UI where there was a width-
stretching problem, as mentioned in the description of a prior change:
https://codereview.chromium.org/2734043003 (commit
caa6a6165d28c3bb499fc0aec9bf391c6bda1578). Now the tab transition UI
thumbnails are *perfectly* aligned with the actual content rendering.

2. Devtools remote screen cast impl: The calculations in PageHandler
were rather confusing, and this was masking a bug where the wrong scale
factor was being applied to the remote's requested image size. This
change eliminates most of the calculations, since all that matters is
that whatever the Android device has locally is being scaled-to-fit
within the remote's requested width+height.

Manual Testing (on a Pixel XL; device scale factor of 3.5):

1. Android tab switcher UI.
2. DevTools remote screen cast inspector tool (resized window during
session to make sure it works for several remote image sizes).

Bug:  759310 ,  812059 ,  582955 
Change-Id: If0726d4947edfc1efa58fe926123b443b6649e55
Reviewed-on: https://chromium-review.googlesource.com/924388
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538610}
[modify] https://crrev.com/419ed0fc82d4f2ddde0a3a54a721e0c623882eff/chrome/browser/android/compositor/tab_content_manager.cc
[modify] https://crrev.com/419ed0fc82d4f2ddde0a3a54a721e0c623882eff/content/browser/devtools/protocol/page_handler.cc
[modify] https://crrev.com/419ed0fc82d4f2ddde0a3a54a721e0c623882eff/content/browser/devtools/protocol/page_handler.h
[modify] https://crrev.com/419ed0fc82d4f2ddde0a3a54a721e0c623882eff/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/419ed0fc82d4f2ddde0a3a54a721e0c623882eff/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/419ed0fc82d4f2ddde0a3a54a721e0c623882eff/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/419ed0fc82d4f2ddde0a3a54a721e0c623882eff/ui/android/delegated_frame_host_android.h

Comment 4 by m...@chromium.org, Mar 6 2018

Owner: samans@chromium.org
Status: Started (was: Available)
Looks like samans@ started this work with https://chromium-review.googlesource.com/c/chromium/src/+/949068.
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 6 2018 

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

commit be87e99ca72e9b8f015294e9ea951cf1785f88ba
Author: Saman Sami <samans@chromium.org>
Date: Tue Mar 06 19:08:20 2018

viz: Add RequestCopyOfOutput to HostFrameSinkManager

Also rename RequestCopyOfSurface to RequestCopyOfOutput in CFSS to make
it clear the CopyOutputRequest is not guaranteed to run on any
particular surface.

BUG= 812059 
TBR=khushalsagar@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I99c42b497f9985d37916a7cdbab0a8b2aa3e3f7d
Reviewed-on: https://chromium-review.googlesource.com/949068
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
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@{#541167}
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/host/host_frame_sink_manager.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/host/host_frame_sink_manager.h
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/service/display/surface_aggregator_unittest.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/service/frame_sinks/compositor_frame_sink_support.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/service/frame_sinks/compositor_frame_sink_support.h
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/service/frame_sinks/frame_sink_manager_impl.h
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/service/frame_sinks/video_capture/capturable_frame_sink.h
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/service/frame_sinks/video_capture/frame_sink_video_capturer_impl_unittest.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/service/surfaces/surface_unittest.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/test/test_frame_sink_manager.h
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/components/viz/test/test_layer_tree_frame_sink.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/services/viz/privileged/interfaces/compositing/frame_sink_manager.mojom
[modify] https://crrev.com/be87e99ca72e9b8f015294e9ea951cf1785f88ba/ui/android/delegated_frame_host_android.cc

Comment 6 by m...@chromium.org, Mar 27 2018

Status: Fixed (was: Started)
I believe this is finished. samans: Feel free to re-open if there's anything left to do.
►

Sign in to add a comment