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

Issue 637035 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , All
Pri: 3
Type: Bug

Blocking:
issue 636630



Sign in to add a comment

cc: Allow finishing copy requests for non-root Surfaces

Project Member Reported by siev...@chromium.org, Aug 11 2016

Issue description

Capturing the contents through CopyOutputRequests while the context is going away is currently difficult.

(For a use case see  crbug.com/636630 . In general it seems useful to be able to capture a placeholder of the most recent contents exactly when your display context is going away for glitch-free resume.)

It would be nice to either have an API or make a best effort to finish pending readback requests when a Display is torn down, esp. if these are on non-root surfaces.

For that it would be nice to be able to handle a sort of offscreen draw which doesn't care about swaps (and doesn't therefore need to respect max. pending swaps). Also the readback is likely on a tab's SurfaceLayer, so we could also handle these even if the root Surface resources are in use by the browser compositor.

 
Components: Internals>Compositing
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 12 2016

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

commit 962aa721a0de39986c7c393e154f773f7b74bdf4
Author: sievers <sievers@chromium.org>
Date: Fri Aug 12 18:26:14 2016

Android: Force draw for pending readbacks during Display teardown

The Surface goes away when the activity is stopped.
Since this is an interesting point in time to grab a placeholder
for the web contents, try to complete pending readbacks that might
have just been issues before tearing down the Display.

BUG= 636630 ,637035
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

Review-Url: https://codereview.chromium.org/2242633002
Cr-Commit-Position: refs/heads/master@{#411706}

[modify] https://crrev.com/962aa721a0de39986c7c393e154f773f7b74bdf4/cc/surfaces/display.cc
[modify] https://crrev.com/962aa721a0de39986c7c393e154f773f7b74bdf4/cc/surfaces/display.h
[modify] https://crrev.com/962aa721a0de39986c7c393e154f773f7b74bdf4/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/962aa721a0de39986c7c393e154f773f7b74bdf4/content/browser/renderer_host/compositor_impl_android.h

Components: -Internals>Compositing Internals>Compositing>Rasterization
Owner: siev...@chromium.org
Status: Assigned (was: Untriaged)
Labels: OS-Android
So I landed sort of a workaround for this. But it still doesn't guarantee that we complete a readback when the OutputSurface goes away. (And on Android the OutputSurface is taken away by the system when the application goes to the background.)

Now it turns out that we currently keep the last frame w/tiles that the browser has when pausing the application. I think this is somewhat of an oversight, since we want to be aggressive with freeing memory on Android.

In order to change this behavior, we'd want to keep a low-quality placeholder that we can show on resume until the first re-rasterized frame arrives. But for being able to make that change, I *think* I'd want to be able to convince myself that we can *always* capture and up-to-date frame when going to the background, or it might cause interesting occasional glitches on resume when the readback didn't succeed.

Marking 'OS-Android' for now although Dana pitched the idea of freeing UI contexts when becoming invisible more aggressively also on desktop.
Cc: aelias@chromium.org
Owner: khushals...@chromium.org
Over to Khushal since he knows this code.

The ForceImmediateDrawAndSwapIfPossible() that I added in compositor_impl_android.cc mostly takes care of that.

But for it to be bullet-proof it should from this call site also ignore the swap limit (and maybe check that there are no resource locks?).

Sign in to add a comment