Improve picture capture debug path

* Fixes hardware bitmap capture
* Fixes mutable bitmap capture (no flickering)
* Adds basic single-frame LRU cache to avoid
  repeated readbacks of GPU resources
* Does up-front readback of GPU resources
* Moves serialization off RenderThread again thanks
  to up-front readback avoiding needing GPU access
  off-thread
* Reduces RAM usage & improves performance by serializing
  directly to output stream instead of first copying to
  a byte[]

Bug: 174223722
Test: PictureCaptureDemo mirrors the content
Change-Id: If7ec208b61d5b917e82087cc312880fc5a38c943
diff --git a/core/java/android/view/ViewDebug.java b/core/java/android/view/ViewDebug.java
index 538b888..73294b3 100644
--- a/core/java/android/view/ViewDebug.java
+++ b/core/java/android/view/ViewDebug.java
@@ -953,8 +953,7 @@
         private final Callable<OutputStream> mCallback;
         private final Executor mExecutor;
         private final ReentrantLock mLock = new ReentrantLock(false);
-        private final ArrayDeque<byte[]> mQueue = new ArrayDeque<>(3);
-        private final ByteArrayOutputStream mByteStream = new ByteArrayOutputStream();
+        private final ArrayDeque<Picture> mQueue = new ArrayDeque<>(3);
         private boolean mStopListening;
         private Thread mRenderThread;
 
@@ -990,9 +989,7 @@
                 mQueue.removeLast();
                 needsInvoke = false;
             }
