Allow PixelCopy for a window from any View

Also make it actually async, and allow the bitmap
to be auto-allocated

Bug: 195673633
Test: PixelCopyTest CTS suite

Change-Id: Ie872f20c809eaaeb8dc32f3ec6347f21a9a7bc1a

core/api/current.txt

@@ -49219,6 +49219,10 @@ package android.view {
   }
 
   public final class PixelCopy {
+    method @NonNull public static android.view.PixelCopy.Request ofSurface(@NonNull android.view.Surface, @NonNull java.util.concurrent.Executor, @NonNull java.util.function.Consumer<android.view.PixelCopy.CopyResult>);
+    method @NonNull public static android.view.PixelCopy.Request ofSurface(@NonNull android.view.SurfaceView, @NonNull java.util.concurrent.Executor, @NonNull java.util.function.Consumer<android.view.PixelCopy.CopyResult>);
+    method @NonNull public static android.view.PixelCopy.Request ofWindow(@NonNull android.view.Window, @NonNull java.util.concurrent.Executor, @NonNull java.util.function.Consumer<android.view.PixelCopy.CopyResult>);
+    method @NonNull public static android.view.PixelCopy.Request ofWindow(@NonNull android.view.View, @NonNull java.util.concurrent.Executor, @NonNull java.util.function.Consumer<android.view.PixelCopy.CopyResult>);
     method public static void request(@NonNull android.view.SurfaceView, @NonNull android.graphics.Bitmap, @NonNull android.view.PixelCopy.OnPixelCopyFinishedListener, @NonNull android.os.Handler);
     method public static void request(@NonNull android.view.SurfaceView, @Nullable android.graphics.Rect, @NonNull android.graphics.Bitmap, @NonNull android.view.PixelCopy.OnPixelCopyFinishedListener, @NonNull android.os.Handler);
     method public static void request(@NonNull android.view.Surface, @NonNull android.graphics.Bitmap, @NonNull android.view.PixelCopy.OnPixelCopyFinishedListener, @NonNull android.os.Handler);
@@ -49233,10 +49237,21 @@ package android.view {
     field public static final int SUCCESS = 0; // 0x0
   }
 
+  public static final class PixelCopy.CopyResult {
+    method @NonNull public android.graphics.Bitmap getBitmap();
+    method public int getStatus();
+  }
+
   public static interface PixelCopy.OnPixelCopyFinishedListener {
     method public void onPixelCopyFinished(int);
   }
 
+  public static final class PixelCopy.Request {
+    method public void request();
+    method @NonNull public android.view.PixelCopy.Request setDestinationBitmap(@Nullable android.graphics.Bitmap);
+    method @NonNull public android.view.PixelCopy.Request setSourceRect(@Nullable android.graphics.Rect);
+  }
+
   public final class PointerIcon implements android.os.Parcelable {
     method @NonNull public static android.view.PointerIcon create(@NonNull android.graphics.Bitmap, float, float);
     method public int describeContents();

graphics/java/android/graphics/HardwareRenderer.java

@@ -1046,14 +1046,39 @@ public class HardwareRenderer {
     }
 
     /** @hide */
-    public static int copySurfaceInto(Surface surface, Rect srcRect, Bitmap bitmap) {
-        if (srcRect == null) {
-            // Empty rect means entire surface
-            return nCopySurfaceInto(surface, 0, 0, 0, 0, bitmap.getNativeInstance());
-        } else {
-            return nCopySurfaceInto(surface, srcRect.left, srcRect.top,
-                    srcRect.right, srcRect.bottom, bitmap.getNativeInstance());
+    public abstract static class CopyRequest {
+        protected Bitmap mDestinationBitmap;
+        final Rect mSrcRect;
+
+        protected CopyRequest(Rect srcRect, Bitmap destinationBitmap) {
+            mDestinationBitmap = destinationBitmap;
+            if (srcRect != null) {
+                mSrcRect = srcRect;
+            } else {
+                mSrcRect = new Rect();
+            }
         }
+
+        /**
+         * Retrieve the bitmap in which to store the result of the copy request
+         */
+        public long getDestinationBitmap(int srcWidth, int srcHeight) {
+            if (mDestinationBitmap == null) {
+                mDestinationBitmap =
+                        Bitmap.createBitmap(srcWidth, srcHeight, Bitmap.Config.ARGB_8888);
+            }
+            return mDestinationBitmap.getNativeInstance();
+        }
+
+        /** Called when the copy is completed */
+        public abstract void onCopyFinished(int result);
+    }
+
+    /** @hide */
+    public static void copySurfaceInto(Surface surface, CopyRequest copyRequest) {
+        final Rect srcRect = copyRequest.mSrcRect;
+        nCopySurfaceInto(surface, srcRect.left, srcRect.top, srcRect.right, srcRect.bottom,
+                copyRequest);
     }
 
     /**
@@ -1464,8 +1489,8 @@ public class HardwareRenderer {
 
     private static native void nRemoveObserver(long nativeProxy, long nativeObserver);
 
-    private static native int nCopySurfaceInto(Surface surface,
-            int srcLeft, int srcTop, int srcRight, int srcBottom, long bitmapHandle);
+    private static native void nCopySurfaceInto(Surface surface,
+            int srcLeft, int srcTop, int srcRight, int srcBottom, CopyRequest request);
 
     private static native Bitmap nCreateHardwareBitmap(long renderNode, int width, int height);
 

graphics/java/android/view/PixelCopy.java

@@ -20,12 +20,15 @@ import android.annotation.IntDef;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.graphics.Bitmap;
+import android.graphics.HardwareRenderer;
 import android.graphics.Rect;
 import android.os.Handler;
 import android.view.ViewTreeObserver.OnDrawListener;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
+import java.util.concurrent.Executor;
+import java.util.function.Consumer;
 
 /**
  * Provides a mechanisms to issue pixel copy requests to allow for copy
@@ -183,12 +186,10 @@ public final class PixelCopy {
         if (srcRect != null && srcRect.isEmpty()) {
             throw new IllegalArgumentException("sourceRect is empty");
         }
-        // TODO: Make this actually async and fast and cool and stuff
-        int result = ThreadedRenderer.copySurfaceInto(source, srcRect, dest);
-        listenerThread.post(new Runnable() {
+        HardwareRenderer.copySurfaceInto(source, new HardwareRenderer.CopyRequest(srcRect, dest) {
             @Override
-            public void run() {
-                listener.onPixelCopyFinished(result);
+            public void onCopyFinished(int result) {
+                listenerThread.post(() -> listener.onPixelCopyFinished(result));
             }
         });
     }
@@ -255,30 +256,10 @@ public final class PixelCopy {
             @NonNull Bitmap dest, @NonNull OnPixelCopyFinishedListener listener,
             @NonNull Handler listenerThread) {
         validateBitmapDest(dest);
-        if (source == null) {
-            throw new IllegalArgumentException("source is null");
-        }
-        if (source.peekDecorView() == null) {
-            throw new IllegalArgumentException(
-                    "Only able to copy windows with decor views");
-        }
-        Surface surface = null;
-        final ViewRootImpl root = source.peekDecorView().getViewRootImpl();
-        if (root != null) {
-            surface = root.mSurface;
-            final Rect surfaceInsets = root.mWindowAttributes.surfaceInsets;
-            if (srcRect == null) {
-                srcRect = new Rect(surfaceInsets.left, surfaceInsets.top,
-                        root.mWidth + surfaceInsets.left, root.mHeight + surfaceInsets.top);
-            } else {
-                srcRect.offset(surfaceInsets.left, surfaceInsets.top);
-            }
-        }
-        if (surface == null || !surface.isValid()) {
-            throw new IllegalArgumentException(
-                    "Window doesn't have a backing surface!");
-        }
-        request(surface, srcRect, dest, listener, listenerThread);
+        final Rect insets = new Rect();
+        final Surface surface = sourceForWindow(source, insets);
+        request(surface, adjustSourceRectForInsets(insets, srcRect), dest, listener,
+                listenerThread);
     }
 
     private static void validateBitmapDest(Bitmap bitmap) {
@@ -294,5 +275,235 @@ public final class PixelCopy {
         }
     }
 
+    private static Surface sourceForWindow(Window source, Rect outInsets) {
+        if (source == null) {
+            throw new IllegalArgumentException("source is null");
+        }
+        if (source.peekDecorView() == null) {
+            throw new IllegalArgumentException(
+                    "Only able to copy windows with decor views");
+        }
+        Surface surface = null;
+        final ViewRootImpl root = source.peekDecorView().getViewRootImpl();
+        if (root != null) {
+            surface = root.mSurface;
+            final Rect surfaceInsets = root.mWindowAttributes.surfaceInsets;
+            outInsets.set(surfaceInsets.left, surfaceInsets.top,
+                    root.mWidth + surfaceInsets.left, root.mHeight + surfaceInsets.top);
+        }
+        if (surface == null || !surface.isValid()) {
+            throw new IllegalArgumentException(
+                    "Window doesn't have a backing surface!");
+        }
+        return surface;
+    }
+
+    private static Rect adjustSourceRectForInsets(Rect insets, Rect srcRect) {
+        if (srcRect == null) {
+            return insets;
+        }
+        if (insets != null) {
+            srcRect.offset(insets.left, insets.top);
+        }
+        return srcRect;
+    }
+
+    /**
+     * Contains the result of a PixelCopy request
+     */
+    public static final class CopyResult {
+        private int mStatus;
+        private Bitmap mBitmap;
+
+        private CopyResult(@CopyResultStatus int status, Bitmap bitmap) {
+            mStatus = status;
+            mBitmap = bitmap;
+        }
+
+        /**
+         * Returns the {@link CopyResultStatus} of the copy request.
+         */
+        public @CopyResultStatus int getStatus() {
+            return mStatus;
+        }
+
+        private void validateStatus() {
+            if (mStatus != SUCCESS) {
+                throw new IllegalStateException("Copy request didn't succeed, status = " + mStatus);
+            }
+        }
+
+        /**
+         * If the PixelCopy {@link Request} was given a destination bitmap with
+         * {@link Request#setDestinationBitmap(Bitmap)} then the returned bitmap will be the same
+         * as the one given. If no destination bitmap was provided, then this
+         * will contain the automatically allocated Bitmap to hold the result.
+         *
+         * @return the Bitmap the copy request was stored in.
+         * @throws IllegalStateException if {@link #getStatus()} is not SUCCESS
+         */
+        public @NonNull Bitmap getBitmap() {
+            validateStatus();
+            return mBitmap;
+        }
+    }
+
+    /**
+     * A builder to create the complete PixelCopy request, which is then executed by calling
+     * {@link #request()}
+     */
+    public static final class Request {
+        private Request(Surface source, Rect sourceInsets, Executor listenerThread,
+                        Consumer<CopyResult> listener) {
+            this.mSource = source;
+            this.mSourceInsets = sourceInsets;
+            this.mListenerThread = listenerThread;
+            this.mListener = listener;
+        }
+
+        private final Surface mSource;
+        private final Consumer<CopyResult> mListener;
+        private final Executor mListenerThread;
+        private final Rect mSourceInsets;
+        private Rect mSrcRect;
+        private Bitmap mDest;
+
+        /**
+         * Sets the region of the source to copy from. By default, the entire source is copied to
+         * the output. If only a subset of the source is necessary to be copied, specifying a
+         * srcRect will improve performance by reducing
+         * the amount of data being copied.
+         *
+         * @param srcRect The area of the source to read from. Null or empty will be treated to
+         *                mean the entire source
+         * @return this
+         */
+        public @NonNull Request setSourceRect(@Nullable Rect srcRect) {
+            this.mSrcRect = srcRect;
+            return this;
+        }
+
+        /**
+         * Specifies the output bitmap in which to store the result. By default, a Bitmap of format
+         * {@link android.graphics.Bitmap.Config#ARGB_8888} with a width & height matching that
+         * of the {@link #setSourceRect(Rect) source area} will be created to place the result.
+         *
+         * @param destination The bitmap to store the result, or null to have a bitmap
+         *                    automatically created of the appropriate size. If not null, must not
+         *                    be {@link Bitmap#isRecycled() recycled} and must be
+         *                    {@link Bitmap#isMutable() mutable}.
+         * @return this
+         */
+        public @NonNull Request setDestinationBitmap(@Nullable Bitmap destination) {
+            if (destination != null) {
+                validateBitmapDest(destination);
+            }
+            this.mDest = destination;
+            return this;
+        }
+
+        /**
+         * Executes the request.
+         */
+        public void request() {
+            if (!mSource.isValid()) {
+                mListenerThread.execute(() -> mListener.accept(
+                        new CopyResult(ERROR_SOURCE_INVALID, null)));
+                return;
+            }
+            HardwareRenderer.copySurfaceInto(mSource, new HardwareRenderer.CopyRequest(
+                    adjustSourceRectForInsets(mSourceInsets, mSrcRect), mDest) {
+                @Override
+                public void onCopyFinished(int result) {
+                    mListenerThread.execute(() -> mListener.accept(
+                            new CopyResult(result, mDestinationBitmap)));
+                }
+            });
+        }
+    }
+
+    /**
+     * Creates a PixelCopy request for the given {@link Window}
+     * @param source The Window to copy from
+     * @param callbackExecutor The executor to run the callback on
+     * @param listener The callback for when the copy request is completed
+     * @return A {@link Request} builder to set the optional params & execute the request
+     */
+    public static @NonNull Request ofWindow(@NonNull Window source,
+                                            @NonNull Executor callbackExecutor,
+                                            @NonNull Consumer<CopyResult> listener) {
+        final Rect insets = new Rect();
+        final Surface surface = sourceForWindow(source, insets);
+        return new Request(surface, insets, callbackExecutor, listener);
+    }
+
+    /**
+     * Creates a PixelCopy request for the {@link Window} that the given {@link View} is
+     * attached to.
+     *
+     * Note that this copy request is not cropped to the area the View occupies by default. If that
+     * behavior is desired, use {@link View#getLocationInWindow(int[])} combined with
+     * {@link Request#setSourceRect(Rect)} to set a crop area to restrict the copy operation.
+     *
+     * @param source A View that {@link View#isAttachedToWindow() is attached} to a window that
+     *               will be used to retrieve the window to copy from.
+     * @param callbackExecutor The executor to run the callback on
+     * @param listener The callback for when the copy request is completed
+     * @return A {@link Request} builder to set the optional params & execute the request
+     */
+    public static @NonNull Request ofWindow(@NonNull View source,
+                                            @NonNull Executor callbackExecutor,
+                                            @NonNull Consumer<CopyResult> listener) {
+        if (source == null || !source.isAttachedToWindow()) {
+            throw new IllegalArgumentException(
+                    "View must not be null & must be attached to window");
+        }
+        final Rect insets = new Rect();
+        Surface surface = null;
+        final ViewRootImpl root = source.getViewRootImpl();
+        if (root != null) {
+            surface = root.mSurface;
+            insets.set(root.mWindowAttributes.surfaceInsets);
+        }
+        if (surface == null || !surface.isValid()) {
+            throw new IllegalArgumentException(
+                    "Window doesn't have a backing surface!");
+        }
+        return new Request(surface, insets, callbackExecutor, listener);
+    }
+
+    /**
+     * Creates a PixelCopy request for the given {@link Surface}
+     *
+     * @param source The Surface to copy from. Must be {@link Surface#isValid() valid}.
+     * @param callbackExecutor The executor to run the callback on
+     * @param listener The callback for when the copy request is completed
+     * @return A {@link Request} builder to set the optional params & execute the request
+     */
+    public static @NonNull Request ofSurface(@NonNull Surface source,
+                                             @NonNull Executor callbackExecutor,
+                                             @NonNull Consumer<CopyResult> listener) {
+        if (source == null || !source.isValid()) {
+            throw new IllegalArgumentException("Source must not be null & must be valid");
+        }
+        return new Request(source, null, callbackExecutor, listener);
+    }
+
+    /**
+     * Creates a PixelCopy request for the {@link Surface} belonging to the
+     * given {@link SurfaceView}
+     *
+     * @param source The SurfaceView to copy from. The backing surface must be
+     *               {@link Surface#isValid() valid}
+     * @param callbackExecutor The executor to run the callback on
+     * @param listener The callback for when the copy request is completed
+     * @return A {@link Request} builder to set the optional params & execute the request
+     */
+    public static @NonNull Request ofSurface(@NonNull SurfaceView source,
+                                             @NonNull Executor callbackExecutor,
+                                             @NonNull Consumer<CopyResult> listener) {
+        return ofSurface(source.getHolder().getSurface(), callbackExecutor, listener);
+    }
+
     private PixelCopy() {}
 }

libs/hwui/CopyRequest.h

@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+#include "Rect.h"
+#include "hwui/Bitmap.h"
+
+namespace android::uirenderer {
+
+// Keep in sync with PixelCopy.java codes
+enum class CopyResult {
+    Success = 0,
+    UnknownError = 1,
+    Timeout = 2,
+    SourceEmpty = 3,
+    SourceInvalid = 4,
+    DestinationInvalid = 5,
+};
+
+struct CopyRequest {
+    Rect srcRect;
+    CopyRequest(Rect srcRect) : srcRect(srcRect) {}
+    virtual ~CopyRequest() {}
+    virtual SkBitmap getDestinationBitmap(int srcWidth, int srcHeight) = 0;
+    virtual void onCopyFinished(CopyResult result) = 0;
+};
+
+}  // namespace android::uirenderer

libs/hwui/Readback.cpp

@@ -49,8 +49,7 @@ namespace uirenderer {
 
 #define ARECT_ARGS(r) float((r).left), float((r).top), float((r).right), float((r).bottom)
 
-CopyResult Readback::copySurfaceInto(ANativeWindow* window, const Rect& inSrcRect,
-                                     SkBitmap* bitmap) {
+void Readback::copySurfaceInto(ANativeWindow* window, const std::shared_ptr<CopyRequest>& request) {
     ATRACE_CALL();
     // Setup the source
     AHardwareBuffer* rawSourceBuffer;
@@ -63,30 +62,33 @@ CopyResult Readback::copySurfaceInto(ANativeWindow* window, const Rect& inSrcRec
     // Really this shouldn't ever happen, but better safe than sorry.
     if (err == UNKNOWN_TRANSACTION) {
         ALOGW("Readback failed to ANativeWindow_getLastQueuedBuffer2 - who are we talking to?");
-        return copySurfaceIntoLegacy(window, inSrcRect, bitmap);
+        return request->onCopyFinished(CopyResult::SourceInvalid);
     }
     ALOGV("Using new path, cropRect=" RECT_STRING ", transform=%x", ARECT_ARGS(cropRect),
           windowTransform);
 
     if (err != NO_ERROR) {
         ALOGW("Failed to get last queued buffer, error = %d", err);
-        return CopyResult::UnknownError;
+        return request->onCopyFinished(CopyResult::SourceInvalid);
     }
     if (rawSourceBuffer == nullptr) {
         ALOGW("Surface doesn't have any previously queued frames, nothing to readback from");
-        return CopyResult::SourceEmpty;
+        return request->onCopyFinished(CopyResult::SourceEmpty);
     }
     UniqueAHardwareBuffer sourceBuffer{rawSourceBuffer};
     AHardwareBuffer_Desc description;
     AHardwareBuffer_describe(sourceBuffer.get(), &description);
     if (description.usage & AHARDWAREBUFFER_USAGE_PROTECTED_CONTENT) {
         ALOGW("Surface is protected, unable to copy from it");
-        return CopyResult::SourceInvalid;
+        return request->onCopyFinished(CopyResult::SourceInvalid);
     }
 
-    if (sourceFence != -1 && sync_wait(sourceFence.get(), 500 /* ms */) != NO_ERROR) {
-        ALOGE("Timeout (500ms) exceeded waiting for buffer fence, abandoning readback attempt");
-        return CopyResult::Timeout;
+    {
+        ATRACE_NAME("sync_wait");
+        if (sourceFence != -1 && sync_wait(sourceFence.get(), 500 /* ms */) != NO_ERROR) {
+            ALOGE("Timeout (500ms) exceeded waiting for buffer fence, abandoning readback attempt");
+            return request->onCopyFinished(CopyResult::Timeout);
+        }
     }
 
     sk_sp<SkColorSpace> colorSpace = DataSpaceToColorSpace(
@@ -95,12 +97,12 @@ CopyResult Readback::copySurfaceInto(ANativeWindow* window, const Rect& inSrcRec
             SkImage::MakeFromAHardwareBuffer(sourceBuffer.get(), kPremul_SkAlphaType, colorSpace);
 
     if (!image.get()) {
-        return CopyResult::UnknownError;
+        return request->onCopyFinished(CopyResult::UnknownError);
     }
 
     sk_sp<GrDirectContext> grContext = mRenderThread.requireGrContext();
 
-    SkRect srcRect = inSrcRect.toSkRect();
+    SkRect srcRect = request->srcRect.toSkRect();
 
     SkRect imageSrcRect = SkRect::MakeIWH(description.width, description.height);
     SkISize imageWH = SkISize::Make(description.width, description.height);
@@ -148,10 +150,12 @@ CopyResult Readback::copySurfaceInto(ANativeWindow* window, const Rect& inSrcRec
         ALOGV("intersecting " RECT_STRING " with " RECT_STRING, SK_RECT_ARGS(srcRect),
               SK_RECT_ARGS(textureRect));
         if (!srcRect.intersect(textureRect)) {
-            return CopyResult::UnknownError;
+            return request->onCopyFinished(CopyResult::UnknownError);
         }
     }
 
+    SkBitmap skBitmap = request->getDestinationBitmap(srcRect.width(), srcRect.height());
+    SkBitmap* bitmap = &skBitmap;
     sk_sp<SkSurface> tmpSurface =
             SkSurface::MakeRenderTarget(mRenderThread.getGrContext(), SkBudgeted::kYes,
                                         bitmap->info(), 0, kTopLeft_GrSurfaceOrigin, nullptr);
@@ -164,7 +168,7 @@ CopyResult Readback::copySurfaceInto(ANativeWindow* window, const Rect& inSrcRec
                                                  tmpInfo, 0, kTopLeft_GrSurfaceOrigin, nullptr);
         if (!tmpSurface.get()) {
             ALOGW("Unable to generate GPU buffer in a format compatible with the provided bitmap");
-            return CopyResult::UnknownError;
+            return request->onCopyFinished(CopyResult::UnknownError);
         }
     }
 
@@ -235,52 +239,13 @@ CopyResult Readback::copySurfaceInto(ANativeWindow* window, const Rect& inSrcRec
             !tmpBitmap.tryAllocPixels(tmpInfo) || !tmpSurface->readPixels(tmpBitmap, 0, 0) ||
             !tmpBitmap.readPixels(bitmap->info(), bitmap->getPixels(), bitmap->rowBytes(), 0, 0)) {
             ALOGW("Unable to convert content into the provided bitmap");
-            return CopyResult::UnknownError;
+            return request->onCopyFinished(CopyResult::UnknownError);
         }
     }
 
     bitmap->notifyPixelsChanged();
 
-    return CopyResult::Success;
-}
-
-CopyResult Readback::copySurfaceIntoLegacy(ANativeWindow* window, const Rect& srcRect,
-                                           SkBitmap* bitmap) {
-    // Setup the source
-    AHardwareBuffer* rawSourceBuffer;
-    int rawSourceFence;
-    Matrix4 texTransform;
-    status_t err = ANativeWindow_getLastQueuedBuffer(window, &rawSourceBuffer, &rawSourceFence,
-                                                     texTransform.data);
-    base::unique_fd sourceFence(rawSourceFence);
-    texTransform.invalidateType();
-    if (err != NO_ERROR) {
-        ALOGW("Failed to get last queued buffer, error = %d", err);
-        return CopyResult::UnknownError;
-    }
-    if (rawSourceBuffer == nullptr) {
-        ALOGW("Surface doesn't have any previously queued frames, nothing to readback from");
-        return CopyResult::SourceEmpty;
-    }
-
-    UniqueAHardwareBuffer sourceBuffer{rawSourceBuffer};
-    AHardwareBuffer_Desc description;
-    AHardwareBuffer_describe(sourceBuffer.get(), &description);
-    if (description.usage & AHARDWAREBUFFER_USAGE_PROTECTED_CONTENT) {
-        ALOGW("Surface is protected, unable to copy from it");
-        return CopyResult::SourceInvalid;
-    }
-
-    if (sourceFence != -1 && sync_wait(sourceFence.get(), 500 /* ms */) != NO_ERROR) {
-        ALOGE("Timeout (500ms) exceeded waiting for buffer fence, abandoning readback attempt");
-        return CopyResult::Timeout;
-    }
-
-    sk_sp<SkColorSpace> colorSpace = DataSpaceToColorSpace(
-            static_cast<android_dataspace>(ANativeWindow_getBuffersDataSpace(window)));
-    sk_sp<SkImage> image =
-            SkImage::MakeFromAHardwareBuffer(sourceBuffer.get(), kPremul_SkAlphaType, colorSpace);
-    return copyImageInto(image, srcRect, bitmap);
+    return request->onCopyFinished(CopyResult::Success);
 }
 
 CopyResult Readback::copyHWBitmapInto(Bitmap* hwBitmap, SkBitmap* bitmap) {
@@ -318,14 +283,14 @@ CopyResult Readback::copyImageInto(const sk_sp<SkImage>& image, SkBitmap* bitmap
 CopyResult Readback::copyImageInto(const sk_sp<SkImage>& image, const Rect& srcRect,
                                    SkBitmap* bitmap) {
     ATRACE_CALL();
+    if (!image.get()) {
+        return CopyResult::UnknownError;
+    }
     if (Properties::getRenderPipelineType() == RenderPipelineType::SkiaGL) {
         mRenderThread.requireGlContext();
     } else {
         mRenderThread.requireVkContext();
     }
-    if (!image.get()) {
-        return CopyResult::UnknownError;
-    }
     int imgWidth = image->width();
     int imgHeight = image->height();
     sk_sp<GrDirectContext> grContext = sk_ref_sp(mRenderThread.getGrContext());

libs/hwui/Readback.h

@@ -16,12 +16,13 @@
 
 #pragma once
 
+#include <SkRefCnt.h>
+
+#include "CopyRequest.h"
 #include "Matrix.h"
 #include "Rect.h"
 #include "renderthread/RenderThread.h"
 
-#include <SkRefCnt.h>
-
 class SkBitmap;
 class SkImage;
 struct SkRect;
@@ -35,23 +36,13 @@ namespace uirenderer {
 class DeferredLayerUpdater;
 class Layer;
 
-// Keep in sync with PixelCopy.java codes
-enum class CopyResult {
-    Success = 0,
-    UnknownError = 1,
-    Timeout = 2,
-    SourceEmpty = 3,
-    SourceInvalid = 4,
-    DestinationInvalid = 5,
-};
-
 class Readback {
 public:
     explicit Readback(renderthread::RenderThread& thread) : mRenderThread(thread) {}
     /**
      * Copies the surface's most recently queued buffer into the provided bitmap.
      */
-    CopyResult copySurfaceInto(ANativeWindow* window, const Rect& srcRect, SkBitmap* bitmap);
+    void copySurfaceInto(ANativeWindow* window, const std::shared_ptr<CopyRequest>& request);
 
     CopyResult copyHWBitmapInto(Bitmap* hwBitmap, SkBitmap* bitmap);
     CopyResult copyImageInto(const sk_sp<SkImage>& image, SkBitmap* bitmap);
@@ -59,7 +50,6 @@ public:
     CopyResult copyLayerInto(DeferredLayerUpdater* layer, SkBitmap* bitmap);
 
 private:
-    CopyResult copySurfaceIntoLegacy(ANativeWindow* window, const Rect& srcRect, SkBitmap* bitmap);
     CopyResult copyImageInto(const sk_sp<SkImage>& image, const Rect& srcRect, SkBitmap* bitmap);
 
     bool copyLayerInto(Layer* layer, const SkRect* srcRect, const SkRect* dstRect,

libs/hwui/jni/android_graphics_HardwareRenderer.cpp

@@ -88,6 +88,11 @@ struct {
     jmethodID onFrameComplete;
 } gFrameCompleteCallback;
 
+struct {
+    jmethodID onCopyFinished;
+    jmethodID getDestinationBitmap;
+} gCopyRequest;
+
 static JNIEnv* getenv(JavaVM* vm) {
     JNIEnv* env;
     if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK) {
@@ -672,15 +677,41 @@ static void android_view_ThreadedRenderer_setFrameCompleteCallback(JNIEnv* env,
     }
 }
 
-static jint android_view_ThreadedRenderer_copySurfaceInto(JNIEnv* env,
-        jobject clazz, jobject jsurface, jint left, jint top,
-        jint right, jint bottom, jlong bitmapPtr) {
-    SkBitmap bitmap;
-    bitmap::toBitmap(bitmapPtr).getSkBitmap(&bitmap);
+class CopyRequestAdapter : public CopyRequest {
+public:
+    CopyRequestAdapter(JavaVM* vm, jobject jCopyRequest, Rect srcRect)
+            : CopyRequest(srcRect), mRefHolder(vm, jCopyRequest) {}
+
+    virtual SkBitmap getDestinationBitmap(int srcWidth, int srcHeight) override {
+        JNIEnv* env = getenv(mRefHolder.vm());
+        jlong bitmapPtr = env->CallLongMethod(
+                mRefHolder.object(), gCopyRequest.getDestinationBitmap, srcWidth, srcHeight);
+        SkBitmap bitmap;
+        bitmap::toBitmap(bitmapPtr).getSkBitmap(&bitmap);
+        return bitmap;
+    }
+
+    virtual void onCopyFinished(CopyResult result) override {
+        JNIEnv* env = getenv(mRefHolder.vm());
+        env->CallVoidMethod(mRefHolder.object(), gCopyRequest.onCopyFinished,
+                            static_cast<jint>(result));
+    }
+
+private:
+    JGlobalRefHolder mRefHolder;
+};
+
+static void android_view_ThreadedRenderer_copySurfaceInto(JNIEnv* env, jobject clazz,
+                                                          jobject jsurface, jint left, jint top,
+                                                          jint right, jint bottom,
+                                                          jobject jCopyRequest) {
+    JavaVM* vm = nullptr;
+    LOG_ALWAYS_FATAL_IF(env->GetJavaVM(&vm) != JNI_OK, "Unable to get Java VM");
+    auto copyRequest = std::make_shared<CopyRequestAdapter>(vm, env->NewGlobalRef(jCopyRequest),
+                                                            Rect(left, top, right, bottom));
     ANativeWindow* window = fromSurface(env, jsurface);
-    jint result = RenderProxy::copySurfaceInto(window, left, top, right, bottom, &bitmap);
+    RenderProxy::copySurfaceInto(window, std::move(copyRequest));
     ANativeWindow_release(window);
-    return result;
 }
 
 class ContextFactory : public IContextFactory {
@@ -969,7 +1000,8 @@ static const JNINativeMethod gMethods[] = {
          (void*)android_view_ThreadedRenderer_setFrameCompleteCallback},
         {"nAddObserver", "(JJ)V", (void*)android_view_ThreadedRenderer_addObserver},
         {"nRemoveObserver", "(JJ)V", (void*)android_view_ThreadedRenderer_removeObserver},
-        {"nCopySurfaceInto", "(Landroid/view/Surface;IIIIJ)I",
+        {"nCopySurfaceInto",
+         "(Landroid/view/Surface;IIIILandroid/graphics/HardwareRenderer$CopyRequest;)V",
          (void*)android_view_ThreadedRenderer_copySurfaceInto},
         {"nCreateHardwareBitmap", "(JII)Landroid/graphics/Bitmap;",
          (void*)android_view_ThreadedRenderer_createHardwareBitmapFromRenderNode},
@@ -1042,6 +1074,11 @@ int register_android_view_ThreadedRenderer(JNIEnv* env) {
     gFrameCompleteCallback.onFrameComplete =
             GetMethodIDOrDie(env, frameCompleteClass, "onFrameComplete", "()V");
 
+    jclass copyRequest = FindClassOrDie(env, "android/graphics/HardwareRenderer$CopyRequest");
+    gCopyRequest.onCopyFinished = GetMethodIDOrDie(env, copyRequest, "onCopyFinished", "(I)V");
+    gCopyRequest.getDestinationBitmap =
+            GetMethodIDOrDie(env, copyRequest, "getDestinationBitmap", "(II)J");
+
     void* handle_ = dlopen("libandroid.so", RTLD_NOW | RTLD_NODELETE);
     fromSurface = (ANW_fromSurface)dlsym(handle_, "ANativeWindow_fromSurface");
     LOG_ALWAYS_FATAL_IF(fromSurface == nullptr,

libs/hwui/renderthread/RenderProxy.cpp

@@ -364,12 +364,13 @@ void RenderProxy::setForceDark(bool enable) {
     mRenderThread.queue().post([this, enable]() { mContext->setForceDark(enable); });
 }
 
-int RenderProxy::copySurfaceInto(ANativeWindow* window, int left, int top, int right, int bottom,
-                                 SkBitmap* bitmap) {
+void RenderProxy::copySurfaceInto(ANativeWindow* window, std::shared_ptr<CopyRequest>&& request) {
     auto& thread = RenderThread::getInstance();
-    return static_cast<int>(thread.queue().runSync([&]() -> auto {
-        return thread.readback().copySurfaceInto(window, Rect(left, top, right, bottom), bitmap);
-    }));
+    ANativeWindow_acquire(window);
+    thread.queue().post([&thread, window, request = std::move(request)] {
+        thread.readback().copySurfaceInto(window, request);
+        ANativeWindow_release(window);
+    });
 }
 
 void RenderProxy::prepareToDraw(Bitmap& bitmap) {

libs/hwui/renderthread/RenderProxy.h

@@ -19,13 +19,14 @@
 
 #include <SkRefCnt.h>
 #include <android/native_window.h>
-#include <cutils/compiler.h>
 #include <android/surface_control.h>
+#include <cutils/compiler.h>
 #include <utils/Functor.h>
 
 #include "../FrameMetricsObserver.h"
 #include "../IContextFactory.h"
 #include "ColorMode.h"
+#include "CopyRequest.h"
 #include "DrawFrameTask.h"
 #include "SwapBehavior.h"
 #include "hwui/Bitmap.h"
@@ -137,8 +138,7 @@ public:
     void removeFrameMetricsObserver(FrameMetricsObserver* observer);
     void setForceDark(bool enable);
 
-    static int copySurfaceInto(ANativeWindow* window, int left, int top, int right,
-                                           int bottom, SkBitmap* bitmap);
+    static void copySurfaceInto(ANativeWindow* window, std::shared_ptr<CopyRequest>&& request);
     static void prepareToDraw(Bitmap& bitmap);
 
     static int copyHWBitmapInto(Bitmap* hwBitmap, SkBitmap* bitmap);

libs/hwui/tests/common/scenes/MagnifierAnimation.cpp

@@ -25,17 +25,51 @@
 
 class MagnifierAnimation;
 
+using Rect = android::uirenderer::Rect;
+
 static TestScene::Registrar _Magnifier(TestScene::Info{
         "magnifier", "A sample magnifier using Readback",
         TestScene::simpleCreateScene<MagnifierAnimation>});
 
+class BlockingCopyRequest : public CopyRequest {
+    sk_sp<Bitmap> mDestination;
+    std::mutex mLock;
+    std::condition_variable mCondVar;
+    CopyResult mResult;
+
+public:
+    BlockingCopyRequest(::Rect rect, sk_sp<Bitmap> bitmap)
+            : CopyRequest(rect), mDestination(bitmap) {}
+
+    virtual SkBitmap getDestinationBitmap(int srcWidth, int srcHeight) override {
+        SkBitmap bitmap;
+        mDestination->getSkBitmap(&bitmap);
+        return bitmap;
+    }
+
+    virtual void onCopyFinished(CopyResult result) override {
+        std::unique_lock _lock{mLock};
+        mResult = result;
+        mCondVar.notify_all();
+    }
+
+    CopyResult waitForResult() {
+        std::unique_lock _lock{mLock};
+        mCondVar.wait(_lock);
+        return mResult;
+    }
+};
+
 class MagnifierAnimation : public TestScene {
 public:
     sp<RenderNode> card;
     sp<RenderNode> zoomImageView;
+    sk_sp<Bitmap> magnifier;
+    std::shared_ptr<BlockingCopyRequest> copyRequest;
 
     void createContent(int width, int height, Canvas& canvas) override {
         magnifier = TestUtils::createBitmap(200, 100);
+        setupCopyRequest();
         SkBitmap temp;
         magnifier->getSkBitmap(&temp);
         temp.eraseColor(Color::White);
@@ -65,19 +99,20 @@ public:
         canvas.enableZ(false);
     }
 
+    void setupCopyRequest() {
+        constexpr int x = 90;
+        constexpr int y = 325;
+        copyRequest = std::make_shared<BlockingCopyRequest>(
+                ::Rect(x, y, x + magnifier->width(), y + magnifier->height()), magnifier);
+    }
+
     void doFrame(int frameNr) override {
         int curFrame = frameNr % 150;
         card->mutateStagingProperties().setTranslationX(curFrame);
         card->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
         if (renderTarget) {
-            SkBitmap temp;
-            magnifier->getSkBitmap(&temp);
-            constexpr int x = 90;
-            constexpr int y = 325;
-            RenderProxy::copySurfaceInto(renderTarget.get(), x, y, x + magnifier->width(),
-                                         y + magnifier->height(), &temp);
+            RenderProxy::copySurfaceInto(renderTarget.get(), copyRequest);
+            copyRequest->waitForResult();
         }
     }
-
-    sk_sp<Bitmap> magnifier;
 };

libs/hwui/thread/WorkQueue.h

@@ -57,7 +57,7 @@ private:
 
 public:
     WorkQueue(std::function<void()>&& wakeFunc, std::mutex& lock)
-            : mWakeFunc(move(wakeFunc)), mLock(lock) {}
+            : mWakeFunc(std::move(wakeFunc)), mLock(lock) {}
 
     void process() {
         auto now = clock::now();