summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Chavi Weingarten <chaviw@google.com> 2024-01-23 21:18:03 +0000
committer Chavi Weingarten <chaviw@google.com> 2024-01-23 21:22:39 +0000
commit1a0843083d816dd8ef9a1028ca0194902befb4b0 (patch)
tree72a2c2a7aba3235ced51f5fdc8c610bf3ff5c4c3
parentb431dc66d417d306d6a8d8fae40d54147b921e67 (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
-rw-r--r--core/java/android/window/ScreenCapture.java11
-rw-r--r--core/java/android/window/flags/window_surfaces.aconfig8
-rw-r--r--core/jni/android_window_ScreenCapture.cpp6
-rw-r--r--services/core/java/com/android/server/display/DisplayManagerService.java6
-rw-r--r--services/core/java/com/android/server/wm/DisplayContent.java27
-rw-r--r--services/core/java/com/android/server/wm/WindowManagerService.java24
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);