-            picture.writeToStream(mByteStream);
-            mQueue.add(mByteStream.toByteArray());
-            mByteStream.reset();
+            mQueue.add(picture);
             mLock.unlock();
 
             if (needsInvoke) {
@@ -1003,7 +1000,7 @@
         @Override
         public void run() {
             mLock.lock();
-            final byte[] picture = mQueue.poll();
+            final Picture picture = mQueue.poll();
             final boolean isStopped = mStopListening;
             mLock.unlock();
             if (Thread.currentThread() == mRenderThread) {
@@ -1024,7 +1021,8 @@
             }
             if (stream != null) {
                 try {
-                    stream.write(picture);
+                    picture.writeToStream(stream);
+                    stream.flush();
                 } catch (IOException ex) {
                     Log.w("ViewDebug", "Aborting rendering commands capture "
                             + "due to IOException writing to output stream", ex);
diff --git a/libs/hwui/Readback.cpp b/libs/hwui/Readback.cpp
index 8a8b418..d8735ce 100644
--- a/libs/hwui/Readback.cpp
+++ b/libs/hwui/Readback.cpp
@@ -275,6 +275,14 @@
     return copyResult;
 }
 
+CopyResult Readback::copyImageInto(const sk_sp<SkImage>& image, SkBitmap* bitmap) {
+    Rect srcRect;
+    Matrix4 transform;
+    transform.loadScale(1, -1, 1);
+    transform.translate(0, -1);
+    return copyImageInto(image, transform, srcRect, bitmap);
+}
+
 CopyResult Readback::copyImageInto(const sk_sp<SkImage>& image, Matrix4& texTransform,
                                    const Rect& srcRect, SkBitmap* bitmap) {
     ATRACE_CALL();
diff --git a/libs/hwui/Readback.h b/libs/hwui/Readback.h
index 4cb4bd8..da25269 100644
--- a/libs/hwui/Readback.h
+++ b/libs/hwui/Readback.h
@@ -50,6 +50,7 @@
     CopyResult copySurfaceInto(ANativeWindow* window, const Rect& srcRect, SkBitmap* bitmap);
 
     CopyResult copyHWBitmapInto(Bitmap* hwBitmap, SkBitmap* bitmap);
+    CopyResult copyImageInto(const sk_sp<SkImage>& image, SkBitmap* bitmap);
 
     CopyResult copyLayerInto(DeferredLayerUpdater* layer, SkBitmap* bitmap);
 
diff --git a/libs/hwui/jni/Picture.h b/libs/hwui/jni/Picture.h
index 536f651..87ba397 100644
--- a/libs/hwui/jni/Picture.h
+++ b/libs/hwui/jni/Picture.h
@@ -38,6 +38,7 @@
 public:
     explicit Picture(const Picture* src = NULL);
     explicit Picture(sk_sp<SkPicture>&& src);
+    virtual ~Picture() = default;
 
     Canvas* beginRecording(int width, int height);
 
@@ -49,7 +50,7 @@
 
     static Picture* CreateFromStream(SkStream* stream);
 
-    void serialize(SkWStream* stream) const;
+    virtual void serialize(SkWStream* stream) const;
 
     void draw(Canvas* canvas);
 
diff --git a/libs/hwui/jni/android_graphics_HardwareRenderer.cpp b/libs/hwui/jni/android_graphics_HardwareRenderer.cpp
index 4289c45..602c32a 100644
--- a/libs/hwui/jni/android_graphics_HardwareRenderer.cpp
+++ b/libs/hwui/jni/android_graphics_HardwareRenderer.cpp
@@ -23,6 +23,8 @@
 #include <Picture.h>
 #include <Properties.h>
 #include <RootRenderNode.h>
+#include <SkImagePriv.h>
+#include <SkSerialProcs.h>
 #include <dlfcn.h>
 #include <gui/TraceUtils.h>
 #include <inttypes.h>
@@ -35,6 +37,7 @@
 #include <renderthread/RenderProxy.h>
 #include <renderthread/RenderTask.h>
 #include <renderthread/RenderThread.h>
+#include <src/image/SkImage_Base.h>
 #include <thread/CommonPool.h>
 #include <utils/Color.h>
 #include <utils/RefBase.h>
@@ -497,6 +500,108 @@
     jobject mObject;
 };
 
+using TextureMap = std::unordered_map<uint32_t, sk_sp<SkImage>>;
+
+struct PictureCaptureState {
+    // Each frame we move from the active map to the previous map, essentially an LRU of 1 frame
+    // This avoids repeated readbacks of the same image, but avoids artificially extending the
+    // lifetime of any particular image.
+    TextureMap mActiveMap;
+    TextureMap mPreviousActiveMap;
+};
+
+// TODO: This & Multi-SKP & Single-SKP should all be de-duped into
+// a single "make a SkPicture serailizable-safe" utility somewhere
+class PictureWrapper : public Picture {
+public:
+    PictureWrapper(sk_sp<SkPicture>&& src, const std::shared_ptr<PictureCaptureState>& state)
+            : Picture(), mPicture(std::move(src)) {
+        ATRACE_NAME("Preparing SKP for capture");
+        // Move the active to previous active
+        state->mPreviousActiveMap = std::move(state->mActiveMap);
+        state->mActiveMap.clear();
+        SkSerialProcs tempProc;
+        tempProc.fImageCtx = state.get();
+        tempProc.fImageProc = collectNonTextureImagesProc;
+        auto ns = SkNullWStream();
+        mPicture->serialize(&ns, &tempProc);
+        state->mPreviousActiveMap.clear();
+
+        // Now snapshot a copy of the active map so this PictureWrapper becomes self-sufficient
+        mTextureMap = state->mActiveMap;
+    }
+
+    static sk_sp<SkImage> imageForCache(SkImage* img) {
+        const SkBitmap* bitmap = as_IB(img)->onPeekBitmap();
+        // This is a mutable bitmap pretending to be an immutable SkImage. As we're going to
+        // actually cross thread boundaries here, make a copy so it's immutable proper
+        if (bitmap && !bitmap->isImmutable()) {
+            ATRACE_NAME("Copying mutable bitmap");
+            return SkImage::MakeFromBitmap(*bitmap);
+        }
+        if (img->isTextureBacked()) {
+            ATRACE_NAME("Readback of texture image");
+            return img->makeNonTextureImage();
+        }
+        SkPixmap pm;
+        if (img->isLazyGenerated() && !img->peekPixels(&pm)) {
+            ATRACE_NAME("Readback of HW bitmap");
+            // This is a hardware bitmap probably
+            SkBitmap bm;
+            if (!bm.tryAllocPixels(img->imageInfo())) {
+                // Failed to allocate, just see what happens
+                return sk_ref_sp(img);
+            }
+            if (RenderProxy::copyImageInto(sk_ref_sp(img), &bm)) {
+                // Failed to readback
+                return sk_ref_sp(img);
+            }
+            bm.setImmutable();
+            return SkMakeImageFromRasterBitmap(bm, kNever_SkCopyPixelsMode);
+        }
+        return sk_ref_sp(img);
+    }
+
+    static sk_sp<SkData> collectNonTextureImagesProc(SkImage* img, void* ctx) {
+        PictureCaptureState* context = reinterpret_cast<PictureCaptureState*>(ctx);
+        const uint32_t originalId = img->uniqueID();
+        auto it = context->mActiveMap.find(originalId);
+        if (it == context->mActiveMap.end()) {
+            auto pit = context->mPreviousActiveMap.find(originalId);
+            if (pit == context->mPreviousActiveMap.end()) {
+                context->mActiveMap[originalId] = imageForCache(img);
+            } else {
+                context->mActiveMap[originalId] = pit->second;
+            }
+        }
+        return SkData::MakeEmpty();
+    }
+
+    static sk_sp<SkData> serializeImage(SkImage* img, void* ctx) {
+        PictureWrapper* context = reinterpret_cast<PictureWrapper*>(ctx);
+        const uint32_t id = img->uniqueID();
+        auto iter = context->mTextureMap.find(id);
+        if (iter != context->mTextureMap.end()) {
+            img = iter->second.get();
+        }
+        return img->encodeToData();
+    }
+
+    void serialize(SkWStream* stream) const override {
+        SkSerialProcs procs;
+        procs.fImageProc = serializeImage;
+        procs.fImageCtx = const_cast<PictureWrapper*>(this);
+        procs.fTypefaceProc = [](SkTypeface* tf, void* ctx) {
+            return tf->serialize(SkTypeface::SerializeBehavior::kDoIncludeData);
+        };
+        mPicture->serialize(stream, &procs);
+    }
+
+private:
+    sk_sp<SkPicture> mPicture;
+    TextureMap mTextureMap;
+};
+
 static void android_view_ThreadedRenderer_setPictureCapturedCallbackJNI(JNIEnv* env,
         jobject clazz, jlong proxyPtr, jobject pictureCallback) {
     RenderProxy* proxy = reinterpret_cast<RenderProxy*>(proxyPtr);
@@ -507,9 +612,11 @@
         LOG_ALWAYS_FATAL_IF(env->GetJavaVM(&vm) != JNI_OK, "Unable to get Java VM");
         auto globalCallbackRef = std::make_shared<JGlobalRefHolder>(vm,
                 env->NewGlobalRef(pictureCallback));
-        proxy->setPictureCapturedCallback([globalCallbackRef](sk_sp<SkPicture>&& picture) {
+        auto pictureState = std::make_shared<PictureCaptureState>();
+        proxy->setPictureCapturedCallback([globalCallbackRef,
+                                           pictureState](sk_sp<SkPicture>&& picture) {
             JNIEnv* env = getenv(globalCallbackRef->vm());
-            Picture* wrapper = new Picture{std::move(picture)};
+            Picture* wrapper = new PictureWrapper{std::move(picture), pictureState};
             env->CallStaticVoidMethod(gHardwareRenderer.clazz,
                     gHardwareRenderer.invokePictureCapturedCallback,
                     static_cast<jlong>(reinterpret_cast<intptr_t>(wrapper)),
diff --git a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
index a78cd83..9bca4df 100644
--- a/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp
@@ -75,7 +75,9 @@
                               bool opaque, const LightInfo& lightInfo,
                               const std::vector<sp<RenderNode>>& renderNodes,
                               FrameInfoVisualizer* profiler) {
-    mEglManager.damageFrame(frame, dirty);
+    if (!isCapturingSkp()) {
+        mEglManager.damageFrame(frame, dirty);
+    }
 
     SkColorType colorType = getSurfaceColorType();
     // setup surface for fbo0
diff --git a/libs/hwui/pipeline/skia/SkiaPipeline.cpp b/libs/hwui/pipeline/skia/SkiaPipeline.cpp
index 039b0f9..5462623 100644
--- a/libs/hwui/pipeline/skia/SkiaPipeline.cpp
+++ b/libs/hwui/pipeline/skia/SkiaPipeline.cpp
@@ -420,7 +420,7 @@
                 procs.fTypefaceProc = [](SkTypeface* tf, void* ctx){
                     return tf->serialize(SkTypeface::SerializeBehavior::kDoIncludeData);
                 };
-                auto data = picture->serialize();
+                auto data = picture->serialize(&procs);
                 savePictureAsync(data, mCapturedFile);
                 mCaptureSequence = 0;
                 mCaptureMode = CaptureMode::None;
@@ -470,8 +470,7 @@
                                    const SkMatrix& preTransform) {
     SkAutoCanvasRestore saver(canvas, true);
     auto clipRestriction = preTransform.mapRect(clip).roundOut();
-    if (CC_UNLIKELY(mCaptureMode == CaptureMode::SingleFrameSKP
-         || mCaptureMode == CaptureMode::MultiFrameSKP)) {
+    if (CC_UNLIKELY(isCapturingSkp())) {
         canvas->drawAnnotation(SkRect::Make(clipRestriction), "AndroidDeviceClipRestriction",
             nullptr);
     } else {
diff --git a/libs/hwui/pipeline/skia/SkiaPipeline.h b/libs/hwui/pipeline/skia/SkiaPipeline.h
index 4658035..bc8a565 100644
--- a/libs/hwui/pipeline/skia/SkiaPipeline.h
+++ b/libs/hwui/pipeline/skia/SkiaPipeline.h
@@ -80,6 +80,8 @@
     SkColorType mSurfaceColorType;
     sk_sp<SkColorSpace> mSurfaceColorSpace;
 
+    bool isCapturingSkp() const { return mCaptureMode != CaptureMode::None; }
+
 private:
     void renderFrameImpl(const SkRect& clip,
                          const std::vector<sp<RenderNode>>& nodes, bool opaque,
diff --git a/libs/hwui/renderthread/RenderProxy.cpp b/libs/hwui/renderthread/RenderProxy.cpp
index ac19a15..6fd644b 100644
--- a/libs/hwui/renderthread/RenderProxy.cpp
+++ b/libs/hwui/renderthread/RenderProxy.cpp
@@ -390,6 +390,17 @@
     }
 }
 
+int RenderProxy::copyImageInto(const sk_sp<SkImage>& image, SkBitmap* bitmap) {
+    RenderThread& thread = RenderThread::getInstance();
+    if (gettid() == thread.getTid()) {
+        // TODO: fix everything that hits this. We should never be triggering a readback ourselves.
+        return (int)thread.readback().copyImageInto(image, bitmap);
+    } else {
+        return thread.queue().runSync(
+                [&]() -> int { return (int)thread.readback().copyImageInto(image, bitmap); });
+    }
+}
+
 void RenderProxy::disableVsync() {
     Properties::disableVsync = true;
 }
diff --git a/libs/hwui/renderthread/RenderProxy.h b/libs/hwui/renderthread/RenderProxy.h
index 0681dc5..6d80949 100644
--- a/libs/hwui/renderthread/RenderProxy.h
+++ b/libs/hwui/renderthread/RenderProxy.h
@@ -136,6 +136,7 @@
     static void prepareToDraw(Bitmap& bitmap);
 
     static int copyHWBitmapInto(Bitmap* hwBitmap, SkBitmap* bitmap);
+    static int copyImageInto(const sk_sp<SkImage>& image, SkBitmap* bitmap);
 
     static void disableVsync();
 
diff --git a/tests/HwAccelerationTest/src/com/android/test/hwui/PictureCaptureDemo.java b/tests/HwAccelerationTest/src/com/android/test/hwui/PictureCaptureDemo.java
index 029e302..15568ac 100644
--- a/tests/HwAccelerationTest/src/com/android/test/hwui/PictureCaptureDemo.java
+++ b/tests/HwAccelerationTest/src/com/android/test/hwui/PictureCaptureDemo.java
@@ -34,13 +34,14 @@
 import android.widget.LinearLayout;
 import android.widget.LinearLayout.LayoutParams;
 import android.widget.ProgressBar;
+import android.widget.TextView;
 
-import java.io.PipedInputStream;
-import java.io.PipedOutputStream;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
 import java.util.Random;
-import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
 
 public class PictureCaptureDemo extends Activity {
     @Override
@@ -77,6 +78,12 @@
         iv2.setImageBitmap(Bitmap.createBitmap(picture, 100, 100, Bitmap.Config.HARDWARE));
         inner.addView(iv2, new LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT));
 
+        TextView hello = new TextView(this);
+        hello.setText("I'm on a layer!");
+        hello.setLayerType(View.LAYER_TYPE_HARDWARE, null);
+        inner.addView(hello,
+                new LayoutParams(LayoutParams.WRAP_CONTENT, LayoutParams.WRAP_CONTENT));
+
         layout.addView(inner,
                 new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT));
         // For testing with a functor in the tree
@@ -84,11 +91,13 @@
         wv.setWebViewClient(new WebViewClient());
         wv.setWebChromeClient(new WebChromeClient());
         wv.loadUrl("https://google.com");
-        layout.addView(wv, new LayoutParams(LayoutParams.MATCH_PARENT, 400));
+        LayoutParams wvParams = new LayoutParams(LayoutParams.MATCH_PARENT, 400);
+        wvParams.bottomMargin = 50;
+        layout.addView(wv, wvParams);
 
         SurfaceView mySurfaceView = new SurfaceView(this);
         layout.addView(mySurfaceView,
-                new LayoutParams(LayoutParams.MATCH_PARENT, 600));
+                new LayoutParams(LayoutParams.MATCH_PARENT, 600, 1f));
 
         setContentView(layout);
 
@@ -98,22 +107,29 @@
             @Override
             public void surfaceCreated(SurfaceHolder holder) {
                 final Random rand = new Random();
+                OutputStream renderingStream = new ByteArrayOutputStream() {
+                    @Override
+                    public void flush() throws IOException {
+                        Picture picture = Picture.createFromStream(
+                                new ByteArrayInputStream(buf, 0, count));
+                        Canvas canvas = holder.lockCanvas();
+                        if (canvas != null && picture != null) {
+                            canvas.drawPicture(picture);
+                            holder.unlockCanvasAndPost(canvas);
+                        }
+                        reset();
+                    }
+                };
+
                 mStopCapture = ViewDebug.startRenderingCommandsCapture(mySurfaceView,
-                        mCaptureThread, (picture) -> {
+                        Executors.newSingleThreadExecutor(), () -> {
                             if (rand.nextInt(20) == 0) {
                                 try {
                                     Thread.sleep(100);
                                 } catch (InterruptedException e) {
                                 }
                             }
-                            Canvas canvas = holder.lockCanvas();
-                            if (canvas == null) {
-                                return false;
-                            }
-                            canvas.drawPicture(picture);
-                            holder.unlockCanvasAndPost(canvas);
-                            picture.close();
-                            return true;
+                            return renderingStream;
                         });
             }
 
@@ -134,20 +150,4 @@
             }
         });
     }
-
-    ExecutorService mCaptureThread = Executors.newSingleThreadExecutor();
-    ExecutorService mExecutor = Executors.newSingleThreadExecutor();
-
-    Picture deepCopy(Picture src) {
-        try {
-            PipedInputStream inputStream = new PipedInputStream();
-            PipedOutputStream outputStream = new PipedOutputStream(inputStream);
-            Future<Picture> future = mExecutor.submit(() -> Picture.createFromStream(inputStream));
-            src.writeToStream(outputStream);
-            outputStream.close();
-            return future.get();
-        } catch (Exception ex) {
-            throw new RuntimeException(ex);
-        }
-    }
 }