Always return ImageShader, even from SkShader::MakeBitmapShader

Lessons learned

1. ImageShader (correctly) always compresses (typically via PNG) during serialization. This has the surprise results of
- if the image was marked opaque, but has some non-opaque pixels (i.e. bug in blitter or caller), then compressing may "fix" those pixels, making the deserialized version draw differently. bug filed.
- 565 compressess/decompresses to 8888 (at least on Mac), which draws differently (esp. under some filters). bug filed.

2. BitmapShader did not enforce a copy for mutable bitmaps, but ImageShader does (since it creates an Image). Thus the former would see subsequent changes to the pixels after shader creation, while the latter does not, hence the change to the BlitRow test to avoid this modify-after-create pattern. I sure hope this prev. behavior was a bug/undefined-behavior, since this CL changes that.

BUG=skia:5595
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2195893002

Review-Url: https://codereview.chromium.org/2195893002

11 files changed, 137 insertions, 301 deletions

diff --git a/include/core/SkShader.h b/include/core/SkShader.h
index c442fae4fd..f7ef836e3e 100644
--- a/include/core/SkShader.h
+++ b/include/core/SkShader.h
@@ -499,6 +499,7 @@ public:
 
     SK_TO_STRING_VIRT()
     SK_DEFINE_FLATTENABLE_TYPE(SkShader)
+    SK_DECLARE_FLATTENABLE_REGISTRAR_GROUP()
 
 protected:
     void flatten(SkWriteBuffer&) const override;
@@ -536,7 +537,7 @@ private:
 
     // So the SkLocalMatrixShader can whack fLocalMatrix in its SkReadBuffer constructor.
     friend class SkLocalMatrixShader;
-    friend class SkBitmapProcShader;    // for computeTotalInverse()
+    friend class SkBitmapProcLegacyShader;    // for computeTotalInverse()
 
     typedef SkFlattenable INHERITED;
 };
diff --git a/samplecode/SampleLighting.cpp b/samplecode/SampleLighting.cpp
index c191a6c749..dbab0928cb 100755
--- a/samplecode/SampleLighting.cpp
+++ b/samplecode/SampleLighting.cpp
@@ -70,7 +70,7 @@ protected:
             SkShader::kClamp_TileMode, SkShader::kClamp_TileMode, nullptr, nullptr);
         sk_sp<SkNormalSource> normalSource = SkNormalSource::MakeFromNormalMap(
                 std::move(normalMap), SkMatrix::I());
-        sk_sp<SkShader> diffuseShader = SkBitmapProcShader::MakeBitmapShader(fDiffuseBitmap,
+        sk_sp<SkShader> diffuseShader = SkShader::MakeBitmapShader(fDiffuseBitmap,
                 SkShader::kClamp_TileMode, SkShader::kClamp_TileMode, nullptr);
         paint.setShader(SkLightingShader::Make(std::move(diffuseShader), std::move(normalSource),
                                                std::move(lights)));
diff --git a/samplecode/SampleLitAtlas.cpp b/samplecode/SampleLitAtlas.cpp
index f1882b7890..cb42d392e4 100644
--- a/samplecode/SampleLitAtlas.cpp
+++ b/samplecode/SampleLitAtlas.cpp
@@ -134,7 +134,7 @@ protected:
                     SkShader::kClamp_TileMode, &normalMat, nullptr);
             sk_sp<SkNormalSource> normalSource = SkNormalSource::MakeFromNormalMap(
                     std::move(normalMap), m);
