Revert "Refactor trimming logic for read/writePixels()"

This reverts commit 977f64cbfad1ecd7fd4b1231c694c7e828fda1f0.

Reason for revert: Triggering nanobench asserts

Original change's description:
> Refactor trimming logic for read/writePixels()
> 
> (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.
> 
> BUG=skia:6021
> 
> Change-Id: I748f50c3b726f7c6de5462e2b1ccb54bc387a510
> Reviewed-on: https://skia-review.googlesource.com/7326
> Reviewed-by: Mike Reed <reed@google.com>
> Commit-Queue: Matt Sarett <msarett@google.com>
> 

TBR=msarett@google.com,brianosman@google.com,reed@google.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:6021

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

12 files changed, 120 insertions, 144 deletions

diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 7d62643c5d..00b2f8e503 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -22,7 +22,6 @@
 #include "SkTemplates.h"
 #include "SkUnPreMultiply.h"
 #include "SkWriteBuffer.h"
-#include "SkWritePixelsRec.h"
 
 #include <string.h>
 
@@ -683,6 +682,7 @@ bool SkBitmap::canCopyTo(SkColorType dstCT) const {
         case kBGRA_8888_SkColorType:
             break;
         case kGray_8_SkColorType:
+        case kIndex_8_SkColorType:
             if (!sameConfigs) {
                 return false;
             }
@@ -714,15 +714,13 @@ bool SkBitmap::writePixels(const SkPixmap& src, int dstX, int dstY) {
         return false;
     }
 
-    SkWritePixelsRec rec(src.info(), src.addr(), src.rowBytes(), dstX, dstY);
-    if (!rec.trim(fInfo.width(), fInfo.height())) {
+    SkPixmap subset;
+    if (!dst.pixmap().extractSubset(&subset,
+                                    SkIRect::MakeXYWH(dstX, dstY, src.width(), src.height()))) {
         return false;
     }
 
-    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());
+    return src.readPixels(subset);
 }
 
 bool SkBitmap::copyTo(SkBitmap* dst, SkColorType dstColorType, Allocator* alloc) const {
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 76f4a706c7..0375ab738d 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -30,6 +30,7 @@
 #include "SkRadialShadowMapShader.h"
 #include "SkRasterClip.h"
 #include "SkRasterHandleAllocator.h"
+#include "SkReadPixelsRec.h"
 #include "SkRRect.h"
 #include "SkShadowPaintFilterCanvas.h"
 #include "SkShadowShader.h"
@@ -857,6 +858,10 @@ 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()) {
@@ -903,8 +908,15 @@ bool SkCanvas::readPixels(const SkImageInfo& dstInfo, void* dstP, size_t rowByte
     if (!device) {
         return false;
     }
+    const SkISize size = this->getBaseLayerSize();
 
-    return device->readPixels(dstInfo, dstP, rowBytes, x, y);
+    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);
 }
 
 bool SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
@@ -916,30 +928,49 @@ bool SkCanvas::writePixels(const SkBitmap& bitmap, int x, int y) {
     return false;
 }
 
-bool SkCanvas::writePixels(const SkImageInfo& srcInfo, const void* pixels, size_t rowBytes,
+bool SkCanvas::writePixels(const SkImageInfo& origInfo, 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;
     }
 
-    // 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;
+    // 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;
     }
+    // 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 =
-            srcRect.size() == SkISize::Make(device->width(), device->height());
+    // Tell our owning surface to bump its generation ID
+    const bool completeOverwrite = info.dimensions() == size;
     this->predrawNotify(completeOverwrite);
 
-    // 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);
+    // The device can assert that the requested area is always contained in its bounds
+    return device->writePixels(info, pixels, rowBytes, target.x(), target.y());
 }
 
 //////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkConfig8888.cpp b/src/core/SkConfig8888.cpp
index d75ea27dc6..98e5b7aced 100644
--- a/src/core/SkConfig8888.cpp
+++ b/src/core/SkConfig8888.cpp
@@ -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) {
+    if (srcInfo == dstInfo && kIndex_8_SkColorType != srcInfo.colorType()) {
         size_t bytes = width * srcInfo.bytesPerPixel();
         for (int y = 0; y < height; ++y) {
             memcpy(dstPixels, srcPixels, bytes);
diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp
index ab75dbc0bc..7800c370c1 100644
--- a/src/core/SkDevice.cpp
+++ b/src/core/SkDevice.cpp
@@ -319,11 +319,31 @@ 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);
 }
 
diff --git a/src/core/SkImageInfo.cpp b/src/core/SkImageInfo.cpp
index 1b7c09b2f9..0453c7b38f 100644
--- a/src/core/SkImageInfo.cpp
+++ b/src/core/SkImageInfo.cpp
@@ -98,6 +98,9 @@ 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;
     }
