Initial version of clipped saveLayer in new pipeline

Additionally disables usage of FBO cache, so FBO destruction safely
interacts with renderstate caching.

Change-Id: I25c277cb7afec2ca33bf226445d6c8867a15a915

16 files changed, 356 insertions, 227 deletions

diff --git a/libs/hwui/BakedOpRenderer.cpp b/libs/hwui/BakedOpRenderer.cpp
index 94806cab2fc5..7dbba25a0ec4 100644
--- a/libs/hwui/BakedOpRenderer.cpp
+++ b/libs/hwui/BakedOpRenderer.cpp
@@ -25,6 +25,15 @@
 namespace android {
 namespace uirenderer {
 
+void BakedOpRenderer::Info::setViewport(uint32_t width, uint32_t height) {
+    viewportWidth = width;
+    viewportHeight = height;
+    orthoMatrix.loadOrtho(viewportWidth, viewportHeight);
+
+    renderState.setViewport(width, height);
+    renderState.blend().syncEnabled();
+}
+
 Texture* BakedOpRenderer::Info::getTexture(const SkBitmap* bitmap) {
     Texture* texture = renderState.assetAtlas().getEntryTexture(bitmap);
     if (!texture) {
@@ -45,9 +54,54 @@ void BakedOpRenderer::Info::renderGlop(const BakedOpState& state, const Glop& gl
     didDraw = true;
 }
 
-void BakedOpRenderer::startFrame(Info& info) {
-    info.renderState.setViewport(info.viewportWidth, info.viewportHeight);
-    info.renderState.blend().syncEnabled();
+Layer* BakedOpRenderer::startLayer(Info& info, uint32_t width, uint32_t height) {
+    info.caches.textureState().activateTexture(0);
+    Layer* layer = info.caches.layerCache.get(info.renderState, width, height);
+    LOG_ALWAYS_FATAL_IF(!layer, "need layer...");
+
+    info.layer = layer;
+    layer->texCoords.set(0.0f, width / float(layer->getHeight()),
+            height / float(layer->getWidth()), 0.0f);
+
+    layer->setFbo(info.renderState.genFramebuffer());
+    info.renderState.bindFramebuffer(layer->getFbo());
+    layer->bindTexture();
+
+    // Initialize the texture if needed
+    if (layer->isEmpty()) {
+        layer->allocateTexture();
+        layer->setEmpty(false);
+    }
+
+    // attach the texture to the FBO
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D,
+            layer->getTextureId(), 0);
+    LOG_ALWAYS_FATAL_IF(GLUtils::dumpGLErrors(), "startLayer FAILED");
+    LOG_ALWAYS_FATAL_IF(glCheckFramebufferStatus(GL_FRAMEBUFFER) != GL_FRAMEBUFFER_COMPLETE,
+            "framebuffer incomplete!");
+
+    // Clear the FBO
+    info.renderState.scissor().setEnabled(false);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    // Change the viewport & ortho projection
+    info.setViewport(width, height);
+    return layer;
+}
+
+void BakedOpRenderer::endLayer(Info& info) {
+    Layer* layer = info.layer;
+    info.layer = nullptr;
+
+    // Detach the texture from the FBO
+    glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, 0, 0);
+    LOG_ALWAYS_FATAL_IF(GLUtils::dumpGLErrors(), "endLayer FAILED");
+    layer->removeFbo(false);
+}
+
+void BakedOpRenderer::startFrame(Info& info, uint32_t width, uint32_t height) {
+    info.renderState.bindFramebuffer(0);
+    info.setViewport(width, height);
     Caches::getInstance().clearGarbage();
 
     if (!info.opaque) {
@@ -130,7 +184,31 @@ void BakedOpRenderer::onEndLayerOp(Info& info, const EndLayerOp& op, const Baked
 }
 
 void BakedOpRenderer::onLayerOp(Info& info, const LayerOp& op, const BakedOpState& state) {
-    LOG_ALWAYS_FATAL("unsupported operation");
+    Layer* layer = *op.layerHandle;
+
+    // TODO: make this work for HW layers
+    layer->setPaint(op.paint);
+    layer->setBlend(true);
+    float layerAlpha = (layer->getAlpha() / 255.0f) * state.alpha;
+
+    const bool tryToSnap = state.computedState.transform.isPureTranslate();
+    Glop glop;
+    GlopBuilder(info.renderState, info.caches, &glop)
+            .setRoundRectClipState(state.roundRectClipState)
+            .setMeshTexturedUvQuad(nullptr, layer->texCoords)
+            .setFillLayer(layer->getTexture(), layer->getColorFilter(), layerAlpha, layer->getMode(), Blend::ModeOrderSwap::NoSwap)
+            .setTransform(state.computedState.transform, TransformFlags::None)
+            .setModelViewMapUnitToRectOptionalSnap(tryToSnap, op.unmappedBounds)
+            .build();
+    info.renderGlop(state, glop);
+
+    // return layer to cache, since each clipped savelayer is only drawn once.
+    layer->setConvexMask(nullptr);
+    if (!info.caches.layerCache.put(layer)) {
+        // Failing to add the layer to the cache should happen only if the layer is too large
+        LAYER_LOGD("Deleting layer");
+        layer->decStrong(nullptr);
+    }
 }
 
 } // namespace uirenderer
diff --git a/libs/hwui/BakedOpRenderer.h b/libs/hwui/BakedOpRenderer.h
index f45dbe41f308..616adde51aa0 100644
--- a/libs/hwui/BakedOpRenderer.h
+++ b/libs/hwui/BakedOpRenderer.h
@@ -25,21 +25,21 @@ namespace uirenderer {
 
 class Caches;
 struct Glop;
+class Layer;
 class RenderState;
 
 class BakedOpRenderer {
 public:
     class Info {
     public:
-        Info(Caches& caches, RenderState& renderState, int viewportWidth, int viewportHeight, bool opaque)
+        Info(Caches& caches, RenderState& renderState, bool opaque)
                 : renderState(renderState)
                 , caches(caches)
-                , opaque(opaque)
-                , viewportWidth(viewportWidth)
-                , viewportHeight(viewportHeight) {
-            orthoMatrix.loadOrtho(viewportWidth, viewportHeight);
+                , opaque(opaque) {
         }
 
+        void setViewport(uint32_t width, uint32_t height);
+
         Texture* getTexture(const SkBitmap* bitmap);
 
         void renderGlop(const BakedOpState& state, const Glop& glop);
@@ -47,16 +47,19 @@ public:
         Caches& caches;
 
         bool didDraw = false;
-        bool opaque;
 
+        Layer* layer = nullptr;
 
         // where should these live? layer state object?
-        int viewportWidth;
-        int viewportHeight;
+        bool opaque;
+        uint32_t viewportWidth = 0;
+        uint32_t viewportHeight = 0;
         Matrix4 orthoMatrix;
     };
 
-    static void startFrame(Info& info);
+    static Layer* startLayer(Info& info, uint32_t width, uint32_t height);
+    static void endLayer(Info& info);
+    static void startFrame(Info& info, uint32_t width, uint32_t height);
     static void endFrame(Info& info);
 
     /**
diff --git a/libs/hwui/Layer.cpp b/libs/hwui/Layer.cpp
index f99d92b89420..489ebc116a2a 100644
--- a/libs/hwui/Layer.cpp
+++ b/libs/hwui/Layer.cpp
@@ -155,8 +155,7 @@ void Layer::removeFbo(bool flush) {
 
     if (fbo) {
         if (flush) LayerRenderer::flushLayer(renderState, this);
-        // If put fails the cache will delete the FBO
-        caches.fboCache.put(fbo);
+        renderState.deleteFramebuffer(fbo);
         fbo = 0;
     }
 }
diff --git a/libs/hwui/LayerRenderer.cpp b/libs/hwui/LayerRenderer.cpp
index 227271d83cf8..e9e5d81f1ce2 100644
--- a/libs/hwui/LayerRenderer.cpp
+++ b/libs/hwui/LayerRenderer.cpp
@@ -189,7 +189,7 @@ Layer* LayerRenderer::createRenderLayer(RenderState& renderState, uint32_t width
     LAYER_RENDERER_LOGD("Requesting new render layer %dx%d", width, height);
 
     Caches& caches = Caches::getInstance();
-    GLuint fbo = caches.fboCache.get();
+    GLuint fbo = renderState.genFramebuffer();
     if (!fbo) {
         ALOGW("Could not obtain an FBO");
         return nullptr;
@@ -204,7 +204,7 @@ Layer* LayerRenderer::createRenderLayer(RenderState& renderState, uint32_t width
 
     // We first obtain a layer before comparing against the max texture size
     // because layers are not allocated at the exact desired size. They are
-    // always created slighly larger to improve recycling
+    // always created slightly larger to improve recycling
     const uint32_t maxTextureSize = caches.maxTextureSize;
     if (layer->getWidth() > maxTextureSize || layer->getHeight() > maxTextureSize) {
         ALOGW("Layer exceeds max. dimensions supported by the GPU (%dx%d, max=%dx%d)",
@@ -357,7 +357,7 @@ bool LayerRenderer::copyLayer(RenderState& renderState, Layer* layer, SkBitmap*
             && bitmap->width() <= caches.maxTextureSize
             && bitmap->height() <= caches.maxTextureSize) {
 
-        GLuint fbo = caches.fboCache.get();
+        GLuint fbo = renderState.getFramebuffer();
         if (!fbo) {
             ALOGW("Could not obtain an FBO");
             return false;
@@ -465,7 +465,7 @@ error:
         layer->setAlpha(alpha, mode);
         layer->setFbo(previousLayerFbo);
         caches.textureState().deleteTexture(texture);
-        caches.fboCache.put(fbo);
+        renderState.deleteFramebuffer(fbo);
         renderState.setViewport(previousViewportWidth, previousViewportHeight);
 
         return status;
diff --git a/libs/hwui/OpReorderer.cpp b/libs/hwui/OpReorderer.cpp
index c1417c451895..cde42f85b0f9 100644
--- a/libs/hwui/OpReorderer.cpp
+++ b/libs/hwui/OpReorderer.cpp
@@ -203,7 +203,7 @@ private:
 };
 
 // iterate back toward target to see if anything drawn since should overlap the new op
-// if no target, merging ops still interate to find similar batch to insert after
+// if no target, merging ops still iterate to find similar batch to insert after
 void OpReorderer::LayerReorderer::locateInsertIndex(int batchId, const Rect& clippedBounds,
         BatchBase** targetBatch, size_t* insertBatchIndex) const {
     for (int i = mBatches.size() - 1; i >= 0; i--) {
@@ -292,18 +292,14 @@ void OpReorderer::LayerReorderer::dump() const {
     }
 }
 
-OpReorderer::OpReorderer()
+OpReorderer::OpReorderer(const SkRect& clip, uint32_t viewportWidth, uint32_t viewportHeight,
+        const std::vector< sp<RenderNode> >& nodes)
         : mCanvasState(*this) {
-    mLayerReorderers.emplace_back();
-    mLayerStack.push_back(0);
-}
-
-void OpReorderer::onViewportInitialized() {}
+    ATRACE_NAME("prepare drawing commands");
 
-void OpReorderer::onSnapshotRestored(const Snapshot& removed, const Snapshot& restored) {}
+    mLayerReorderers.emplace_back(viewportWidth, viewportHeight);
+    mLayerStack.push_back(0);
 
-void OpReorderer::defer(const SkRect& clip, int viewportWidth, int viewportHeight,
-        const std::vector< sp<RenderNode> >& nodes) {
     mCanvasState.initializeSaveStack(viewportWidth, viewportHeight,
             clip.fLeft, clip.fTop, clip.fRight, clip.fBottom,
             Vector3());
@@ -321,13 +317,22 @@ void OpReorderer::defer(const SkRect& clip, int viewportWidth, int viewportHeigh
     }
 }
 
-void OpReorderer::defer(int viewportWidth, int viewportHeight, const DisplayList& displayList) {
+OpReorderer::OpReorderer(int viewportWidth, int viewportHeight, const DisplayList& displayList)
+        : mCanvasState(*this) {
     ATRACE_NAME("prepare drawing commands");
+
+    mLayerReorderers.emplace_back(viewportWidth, viewportHeight);
+    mLayerStack.push_back(0);
+
     mCanvasState.initializeSaveStack(viewportWidth, viewportHeight,
             0, 0, viewportWidth, viewportHeight, Vector3());
     deferImpl(displayList);
 }
 
+void OpReorderer::onViewportInitialized() {}
+
+void OpReorderer::onSnapshotRestored(const Snapshot& removed, const Snapshot& restored) {}
+
 /**
  * Used to define a list of lambdas referencing private OpReorderer::onXXXXOp() methods.
  *
@@ -350,11 +355,6 @@ void OpReorderer::deferImpl(const DisplayList& displayList) {
 
 void OpReorderer::replayBakedOpsImpl(void* arg, BakedOpReceiver* receivers) {
     ATRACE_NAME("flush drawing commands");
-    // Relay through layers in reverse order, since layers
-    // later in the list will be drawn by earlier ones
-    for (int i = mLayerReorderers.size() - 1; i >= 0; i--) {
-        mLayerReorderers[i].replayBakedOpsImpl(arg, receivers);
-    }
 }
 
 void OpReorderer::onRenderNodeOp(const RenderNodeOp& op) {
@@ -405,15 +405,17 @@ void OpReorderer::onSimpleRectsOp(const SimpleRectsOp& op) {
 
 // TODO: test rejection at defer time, where the bounds become empty
 void OpReorderer::onBeginLayerOp(const BeginLayerOp& op) {
+    const uint32_t layerWidth = (uint32_t) op.unmappedBounds.getWidth();
+    const uint32_t layerHeight = (uint32_t) op.unmappedBounds.getHeight();
+
     mCanvasState.save(SkCanvas::kClip_SaveFlag | SkCanvas::kMatrix_SaveFlag);
     mCanvasState.writableSnapshot()->transform->loadIdentity();
-    mCanvasState.writableSnapshot()->initializeViewport(
-            (int) op.unmappedBounds.getWidth(), (int) op.unmappedBounds.getHeight());
+    mCanvasState.writableSnapshot()->initializeViewport(layerWidth, layerHeight);
     mCanvasState.writableSnapshot()->roundRectClipState = nullptr;
 
     // create a new layer, and push its index on the stack
     mLayerStack.push_back(mLayerReorderers.size());
-    mLayerReorderers.emplace_back();
+    mLayerReorderers.emplace_back(layerWidth, layerHeight);
     mLayerReorderers.back().beginLayerOp = &op;
 }
 
@@ -432,7 +434,8 @@ void OpReorderer::onEndLayerOp(const EndLayerOp& /* ignored */) {
             beginLayerOp.unmappedBounds,
             beginLayerOp.localMatrix,
             beginLayerOp.localClipRect,
-            beginLayerOp.paint);
+            beginLayerOp.paint,
+            &mLayerReorderers[finishedLayerIndex].layer);
     BakedOpState* bakedOpState = tryBakeOpState(*drawLayerOp);
 
     if (bakedOpState) {
diff --git a/libs/hwui/OpReorderer.h b/libs/hwui/OpReorderer.h
index 73dc9af01f71..f32b8589983b 100644
--- a/libs/hwui/OpReorderer.h
+++ b/libs/hwui/OpReorderer.h
@@ -63,6 +63,10 @@ class OpReorderer : public CanvasStateClient {
      */
     class LayerReorderer {
     public:
+        LayerReorderer(uint32_t width, uint32_t height)
+                : width(width)
+                , height(height) {}
+
         // iterate back toward target to see if anything drawn since should overlap the new op
         // if no target, merging ops still iterate to find similar batch to insert after
         void locateInsertIndex(int batchId, const Rect& clippedBounds,
@@ -77,15 +81,22 @@ class OpReorderer : public CanvasStateClient {
 
         void replayBakedOpsImpl(void* arg, BakedOpReceiver* receivers) const;
 
+        bool empty() const {
+            return mBatches.empty();
+        }
+
         void clear() {
             mBatches.clear();
         }
 
         void dump() const;
 
+        Layer* layer = nullptr;
         const BeginLayerOp* beginLayerOp = nullptr;
-
+        const uint32_t width;
+        const uint32_t height;
     private:
+
         std::vector<BatchBase*> mBatches;
 
         /**
@@ -100,14 +111,13 @@ class OpReorderer : public CanvasStateClient {
 
     };
 public:
-    OpReorderer();
-    virtual ~OpReorderer() {}
-
     // TODO: not final, just presented this way for simplicity. Layers too?
-    void defer(const SkRect& clip, int viewportWidth, int viewportHeight,
+    OpReorderer(const SkRect& clip, uint32_t viewportWidth, uint32_t viewportHeight,
             const std::vector< sp<RenderNode> >& nodes);
 
-    void defer(int viewportWidth, int viewportHeight, const DisplayList& displayList);
+    OpReorderer(int viewportWidth, int viewportHeight, const DisplayList& displayList);
+
+    virtual ~OpReorderer() {}
 
     /**
      * replayBakedOps() is templated based on what class will receive ops being replayed.
@@ -128,8 +138,21 @@ public:
         static BakedOpReceiver receivers[] = {
             MAP_OPS(BAKED_OP_RECEIVER)
         };
-        StaticReceiver::startFrame(arg);
-        replayBakedOpsImpl((void*)&arg, receivers);
+
+        // Relay through layers in reverse order, since layers
+        // later in the list will be drawn by earlier ones
+        for (int i = mLayerReorderers.size() - 1; i >= 1; i--) {
+            LayerReorderer& layer = mLayerReorderers[i];
+            if (!layer.empty()) {
+                layer.layer = StaticReceiver::startLayer(arg, layer.width, layer.height);
+                layer.replayBakedOpsImpl((void*)&arg, receivers);
+                StaticReceiver::endLayer(arg);
+            }
+        }
+
+        const LayerReorderer& fbo0 = mLayerReorderers[0];
+        StaticReceiver::startFrame(arg, fbo0.width, fbo0.height);
+        fbo0.replayBakedOpsImpl((void*)&arg, receivers);
         StaticReceiver::endFrame(arg);
     }
 
diff --git a/libs/hwui/OpenGLRenderer.cpp b/libs/hwui/OpenGLRenderer.cpp
index d4f65b635d4c..8c3603b153b9 100644
--- a/libs/hwui/OpenGLRenderer.cpp
+++ b/libs/hwui/OpenGLRenderer.cpp
@@ -673,7 +673,7 @@ bool OpenGLRenderer::createLayer(float left, float top, float right, float botto
 
 bool OpenGLRenderer::createFboLayer(Layer* layer, Rect& bounds, Rect& clip) {
     layer->clipRect.set(clip);
-    layer->setFbo(mCaches.fboCache.get());
+    layer->setFbo(mRenderState.genFramebuffer());
 
     writableSnapshot()->region = &writableSnapshot()->layer->region;
     writableSnapshot()->flags |= Snapshot::kFlagFboTarget | Snapshot::kFlagIsFboLayer;
diff --git a/libs/hwui/RecordedOp.h b/libs/hwui/RecordedOp.h
index dd016372584a..6c31b429e039 100644
--- a/libs/hwui/RecordedOp.h
+++ b/libs/hwui/RecordedOp.h
@@ -29,6 +29,7 @@ class SkPaint;
 namespace android {
 namespace uirenderer {
 
+class Layer;
 class RenderNode;
 struct Vertex;
 
@@ -136,8 +137,12 @@ struct EndLayerOp : RecordedOp {
 };
 
 struct LayerOp : RecordedOp {
-    LayerOp(BASE_PARAMS)
-            : SUPER(LayerOp) {}
+    LayerOp(BASE_PARAMS, Layer** layerHandle)
+            : SUPER(LayerOp)
+            , layerHandle(layerHandle) {}
+    // Records a handle to the Layer object, since the Layer itself won't be
+    // constructed until after this operation is constructed.
+    Layer** layerHandle;
 };
 
 }; // namespace uirenderer
diff --git a/libs/hwui/microbench/OpReordererBench.cpp b/libs/hwui/microbench/OpReordererBench.cpp
index cf96d44c286f..382c0bd70b52 100644
--- a/libs/hwui/microbench/OpReordererBench.cpp
+++ b/libs/hwui/microbench/OpReordererBench.cpp
@@ -48,8 +48,7 @@ BENCHMARK_NO_ARG(BM_OpReorderer_defer);
 void BM_OpReorderer_defer::Run(int iters) {
     StartBenchmarkTiming();
     for (int i = 0; i < iters; i++) {
-        OpReorderer reorderer;
-        reorderer.defer(200, 200, *sReorderingDisplayList);
+        OpReorderer reorderer(200, 200, *sReorderingDisplayList);
         MicroBench::DoNotOptimize(&reorderer);
     }
     StopBenchmarkTiming();
@@ -60,11 +59,10 @@ void BM_OpReorderer_deferAndRender::Run(int iters) {
     TestUtils::runOnRenderThread([this, iters](RenderState& renderState, Caches& caches) {
         StartBenchmarkTiming();
         for (int i = 0; i < iters; i++) {
-            OpReorderer reorderer;
-            reorderer.defer(200, 200, *sReorderingDisplayList);
+            OpReorderer reorderer(200, 200, *sReorderingDisplayList);
             MicroBench::DoNotOptimize(&reorderer);
 
-            BakedOpRenderer::Info info(caches, renderState, 200, 200, true);
+            BakedOpRenderer::Info info(caches, renderState, true);
             reorderer.replayBakedOps<BakedOpRenderer>(info);
         }
         StopBenchmarkTiming();
diff --git a/libs/hwui/renderstate/RenderState.cpp b/libs/hwui/renderstate/RenderState.cpp
index dfa70ace2f44..9637117bc11a 100644
--- a/libs/hwui/renderstate/RenderState.cpp
+++ b/libs/hwui/renderstate/RenderState.cpp
@@ -125,6 +125,21 @@ void RenderState::bindFramebuffer(GLuint fbo) {
     }
 }
 
+GLuint RenderState::genFramebuffer() {
+    GLuint ret;
+    glGenFramebuffers(1, &ret);
+    return ret;
+}
+
+void RenderState::deleteFramebuffer(GLuint fbo) {
+    if (mFramebuffer == fbo) {
+        // GL defines that deleting the currently bound FBO rebinds FBO 0.
+        // Reflect this in our cached value.
+        mFramebuffer = 0;
+    }
+    glDeleteFramebuffers(1, &fbo);
+}
+
 void RenderState::invokeFunctor(Functor* functor, DrawGlInfo::Mode mode, DrawGlInfo* info) {
     if (mode == DrawGlInfo::kModeProcessNoContext) {
         // If there's no context we don't need to interrupt as there's
diff --git a/libs/hwui/renderstate/RenderState.h b/libs/hwui/renderstate/RenderState.h
index 9ae084506f1d..87a79966df5f 100644
--- a/libs/hwui/renderstate/RenderState.h
+++ b/libs/hwui/renderstate/RenderState.h
@@ -57,7 +57,10 @@ public:
     void getViewport(GLsizei* outWidth, GLsizei* outHeight);
 
     void bindFramebuffer(GLuint fbo);
-    GLint getFramebuffer() { return mFramebuffer; }
+    GLuint getFramebuffer() { return mFramebuffer; }
+    GLuint genFramebuffer();
+    void deleteFramebuffer(GLuint fbo);
+
 
     void invokeFunctor(Functor* functor, DrawGlInfo::Mode mode, DrawGlInfo* info);
 
diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index f5714265032a..7c0f0b67b8a3 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -318,10 +318,8 @@ void CanvasContext::draw() {
     mEglManager.damageFrame(frame, dirty);
 
 #if HWUI_NEW_OPS
-    OpReorderer reorderer;
-    reorderer.defer(dirty, frame.width(), frame.height(), mRenderNodes);
-    BakedOpRenderer::Info info(Caches::getInstance(), mRenderThread.renderState(),
-            frame.width(), frame.height(), mOpaque);
+    OpReorderer reorderer(dirty, frame.width(), frame.height(), mRenderNodes);
+    BakedOpRenderer::Info info(Caches::getInstance(), mRenderThread.renderState(), mOpaque);
     // TODO: profiler().draw(mCanvas);
     reorderer.replayBakedOps<BakedOpRenderer>(info);
 
diff --git a/libs/hwui/tests/TreeContentAnimation.cpp b/libs/hwui/tests/TreeContentAnimation.cpp
index 891af9171518..2eefd37561a6 100644
--- a/libs/hwui/tests/TreeContentAnimation.cpp
+++ b/libs/hwui/tests/TreeContentAnimation.cpp
@@ -54,15 +54,10 @@ public:
     }
 };
 
-static TestCanvas* startRecording(RenderNode* node) {
-    TestCanvas* renderer = new TestCanvas(
-            node->stagingProperties().getWidth(), node->stagingProperties().getHeight());
-    return renderer;
-}
-
-static void endRecording(TestCanvas* renderer, RenderNode* node) {
-    node->setStagingDisplayList(renderer->finishRecording());
-    delete renderer;
+static void recordNode(RenderNode& node, std::function<void(TestCanvas&)> contentCallback) {
+    TestCanvas canvas(node.stagingProperties().getWidth(), node.stagingProperties().getHeight());
+    contentCallback(canvas);
+    node.setStagingDisplayList(canvas.finishRecording());
 }
 
 class TreeContentAnimation {
@@ -75,7 +70,7 @@ public:
             frameCount = fc;
         }
     }
-    virtual void createContent(int width, int height, TestCanvas* renderer) = 0;
+    virtual void createContent(int width, int height, TestCanvas* canvas) = 0;
     virtual void doFrame(int frameNr) = 0;
 
     template <class T>
@@ -108,11 +103,9 @@ public:
         proxy->setup(width, height, dp(800.0f), 255 * 0.075, 255 * 0.15);
         proxy->setLightCenter((Vector3){lightX, dp(-200.0f), dp(800.0f)});
 
-        android::uirenderer::Rect DUMMY;
-
-        TestCanvas* renderer = startRecording(rootNode);
-        animation.createContent(width, height, renderer);
-        endRecording(renderer, rootNode);
+        recordNode(*rootNode, [&animation, width, height](TestCanvas& canvas) {
+            animation.createContent(width, height, &canvas); //TODO: no&
+        });
 
         // Do a few cold runs then reset the stats so that the caches are all hot
         for (int i = 0; i < 3; i++) {
@@ -140,19 +133,19 @@ public:
 class ShadowGridAnimation : public TreeContentAnimation {
 public:
     std::vector< sp<RenderNode> > cards;
-    void createContent(int width, int height, TestCanvas* renderer) override {
-        renderer->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode);
-        renderer->insertReorderBarrier(true);
+    void createContent(int width, int height, TestCanvas* canvas) override {
+        canvas->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode);
+        canvas->insertReorderBarrier(true);
 
         for (int x = dp(16); x < (width - dp(116)); x += dp(116)) {
             for (int y = dp(16); y < (height - dp(116)); y += dp(116)) {
                 sp<RenderNode> card = createCard(x, y, dp(100), dp(100));
-                renderer->drawRenderNode(card.get());
+                canvas->drawRenderNode(card.get());
                 cards.push_back(card);
             }
         }
 
-        renderer->insertReorderBarrier(false);
+        canvas->insertReorderBarrier(false);
     }
     void doFrame(int frameNr) override {
         int curFrame = frameNr % 150;
@@ -171,9 +164,9 @@ private:
         node->mutateStagingProperties().mutableOutline().setShouldClip(true);
         node->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y | RenderNode::Z);
 
-        TestCanvas* renderer = startRecording(node.get());
-        renderer->drawColor(0xFFEEEEEE, SkXfermode::kSrcOver_Mode);
-        endRecording(renderer, node.get());
+        recordNode(*node, [](TestCanvas& canvas) {
+            canvas.drawColor(0xFFEEEEEE, SkXfermode::kSrcOver_Mode);
+        });
         return node;
     }
 };
@@ -187,19 +180,19 @@ static Benchmark _ShadowGrid(BenchmarkInfo{
 class ShadowGrid2Animation : public TreeContentAnimation {
 public:
     std::vector< sp<RenderNode> > cards;
-    void createContent(int width, int height, TestCanvas* renderer) override {
-        renderer->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode);
-        renderer->insertReorderBarrier(true);
+    void createContent(int width, int height, TestCanvas* canvas) override {
+        canvas->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode);
+        canvas->insertReorderBarrier(true);
 
         for (int x = dp(8); x < (width - dp(58)); x += dp(58)) {
             for (int y = dp(8); y < (height - dp(58)); y += dp(58)) {
                 sp<RenderNode> card = createCard(x, y, dp(50), dp(50));
-                renderer->drawRenderNode(card.get());
+                canvas->drawRenderNode(card.get());
                 cards.push_back(card);
             }
         }
 
-        renderer->insertReorderBarrier(false);
+        canvas->insertReorderBarrier(false);
     }
     void doFrame(int frameNr) override {
         int curFrame = frameNr % 150;
@@ -218,9 +211,9 @@ private:
         node->mutateStagingProperties().mutableOutline().setShouldClip(true);
         node->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y | RenderNode::Z);
 
-        TestCanvas* renderer = startRecording(node.get());
-        renderer->drawColor(0xFFEEEEEE, SkXfermode::kSrcOver_Mode);
-        endRecording(renderer, node.get());
+        recordNode(*node, [](TestCanvas& canvas) {
+            canvas.drawColor(0xFFEEEEEE, SkXfermode::kSrcOver_Mode);
+        });
         return node;
     }
 };
@@ -233,15 +226,30 @@ static Benchmark _ShadowGrid2(BenchmarkInfo{
 
 class RectGridAnimation : public TreeContentAnimation {
 public:
-    sp<RenderNode> card;
-    void createContent(int width, int height, TestCanvas* renderer) override {
-        renderer->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode);
-        renderer->insertReorderBarrier(true);
+    sp<RenderNode> card = new RenderNode();
+    void createContent(int width, int height, TestCanvas* canvas) override {
+        canvas->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode);
+        canvas->insertReorderBarrier(true);
+
+        card->mutateStagingProperties().setLeftTopRightBottom(50, 50, 250, 250);
+        card->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
+        recordNode(*card, [](TestCanvas& canvas) {
+            canvas.drawColor(0xFFFF00FF, SkXfermode::kSrcOver_Mode);
+
+            SkRegion region;
+            for (int xOffset = 0; xOffset < 200; xOffset+=2) {
+                for (int yOffset = 0; yOffset < 200; yOffset+=2) {
+                    region.op(xOffset, yOffset, xOffset + 1, yOffset + 1, SkRegion::kUnion_Op);
+                }
+            }
 
-        card = createCard(40, 40, 200, 200);
-        renderer->drawRenderNode(card.get());
+            SkPaint paint;
+            paint.setColor(0xff00ffff);
+            canvas.drawRegion(region, paint);
+        });
+        canvas->drawRenderNode(card.get());
 
-        renderer->insertReorderBarrier(false);
+        canvas->insertReorderBarrier(false);
     }
     void doFrame(int frameNr) override {
         int curFrame = frameNr % 150;
@@ -249,29 +257,6 @@ public:
         card->mutateStagingProperties().setTranslationY(curFrame);
         card->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
     }
-private:
-    sp<RenderNode> createCard(int x, int y, int width, int height) {
-        sp<RenderNode> node = new RenderNode();
-        node->mutateStagingProperties().setLeftTopRightBottom(x, y, x + width, y + height);
-        node->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
-
-        TestCanvas* renderer = startRecording(node.get());
-        renderer->drawColor(0xFFFF00FF, SkXfermode::kSrcOver_Mode);
-
-        SkRegion region;
-        for (int xOffset = 0; xOffset < width; xOffset+=2) {
-            for (int yOffset = 0; yOffset < height; yOffset+=2) {
-                region.op(xOffset, yOffset, xOffset + 1, yOffset + 1, SkRegion::kUnion_Op);
-            }
-        }
-
-        SkPaint paint;
-        paint.setColor(0xff00ffff);
-        renderer->drawRegion(region, paint);
-
-        endRecording(renderer, node.get());
-        return node;
-    }
 };
 static Benchmark _RectGrid(BenchmarkInfo{
     "rectgrid",
@@ -282,15 +267,22 @@ static Benchmark _RectGrid(BenchmarkInfo{
 
 class OvalAnimation : public TreeContentAnimation {
 public:
-    sp<RenderNode> card;
-    void createContent(int width, int height, TestCanvas* renderer) override {
-        renderer->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode);
-        renderer->insertReorderBarrier(true);
-
-        card = createCard(40, 40, 400, 400);
-        renderer->drawRenderNode(card.get());
+    sp<RenderNode> card = new RenderNode();
+    void createContent(int width, int height, TestCanvas* canvas) override {
+        canvas->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode);
+        canvas->insertReorderBarrier(true);
 
-        renderer->insertReorderBarrier(false);
+        card->mutateStagingProperties().setLeftTopRightBottom(0, 0, 200, 200);
+        card->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
+        recordNode(*card, [](TestCanvas& canvas) {
+            SkPaint paint;
+            paint.setAntiAlias(true);
+            paint.setColor(0xFF000000);
+            canvas.drawOval(0, 0, 200, 200, paint);
+        });
+        canvas->drawRenderNode(card.get());
+
+        canvas->insertReorderBarrier(false);
     }
 
     void doFrame(int frameNr) override {
@@ -299,22 +291,6 @@ public:
         card->mutateStagingProperties().setTranslationY(curFrame);
         card->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
     }
-private:
-    sp<RenderNode> createCard(int x, int y, int width, int height) {
-        sp<RenderNode> node = new RenderNode();
-        node->mutateStagingProperties().setLeftTopRightBottom(x, y, x + width, y + height);
-        node->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
-
-        TestCanvas* renderer = startRecording(node.get());
-
-        SkPaint paint;
-        paint.setAntiAlias(true);
-        paint.setColor(0xFF000000);
-        renderer->drawOval(0, 0, width, height, paint);
-
-        endRecording(renderer, node.get());
-        return node;
-    }
 };
 static Benchmark _Oval(BenchmarkInfo{
     "oval",
@@ -325,7 +301,7 @@ static Benchmark _Oval(BenchmarkInfo{
 class PartialDamageTest : public TreeContentAnimation {
 public:
     std::vector< sp<RenderNode> > cards;
-    void createContent(int width, int height, TestCanvas* renderer) override {
+    void createContent(int width, int height, TestCanvas* canvas) override {
         static SkColor COLORS[] = {
                 0xFFF44336,
                 0xFF9C27B0,
@@ -333,13 +309,13 @@ public:
                 0xFF4CAF50,
         };
 
-        renderer->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode);
+        canvas->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode);
 
         for (int x = dp(16); x < (width - dp(116)); x += dp(116)) {
             for (int y = dp(16); y < (height - dp(116)); y += dp(116)) {
                 sp<RenderNode> card = createCard(x, y, dp(100), dp(100),
                         COLORS[static_cast<int>((y / dp(116))) % 4]);
-                renderer->drawRenderNode(card.get());
+                canvas->drawRenderNode(card.get());
                 cards.push_back(card);
             }
         }
@@ -350,10 +326,10 @@ public:
         cards[0]->mutateStagingProperties().setTranslationY(curFrame);
         cards[0]->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
 
-        TestCanvas* renderer = startRecording(cards[0].get());
-        renderer->drawColor(interpolateColor(curFrame / 150.0f, 0xFFF44336, 0xFFF8BBD0),
-                SkXfermode::kSrcOver_Mode);
-        endRecording(renderer, cards[0].get());
+        recordNode(*cards[0], [curFrame](TestCanvas& canvas) {
+            canvas.drawColor(interpolateColor(curFrame / 150.0f, 0xFFF44336, 0xFFF8BBD0),
+                    SkXfermode::kSrcOver_Mode);
+        });
     }
 
     static SkColor interpolateColor(float fraction, SkColor start, SkColor end) {
@@ -378,9 +354,9 @@ private:
         node->mutateStagingProperties().setLeftTopRightBottom(x, y, x + width, y + height);
         node->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
 
-        TestCanvas* renderer = startRecording(node.get());
-        renderer->drawColor(color, SkXfermode::kSrcOver_Mode);
-        endRecording(renderer, node.get());
+        recordNode(*node, [color](TestCanvas& canvas) {
+            canvas.drawColor(color, SkXfermode::kSrcOver_Mode);
+        });
         return node;
     }
 };
@@ -393,16 +369,24 @@ static Benchmark _PartialDamage(BenchmarkInfo{
 });
 
 
-class SimpleRectGridAnimation : public TreeContentAnimation {
+class SaveLayerAnimation : public TreeContentAnimation {
 public:
-    sp<RenderNode> card;
-    void createContent(int width, int height, TestCanvas* renderer) override {
-        SkPaint paint;
-        paint.setColor(0xFF00FFFF);
-        renderer->drawRect(0, 0, width, height, paint);
-
-        card = createCard(40, 40, 200, 200);
-        renderer->drawRenderNode(card.get());
+    sp<RenderNode> card = new RenderNode();
+    void createContent(int width, int height, TestCanvas* canvas) override {
+        canvas->drawColor(0xFFFFFFFF, SkXfermode::kSrcOver_Mode); // background
+
+        card->mutateStagingProperties().setLeftTopRightBottom(0, 0, 200, 200);
+        card->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
+        recordNode(*card, [](TestCanvas& canvas) {
+            canvas.saveLayerAlpha(0, 0, 200, 200, 128, SkCanvas::kClipToLayer_SaveFlag);
+            canvas.drawColor(0xFF00FF00, SkXfermode::kSrcOver_Mode); // outer, unclipped
+            canvas.saveLayerAlpha(50, 50, 150, 150, 128, SkCanvas::kClipToLayer_SaveFlag);
+            canvas.drawColor(0xFF0000FF, SkXfermode::kSrcOver_Mode); // inner, clipped
+            canvas.restore();
+            canvas.restore();
+        });
+
+        canvas->drawRenderNode(card.get());
     }
     void doFrame(int frameNr) override {
         int curFrame = frameNr % 150;
@@ -410,24 +394,10 @@ public:
         card->mutateStagingProperties().setTranslationY(curFrame);
         card->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
     }
-private:
-    sp<RenderNode> createCard(int x, int y, int width, int height) {
-        sp<RenderNode> node = new RenderNode();
-        node->mutateStagingProperties().setLeftTopRightBottom(x, y, x + width, y + height);
-        node->setPropertyFieldsDirty(RenderNode::X | RenderNode::Y);
-
-        TestCanvas* renderer = startRecording(node.get());
-        SkPaint paint;
-        paint.setColor(0xFFFF00FF);
-        renderer->drawRect(0, 0, width, height, paint);
-
-        endRecording(renderer, node.get());
-        return node;
-    }
 };
-static Benchmark _SimpleRectGrid(BenchmarkInfo{
-    "simplerectgrid",
-    "A simple collection of rects. "
-    "Low CPU/GPU load.",
-    TreeContentAnimation::run<SimpleRectGridAnimation>
+static Benchmark _SaveLayer(BenchmarkInfo{
+    "savelayer",
+    "A nested pair of clipped saveLayer operations. "
+    "Tests the clipped saveLayer codepath. Draws content into offscreen buffers and back again.",
+    TreeContentAnimation::run<SaveLayerAnimation>
 });
diff --git a/libs/hwui/unit_tests/BakedOpStateTests.cpp b/libs/hwui/unit_tests/BakedOpStateTests.cpp
index 82aebeabe038..bc1b69fa3729 100644
--- a/libs/hwui/unit_tests/BakedOpStateTests.cpp
+++ b/libs/hwui/unit_tests/BakedOpStateTests.cpp
@@ -85,13 +85,5 @@ TEST(BakedOpState, constructAndReject) {
     }
 }
 
-#define UNSUPPORTED_OP(Info, Type) \
-        static void on##Type(Info*, const Type&, const BakedOpState&) { FAIL(); }
-
-class Info {
-public:
-    int index = 0;
-};
-
 }
 }
diff --git a/libs/hwui/unit_tests/OpReordererTests.cpp b/libs/hwui/unit_tests/OpReordererTests.cpp
index d02f89dd7088..af9c3f20004e 100644
--- a/libs/hwui/unit_tests/OpReordererTests.cpp
+++ b/libs/hwui/unit_tests/OpReordererTests.cpp
@@ -35,19 +35,22 @@ namespace uirenderer {
  * for every test.
  *
  * onXXXOp methods fail by default - tests should override ops they expect
+ * startLayer fails by default - tests should override if expected
  * startFrame/endFrame do nothing by default - tests should override to intercept
  */
 template<class CustomClient, class Arg>
 class TestReceiver {
 public:
 #define CLIENT_METHOD(Type) \
-    virtual void on##Type(Arg&, const Type&, const BakedOpState&) { FAIL(); }
+    virtual void on##Type(Arg&, const Type&, const BakedOpState&) { ADD_FAILURE(); }
     class Client {
     public:
         virtual ~Client() {};
         MAP_OPS(CLIENT_METHOD)
 
-        virtual void startFrame(Arg& info) {}
+        virtual Layer* startLayer(Arg& info, uint32_t width, uint32_t height) { ADD_FAILURE(); return nullptr; }
+        virtual void endLayer(Arg& info) { ADD_FAILURE(); }
+        virtual void startFrame(Arg& info, uint32_t width, uint32_t height) {}
         virtual void endFrame(Arg& info) {}
     };
 
@@ -57,11 +60,18 @@ public:
     }
     MAP_OPS(DISPATCHER_METHOD)
 
-    static void startFrame(Arg& info) {
+    static Layer* startLayer(Arg& info, uint32_t width, uint32_t height) {
         CustomClient client;
-        client.startFrame(info);
+        return client.startLayer(info, width, height);
+    }
+    static void endLayer(Arg& info) {
+        CustomClient client;
+        client.endLayer(info);
+    }
+    static void startFrame(Arg& info, uint32_t width, uint32_t height) {
+        CustomClient client;
+        client.startFrame(info, width, height);
     }
-
     static void endFrame(Arg& info) {
         CustomClient client;
         client.endFrame(info);
@@ -78,8 +88,10 @@ class FailReceiver : public TestReceiver<FailReceiver, Info>::Client {};
 
 class SimpleReceiver : public TestReceiver<SimpleReceiver, Info>::Client {
 public:
-    void startFrame(Info& info) override {
+    void startFrame(Info& info, uint32_t width, uint32_t height) override {
         EXPECT_EQ(0, info.index++);
+        EXPECT_EQ(100u, width);
+        EXPECT_EQ(200u, height);
     }
     void onRectOp(Info& info, const RectOp& op, const BakedOpState& state) override {
         EXPECT_EQ(1, info.index++);
@@ -97,8 +109,7 @@ TEST(OpReorderer, simple) {
         canvas.drawRect(0, 0, 100, 200, SkPaint());
         canvas.drawBitmap(bitmap, 10, 10, nullptr);
     });
-    OpReorderer reorderer;
-    reorderer.defer(200, 200, *dl);
+    OpReorderer reorderer(100, 200, *dl);
 
     Info info;
     reorderer.replayBakedOps<TestReceiver<SimpleReceiver, Info>>(info);
@@ -113,8 +124,7 @@ TEST(OpReorderer, simpleRejection) {
         canvas.drawRect(0, 0, 400, 400, SkPaint());
         canvas.restore();
     });
-    OpReorderer reorderer;
-    reorderer.defer(200, 200, *dl);
+    OpReorderer reorderer(200, 200, *dl);
 
     Info info;
     reorderer.replayBakedOps<TestReceiver<FailReceiver, Info>>(info);
@@ -146,8 +156,7 @@ TEST(OpReorderer, simpleBatching) {
         canvas.restore();
     });
 
-    OpReorderer reorderer;
-    reorderer.defer(200, 200, *dl);
+    OpReorderer reorderer(200, 200, *dl);
 
     Info info;
     reorderer.replayBakedOps<TestReceiver<SimpleBatchingReceiver, Info>>(info);
@@ -167,7 +176,7 @@ public:
             EXPECT_EQ(SK_ColorWHITE, op.paint->getColor());
             break;
         default:
-            FAIL();
+            ADD_FAILURE();
         }
     }
 };
@@ -196,8 +205,7 @@ TEST(OpReorderer, renderNode) {
     std::vector< sp<RenderNode> > nodes;
     nodes.push_back(parent.get());
 
-    OpReorderer reorderer;
-    reorderer.defer(SkRect::MakeWH(200, 200), 200, 200, nodes);
+    OpReorderer reorderer(SkRect::MakeWH(200, 200), 200, 200, nodes);
 
     Info info;
     reorderer.replayBakedOps<TestReceiver<RenderNodeReceiver, Info>>(info);
@@ -221,8 +229,7 @@ TEST(OpReorderer, clipped) {
     std::vector< sp<RenderNode> > nodes;
     nodes.push_back(node.get());
 
-    OpReorderer reorderer;
-    reorderer.defer(SkRect::MakeLTRB(10, 20, 30, 40), // clip to small area, should see in receiver
+    OpReorderer reorderer(SkRect::MakeLTRB(10, 20, 30, 40), // clip to small area, should see in receiver
             200, 200, nodes);
 
     Info info;
@@ -232,8 +239,17 @@ TEST(OpReorderer, clipped) {
 
 class SaveLayerSimpleReceiver : public TestReceiver<SaveLayerSimpleReceiver, Info>::Client {
 public:
-    void onRectOp(Info& info, const RectOp& op, const BakedOpState& state) override {
+    Layer* startLayer(Info& info, uint32_t width, uint32_t height) override {
         EXPECT_EQ(0, info.index++);
+        EXPECT_EQ(180u, width);
+        EXPECT_EQ(180u, height);
+        return nullptr;
+    }
+    void endLayer(Info& info) override {
+        EXPECT_EQ(2, info.index++);
+    }
+    void onRectOp(Info& info, const RectOp& op, const BakedOpState& state) override {
+        EXPECT_EQ(1, info.index++);
         EXPECT_EQ(Rect(10, 10, 190, 190), op.unmappedBounds);
         EXPECT_EQ(Rect(0, 0, 180, 180), state.computedState.clippedBounds);
         EXPECT_EQ(Rect(0, 0, 180, 180), state.computedState.clipRect);
@@ -243,7 +259,7 @@ public:
         EXPECT_MATRIX_APPROX_EQ(expectedTransform, state.computedState.transform);
     }
     void onLayerOp(Info& info, const LayerOp& op, const BakedOpState& state) override {
-        EXPECT_EQ(1, info.index++);
+        EXPECT_EQ(3, info.index++);
         EXPECT_EQ(Rect(10, 10, 190, 190), state.computedState.clippedBounds);
         EXPECT_EQ(Rect(0, 0, 200, 200), state.computedState.clipRect);
         EXPECT_TRUE(state.computedState.transform.isIdentity());
@@ -256,33 +272,61 @@ TEST(OpReorderer, saveLayerSimple) {
         canvas.restore();
     });
 
-    OpReorderer reorderer;
-    reorderer.defer(200, 200, *dl);
+    OpReorderer reorderer(200, 200, *dl);
 
     Info info;
     reorderer.replayBakedOps<TestReceiver<SaveLayerSimpleReceiver, Info>>(info);
-    EXPECT_EQ(2, info.index);
+    EXPECT_EQ(4, info.index);
 }
 
 
-// saveLayer1 {rect1, saveLayer2 { rect2 } } will play back as rect2, rect1, layerOp2, layerOp1
+/* saveLayer1 {rect1, saveLayer2 { rect2 } } will play back as:
+ * - startLayer2, rect2 endLayer2
+ * - startLayer1, rect1, drawLayer2, endLayer1
+ * - startFrame, layerOp1, endFrame
+ */
 class SaveLayerNestedReceiver : public TestReceiver<SaveLayerNestedReceiver, Info>::Client {
 public:
-    void onRectOp(Info& info, const RectOp& op, const BakedOpState& state) override {
+    Layer* startLayer(Info& info, uint32_t width, uint32_t height) override {
         const int index = info.index++;
         if (index == 0) {
+            EXPECT_EQ(400u, width);
+            EXPECT_EQ(400u, height);
+            return (Layer*) 0x400;
+        } else if (index == 3) {
+            EXPECT_EQ(800u, width);
+            EXPECT_EQ(800u, height);
+            return (Layer*) 0x800;
+        } else { ADD_FAILURE(); }
+        return (Layer*) nullptr;
+    }
+    void endLayer(Info& info) override {
+        int index = info.index++;
+        EXPECT_TRUE(index == 2 || index == 6);
+    }
+    void startFrame(Info& info, uint32_t width, uint32_t height) override {
+        EXPECT_EQ(7, info.index++);
+    }
+    void endFrame(Info& info) override {
+        EXPECT_EQ(9, info.index++);
+    }
+    void onRectOp(Info& info, const RectOp& op, const BakedOpState& state) override {
+        const int index = info.index++;
+        if (index == 1) {
             EXPECT_EQ(Rect(0, 0, 400, 400), op.unmappedBounds); // inner rect
-        } else if (index == 1) {
+        } else if (index == 4) {
             EXPECT_EQ(Rect(0, 0, 800, 800), op.unmappedBounds); // outer rect
-        } else { FAIL(); }
+        } else { ADD_FAILURE(); }
     }
     void onLayerOp(Info& info, const LayerOp& op, const BakedOpState& state) override {
         const int index = info.index++;
-        if (index == 2) {
+        if (index == 5) {
+            EXPECT_EQ((Layer*)0x400, *op.layerHandle);
             EXPECT_EQ(Rect(0, 0, 400, 400), op.unmappedBounds); // inner layer
-        } else if (index == 3) {
+        } else if (index == 8) {
+            EXPECT_EQ((Layer*)0x800, *op.layerHandle);
             EXPECT_EQ(Rect(0, 0, 800, 800), op.unmappedBounds); // outer layer
-        } else { FAIL(); }
+        } else { ADD_FAILURE(); }
     }
 };
 TEST(OpReorderer, saveLayerNested) {
@@ -299,12 +343,11 @@ TEST(OpReorderer, saveLayerNested) {
         canvas.restore();
     });
 
-    OpReorderer reorderer;
-    reorderer.defer(800, 800, *dl);
+    OpReorderer reorderer(800, 800, *dl);
 
     Info info;
     reorderer.replayBakedOps<TestReceiver<SaveLayerNestedReceiver, Info>>(info);
-    EXPECT_EQ(4, info.index);
+    EXPECT_EQ(10, info.index);
 }
 
 TEST(OpReorderer, saveLayerContentRejection) {
@@ -319,8 +362,7 @@ TEST(OpReorderer, saveLayerContentRejection) {
         canvas.restore();
         canvas.restore();
     });
-    OpReorderer reorderer;
-    reorderer.defer(200, 200, *dl);
+    OpReorderer reorderer(200, 200, *dl);
     Info info;
 
     // should see no ops, even within the layer, since the layer should be rejected
diff --git a/libs/hwui/unit_tests/RecordingCanvasTests.cpp b/libs/hwui/unit_tests/RecordingCanvasTests.cpp
index c0231235027a..e8cdf461c783 100644
--- a/libs/hwui/unit_tests/RecordingCanvasTests.cpp
+++ b/libs/hwui/unit_tests/RecordingCanvasTests.cpp
@@ -38,7 +38,7 @@ TEST(RecordingCanvas, emptyPlayback) {
         canvas.save(SkCanvas::kMatrix_SaveFlag | SkCanvas::kClip_SaveFlag);
         canvas.restore();
     });
-    playbackOps(*dl, [](const RecordedOp& op) { FAIL(); });
+    playbackOps(*dl, [](const RecordedOp& op) { ADD_FAILURE(); });
 }
 
 TEST(RecordingCanvas, testSimpleRectRecord) {
@@ -135,7 +135,7 @@ TEST(RecordingCanvas, saveLayerSimple) {
             // TODO: add asserts
             break;
         default:
-            FAIL();
+            ADD_FAILURE();
         }
     });
     EXPECT_EQ(3, count);