Fold alpha to the draw in savelayer-draw-restore patterns with non-opaque draw

gm/recordopts.cpp

Index: gm/recordopts.cpp
diff --git a/gm/recordopts.cpp b/gm/recordopts.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..a228b617fc0369f2636e8a3647a65935af1d1b10
--- /dev/null
+++ b/gm/recordopts.cpp
@@ -0,0 +1,281 @@
+/*
+ * Copyright 2014 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#include "gm.h"
+#include "SkCanvas.h"
+#include "SkPath.h"
+#include "SkPictureRecorder.h"
+
+/** A call pattern that should be optimized to a single draw call. */
+static void draw_save_layer_draw_restore(SkCanvas* canvas, int layerAlpha, int drawAlpha,
+                                         bool isForOptimizedRecord) {
+    SkRect targetRect(SkRect::MakeWH(SK_Scalar1, SK_Scalar1));
+    SkPaint layerPaint;
+    layerPaint.setColor(SkColorSetARGB(layerAlpha, 0x0, 0x00, 0x0));
+    if (!isForOptimizedRecord) {
+        // When drawing without the picture optimization, eg. for example directly to a canvas, we
+        // can not save layer for every call. This takes too much time for a gm test. The drawing

[mtklein, 2015/01/14 13:28:30]

... can not save the full layer ...  ?

+        // needs to constrain the saveLayer.
+        canvas->saveLayer(&targetRect, &layerPaint);
+    } else {
+        // The optimization can not handle rect constraints yet.
+        canvas->saveLayer(NULL, &layerPaint);
+    }
+    SkPaint drawPaint;
+    // Using just one color should not lose any generality. This way it is just easier to reason
+    // about the magnitude of the error. 255 should have the biggest error.
+    drawPaint.setColor(SkColorSetARGB(drawAlpha, 0, 255, 0));
+
+    canvas->drawRect(targetRect, drawPaint);
+    canvas->restore();
+}
+
+/** Draws all (layer alpha, draw alpha) combinations as pixel-wide rects.  This should identify the
+ * combinations that show difference between "savelayer-draw-restore" sequence and optimized "draw"
+ * sequence. */
+static void draw_all_save_layer_draw_restore_combinations(SkCanvas* canvas,
+                                                          bool isForOptimizedRecord) {
+    canvas->save();
+    for (int layerAlpha = 0; layerAlpha < 256; ++layerAlpha) {
+        canvas->save();
+        for (int drawAlpha = 0; drawAlpha < 256; ++drawAlpha) {
+            draw_save_layer_draw_restore(canvas, layerAlpha, drawAlpha, isForOptimizedRecord);
+            canvas->translate(SK_Scalar1, 0);
+        }
+        canvas->restore();
+        canvas->translate(0, SK_Scalar1);
+    }
+    canvas->restore();
+}
+
+
+/** Xfermode that highlights small differences between the pixel values that are being drawn.
+ * HighlightSmallDifferencesXfermode shows difference between existing image and new image pixel
+ * values. The image is the component-wise manhattan distance between the what is there in the
+ * backbuffer and the new shape. The distance is visualized as step colors: distance of 0 is black,
+ * 1 is dark gray (100), 2 is gray (200) and distance > 2 is white (255).  In order to debug this:
+ * This should produce black image (with a deterministic backend):
+ *
+ * SkCanvas canvas(...);
+ * SkPicture picture(...);
+ * picture.draw(canvas);
+ * SkPaint paint;
+ * paint.setXfermode(HighlightSmallDifferencesXfermode::Create());
+ * canvas.saveLayer(NULL, &paint);
+ * picture.draw(canvas);
+ * canvas.restore();
+ */
+class HighlightSmallDifferencesXfermode : public SkXfermode {
+  public:
+    static HighlightSmallDifferencesXfermode* Create() {
+        return SkNEW(HighlightSmallDifferencesXfermode);
+    }
+
+    SK_TO_STRING_OVERRIDE()
+    SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkPixelXorXfermode)
+
+protected:
+    HighlightSmallDifferencesXfermode() { }
+    void flatten(SkWriteBuffer&) const SK_OVERRIDE { }
+
+    SkPMColor xferColor(SkPMColor src, SkPMColor dst) const SK_OVERRIDE {
+        int d = SkAbs32(SkGetPackedR32(dst) - SkGetPackedR32(src))
+                + SkAbs32(SkGetPackedG32(dst) - SkGetPackedG32(src))
+                + SkAbs32(SkGetPackedB32(dst) - SkGetPackedB32(src));
+        d = SkClampMax(d * 100, 255);
+        return SkPackARGB32(0xFF, d, d, d);
+    }
+#if SK_SUPPORT_GPU
+    bool asFragmentProcessor(GrFragmentProcessor** fp, GrTexture* background) const;
+#endif
+    typedef SkXfermode INHERITED;
+};
+
+SkFlattenable* HighlightSmallDifferencesXfermode::CreateProc(SkReadBuffer& buffer) {
+    return Create();
+}
+
+#ifndef SK_IGNORE_TO_STRING
+void HighlightSmallDifferencesXfermode::toString(SkString* str) const {
+    str->append("HighlightSmallDifferencesXfermode: ()");
+}
+#endif
+
+#if SK_SUPPORT_GPU
+
+#include "GrFragmentProcessor.h"
+#include "GrCoordTransform.h"
+#include "GrInvariantOutput.h"
+#include "GrProcessorUnitTest.h"
+#include "gl/GrGLProcessor.h"
+#include "gl/builders/GrGLProgramBuilder.h"
+
+class HighlightSmallDifferencesXferEffect : public GrFragmentProcessor {
+public:
+    static GrFragmentProcessor* Create(GrTexture* background) {
+        return SkNEW_ARGS(HighlightSmallDifferencesXferEffect, (background));
+    }
+
+    void getGLProcessorKey(const GrGLCaps& caps,
+                                   GrProcessorKeyBuilder* b) const SK_OVERRIDE {
+        GLProcessor::GenKey(*this, caps, b);
+    }
+
+    GrGLFragmentProcessor* createGLInstance() const SK_OVERRIDE {
+        return SkNEW_ARGS(GLProcessor, (*this));
+    }
+
+    const char* name() const SK_OVERRIDE { return "HighlightSmallDifferencesXferEffect"; }
+
+    const GrTextureAccess&  backgroundAccess() const { return fBackgroundAccess; }
+
+    class GLProcessor : public GrGLFragmentProcessor {
+    public:
+        GLProcessor(const GrFragmentProcessor&) {}
+
+        void emitCode(GrGLFPBuilder* builder,
+                      const GrFragmentProcessor& fp,
+                      const char* outputColor,
+                      const char* inputColor,
+                      const TransformedCoordsArray& coords,
+                      const TextureSamplerArray& samplers) SK_OVERRIDE {
+            const GrTexture* backgroundTex =
+                    fp.cast<HighlightSmallDifferencesXferEffect>().backgroundAccess().getTexture();
+            GrGLFPFragmentBuilder* fsBuilder = builder->getFragmentShaderBuilder();
+            const char* dstColor;
+            if (backgroundTex) {
+                dstColor = "bgColor";
+                fsBuilder->codeAppendf("\t\tvec4 %s = ", dstColor);
+                fsBuilder->appendTextureLookup(samplers[0], coords[0].c_str(), coords[0].getType());
+                fsBuilder->codeAppendf(";\n");
+            } else {
+                dstColor = fsBuilder->dstColor();
+            }
+            SkASSERT(dstColor);
+
+            // We don't try to optimize for this case at all
+            if (NULL == inputColor) {
+                fsBuilder->codeAppendf("\t\tconst vec4 ones = vec4(1);\n");
+                inputColor = "ones";
+            }
+            fsBuilder->codeAppendf("\t\t%s = vec4(vec3(dot(abs(%s.rgb - %s.rgb), vec3(1)) * 100.0),"
+                                   " 1.0);\n", outputColor, dstColor, inputColor);
+        }
+
+        static inline void GenKey(const GrProcessor& proc, const GrGLCaps&,
+                                  GrProcessorKeyBuilder* b) {
+            // The background may come from the dst or from a texture.
+            uint32_t key = proc.numTextures();
+            SkASSERT(key <= 1);
+            b->add32(key);
+        }
+
+        typedef GrGLFragmentProcessor INHERITED;
+    };
+
+private:
+    HighlightSmallDifferencesXferEffect(GrTexture* background) {
+        this->initClassID<HighlightSmallDifferencesXferEffect>();
+        if (background) {
+            fBackgroundTransform.reset(kLocal_GrCoordSet, background,
+                                       GrTextureParams::kNone_FilterMode);
+            this->addCoordTransform(&fBackgroundTransform);
+            fBackgroundAccess.reset(background);
+            this->addTextureAccess(&fBackgroundAccess);
+        } else {
+            this->setWillReadDstColor();
+        }
+    }
+
+    bool onIsEqual(const GrFragmentProcessor& other) const SK_OVERRIDE {
+        return true;
+    }
+
+    void onComputeInvariantOutput(GrInvariantOutput* inout) const SK_OVERRIDE {
+        inout->setToUnknown(GrInvariantOutput::kWill_ReadInput);
+    }
+
+    GrCoordTransform fBackgroundTransform;
+    GrTextureAccess  fBackgroundAccess;
+
+    typedef GrFragmentProcessor INHERITED;
+};
+
+bool HighlightSmallDifferencesXfermode::asFragmentProcessor(GrFragmentProcessor** fp,
+                                       GrTexture* background) const {
+    *fp = HighlightSmallDifferencesXferEffect::Create(background);
+    SkASSERT(*fp);
+    return true;
+};
+
+#endif
+
+/**
+   Tests record optimizations. Draws shapes using a call patterns that get
+   optimized in SkRecord. Used to ensure that the optimization does not change the
+   pixels (too drasticly).
+*/
+
+class RecordOptsGM : public skiagm::GM {
+public:
+    RecordOptsGM() {
+    }
+
+protected:
+    SkString onShortName() SK_OVERRIDE {
+        return SkString("recordopts");
+    }
+
+    SkISize onISize() SK_OVERRIDE {
+        return SkISize::Make(256, 256);
+    }
+
+    void onOnceBeforeDraw() SK_OVERRIDE {
+        // Create a more optimal picture of unoptimal call sequence.
+        SkPictureRecorder recorder;
+        draw_all_save_layer_draw_restore_combinations(recorder.beginRecording(256, 256),
+                                                      true /*isForOptimizedRecord*/);
+        fSaveLayerDrawRestorePicture.reset(recorder.endRecordingAsPicture());
+
+        // Create a reference bitmap image from the same calls, without the optimization.
+        // The reference must be a bitmap image instead, see below.
+        fSaveLayerDrawRestoreReferenceBitmap.allocN32Pixels(256, 256);
+        fSaveLayerDrawRestoreReferenceBitmap.eraseColor(SK_ColorBLACK);
+        SkCanvas referenceCanvas(fSaveLayerDrawRestoreReferenceBitmap);
+        draw_all_save_layer_draw_restore_combinations(&referenceCanvas,
+                                                      false /*isForOptimizedRecord*/);
+        referenceCanvas.flush();
+    }
+
+    void onDraw(SkCanvas* canvas) SK_OVERRIDE {
+        canvas->clear(SK_ColorTRANSPARENT);
+
+        // Test savelayer-draw-restore picture optimization.
+        // Draw optimized picture.
+        fSaveLayerDrawRestorePicture->playback(canvas);
+        // Visualize the difference between optimized picture and unoptimized commands by drawing
+        // the unoptimized commands on top of the existing picture using the
+        // HighlightSmallDifferences xfermode. If there would not be any differences the resulting
+        // bitmap would be empty.
+        SkPaint diffPaint;
+        diffPaint.setXfermode(HighlightSmallDifferencesXfermode::Create());
+        // Unfortunately we can not draw the unoptimized commands, because GPU backend can not
+        // execute so many save layers in time. Thus our difference image is correct only for the

[mtklein, 2015/01/14 13:28:30]

If we're only going to get a useful diff with the 8888 backend, let's simplify
this.  As cool as the HighlighSmallDifferencesXfermode is, it's a rather large
maintenance burden for such a one-off test, particularly if it doesn't give us
useful results.  Here's what I'd suggest

onDraw(canvas) {
  SkBitmap direct, optimized, diff;
  ... setup bitmaps ...

  SkCanvas dcanvas(direct), ocanvas(optimized);
  draw_all_save_layer_draw_restore_combinations(&dcanvas, false);

  SkPictureRecorder rec;
  draw_all_save_layer_draw_restore_combinations(rec.beginRecording(...), true);
  SkAutoTDelete<SkPicture> pic(rec.endRecording());
  pic->playback(&ocanvas);

  // Because the GPU backend can't handle so many save layers, we just do
everything on the CPU and draw
  // the final diff result as a bitmap.
  diff = apply_highlightsmalldifferences_xfermode_logic(direct, optimized);
  canvas->drawBitmap(diff);
}

[Kimmo Kinnunen, 2015/01/14 14:07:20]

Right. Well, with my gpu and the current code it gives the same result.
The original idea was that the file wouldn't be for just this one test, rather
also the future tests. Thus the diff might be useful in scenarios where gpu
would be slow (-->reason to optimize) but the testcase not so heavy (-->direct
xfermode useful).

But I can also spend time some more time rewriting this again to be simpler, of
course.. 

[mtklein, 2015/01/14 14:53:10]

Hmmm.  I think I misunderstood the downside of the setup.  This seems fine. 
Let's land this, see how close the 8888 backend and all the other GPUs are, and
then decide to either baseline this as-is or rewrite it to force 8888.

+        // 8888 backend. For GPU backends, the difference image is the difference between optimized
+        // GPU picture and unoptimized 8888 commands.
+        canvas->drawBitmap(fSaveLayerDrawRestoreReferenceBitmap, 0, 0, &diffPaint);
+    }
+
+private:
+    SkAutoTUnref<SkPicture> fSaveLayerDrawRestorePicture;
+    SkBitmap fSaveLayerDrawRestoreReferenceBitmap;
+
+    typedef skiagm::GM INHERITED;
+};
+
+DEF_GM( return SkNEW(RecordOptsGM); )
+