@@ -128,39 +131,3 @@ 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;
-}
diff --git a/src/core/SkImageInfoPriv.h b/src/core/SkImageInfoPriv.h
index f04acc5484..151de82632 100644
--- a/src/core/SkImageInfoPriv.h
+++ b/src/core/SkImageInfoPriv.h
@@ -39,7 +39,8 @@ 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.  Do we overwrite the input color table?
+ *      We will not convert to kIndex8 unless |src| is kIndex8.  This is possible only
+ *      in some cases and likley inefficient.
  *      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.
@@ -56,7 +57,7 @@ static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkIm
         return false;
     }
 
-    if (kIndex_8_SkColorType == dst.colorType()) {
+    if (kIndex_8_SkColorType == dst.colorType() && kIndex_8_SkColorType != src.colorType()) {
         return false;
     }
 
diff --git a/src/core/SkPixmap.cpp b/src/core/SkPixmap.cpp
index c0889cfd84..1e24b93a60 100644
--- a/src/core/SkPixmap.cpp
+++ b/src/core/SkPixmap.cpp
@@ -16,7 +16,6 @@
 #include "SkNx.h"
 #include "SkPM4f.h"
 #include "SkPixmap.h"
-#include "SkReadPixelsRec.h"
 #include "SkSurface.h"
 #include "SkUtils.h"
 
@@ -84,20 +83,37 @@ bool SkPixmap::extractSubset(SkPixmap* result, const SkIRect& subset) const {
     return true;
 }
 
-bool SkPixmap::readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB, int x, int y)
-const {
-    if (!SkImageInfoValidConversion(dstInfo, fInfo)) {
+bool SkPixmap::readPixels(const SkImageInfo& requestedDstInfo, void* dstPixels, size_t dstRB,
+                          int x, int y) const {
+    if (!SkImageInfoValidConversion(requestedDstInfo, fInfo)) {
         return false;
     }
 
-    SkReadPixelsRec rec(dstInfo, dstPixels, dstRB, x, y);
-    if (!rec.trim(fInfo.width(), fInfo.height())) {
+    if (nullptr == dstPixels || dstRB < requestedDstInfo.minRowBytes()) {
         return false;
     }
 
-    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,
+    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,
                                    srcInfo, srcPixels, this->rowBytes(), this->ctable());
 }
 
diff --git a/src/core/SkWritePixelsRec.h b/src/core/SkWritePixelsRec.h
deleted file mode 100644
index 652a13a822..0000000000
--- a/src/core/SkWritePixelsRec.h
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * 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
diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp
index c5ade9e7b1..85eb671206 100644
--- a/src/gpu/SkGpuDevice.cpp
+++ b/src/gpu/SkGpuDevice.cpp
@@ -35,7 +35,6 @@
 #include "SkPictureData.h"
 #include "SkRRect.h"
 #include "SkRasterClip.h"
-#include "SkReadPixelsRec.h"
 #include "SkRecord.h"
 #include "SkSpecialImage.h"
 #include "SkStroke.h"
@@ -44,7 +43,6 @@
 #include "SkTLazy.h"
 #include "SkUtils.h"
 #include "SkVertState.h"
-#include "SkWritePixelsRec.h"
 #include "effects/GrBicubicEffect.h"
 #include "effects/GrSimpleTextureEffect.h"
 #include "effects/GrTextureDomain.h"
@@ -201,12 +199,7 @@ bool SkGpuDevice::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size
         return false;
     }
 
-    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);
+    return fRenderTargetContext->readPixels(dstInfo, dstPixels, dstRowBytes, x, y);
 }
 
 bool SkGpuDevice::onWritePixels(const SkImageInfo& srcInfo, const void* srcPixels,
@@ -217,12 +210,7 @@ bool SkGpuDevice::onWritePixels(const SkImageInfo& srcInfo, const void* srcPixel
         return false;
     }
 
-    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);
+    return fRenderTargetContext->writePixels(srcInfo, srcPixels, srcRowBytes, x, y);
 }
 
 bool SkGpuDevice::onAccessPixels(SkPixmap* pmap) {
diff --git a/src/image/SkImage.cpp b/src/image/SkImage.cpp
index 8efe91a706..137f9166ba 100644
--- a/src/image/SkImage.cpp
+++ b/src/image/SkImage.cpp
@@ -51,7 +51,11 @@ bool SkImage::peekPixels(SkPixmap* pm) const {
 
 bool SkImage::readPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRowBytes,
                            int srcX, int srcY, CachingHint chint) const {
-    return as_IB(this)->onReadPixels(dstInfo, dstPixels, dstRowBytes, srcX, srcY, chint);
+    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);
 }
 
 bool SkImage::scalePixels(const SkPixmap& dst, SkFilterQuality quality, CachingHint chint) const {
diff --git a/src/image/SkImage_Gpu.cpp b/src/image/SkImage_Gpu.cpp
index fb93350039..7247ae36fa 100644
--- a/src/image/SkImage_Gpu.cpp
+++ b/src/image/SkImage_Gpu.cpp
@@ -27,7 +27,6 @@
 #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)
@@ -129,26 +128,20 @@ static void apply_premul(const SkImageInfo& info, void* pixels, size_t rowBytes)
     }
 }
 