-            sk_sp<SkShader> diffuseShader = SkBitmapProcShader::MakeBitmapShader(fAtlas,
+            sk_sp<SkShader> diffuseShader = SkShader::MakeBitmapShader(fAtlas,
                     SkShader::kClamp_TileMode, SkShader::kClamp_TileMode, &diffMat);
             paint.setShader(SkLightingShader::Make(std::move(diffuseShader),
                     std::move(normalSource), fLights));
diff --git a/src/core/SkBitmapProcShader.cpp b/src/core/SkBitmapProcShader.cpp
index 3ad9d5a626..e0d281b026 100644
--- a/src/core/SkBitmapProcShader.cpp
+++ b/src/core/SkBitmapProcShader.cpp
@@ -8,17 +8,6 @@
 #include "SkBitmapProcShader.h"
 #include "SkBitmapProcState.h"
 #include "SkBitmapProvider.h"
-#include "SkColorPriv.h"
-#include "SkErrorInternals.h"
-#include "SkPixelRef.h"
-#include "SkReadBuffer.h"
-#include "SkWriteBuffer.h"
-
-#if SK_SUPPORT_GPU
-#include "SkGrPriv.h"
-#include "effects/GrBicubicEffect.h"
-#include "effects/GrSimpleTextureEffect.h"
-#endif
 
 static bool only_scale_and_translate(const SkMatrix& matrix) {
     unsigned mask = SkMatrix::kTranslate_Mask | SkMatrix::kScale_Mask;
@@ -211,13 +200,15 @@ static bool choose_linear_pipeline(const SkShader::ContextRec& rec, const SkImag
            || needsPremul || needsSwizzle;
 }
 
-size_t SkBitmapProcShader::ContextSize(const ContextRec& rec, const SkImageInfo& srcInfo) {
+size_t SkBitmapProcLegacyShader::ContextSize(const ContextRec& rec, const SkImageInfo& srcInfo) {
     size_t size0 = sizeof(BitmapProcShaderContext) + sizeof(SkBitmapProcState);
     size_t size1 = sizeof(LinearPipelineContext) + sizeof(SkBitmapProcInfo);
+    size_t s = SkTMax(size0, size1);
+    return s;
     return SkTMax(size0, size1);
 }
 
-SkShader::Context* SkBitmapProcShader::MakeContext(const SkShader& shader,
+SkShader::Context* SkBitmapProcLegacyShader::MakeContext(const SkShader& shader,
                                                    TileMode tmx, TileMode tmy,
                                                    const SkBitmapProvider& provider,
                                                    const ContextRec& rec, void* storage) {
@@ -251,225 +242,3 @@ SkShader::Context* SkBitmapProcShader::MakeContext(const SkShader& shader,
         return new (storage) BitmapProcShaderContext(shader, rec, state);
     }
 }
-
-SkShader::Context* SkBitmapProcShader::onCreateContext(const ContextRec& rec, void* storage) const {
-    return MakeContext(*this, (TileMode)fTileModeX, (TileMode)fTileModeY,
-                       SkBitmapProvider(fRawBitmap), rec, storage);
-}
-
-///////////////////////////////////////////////////////////////////////////////////////////////////
-
-SkBitmapProcShader::SkBitmapProcShader(const SkBitmap& src, TileMode tmx, TileMode tmy,
-                                       const SkMatrix* localMatrix)
-        : INHERITED(localMatrix) {
-    fRawBitmap = src;
-    fTileModeX = (uint8_t)tmx;
-    fTileModeY = (uint8_t)tmy;
-}
-
-bool SkBitmapProcShader::onIsABitmap(SkBitmap* texture, SkMatrix* texM, TileMode xy[]) const {
-    if (texture) {
-        *texture = fRawBitmap;
-    }
-    if (texM) {
-        *texM = this->getLocalMatrix();
-    }
-    if (xy) {
-        xy[0] = (TileMode)fTileModeX;
-        xy[1] = (TileMode)fTileModeY;
-    }
-    return true;
-}
-
-sk_sp<SkFlattenable> SkBitmapProcShader::CreateProc(SkReadBuffer& buffer) {
-    SkMatrix lm;
-    buffer.readMatrix(&lm);
-    SkBitmap bm;
-    if (!buffer.readBitmap(&bm)) {
-        return nullptr;
-    }
-    bm.setImmutable();
-    TileMode mx = (TileMode)buffer.readUInt();
-    TileMode my = (TileMode)buffer.readUInt();
-    return SkShader::MakeBitmapShader(bm, mx, my, &lm);
-}
-
-void SkBitmapProcShader::flatten(SkWriteBuffer& buffer) const {
-    buffer.writeMatrix(this->getLocalMatrix());
-    buffer.writeBitmap(fRawBitmap);
-    buffer.writeUInt(fTileModeX);
-    buffer.writeUInt(fTileModeY);
-}
-
-bool SkBitmapProcShader::isOpaque() const {
-    return fRawBitmap.isOpaque();
-}
-
-bool SkBitmapProcShader::BitmapIsTooBig(const SkBitmap& bm) {
-    static const int kMaxSize = 65535;
-
-    return bm.width() > kMaxSize || bm.height() > kMaxSize;
-}
-
-///////////////////////////////////////////////////////////////////////////////////////////////////
-
-#include "SkUnPreMultiply.h"
-#include "SkColorShader.h"
-#include "SkEmptyShader.h"
-
-// returns true and set color if the bitmap can be drawn as a single color
-// (for efficiency)
-static bool can_use_color_shader(const SkBitmap& bm, SkColor* color) {
-#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
-    // HWUI does not support color shaders (see b/22390304)
-    return false;
-#endif
-
-    if (1 != bm.width() || 1 != bm.height()) {
-        return false;
-    }
-
-    SkAutoLockPixels alp(bm);
-    if (!bm.readyToDraw()) {
-        return false;
-    }
-
-    switch (bm.colorType()) {
-        case kN32_SkColorType:
-            *color = SkUnPreMultiply::PMColorToColor(*bm.getAddr32(0, 0));
-            return true;
-        case kRGB_565_SkColorType:
-            *color = SkPixel16ToColor(*bm.getAddr16(0, 0));
-            return true;
-        case kIndex_8_SkColorType:
-            *color = SkUnPreMultiply::PMColorToColor(bm.getIndex8Color(0, 0));
-            return true;
-        default: // just skip the other configs for now
-            break;
-    }
-    return false;
-}
-
-static bool bitmap_is_too_big(const SkBitmap& bm) {
-    // SkBitmapProcShader stores bitmap coordinates in a 16bit buffer, as it
-    // communicates between its matrix-proc and its sampler-proc. Until we can
-    // widen that, we have to reject bitmaps that are larger.
-    //
-    static const int kMaxSize = 65535;
-
-    return bm.width() > kMaxSize || bm.height() > kMaxSize;
-}
-
-sk_sp<SkShader> SkMakeBitmapShader(const SkBitmap& src, SkShader::TileMode tmx,
-                                   SkShader::TileMode tmy, const SkMatrix* localMatrix,
-                                   SkTBlitterAllocator* allocator) {
-    SkShader* shader;
-    SkColor color;
-    if (src.isNull() || bitmap_is_too_big(src)) {
-        if (nullptr == allocator) {
-            shader = new SkEmptyShader;
-        } else {
-            shader = allocator->createT<SkEmptyShader>();
-        }
-    } else if (can_use_color_shader(src, &color)) {
-        if (nullptr == allocator) {
-            shader = new SkColorShader(color);
-        } else {
-            shader = allocator->createT<SkColorShader>(color);
-        }
-    } else {
-        if (nullptr == allocator) {
-            shader = new SkBitmapProcShader(src, tmx, tmy, localMatrix);
-        } else {
-            shader = allocator->createT<SkBitmapProcShader>(src, tmx, tmy, localMatrix);
-        }
-    }
-    return sk_sp<SkShader>(shader);
-}
-
-///////////////////////////////////////////////////////////////////////////////
-
-#ifndef SK_IGNORE_TO_STRING
-void SkBitmapProcShader::toString(SkString* str) const {
-    static const char* gTileModeName[SkShader::kTileModeCount] = {
-        "clamp", "repeat", "mirror"
-    };
-
-    str->append("BitmapShader: (");
-
-    str->appendf("(%s, %s)",
-                 gTileModeName[fTileModeX],
-                 gTileModeName[fTileModeY]);
-
-    str->append(" ");
-    fRawBitmap.toString(str);
-
-    this->INHERITED::toString(str);
-
-    str->append(")");
-}
-#endif
-
-///////////////////////////////////////////////////////////////////////////////
-
-#if SK_SUPPORT_GPU
-
-#include "GrTextureAccess.h"
-#include "SkGr.h"
-#include "effects/GrSimpleTextureEffect.h"
-
-sk_sp<GrFragmentProcessor> SkBitmapProcShader::asFragmentProcessor(const AsFPArgs& args) const {
-    SkMatrix matrix;
-    matrix.setIDiv(fRawBitmap.width(), fRawBitmap.height());
-
-    SkMatrix lmInverse;
-    if (!this->getLocalMatrix().invert(&lmInverse)) {
-        return nullptr;
-    }
-    if (args.fLocalMatrix) {
-        SkMatrix inv;
-        if (!args.fLocalMatrix->invert(&inv)) {
-            return nullptr;
-        }
-        lmInverse.postConcat(inv);
-    }
-    matrix.preConcat(lmInverse);
-
-    SkShader::TileMode tm[] = {
-        (TileMode)fTileModeX,
-        (TileMode)fTileModeY,
-    };
-
-    // Must set wrap and filter on the sampler before requesting a texture. In two places below
-    // we check the matrix scale factors to determine how to interpret the filter quality setting.
-    // This completely ignores the complexity of the drawVertices case where explicit local coords
-    // are provided by the caller.
-    bool doBicubic;
-    GrTextureParams::FilterMode textureFilterMode =
-            GrSkFilterQualityToGrFilterMode(args.fFilterQuality, *args.fViewMatrix,
-                                            this->getLocalMatrix(), &doBicubic);
-    GrTextureParams params(tm, textureFilterMode);
-    SkAutoTUnref<GrTexture> texture(GrRefCachedBitmapTexture(args.fContext, fRawBitmap, params,
-                                                             args.fGammaTreatment));
-
-    if (!texture) {
-        SkErrorInternals::SetError( kInternalError_SkError,
-                                    "Couldn't convert bitmap to texture.");
-        return nullptr;
-    }
-    sk_sp<GrColorSpaceXform> colorSpaceXform = GrColorSpaceXform::Make(fRawBitmap.colorSpace(),
-                                                                       args.fDstColorSpace);
-    sk_sp<GrFragmentProcessor> inner;
-    if (doBicubic) {
-        inner = GrBicubicEffect::Make(texture, std::move(colorSpaceXform), matrix, tm);
-    } else {
-        inner = GrSimpleTextureEffect::Make(texture, std::move(colorSpaceXform), matrix, params);
-    }
-
-    if (kAlpha_8_SkColorType == fRawBitmap.colorType()) {
-        return GrFragmentProcessor::MulOutputByInputUnpremulColor(std::move(inner));
-    }
-    return GrFragmentProcessor::MulOutputByInputAlpha(std::move(inner));
-}
-
-#endif
diff --git a/src/core/SkBitmapProcShader.h b/src/core/SkBitmapProcShader.h
index 624034851d..c0e6a407ce 100644
--- a/src/core/SkBitmapProcShader.h
+++ b/src/core/SkBitmapProcShader.h
@@ -4,7 +4,6 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
-
 #ifndef SkBitmapProcShader_DEFINED
 #define SkBitmapProcShader_DEFINED
 
@@ -14,36 +13,7 @@
 struct SkBitmapProcState;
 class SkBitmapProvider;
 
-class SkBitmapProcShader : public SkShader {
-public:
-    SkBitmapProcShader(const SkBitmap& src, TileMode tx, TileMode ty,
-                       const SkMatrix* localMatrix = nullptr);
-
-    bool isOpaque() const override;
-
-    // SkBitmapProcShader stores bitmap coordinates in a 16bit buffer, as it
-    // communicates between its matrix-proc and its sampler-proc. Until we can
-    // widen that, we have to reject bitmaps that are larger.
-    static bool BitmapIsTooBig(const SkBitmap&);
-
-    SK_TO_STRING_OVERRIDE()
-    SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkBitmapProcShader)
-
-#if SK_SUPPORT_GPU
-    sk_sp<GrFragmentProcessor> asFragmentProcessor(const AsFPArgs&) const override;
-#endif
-
-protected:
-    void flatten(SkWriteBuffer&) const override;
-    size_t onContextSize(const ContextRec& rec) const override {
-        return ContextSize(rec, fRawBitmap.info());
-    }
-    Context* onCreateContext(const ContextRec&, void* storage) const override;
-    bool onIsABitmap(SkBitmap*, SkMatrix*, TileMode*) const override;
-
-    SkBitmap    fRawBitmap;
-    uint8_t     fTileModeX, fTileModeY;
-
+class SkBitmapProcLegacyShader : public SkShader {
 private:
     friend class SkImageShader;
 
diff --git a/src/core/SkGlobalInitialization_core.cpp b/src/core/SkGlobalInitialization_core.cpp
index 022c4972c4..21c4d16c4a 100644
--- a/src/core/SkGlobalInitialization_core.cpp
+++ b/src/core/SkGlobalInitialization_core.cpp
@@ -29,13 +29,11 @@
  */
 void SkFlattenable::PrivateInitializer::InitCore() {
     // Shader
-    SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkBitmapProcShader)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkColorFilterShader)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkColorShader)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkColor4Shader)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkComposeShader)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkEmptyShader)
-    SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkImageShader)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkLocalMatrixShader)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkPictureShader)
 
