New issue
Advanced search Search tips

Issue 781986 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Dec 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 754872



Sign in to add a comment

GLRendererCopier::CacheEntry clean-up (pass CacheEntry around on stack)

Project Member Reported by m...@chromium.org, Nov 6 2017

Issue description

In viz::GLRendererCopier code, there are several methods that each perform TakeOrCreate() and CacheOrDelete() operations on various objects used in the execution of CopyOutputRequests. Instead, it would be simpler if the code were refactored to just create or find a CacheEntry that goes with a CopyOutputRequest, and pass a pointer to that around on the stack. Then, once request execution has finished, there can be a simple decision at the end of the workflow to decide whether to keep the CacheEntry, or destroy it (and all the objects it owns).

 

Comment 1 by m...@chromium.org, Nov 6 2017

Status: Assigned (was: Untriaged)

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

Components: Internals>Media>ScreenCapture
Labels: -Proj-Mustash Proj-Mash-MultiProcess
Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 27

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

commit 82751409267d5f4bfc718aebb728ae42f04e2ba6
Author: Yuri Wiitala <miu@chromium.org>
Date: Tue Nov 27 06:16:51 2018

GLRendererCopier: Replace use of GLHelper with GLScaler/GLI420Converter.

Switches-over GLRendererCopier to use the new GLScaler and
GLI420Converter when scaling and/or format-converting images for
CopyOutputRequests. This change also addresses:

1. Simpler handling of cached GL resources (used across copy requests
having the same requester). Rather than separate methods for each of the
possible cached resources (with some duplicated logic), there is now one
"ReusableThings" struct to hold them all (and is passed around on the
stack).

2. Simplified the I420 code around handling of "packed textures" and
"aligned rects".

3. Simplified the detection/execution of whether to byte-swizzle on the
CPU (after readback) or GPU (before readback). Simplified the RGBA
bitmap copy-from-pixel-buffer code (w.r.t. upside-down row-by-row copy
and byte-swizzling).

Testing: Confirmed existing unit and pixel tests are thoroughly-testing
all possible code paths in GLRendererCopier. Manually confirmed tab and
desktop capture (incl. CrOS), and screenshots work.

Performance improvement: A local run of performance_browser_tests, on a
machine with a high-end nVidia GPU, has shown a ~20% improvement in
capture latency (i.e., the interval of time from when copy request was
issued until an SkBitmap result is ready in system memory).
http://chromeperf.appspot.com/ will reveal how other platform
configurations are affected.

Bug: 870036, 810131 , 781986 ,758057
Binary-Size: Increase is temporary (linking in new impl, future CLs will switch-over other clients to stop using old impl).
Change-Id: Ibcb16c1382ea0cffb00bc7605e0594b1fe28356a
Reviewed-on: https://chromium-review.googlesource.com/c/1343247
Commit-Queue: Yuri Wiitala <miu@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Ria Jiang <riajiang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611027}
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/chrome/browser/extensions/api/cast_streaming/cast_streaming_apitest.cc
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/common/gl_scaler.cc
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/common/gl_scaler_pixeltest.cc
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/service/display/copy_output_scaling_pixeltest.cc
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/service/display/gl_renderer_copier.cc
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/service/display/gl_renderer_copier.h
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/service/display/gl_renderer_copier_pixeltest.cc
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/components/viz/service/display/gl_renderer_copier_unittest.cc
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/content/browser/media/capture/fake_video_capture_stack.cc
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/content/browser/media/capture/frame_test_util.h
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/content/browser/media/capture/web_contents_video_capture_device_browsertest.cc
[modify] https://crrev.com/82751409267d5f4bfc718aebb728ae42f04e2ba6/content/browser/renderer_host/render_widget_host_view_browsertest.cc

Status: Fixed (was: Started)

Sign in to add a comment