-bool SkImage_Gpu::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size_t dstRB,
+bool SkImage_Gpu::onReadPixels(const SkImageInfo& info, void* pixels, size_t rowBytes,
                                int srcX, int srcY, CachingHint) const {
-    if (!SkImageInfoValidConversion(dstInfo, this->onImageInfo())) {
+    if (!SkImageInfoValidConversion(info, this->onImageInfo())) {
         return false;
     }
 
-    SkReadPixelsRec rec(dstInfo, dstPixels, dstRB, srcX, srcY);
-    if (!rec.trim(this->width(), this->height())) {
-        return false;
-    }
-
-    GrPixelConfig config = SkImageInfo2GrPixelConfig(rec.fInfo, *fTexture->getContext()->caps());
+    GrPixelConfig config = SkImageInfo2GrPixelConfig(info, *fTexture->getContext()->caps());
     uint32_t flags = 0;
-    if (kUnpremul_SkAlphaType == rec.fInfo.alphaType() && kPremul_SkAlphaType == fAlphaType) {
+    if (kUnpremul_SkAlphaType == info.alphaType() && kPremul_SkAlphaType == fAlphaType) {
         // let the GPU perform this transformation for us
         flags = GrContext::kUnpremul_PixelOpsFlag;
     }
-    if (!fTexture->readPixels(fColorSpace.get(), rec.fX, rec.fY, rec.fInfo.width(),
-                              rec.fInfo.height(), config, rec.fInfo.colorSpace(), rec.fPixels,
-                              rec.fRowBytes, flags)) {
+    if (!fTexture->readPixels(fColorSpace.get(), srcX, srcY, info.width(), info.height(), config,
+                              info.colorSpace(), pixels, rowBytes, flags)) {
         return false;
     }
     // do we have to manually fix-up the alpha channel?
@@ -159,8 +152,8 @@ bool SkImage_Gpu::onReadPixels(const SkImageInfo& dstInfo, void* dstPixels, size
     //
     // Should this be handled by Ganesh? todo:?
     //
-    if (kPremul_SkAlphaType == rec.fInfo.alphaType() && kUnpremul_SkAlphaType == fAlphaType) {
-        apply_premul(rec.fInfo, rec.fPixels, rec.fRowBytes);
+    if (kPremul_SkAlphaType == info.alphaType() && kUnpremul_SkAlphaType == fAlphaType) {
+        apply_premul(info, pixels, rowBytes);
     }
     return true;
 }
diff --git a/tests/BitmapCopyTest.cpp b/tests/BitmapCopyTest.cpp
index d69f7df76e..ab0ec30e16 100644
--- a/tests/BitmapCopyTest.cpp
+++ b/tests/BitmapCopyTest.cpp
@@ -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,     "010111"  },
+    { kIndex_8_SkColorType,     "011111"  },
     { kRGB_565_SkColorType,     "010101"  },
     { kARGB_4444_SkColorType,   "010111"  },
     { kN32_SkColorType,         "010111"  },
@@ -235,8 +235,7 @@ DEF_TEST(BitmapCopy_extractSubset, reporter) {
                 if (!success) {
                     // Skip checking that success matches fValid, which is redundant
                     // with the code below.
-                    REPORTER_ASSERT(reporter, kIndex_8_SkColorType == gPairs[i].fColorType ||
-                                              gPairs[i].fColorType != gPairs[j].fColorType);
+                    REPORTER_ASSERT(reporter, gPairs[i].fColorType != gPairs[j].fColorType);
                     continue;
                 }