Reland "Refactor trimming logic for read/writePixels()"
authorMatt Sarett <msarett@google.com>
Mon, 23 Jan 2017 17:15:09 +0000 (12:15 -0500)
committerSkia Commit-Bot <skia-commit-bot@chromium.org>
Mon, 23 Jan 2017 18:12:46 +0000 (18:12 +0000)
Original CL: https://skia-review.googlesource.com/c/7326/

(1) Move trimming logic into Bitmap/Pixmap level for
    raster.  Everything goes through here, so we'll
    only do the work once.
(2) This means it also goes to GPU level.
(3) Always use SkReadPixelsRec rather than inlining
    the logic.
(4) Create an SkWritePixelsRec to encapsulate write
    trimming.
(5) Disabled kIndex8 as a dst - always.

CQ_INCLUDE_TRYBOTS=skia.primary:Perf-Ubuntu-Clang-GCE-CPU-AVX2-x86_64-Debug

BUG=skia:6021

Change-Id: I25a964e3c610c4e36d195a255e2150657baec649
Reviewed-on: https://skia-review.googlesource.com/7404
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Matt Sarett <msarett@google.com>

12 files changed:
src/core/SkBitmap.cpp
src/core/SkCanvas.cpp
src/core/SkConfig8888.cpp
src/core/SkDevice.cpp
src/core/SkImageInfo.cpp
src/core/SkImageInfoPriv.h
src/core/SkPixmap.cpp
src/core/SkWritePixelsRec.h [new file with mode: 0644]
src/gpu/SkGpuDevice.cpp
src/image/SkImage.cpp
src/image/SkImage_Gpu.cpp
tests/BitmapCopyTest.cpp

index 00b2f8e503499b88d4f1613704dfe7f9fd7ed3de..f54488d0a518a496f0143b6d004c738798803ab2 100644 (file)
@@ -12,6 +12,7 @@
 #include "SkData.h"
 #include "SkFilterQuality.h"
 #include "SkHalf.h"
+#include "SkImageInfoPriv.h"
 #include "SkMallocPixelRef.h"
 #include "SkMask.h"
 #include "SkMath.h"
@@ -22,6 +23,7 @@
 #include "SkTemplates.h"
 #include "SkUnPreMultiply.h"
 #include "SkWriteBuffer.h"
+#include "SkWritePixelsRec.h"
 
 #include <string.h>
 
