diff options
| author | 2024-01-23 21:18:03 +0000 | |
|---|---|---|
| committer | 2024-01-23 21:22:39 +0000 | |
| commit | 1a0843083d816dd8ef9a1028ca0194902befb4b0 (patch) | |
| tree | 72a2c2a7aba3235ced51f5fdc8c610bf3ff5c4c3 | |
| parent | b431dc66d417d306d6a8d8fae40d54147b921e67 (diff) | |
Use sync screen capture when needed
The ideal way to request screenshots is via the async oneway call into
SurfaceFlinger. However, a lot of the legacy code requires the buffer to
be available before proceeding. This means we still need to handle sync
screenshots even when there's performance overhead.
This is needed because there are places in system server that request
screenshots while holding a lock and then wait synchronously on the
results. While waiting on the buffer and holding the lock, additional
two way binder calls can be made into system server that are waiting to
acquire the same lock. If there are enough requests, we may run out of
binder threads and then the screen capture result can't be posted back
to system server because it needs a free binder thread. This will result
in a deadlock because the lock that the screenshot request is holding
can never be unlocked without a free binder thread. Binder threads will
never be freed up because they are waiting to acquire the lock.
The async screencapture code is still useful for cases where there's no
global lock being held while waiting on results or the results is
posted.
Removed simple places where the global lock is held when making screenshot
requests
Bug: 321263247
Test: Screenshots
Change-Id: I027cec03bf83db10da29c4ef1195a9e4c8867df8
6 files changed, 47 insertions, 35 deletions
diff --git a/core/java/android/window/ScreenCapture.java b/core/java/android/window/ScreenCapture.java index befb0023ebe6..544642811a39 100644 --- a/core/java/android/window/ScreenCapture.java +++ b/core/java/android/window/ScreenCapture.java @@ -30,6 +30,8 @@ import android.os.Parcelable; import android.util.Log; import android.view.SurfaceControl; +import com.android.window.flags.Flags; + import libcore.util.NativeAllocationRegistry; import java.util.concurrent.CountDownLatch; @@ -48,7 +50,7 @@ public class ScreenCapture { private static native int nativeCaptureDisplay(DisplayCaptureArgs captureArgs, long captureListener); private static native int nativeCaptureLayers(LayerCaptureArgs captureArgs, - long captureListener); + long captureListener, boolean sync); private static native long nativeCreateScreenCaptureListener( ObjIntConsumer<ScreenshotHardwareBuffer> consumer); private static native void nativeWriteListenerToParcel(long nativeObject, Parcel out); @@ -134,7 +136,8 @@ public class ScreenCapture { */ public static ScreenshotHardwareBuffer captureLayers(LayerCaptureArgs captureArgs) { SynchronousScreenCaptureListener syncScreenCapture = createSyncCaptureListener(); - int status = captureLayers(captureArgs, syncScreenCapture); + int status = nativeCaptureLayers(captureArgs, syncScreenCapture.mNativeObject, + Flags.syncScreenCapture()); if (status != 0) { return null; } @@ -171,7 +174,7 @@ public class ScreenCapture { */ public static int captureLayers(@NonNull LayerCaptureArgs captureArgs, @NonNull ScreenCaptureListener captureListener) { - return nativeCaptureLayers(captureArgs, captureListener.mNativeObject); + return nativeCaptureLayers(captureArgs, captureListener.mNativeObject, false /* sync */); } /** @@ -674,7 +677,7 @@ public class ScreenCapture { * This listener can only be used for a single call to capture content call. */ public static class ScreenCaptureListener implements Parcelable { - private final long mNativeObject; + final long mNativeObject; private static final NativeAllocationRegistry sRegistry = NativeAllocationRegistry.createMalloced( ScreenCaptureListener.class.getClassLoader(), getNativeListenerFinalizer()); diff --git a/core/java/android/window/flags/window_surfaces.aconfig b/core/java/android/window/flags/window_surfaces.aconfig index 751c1a8dd0ff..069affb4c06c 100644 --- a/core/java/android/window/flags/window_surfaces.aconfig +++ b/core/java/android/window/flags/window_surfaces.aconfig @@ -88,3 +88,11 @@ flag { is_fixed_read_only: true bug: "304574518" } + +flag { + namespace: "window_surfaces" + name: "sync_screen_capture" + description: "Create a screen capture API that blocks in SurfaceFlinger" + is_fixed_read_only: true + bug: "321263247" +} diff --git a/core/jni/android_window_ScreenCapture.cpp b/core/jni/android_window_ScreenCapture.cpp index 6e903b3ab56d..1031542eb2e6 100644 --- a/core/jni/android_window_ScreenCapture.cpp +++ b/core/jni/android_window_ScreenCapture.cpp @@ -211,7 +211,7 @@ static jint nativeCaptureDisplay(JNIEnv* env, jclass clazz, jobject displayCaptu } static jint nativeCaptureLayers(JNIEnv* env, jclass clazz, jobject layerCaptureArgsObject, - jlong screenCaptureListenerObject) { + jlong screenCaptureListenerObject, jboolean sync) { LayerCaptureArgs captureArgs; getCaptureArgs(env, layerCaptureArgsObject, captureArgs); @@ -227,7 +227,7 @@ static jint nativeCaptureLayers(JNIEnv* env, jclass clazz, jobject layerCaptureA sp<gui::IScreenCaptureListener> captureListener = reinterpret_cast<gui::IScreenCaptureListener*>(screenCaptureListenerObject); - return ScreenshotClient::captureLayers(captureArgs, captureListener); + return ScreenshotClient::captureLayers(captureArgs, captureListener, sync); } static jlong nativeCreateScreenCaptureListener(JNIEnv* env, jclass clazz, jobject consumerObj) { @@ -281,7 +281,7 @@ static const JNINativeMethod sScreenCaptureMethods[] = { // clang-format off {"nativeCaptureDisplay", "(Landroid/window/ScreenCapture$DisplayCaptureArgs;J)I", (void*)nativeCaptureDisplay }, - {"nativeCaptureLayers", "(Landroid/window/ScreenCapture$LayerCaptureArgs;J)I", + {"nativeCaptureLayers", "(Landroid/window/ScreenCapture$LayerCaptureArgs;JZ)I", (void*)nativeCaptureLayers }, {"nativeCreateScreenCaptureListener", "(Ljava/util/function/ObjIntConsumer;)J", (void*)nativeCreateScreenCaptureListener }, diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index 9cf9119aff82..245fcea06eac 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -2776,17 +2776,17 @@ public final class DisplayManagerService extends SystemService { } private ScreenCapture.ScreenshotHardwareBuffer userScreenshotInternal(int displayId) { + final ScreenCapture.DisplayCaptureArgs captureArgs; synchronized (mSyncRoot) { final IBinder token = getDisplayToken(displayId); if (token == null) { return null; } - final ScreenCapture.DisplayCaptureArgs captureArgs = - new ScreenCapture.DisplayCaptureArgs.Builder(token) + captureArgs = new ScreenCapture.DisplayCaptureArgs.Builder(token) .build(); - return ScreenCapture.captureDisplay(captureArgs); } + return ScreenCapture.captureDisplay(captureArgs); } @VisibleForTesting diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index e7ecf520425d..5dcd335baf2d 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -160,6 +160,7 @@ import static com.android.server.wm.utils.DisplayInfoOverrides.WM_OVERRIDE_FIELD import static com.android.server.wm.utils.DisplayInfoOverrides.copyDisplayInfoFields; import static com.android.server.wm.utils.RegionUtils.forEachRectReverse; import static com.android.server.wm.utils.RegionUtils.rectListToRegion; +import static com.android.window.flags.Flags.deferDisplayUpdates; import static com.android.window.flags.Flags.explicitRefreshRateHints; import android.annotation.IntDef; @@ -174,7 +175,6 @@ import android.content.pm.ActivityInfo.ScreenOrientation; import android.content.res.CompatibilityInfo; import android.content.res.Configuration; import android.content.res.Resources; -import android.graphics.Bitmap; import android.graphics.ColorSpace; import android.graphics.Insets; import android.graphics.Matrix; @@ -246,7 +246,6 @@ import android.view.inputmethod.ImeTracker; import android.window.DisplayWindowPolicyController; import android.window.IDisplayAreaOrganizer; import android.window.ScreenCapture; -import android.window.ScreenCapture.SynchronousScreenCaptureListener; import android.window.SystemPerformanceHinter; import android.window.TransitionRequestInfo; @@ -276,7 +275,6 @@ import java.util.Objects; import java.util.Set; import java.util.function.Consumer; import java.util.function.Predicate; -import static com.android.window.flags.Flags.deferDisplayUpdates; /** * Utility class for keeping track of the WindowStates and other pertinent contents of a @@ -5207,10 +5205,9 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp } /** - * Takes a snapshot of the display. In landscape mode this grabs the whole screen. - * In portrait mode, it grabs the full screenshot. + * Creates a LayerCaptureArgs object to represent the entire DisplayContent */ - Bitmap screenshotDisplayLocked() { + ScreenCapture.LayerCaptureArgs getLayerCaptureArgs() { if (!mWmService.mPolicy.isScreenOn()) { if (DEBUG_SCREENSHOT) { Slog.i(TAG_WM, "Attempted to take screenshot while display was off."); @@ -5218,24 +5215,10 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp return null; } - SynchronousScreenCaptureListener syncScreenCapture = - ScreenCapture.createSyncCaptureListener(); - getBounds(mTmpRect); mTmpRect.offsetTo(0, 0); - ScreenCapture.LayerCaptureArgs args = - new ScreenCapture.LayerCaptureArgs.Builder(getSurfaceControl()) - .setSourceCrop(mTmpRect).build(); - - ScreenCapture.captureLayers(args, syncScreenCapture); - - final ScreenCapture.ScreenshotHardwareBuffer screenshotBuffer = - syncScreenCapture.getBuffer(); - final Bitmap bitmap = screenshotBuffer == null ? null : screenshotBuffer.asBitmap(); - if (bitmap == null) { - Slog.w(TAG_WM, "Failed to take screenshot"); - } - return bitmap; + return new ScreenCapture.LayerCaptureArgs.Builder(getSurfaceControl()) + .setSourceCrop(mTmpRect).build(); } @Override diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index f8ac8da710c8..581d931ba454 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -4087,7 +4087,7 @@ public class WindowManagerService extends IWindowManager.Stub throw new SecurityException("Requires READ_FRAME_BUFFER permission"); } - final Bitmap bm; + ScreenCapture.LayerCaptureArgs captureArgs; synchronized (mGlobalLock) { final DisplayContent displayContent = mRoot.getDisplayContent(DEFAULT_DISPLAY); if (displayContent == null) { @@ -4095,12 +4095,30 @@ public class WindowManagerService extends IWindowManager.Stub Slog.i(TAG_WM, "Screenshot returning null. No Display for displayId=" + DEFAULT_DISPLAY); } - bm = null; + captureArgs = null; } else { - bm = displayContent.screenshotDisplayLocked(); + captureArgs = displayContent.getLayerCaptureArgs(); } } + final Bitmap bm; + if (captureArgs != null) { + ScreenCapture.SynchronousScreenCaptureListener syncScreenCapture = + ScreenCapture.createSyncCaptureListener(); + + ScreenCapture.captureLayers(captureArgs, syncScreenCapture); + + final ScreenCapture.ScreenshotHardwareBuffer screenshotBuffer = + syncScreenCapture.getBuffer(); + bm = screenshotBuffer == null ? null : screenshotBuffer.asBitmap(); + } else { + bm = null; + } + + if (bm == null) { + Slog.w(TAG_WM, "Failed to take screenshot"); + } + FgThread.getHandler().post(() -> { try { receiver.onHandleAssistScreenshot(bm); |