Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1a084308 authored by Chavi Weingarten's avatar Chavi Weingarten
Browse files

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
parent b431dc66
Loading
Loading
Loading
Loading
+7 −4
Original line number Diff line number Diff line
@@ -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());
+8 −0
Original line number Diff line number Diff line
@@ -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"
}
+3 −3
Original line number Diff line number Diff line
@@ -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 },
+3 −3
Original line number Diff line number Diff line
@@ -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
+5 −22
Original line number Diff line number Diff line
@@ -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())
        return 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;
    }

    @Override
Loading