@@ -682,7 +684,6 @@ bool SkBitmap::canCopyTo(SkColorType dstCT) const {
         case kBGRA_8888_SkColorType:
             break;
         case kGray_8_SkColorType:
-        case kIndex_8_SkColorType:
             if (!sameConfigs) {
                 return false;
             }
@@ -714,13 +715,19 @@ bool SkBitmap::writePixels(const SkPixmap& src, int dstX, int dstY) {
         return false;
     }
 
-    SkPixmap subset;
-    if (!dst.pixmap().extractSubset(&subset,
-                                    SkIRect::MakeXYWH(dstX, dstY, src.width(), src.height()))) {
+    if (!SkImageInfoValidConversion(fInfo, src.info())) {
+        return false;
+    }
+
+    SkWritePixelsRec rec(src.info(), src.addr(), src.rowBytes(), dstX, dstY);
+    if (!rec.trim(fInfo.width(), fInfo.height())) {
         return false;
     }
 
-    return src.readPixels(subset);
+    void* dstPixels = this->getAddr(rec.fX, rec.fY);
+    const SkImageInfo dstInfo = fInfo.makeWH(rec.fInfo.width(), rec.fInfo.height());
+    return SkPixelInfo::CopyPixels(dstInfo, dstPixels, this->rowBytes(),
+                                   rec.fInfo, rec.fPixels, rec.fRowBytes, src.ctable());
 }
 
 bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc) const {
index d57b62fa99f70068aad2142e28c7ab638c5451d6..7596c3a70247d5f34c56b4a66290c5159fbba472 100644 (file)
@@ -30,7 +30,6 @@
 #include "SkRadialShadowMapShader.h"
 #include "SkRasterClip.h"
 #include "SkRasterHandleAllocator.h"
-#include "SkReadPixelsRec.h"
 #include "SkRRect.h"
 #include "SkShadowPaintFilterCanvas.h"
 #include "SkShadowShader.h"
@@ -858,10 +857,6 @@ SkBaseDevice* SkCanvas::getTopDevice() const {
 }
 
 bool SkCanvas::readPixels(SkBitmap* bitmap, int x, int y) {
-    if (kUnknown_SkColorType == bitmap->colorType()) {
-        return false;
-    }
-
     bool weAllocated = false;
     if (nullptr == bitmap->pixelRef()) {
         if (!bitmap->tryAllocPixels()) {
@@ -908,15 +903,8 @@ bool SkCanvas::readPixels(const SkImageInfo& dstInfo, void* dstP, size_t rowByte
     if (!device) {
         return false;
     }
-    const SkISize size = this->getBaseLayerSize();
 
-    SkReadPixelsRec rec(dstInfo, dstP, rowBytes, x, y);
-    if (!rec.trim(size.width(), size.height())) {
-        return false;
-    }
-
-    // The device can assert that the requested area is always contained in its bounds
-    return device->readPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY);
+    return device->readPixels(dstInfo, dstP, rowBytes, x, y);
 }
 
 bool SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
@@ -928,49 +916,30 @@ bool SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
     return false;
 }
 
-bool SkCanvas::writePixels(const SkImageInfo& origInfo, const void* pixels, size_t rowBytes,
+bool SkCanvas::writePixels(const SkImageInfo& srcInfo, const void* pixels, size_t rowBytes,
                            int x, int y) {
-    switch (origInfo.colorType()) {
-        case kUnknown_SkColorType:
-        case kIndex_8_SkColorType:
-            return false;
-        default:
-            break;
-    }
-    if (nullptr == pixels || rowBytes < origInfo.minRowBytes()) {
-        return false;
-    }
-
-    const SkISize size = this->getBaseLayerSize();
-    SkIRect target = SkIRect::MakeXYWH(x, y, origInfo.width(), origInfo.height());
-    if (!target.intersect(0, 0, size.width(), size.height())) {
-        return false;
-    }
-
     SkBaseDevice* device = this->getDevice();
     if (!device) {
         return false;
     }
 
-    // the intersect may have shrunk info's logical size
-    const SkImageInfo info = origInfo.makeWH(target.width(), target.height());
-
-    // if x or y are negative, then we have to adjust pixels
-    if (x > 0) {
-        x = 0;
-    }
-    if (y > 0) {
-        y = 0;
+    // This check gives us an early out and prevents generation ID churn on the surface.
+    // This is purely optional: it is a subset of the checks performed by SkWritePixelsRec.
+    SkIRect srcRect = SkIRect::MakeXYWH(x, y, srcInfo.width(), srcInfo.height());
+    if (!srcRect.intersect(0, 0, device->width(), device->height())) {
+        return false;
     }
-    // here x,y are either 0 or negative
-    pixels = ((const char*)pixels - y * rowBytes - x * info.bytesPerPixel());
 
-    // Tell our owning surface to bump its generation ID
-    const bool completeOverwrite = info.dimensions() == size;
+    // Tell our owning surface to bump its generation ID.
+    const bool completeOverwrite =
+            srcRect.size() == SkISize::Make(device->width(), device->height());
     this->predrawNotify(completeOverwrite);
 
-    // The device can assert that the requested area is always contained in its bounds
-    return device->writePixels(info, pixels, rowBytes, target.x(), target.y());
+    // This can still fail, most notably in the case of a invalid color type or alpha type
+    // conversion.  We could pull those checks into this function and avoid the unnecessary
+    // generation ID bump.  But then we would be performing those checks twice, since they
+    // are also necessary at the bitmap/pixmap entry points.
+    return device->writePixels(srcInfo, pixels, rowBytes, x, y);
 }
 
 //////////////////////////////////////////////////////////////////////////////
index 98e5b7aced899f64758631061c661a62d5d5dda5..d75ea27dc67dad9a561235f96a2b08d7531d9403 100644 (file)
@@ -371,7 +371,7 @@ bool SkPixelInfo::CopyPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t
     const int height = srcInfo.height();
 
     // Do the easiest one first : both configs are equal
-    if (srcInfo == dstInfo && kIndex_8_SkColorType != srcInfo.colorType()) {
+    if (srcInfo == dstInfo) {
         size_t bytes = width * srcInfo.bytesPerPixel();
         for (int y = 0; y < height; ++y) {
             memcpy(dstPixels, srcPixels, bytes);
index 7800c370c18eff0e3de75e78be4dfc260f85cf99..ab75dbc0bc94a529212118a8baaf9bb6c5212d7c 100644 (file)
@@ -319,31 +319,11 @@ sk_sp<SkSpecialImage> SkBaseDevice::snapSpecial() { return nullptr; }
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 bool SkBaseDevice::readPixels(const SkImageInfo& info, void* dstP, size_t rowBytes, int x, int y) {
-#ifdef SK_DEBUG
-    SkASSERT(info.width() > 0 && info.height() > 0);
-    SkASSERT(dstP);
-    SkASSERT(rowBytes >= info.minRowBytes());
-    SkASSERT(x >= 0 && y >= 0);
-
-    const SkImageInfo& srcInfo = this->imageInfo();
-    SkASSERT(x + info.width() <= srcInfo.width());
-    SkASSERT(y + info.height() <= srcInfo.height());
-#endif
     return this->onReadPixels(info, dstP, rowBytes, x, y);
 }
 
 bool SkBaseDevice::writePixels(const SkImageInfo& info, const void* pixels, size_t rowBytes,
                                int x, int y) {
-#ifdef SK_DEBUG
-    SkASSERT(info.width() > 0 && info.height() > 0);
-    SkASSERT(pixels);
-    SkASSERT(rowBytes >= info.minRowBytes());
-    SkASSERT(x >= 0 && y >= 0);
-
-    const SkImageInfo& dstInfo = this->imageInfo();
-    SkASSERT(x + info.width() <= dstInfo.width());
-    SkASSERT(y + info.height() <= dstInfo.height());
-#endif
     return this->onWritePixels(info, pixels, rowBytes, x, y);
 }
 
index 0453c7b38f66bb4da8a2f2a117f1ff57dadbba31..1b7c09b2f9be09cdb3f4781750b31b62dbbc8d82 100644 (file)
@@ -98,9 +98,6 @@ bool SkColorTypeValidateAlphaType(SkColorType colorType, SkAlphaType alphaType,
 #include "SkReadPixelsRec.h"
 
 bool SkReadPixelsRec::trim(int srcWidth, int srcHeight) {
-    if (kIndex_8_SkColorType == fInfo.colorType()) {
-        return false;
-    }
     if (nullptr == fPixels || fRowBytes < fInfo.minRowBytes()) {
         return false;
     }
@@ -131,3 +128,39 @@ bool SkReadPixelsRec::trim(int srcWidth, int srcHeight) {
 
     return true;
 }
+
+///////////////////////////////////////////////////////////////////////////////////////////////////
+
+#include "SkWritePixelsRec.h"
+
+bool SkWritePixelsRec::trim(int dstWidth, int dstHeight) {
+    if (nullptr == fPixels || fRowBytes < fInfo.minRowBytes()) {
+        return false;
+    }
+    if (0 >= fInfo.width() || 0 >= fInfo.height()) {
+        return false;
+    }
+
+    int x = fX;
+    int y = fY;
+    SkIRect dstR = SkIRect::MakeXYWH(x, y, fInfo.width(), fInfo.height());
+    if (!dstR.intersect(0, 0, dstWidth, dstHeight)) {
+        return false;
+    }
+
+    // if x or y are negative, then we have to adjust pixels
+    if (x > 0) {
+        x = 0;
+    }
+    if (y > 0) {
+        y = 0;
+    }
+    // here x,y are either 0 or negative
+    fPixels = ((const char*)fPixels - y * fRowBytes - x * fInfo.bytesPerPixel());
+    // the intersect may have shrunk info's logical size
+    fInfo = fInfo.makeWH(dstR.width(), dstR.height());
+    fX = dstR.x();
+    fY = dstR.y();
+
+    return true;
+}
index 151de82632e6db264c3d95261da045392416ef95..f04acc54840d0c53ea2649567a643888f0b71568 100644 (file)
@@ -39,8 +39,7 @@ static inline bool SkImageInfoIsValid(const SkImageInfo& info) {
 /**
  *  Returns true if Skia has defined a pixel conversion from the |src| to the |dst|.
  *  Returns false otherwise.  Some discussion of false cases:
- *      We will not convert to kIndex8 unless |src| is kIndex8.  This is possible only
- *      in some cases and likley inefficient.
+ *      We will not convert to kIndex8.  Do we overwrite the input color table?
  *      We do not convert to kGray8 when the |src| is not kGray8.  We may add this
  *      feature - it just requires some work to convert to luminance while handling color
  *      spaces correctly.  Currently no one is asking for this.
@@ -57,7 +56,7 @@ static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkIm
         return false;
     }
 
-    if (kIndex_8_SkColorType == dst.colorType() && kIndex_8_SkColorType != src.colorType()) {
+    if (kIndex_8_SkColorType == dst.colorType()) {
         return false;
     }
 
index 1e24b93a603e1d689b3f91793576ada01c0537c2..c0889cfd84e3445eb9c4b8792d3fd60bc40a0492 100644 (file)
@@ -16,6 +16,7 @@
 #include "SkNx.h"
 #include "SkPM4f.h"
 #include "SkPixmap.h"
+#include "SkReadPixelsRec.h"
 #include "SkSurface.h"
 #include "SkUtils.h"
 
@@ -83,37 +84,20 @@ bool SkPixmap::extractSubset(SkPixmap* result, const SkIRect& subset) const {
     return true;
 }
 
-bool SkPixmap::readPixels(const SkImageInfo& requestedDstInfo, void* dstPixels, size_t dstRB,
-                          int x, int y) const {
-    if (!SkImageInfoValidConversion(requestedDstInfo, fInfo)) {
+bool SkPixmap::readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB, int x, int y)
+const {
+    if (!SkImageInfoValidConversion(dstInfo, fInfo)) {
         return false;
     }
 
-    if (nullptr == dstPixels || dstRB < requestedDstInfo.minRowBytes()) {
+    SkReadPixelsRec rec(dstInfo, dstPixels, dstRB, x, y);
+    if (!rec.trim(fInfo.width(), fInfo.height())) {
         return false;
     }
 
-    SkIRect srcR = SkIRect::MakeXYWH(x, y, requestedDstInfo.width(), requestedDstInfo.height());
-    if (!srcR.intersect(0, 0, this->width(), this->height())) {
-        return false;
-    }
-
-    // the intersect may have shrunk info's logical size
-    const SkImageInfo dstInfo = requestedDstInfo.makeWH(srcR.width(), srcR.height());
-
-    // if x or y are negative, then we have to adjust pixels
-    if (x > 0) {
-        x = 0;
-    }
-    if (y > 0) {
-        y = 0;
-    }
-    // here x,y are either 0 or negative
-    dstPixels = ((char*)dstPixels - y * dstRB - x * dstInfo.bytesPerPixel());
-
-    const SkImageInfo srcInfo = this->info().makeWH(dstInfo.width(), dstInfo.height());
-    const void* srcPixels = this->addr(srcR.x(), srcR.y());
-    return SkPixelInfo::CopyPixels(dstInfo, dstPixels, dstRB,
+    const void* srcPixels = this->addr(rec.fX, rec.fY);
+    const SkImageInfo srcInfo = fInfo.makeWH(rec.fInfo.width(), rec.fInfo.height());
+    return SkPixelInfo::CopyPixels(rec.fInfo, rec.fPixels, rec.fRowBytes,
                                    srcInfo, srcPixels, this->rowBytes(), this->ctable());
 }
 
diff --git a/src/core/SkWritePixelsRec.h b/src/core/SkWritePixelsRec.h
new file mode 100644 (file)
index 0000000..652a13a
--- /dev/null
@@ -0,0 +1,41 @@
+/*
+ * Copyright 2017 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifndef SkWritePixelsRec_DEFINED
+#define SkWritePixelsRec_DEFINED
+
+#include "SkImageInfo.h"
+
+/**
+ *  Helper class to package and trim the parameters passed to writePixels()
+ */
+struct SkWritePixelsRec {
+    SkWritePixelsRec(const SkImageInfo& info, const void* pixels, size_t rowBytes, int x, int y)
+        : fPixels(pixels)
+        , fRowBytes(rowBytes)
+        , fInfo(info)
+        , fX(x)
+        , fY(y)
+    {}
+
+    const void* fPixels;
+    size_t      fRowBytes;
+    SkImageInfo fInfo;
+    int         fX;
+    int         fY;
+
+    /*
+     *  On true, may have modified its fields (except fRowBytes) to make it a legal subset
+     *  of the specified dst width/height.
+     *
+     *  On false, leaves self unchanged, but indicates that it does not overlap dst, or
+     *  is not valid (e.g. bad fInfo) for writePixels().
+     */
+    bool trim(int dstWidth, int dstHeight);
+};
+
+#endif
index 85eb671206eba33c06afa0448ae048e63b295881..c5ade9e7b1f6167a778c40e34b4af6c982100130 100644 (file)
@@ -35,6 +35,7 @@
 #include "SkPictureData.h"
 #include "SkRRect.h"
 #include "SkRasterClip.h"
+#include "SkReadPixelsRec.h"
 #include "SkRecord.h"
 #include "SkSpecialImage.h"
 #include "SkStroke.h"
@@ -43,6 +44,7 @@
 #include "SkTLazy.h"
 #include "SkUtils.h"
 #include "SkVertState.h"
+#include "SkWritePixelsRec.h"
 #include "effects/GrBicubicEffect.h"
 #include "effects/GrSimpleTextureEffect.h"
 #include "effects/GrTextureDomain.h"
@@ -199,7 +201,12 @@ bool SkGpuDevice::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size
         return false;
     }
 
-    return fRenderTargetContext->readPixels(dstInfo, dstPixels, dstRowBytes, x, y);
+    SkReadPixelsRec rec(dstInfo, dstPixels, dstRowBytes, x, y);
+    if (!rec.trim(this->width(), this->height())) {
+        return false;
+    }
+
+    return fRenderTargetContext->readPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY);
 }
 
 bool SkGpuDevice::onWritePixels(const SkImageInfo& srcInfo, const void* srcPixels,
@@ -210,7 +217,12 @@ bool SkGpuDevice::onWritePixels(const SkImageInfo& srcInfo, const void* srcPixel
         return false;
     }
 
-    return fRenderTargetContext->writePixels(srcInfo, srcPixels, srcRowBytes, x, y);
+    SkWritePixelsRec rec(srcInfo, srcPixels, srcRowBytes, x, y);
+    if (!rec.trim(this->width(), this->height())) {
+        return false;
+    }
+
+    return fRenderTargetContext->writePixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY);
 }
 
 bool SkGpuDevice::onAccessPixels(SkPixmap* pmap) {
index 137f9166ba0b1e46beb5b93dd2272779ff7745f4..8efe91a706d9a2c2d9032811cf333654f2254e0f 100644 (file)
@@ -51,11 +51,7 @@ bool SkImage::peekPixels(SkPixmap* pm) const {
 
 bool SkImage::readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
                            int srcX, int srcY, CachingHint chint) const {
-    SkReadPixelsRec rec(dstInfo, dstPixels, dstRowBytes, srcX, srcY);
-    if (!rec.trim(this->width(), this->height())) {
-        return false;
-    }
-    return as_IB(this)->onReadPixels(rec.fInfo, rec.fPixels, rec.fRowBytes, rec.fX, rec.fY, chint);
+    return as_IB(this)->onReadPixels(dstInfo, dstPixels, dstRowBytes, srcX, srcY, chint);
 }
 
 bool SkImage::scalePixels(const SkPixmap& dst, SkFilterQuality quality, CachingHint chint) const {
index 7247ae36fa013152e5d3cab3af203bde061f2d48..fb93350039e6985b60a79d00fb206a2e1ae8dcb5 100644 (file)
@@ -27,6 +27,7 @@
 #include "SkImageInfoPriv.h"
 #include "SkMipMap.h"
 #include "SkPixelRef.h"
+#include "SkReadPixelsRec.h"
 
 SkImage_Gpu::SkImage_Gpu(int w, int h, uint32_t uniqueID, SkAlphaType at, sk_sp<GrTexture> tex,
                          sk_sp<SkColorSpace> colorSpace, SkBudgeted budgeted)
@@ -128,20 +129,26 @@ static void apply_premul(const SkImageInfo& info, void* pixels, size_t rowBytes)
     }
 }
 
-bool SkImage_Gpu::onReadPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
+bool SkImage_Gpu::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
                                int srcX, int srcY, CachingHint) const {
-    if (!SkImageInfoValidConversion(info, this->onImageInfo())) {
+    if (!SkImageInfoValidConversion(dstInfo, this->onImageInfo())) {
         return false;
     }
 
-    GrPixelConfig config = SkImageInfo2GrPixelConfig(info, *fTexture->getContext()->caps());
+    SkReadPixelsRec rec(dstInfo, dstPixels, dstRB, srcX, srcY);
+    if (!rec.trim(this->width(), this->height())) {
+        return false;
+    }
+
+    GrPixelConfig config = SkImageInfo2GrPixelConfig(rec.fInfo, *fTexture->getContext()->caps());
     uint32_t flags = 0;
-    if (kUnpremul_SkAlphaType == info.alphaType() && kPremul_SkAlphaType == fAlphaType) {
+    if (kUnpremul_SkAlphaType == rec.fInfo.alphaType() && kPremul_SkAlphaType == fAlphaType) {
         // let the GPU perform this transformation for us
         flags = GrContext::kUnpremul_PixelOpsFlag;
     }
-    if (!fTexture->readPixels(fColorSpace.get(), srcX, srcY, info.width(), info.height(), config,
-                              info.colorSpace(), pixels, rowBytes, flags)) {
+    if (!fTexture->readPixels(fColorSpace.get(), rec.fX, rec.fY, rec.fInfo.width(),
+                              rec.fInfo.height(), config, rec.fInfo.colorSpace(), rec.fPixels,
+                              rec.fRowBytes, flags)) {
         return false;
     }
     // do we have to manually fix-up the alpha channel?
@@ -152,8 +159,8 @@ bool SkImage_Gpu::onReadPixels(const SkImageInfo& info, void* pixels, size_t row
     //
     // Should this be handled by Ganesh? todo:?
     //
-    if (kPremul_SkAlphaType == info.alphaType() && kUnpremul_SkAlphaType == fAlphaType) {
-        apply_premul(info, pixels, rowBytes);
+    if (kPremul_SkAlphaType == rec.fInfo.alphaType() && kUnpremul_SkAlphaType == fAlphaType) {
+        apply_premul(rec.fInfo, rec.fPixels, rec.fRowBytes);
     }
     return true;
 }
index ab0ec30e16e7495ecfadfae45dbdaaebab120f79..d69f7df76e27be5944bd13beb16b61a67e42e019 100644 (file)
@@ -184,7 +184,7 @@ static void writeCoordPixels(SkBitmap& bm, const Coordinates& coords) {
 static const Pair gPairs[] = {
     { kUnknown_SkColorType,     "000000"  },
     { kAlpha_8_SkColorType,     "010000"  },
-    { kIndex_8_SkColorType,     "011111"  },
+    { kIndex_8_SkColorType,     "010111"  },
     { kRGB_565_SkColorType,     "010101"  },
     { kARGB_4444_SkColorType,   "010111"  },
     { kN32_SkColorType,         "010111"  },
@@ -235,7 +235,8 @@ DEF_TEST(BitmapCopy_extractSubset, reporter) {
                 if (!success) {
                     // Skip checking that success matches fValid, which is redundant
                     // with the code below.
-                    REPORTER_ASSERT(reporter, gPairs[i].fColorType != gPairs[j].fColorType);
+                    REPORTER_ASSERT(reporter, kIndex_8_SkColorType == gPairs[i].fColorType ||
+                                              gPairs[i].fColorType != gPairs[j].fColorType);
                     continue;
                 }