@@ -48,6 +46,8 @@ void SkFlattenable::PrivateInitializer::InitCore() {
     // ColorFilter
     SkColorFilter::InitializeFlattenables();
 
+    SkShader::InitializeFlattenables();
+
     // Xfermode
     SkXfermode::InitializeFlattenables();
 
diff --git a/src/image/SkImageShader.cpp b/src/image/SkImageShader.cpp
index 12caa7a3d2..a486f71dfd 100644
--- a/src/image/SkImageShader.cpp
+++ b/src/image/SkImageShader.cpp
@@ -7,6 +7,9 @@
 
 #include "SkBitmapProcShader.h"
 #include "SkBitmapProvider.h"
+#include "SkColorShader.h"
+#include "SkColorTable.h"
+#include "SkEmptyShader.h"
 #include "SkImage_Base.h"
 #include "SkImageShader.h"
 #include "SkReadBuffer.h"
@@ -43,11 +46,11 @@ bool SkImageShader::isOpaque() const {
 }
 
 size_t SkImageShader::onContextSize(const ContextRec& rec) const {
-    return SkBitmapProcShader::ContextSize(rec, SkBitmapProvider(fImage).info());
+    return SkBitmapProcLegacyShader::ContextSize(rec, SkBitmapProvider(fImage).info());
 }
 
 SkShader::Context* SkImageShader::onCreateContext(const ContextRec& rec, void* storage) const {
-    return SkBitmapProcShader::MakeContext(*this, fTileModeX, fTileModeY,
+    return SkBitmapProcLegacyShader::MakeContext(*this, fTileModeX, fTileModeY,
                                            SkBitmapProvider(fImage), rec, storage);
 }
 
@@ -81,12 +84,76 @@ bool SkImageShader::onIsABitmap(SkBitmap* texture, SkMatrix* texM, TileMode xy[]
     return true;
 }
 
+static bool bitmap_is_too_big(int w, int h) {
+    // SkBitmapProcShader stores bitmap coordinates in a 16bit buffer, as it
+    // communicates between its matrix-proc and its sampler-proc. Until we can
+    // widen that, we have to reject bitmaps that are larger.
+    //
+    static const int kMaxSize = 65535;
+    
+    return w > kMaxSize || h > kMaxSize;
+}
+
+// returns true and set color if the bitmap can be drawn as a single color
+// (for efficiency)
+static bool can_use_color_shader(const SkImage* image, SkColor* color) {
+#ifdef SK_BUILD_FOR_ANDROID_FRAMEWORK
+    // HWUI does not support color shaders (see b/22390304)
+    return false;
+#endif
+    
+    if (1 != image->width() || 1 != image->height()) {
+        return false;
+    }
+    
+    SkPixmap pmap;
+    if (!image->peekPixels(&pmap)) {
+        return false;
+    }
+    
+    switch (pmap.colorType()) {
+        case kN32_SkColorType:
+            *color = SkUnPreMultiply::PMColorToColor(*pmap.addr32(0, 0));
+            return true;
+        case kRGB_565_SkColorType:
+            *color = SkPixel16ToColor(*pmap.addr16(0, 0));
+            return true;
+        case kIndex_8_SkColorType: {
+            const SkColorTable& ctable = *pmap.ctable();
+            *color = SkUnPreMultiply::PMColorToColor(ctable[*pmap.addr8(0, 0)]);
+            return true;
+        }
+        default: // just skip the other configs for now
+            break;
+    }
+    return false;
+}
+
 sk_sp<SkShader> SkImageShader::Make(const SkImage* image, TileMode tx, TileMode ty,
-                                    const SkMatrix* localMatrix) {
-    if (!image) {
-        return nullptr;
+                                    const SkMatrix* localMatrix,
+                                    SkTBlitterAllocator* allocator) {
+    SkShader* shader;
+    SkColor color;
+    if (!image || bitmap_is_too_big(image->width(), image->height())) {
+        if (nullptr == allocator) {
+            shader = new SkEmptyShader;
+        } else {
+            shader = allocator->createT<SkEmptyShader>();
+        }
+    } else if (can_use_color_shader(image, &color)) {
+        if (nullptr == allocator) {
+            shader = new SkColorShader(color);
+        } else {
+            shader = allocator->createT<SkColorShader>(color);
+        }
+    } else {
+        if (nullptr == allocator) {
+            shader = new SkImageShader(image, tx, ty, localMatrix);
+        } else {
+            shader = allocator->createT<SkImageShader>(image, tx, ty, localMatrix);
+        }
     }
-    return sk_sp<SkShader>(new SkImageShader(image, tx, ty, localMatrix));
+    return sk_sp<SkShader>(shader);
 }
 
 #ifndef SK_IGNORE_TO_STRING
@@ -161,3 +228,33 @@ sk_sp<GrFragmentProcessor> SkImageShader::asFragmentProcessor(const AsFPArgs& ar
 }
 
 #endif
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+#include "SkImagePriv.h"
+
+sk_sp<SkShader> SkMakeBitmapShader(const SkBitmap& src, SkShader::TileMode tmx,
+                                   SkShader::TileMode tmy, const SkMatrix* localMatrix,
+                                   SkTBlitterAllocator* allocator) {
+    ForceCopyMode mode = allocator ? kNever_ForceCopyMode : kNo_ForceCopyMode;
+    return SkImageShader::Make(SkMakeImageFromRasterBitmap(src, mode).get(),
+                               tmx, tmy, localMatrix, allocator);
+}
+
+static sk_sp<SkFlattenable> SkBitmapProcShader_CreateProc(SkReadBuffer& buffer) {
+    SkMatrix lm;
+    buffer.readMatrix(&lm);
+    SkBitmap bm;
+    if (!buffer.readBitmap(&bm)) {
+        return nullptr;
+    }
+    bm.setImmutable();
+    SkShader::TileMode mx = (SkShader::TileMode)buffer.readUInt();
+    SkShader::TileMode my = (SkShader::TileMode)buffer.readUInt();
+    return SkShader::MakeBitmapShader(bm, mx, my, &lm);
+}
+
+SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_START(SkShader)
+SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkImageShader)
+SkFlattenable::Register("SkBitmapProcShader", SkBitmapProcShader_CreateProc, kSkShader_Type);
+SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_END
+
diff --git a/src/image/SkImageShader.h b/src/image/SkImageShader.h
index 160de7ac9c..e9911e6c25 100644
--- a/src/image/SkImageShader.h
+++ b/src/image/SkImageShader.h
@@ -10,11 +10,12 @@
 
 #include "SkImage.h"
 #include "SkShader.h"
+#include "SkBitmapProcShader.h"
 
 class SkImageShader : public SkShader {
 public:
     static sk_sp<SkShader> Make(const SkImage*, TileMode tx, TileMode ty,
-                                const SkMatrix* localMatrix);
+                                const SkMatrix* localMatrix, SkTBlitterAllocator* = nullptr);
 
     bool isOpaque() const override;
 
@@ -25,6 +26,8 @@ public:
     sk_sp<GrFragmentProcessor> asFragmentProcessor(const AsFPArgs&) const override;
 #endif
 
+    SkImageShader(const SkImage*, TileMode tx, TileMode ty, const SkMatrix* localMatrix);
+
 protected:
     void flatten(SkWriteBuffer&) const override;
     size_t onContextSize(const ContextRec&) const override;
@@ -37,7 +40,7 @@ protected:
     const TileMode              fTileModeY;
 
 private:
-    SkImageShader(const SkImage*, TileMode tx, TileMode ty, const SkMatrix* localMatrix);
+    friend class SkShader;
 
     typedef SkShader INHERITED;
 };
diff --git a/tests/BlitRowTest.cpp b/tests/BlitRowTest.cpp
index 032acdac39..99b79217e0 100644
--- a/tests/BlitRowTest.cpp
+++ b/tests/BlitRowTest.cpp
@@ -195,19 +195,11 @@ static void test_diagonal(skiatest::Reporter* reporter) {
     };
 
     static const SkColor gDstBG[] = { 0, 0xFFFFFFFF };
-
-    SkPaint paint;
+    const SkRect srcR = SkRect::MakeIWH(W, H);
 
     SkBitmap srcBM;
     srcBM.allocN32Pixels(W, H);
-    SkRect srcR = {
-        0, 0, SkIntToScalar(srcBM.width()), SkIntToScalar(srcBM.height()) };
-
-    // cons up a mesh to draw the bitmap with
-    Mesh mesh(srcBM, &paint);
-
-    SkImageInfo info = SkImageInfo::Make(W, H, kUnknown_SkColorType,
-                                         kPremul_SkAlphaType);
+    SkImageInfo info = SkImageInfo::Make(W, H, kUnknown_SkColorType, kPremul_SkAlphaType);
 
     for (size_t i = 0; i < SK_ARRAY_COUNT(gDstColorType); i++) {
         info = info.makeColorType(gDstColorType[i]);
@@ -224,7 +216,10 @@ static void test_diagonal(skiatest::Reporter* reporter) {
             bgColor = gDstBG[j];
 
             for (int c = 0; c <= 0xFF; c++) {
+                // cons up a mesh to draw the bitmap with
+                SkPaint paint;
                 srcBM.eraseARGB(0xFF, c, c, c);
+                Mesh mesh(srcBM, &paint);
 
                 for (int k = 0; k < 4; k++) {
                     bool dither = (k & 1) != 0;
diff --git a/tests/ShaderTest.cpp b/tests/ShaderTest.cpp
index d3f74ffb3d..83240478ad 100644
--- a/tests/ShaderTest.cpp
+++ b/tests/ShaderTest.cpp
@@ -15,8 +15,7 @@
 static void check_isabitmap(skiatest::Reporter* reporter, SkShader* shader,
                             int expectedW, int expectedH,
                             SkShader::TileMode expectedX, SkShader::TileMode expectedY,
-                            const SkMatrix& expectedM,
-                            bool expectedImage) {
+                            const SkMatrix& expectedM) {
     SkBitmap bm;
     SkShader::TileMode tileModes[2];
     SkMatrix localM;
@@ -32,14 +31,12 @@ static void check_isabitmap(skiatest::Reporter* reporter, SkShader* shader,
     tileModes[0] = tileModes[1] = (SkShader::TileMode)99;
 
     SkImage* image = shader->isAImage(&localM, tileModes);
-    REPORTER_ASSERT(reporter, (image != nullptr) == expectedImage);
-    if (image) {
-        REPORTER_ASSERT(reporter, image->width() == expectedW);
-        REPORTER_ASSERT(reporter, image->height() == expectedH);
-        REPORTER_ASSERT(reporter, localM == expectedM);
-        REPORTER_ASSERT(reporter, tileModes[0] == expectedX);
-        REPORTER_ASSERT(reporter, tileModes[1] == expectedY);
-    }
+    REPORTER_ASSERT(reporter, image);
+    REPORTER_ASSERT(reporter, image->width() == expectedW);
+    REPORTER_ASSERT(reporter, image->height() == expectedH);
+    REPORTER_ASSERT(reporter, localM == expectedM);
+    REPORTER_ASSERT(reporter, tileModes[0] == expectedX);
+    REPORTER_ASSERT(reporter, tileModes[1] == expectedY);
 }
 
 DEF_TEST(Shader_isABitmap, reporter) {
@@ -55,6 +52,6 @@ DEF_TEST(Shader_isABitmap, reporter) {
     auto shader0 = SkShader::MakeBitmapShader(bm, tmx, tmy, &localM);
     auto shader1 = SkImage::MakeFromBitmap(bm)->makeShader(tmx, tmy, &localM);
 
-    check_isabitmap(reporter, shader0.get(), W, H, tmx, tmy, localM, false);
-    check_isabitmap(reporter, shader1.get(), W, H, tmx, tmy, localM, true);
+    check_isabitmap(reporter, shader0.get(), W, H, tmx, tmy, localM);
+    check_isabitmap(reporter, shader1.get(), W, H, tmx, tmy, localM);
 }
diff --git a/tools/dm_flags.py b/tools/dm_flags.py
index 2ee9358fe5..fd3b2b791a 100755
--- a/tools/dm_flags.py
+++ b/tools/dm_flags.py
@@ -208,6 +208,10 @@ def get_args(bot):
                             'filterbitmap_image_mandrill_64',
                             'shadows',
                             'simpleaaclip_aaclip'])
+  # skia:5595
+  bad_serialize_gms.extend(['composeshader_bitmap',
+                            'scaled_tilemodes_npot',
+                            'scaled_tilemodes'])
   for test in bad_serialize_gms:
     blacklist.extend(['serialize-8888', 'gm', '_', test])