Support GL_RGB textures and render targets.

Bug= skia:7533

Change-Id: Iba30e90dbf2574368b773bb5cf2ebd5219559717
Reviewed-on: https://skia-review.googlesource.com/108188
Commit-Queue: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>

17 files changed, 161 insertions, 275 deletions

diff --git a/src/core/SkGpuBlurUtils.cpp b/src/core/SkGpuBlurUtils.cpp
index b42bb04782..fe614bd0df 100644
--- a/src/core/SkGpuBlurUtils.cpp
+++ b/src/core/SkGpuBlurUtils.cpp
@@ -233,10 +233,10 @@ sk_sp<GrRenderTargetContext> GaussianBlur(GrContext* context,
     }
 
     SkASSERT(kBGRA_8888_GrPixelConfig == config || kRGBA_8888_GrPixelConfig == config ||
-             kRGBA_4444_GrPixelConfig == config || kRGB_565_GrPixelConfig == config ||
-             kSRGBA_8888_GrPixelConfig == config || kSBGRA_8888_GrPixelConfig == config ||
-             kRGBA_half_GrPixelConfig == config || kAlpha_8_GrPixelConfig == config ||
-             kRGBA_1010102_GrPixelConfig == config);
+             kRGB_888_GrPixelConfig == config || kRGBA_4444_GrPixelConfig == config ||
+             kRGB_565_GrPixelConfig == config || kSRGBA_8888_GrPixelConfig == config ||
+             kSBGRA_8888_GrPixelConfig == config || kRGBA_half_GrPixelConfig == config ||
+             kAlpha_8_GrPixelConfig == config || kRGBA_1010102_GrPixelConfig == config);
 
     const int width = dstBounds.width();
     const int height = dstBounds.height();
diff --git a/src/core/SkImageInfo.cpp b/src/core/SkImageInfo.cpp
index f9933736e3..fd5e8087ba 100644
--- a/src/core/SkImageInfo.cpp
+++ b/src/core/SkImageInfo.cpp
@@ -45,16 +45,7 @@ enum Stored_SkColorType {
 };
 
 bool SkColorTypeIsAlwaysOpaque(SkColorType ct) {
-    switch (ct) {
-        case kRGB_565_SkColorType:
-        case kRGB_888x_SkColorType:
-        case kRGB_101010x_SkColorType:
-        case kGray_8_SkColorType:
-            return true;
-        default:
-            break;
-    }
-    return false;
+    return !(kAlpha_SkColorTypeComponentFlag & SkColorTypeComponentFlags(ct));
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkImageInfoPriv.h b/src/core/SkImageInfoPriv.h
deleted file mode 100644
index 04779ff3fb..0000000000
--- a/src/core/SkImageInfoPriv.h
+++ /dev/null
@@ -1,179 +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 SkImageInfoPriv_DEFINED
-#define SkImageInfoPriv_DEFINED
-
-#include "SkImageInfo.h"
-
-enum class SkDestinationSurfaceColorMode {
-    kLegacy,
-    kGammaAndColorSpaceAware,
-};
-
-static inline bool SkAlphaTypeIsValid(unsigned value) {
-    return value <= kLastEnum_SkAlphaType;
-}
-
-static int SkColorTypeShiftPerPixel(SkColorType ct) {
-    switch (ct) {
-        case kUnknown_SkColorType:      return 0;
-        case kAlpha_8_SkColorType:      return 0;
-        case kRGB_565_SkColorType:      return 1;
-        case kARGB_4444_SkColorType:    return 1;
-        case kRGBA_8888_SkColorType:    return 2;
-        case kRGB_888x_SkColorType:     return 2;
-        case kBGRA_8888_SkColorType:    return 2;
-        case kRGBA_1010102_SkColorType: return 2;
-        case kRGB_101010x_SkColorType:  return 2;
-        case kGray_8_SkColorType:       return 0;
-        case kRGBA_F16_SkColorType:     return 3;
-    }
-    return 0;
-}
-
-static inline size_t SkColorTypeMinRowBytes(SkColorType ct, int width) {
-    return width * SkColorTypeBytesPerPixel(ct);
-}
-
-static inline bool SkColorTypeIsValid(unsigned value) {
-    return value <= kLastEnum_SkColorType;
-}
-
-static inline size_t SkColorTypeComputeOffset(SkColorType ct, int x, int y, size_t rowBytes) {
-    if (kUnknown_SkColorType == ct) {
-        return 0;
-    }
-    return y * rowBytes + (x << SkColorTypeShiftPerPixel(ct));
-}
-
-/**
- *  This contains shared checks on SkImageInfo.  Depending on the desired color space behavior,
- *  the caller should choose one of the two versions below.
- */
-static inline bool SkImageInfoIsValidCommon(const SkImageInfo& info) {
-    if (info.width() <= 0 || info.height() <= 0) {
-        return false;
-    }
-
-    const int kMaxDimension = SK_MaxS32 >> 2;
-    if (info.width() > kMaxDimension || info.height() > kMaxDimension) {
-        return false;
-    }
-
-    if (kUnknown_SkColorType == info.colorType() || kUnknown_SkAlphaType == info.alphaType()) {
-        return false;
-    }
-
-    if (kOpaque_SkAlphaType != info.alphaType() &&
-       (kRGB_565_SkColorType == info.colorType() || kGray_8_SkColorType == info.colorType())) {
-        return false;
-    }
-
-    if (kRGBA_F16_SkColorType == info.colorType() &&
-       (info.colorSpace() && (!info.colorSpace()->gammaIsLinear()))) {
-        return false;
-    }
-
-    return true;
-}
-
-/**
- *  Returns true if |info| contains a valid combination of width, height, colorType, alphaType,
- *  colorSpace.  Allows numerical color spaces.  Returns false otherwise.
- */
-static inline bool SkImageInfoIsValidAllowNumericalCS(const SkImageInfo& info) {
-    if (!SkImageInfoIsValidCommon(info)) {
-        return false;
-    }
-
-    SkColorSpaceTransferFn fn;
-    if (info.colorSpace() && !info.colorSpace()->isNumericalTransferFn(&fn)) {
-        return false;
-    }
-
-    return true;
-}
-
-/**
- *  Returns true if |info| contains a valid combination of width, height, colorType, alphaType,
- *  colorSpace.  Only supports rendering color spaces.  Returns false otherwise.
- */
-static inline bool SkImageInfoIsValidRenderingCS(const SkImageInfo& info) {
-    if (!SkImageInfoIsValidCommon(info)) {
-        return false;
-    }
-
-    if (info.colorSpace() &&
-       (!info.colorSpace()->gammaCloseToSRGB() && !info.colorSpace()->gammaIsLinear())) {
-        return false;
-    }
-
-    return true;
-}
-
-/**
- *  Returns true if |info| contains a valid combination of width, height, colorType, alphaType,
- *  colorSpace.  Uses |colorMode| to decide how to treat color spaces.
- */
-static inline bool SkImageInfoIsValid(const SkImageInfo& info,
-                                      SkDestinationSurfaceColorMode colorMode) {
-    if (SkDestinationSurfaceColorMode::kGammaAndColorSpaceAware == colorMode) {
-        return SkImageInfoIsValidRenderingCS(info);
-    }
-
-    return SkImageInfoIsValidAllowNumericalCS(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 it exactly matches the src, since color tables
- *      are immutable.
- *      We do not convert to kGray8 when the |src| is not kGray8 in the same color space.
- *      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.
- *      We will not convert from kAlpha8 when the |dst| is not kAlpha8.  This would require
- *      inventing color information.
- *      We will not convert to kOpaque when the |src| is not kOpaque.  This could be
- *      implemented to set all the alpha values to 1, but there is still some ambiguity -
- *      should we use kPremul or kUnpremul color values with the opaque alphas?  Or should
- *      we just use whatever the |src| alpha is?  In the future, we could choose to clearly
- *      define this, but currently no one is asking for this feature.
- *      We will not convert to a particular color space if |src| is nullptr.  The color space
- *      conversion is not well-defined.
- */
-static inline bool SkImageInfoValidConversion(const SkImageInfo& dst, const SkImageInfo& src) {
-    if (!SkImageInfoIsValidAllowNumericalCS(dst) || !SkImageInfoIsValidAllowNumericalCS(src)) {
-        return false;
-    }
-
-    if (kGray_8_SkColorType == dst.colorType()) {
-        if (kGray_8_SkColorType != src.colorType()) {
-            return false;
-        }
-
-        if (dst.colorSpace() && !SkColorSpace::Equals(dst.colorSpace(), src.colorSpace())) {
-            return false;
-        }
-    }
-
-    if (kAlpha_8_SkColorType != dst.colorType() && kAlpha_8_SkColorType == src.colorType()) {
-        return false;
-    }
-
-    if (kOpaque_SkAlphaType == dst.alphaType() && kOpaque_SkAlphaType != src.alphaType()) {
-        return false;
-    }
-
-    if (dst.colorSpace() && !src.colorSpace()) {
-        return false;
-    }
-
-    return true;
-}
-#endif  // SkImageInfoPriv_DEFINED
diff --git a/src/gpu/GrCaps.cpp b/src/gpu/GrCaps.cpp
index 1d25c1a791..0507a3fdfb 100644
--- a/src/gpu/GrCaps.cpp
+++ b/src/gpu/GrCaps.cpp
@@ -24,6 +24,7 @@ static const char* pixel_config_name(GrPixelConfig config) {
         case kRGB_565_GrPixelConfig: return "RGB565";
         case kRGBA_4444_GrPixelConfig: return "RGBA444";
         case kRGBA_8888_GrPixelConfig: return "RGBA8888";
+        case kRGB_888_GrPixelConfig: return "RGB888";
         case kBGRA_8888_GrPixelConfig: return "BGRA8888";
         case kSRGBA_8888_GrPixelConfig: return "SRGBA8888";
         case kSBGRA_8888_GrPixelConfig: return "SBGRA8888";
diff --git a/src/gpu/GrContext.cpp b/src/gpu/GrContext.cpp
index 626b3f6ecd..3baf0b9737 100644
--- a/src/gpu/GrContext.cpp
+++ b/src/gpu/GrContext.cpp
@@ -358,6 +358,7 @@ static bool valid_premul_config(GrPixelConfig config) {
         case kRGB_565_GrPixelConfig:            return false;
         case kRGBA_4444_GrPixelConfig:          return true;
         case kRGBA_8888_GrPixelConfig:          return true;
+        case kRGB_888_GrPixelConfig:            return false;
         case kBGRA_8888_GrPixelConfig:          return true;
         case kSRGBA_8888_GrPixelConfig:         return true;
         case kSBGRA_8888_GrPixelConfig:         return true;
@@ -383,6 +384,7 @@ static bool valid_premul_color_type(GrColorType ct) {
         case GrColorType::kRGB_565:      return false;
         case GrColorType::kABGR_4444:    return true;
         case GrColorType::kRGBA_8888:    return true;
+        case GrColorType::kRGB_888x:     return false;
         case GrColorType::kBGRA_8888:    return true;
         case GrColorType::kRGBA_1010102: return true;
         case GrColorType::kGray_8:       return false;
@@ -402,7 +404,6 @@ static bool valid_pixel_conversion(GrColorType cpuColorType, GrPixelConfig gpuCo
         (!valid_premul_color_type(cpuColorType) || !valid_premul_config(gpuConfig))) {
         return false;
     }
-
     return true;
 }
 
@@ -471,10 +472,20 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceContext* dst, int left, int top,
                                        int height, GrColorType srcColorType,
                                        SkColorSpace* srcColorSpace, const void* buffer,
                                        size_t rowBytes, uint32_t pixelOpsFlags) {
-#ifndef SK_LEGACY_GPU_PIXEL_OPS
-    return this->writeSurfacePixels2(dst, left, top, width, height, srcColorType, srcColorSpace,
-                                     buffer, rowBytes, pixelOpsFlags);
+    bool useLegacyPath = false;
+#ifdef SK_LEGACY_GPU_PIXEL_OPS
+    useLegacyPath = true;
 #endif
+    // Newly added color types/configs are only supported by the new code path.
+    if (srcColorType == GrColorType::kRGB_888x ||
+        dst->asSurfaceProxy()->config() == kRGB_888_GrPixelConfig) {
+        useLegacyPath = false;
+    }
+
+    if (!useLegacyPath) {
+        return this->writeSurfacePixels2(dst, left, top, width, height, srcColorType, srcColorSpace,
+                                         buffer, rowBytes, pixelOpsFlags);
+    }
 
     // TODO: Color space conversion
 
@@ -497,6 +508,14 @@ bool GrContextPriv::writeSurfacePixels(GrSurfaceContext* dst, int left, int top,
     if (!valid_pixel_conversion(srcColorType, dstProxy->config(), premul)) {
         return false;
     }
+    // There is no way to store alpha values in the dst.
+    if (GrColorTypeHasAlpha(srcColorType) && GrPixelConfigIsOpaque(dstProxy->config())) {
+        return false;
+    }
+    // The source has no alpha value and the dst is only alpha
+    if (!GrColorTypeHasAlpha(srcColorType) && GrPixelConfigIsAlphaOnly(dstProxy->config())) {
+        return false;
+    }
 
     // We need to guarantee round-trip conversion if we are reading and writing 8888 non-sRGB data,
     // without any color spaces attached, and the caller wants us to premul.
@@ -612,10 +631,21 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, int left, int top,
                                       int height, GrColorType dstColorType,
                                       SkColorSpace* dstColorSpace, void* buffer, size_t rowBytes,
                                       uint32_t flags) {
-#ifndef SK_LEGACY_GPU_PIXEL_OPS
-    return this->readSurfacePixels2(src, left, top, width, height, dstColorType, dstColorSpace,
-                                    buffer, rowBytes, flags);
+    bool useLegacyPath = false;
+#ifdef SK_LEGACY_GPU_PIXEL_OPS
+    useLegacyPath = true;
 #endif
+    // Newly added color types/configs are only supported by the new code path.
+    if (dstColorType == GrColorType::kRGB_888x ||
+        src->asSurfaceProxy()->config() == kRGB_888_GrPixelConfig) {
+        useLegacyPath = false;
+    }
+
+    if (!useLegacyPath) {
+        return this->readSurfacePixels2(src, left, top, width, height, dstColorType, dstColorSpace,
+                                        buffer, rowBytes, flags);
+    }
+
     // TODO: Color space conversion
 
     ASSERT_SINGLE_OWNER_PRIV
@@ -642,6 +672,18 @@ bool GrContextPriv::readSurfacePixels(GrSurfaceContext* src, int left, int top,
     if (!valid_pixel_conversion(dstColorType, srcProxy->config(), unpremul)) {
         return false;
     }
+    // The source is alpha-only but the dst is not. TODO: Make non-alpha channels in the dst be 0?
+    if (GrPixelConfigIsAlphaOnly(srcProxy->config()) && !GrColorTypeIsAlphaOnly(dstColorType)) {
+        return false;
+    }
+    // The source has no alpha, the dst is alpha-only. TODO: set all values in dst to 1?
+    if (GrPixelConfigIsOpaque(srcProxy->config()) && GrColorTypeIsAlphaOnly(dstColorType)) {
+        return false;
+    }
+    // Not clear if we should unpremul in this case.
+    if (!GrPixelConfigIsOpaque(srcProxy->config()) && !GrColorTypeHasAlpha(dstColorType)) {
+        return false;
+    }
 
     // We need to guarantee round-trip conversion if we are reading and writing 8888 non-sRGB data,
     // without any color spaces attached, and the caller wants us to unpremul.
@@ -913,10 +955,6 @@ bool GrContextPriv::readSurfacePixels2(GrSurfaceContext* src, int left, int top,
                                        int height, GrColorType dstColorType,
                                        SkColorSpace* dstColorSpace, void* buffer, size_t rowBytes,
                                        uint32_t flags) {
-    SkASSERT(!(flags & kDontFlush_PixelOpsFlag));
-    if (flags & kDontFlush_PixelOpsFlag){
-        return false;
-    }
     ASSERT_SINGLE_OWNER_PRIV
     RETURN_FALSE_IF_ABANDONED_PRIV
     SkASSERT(src);
@@ -924,6 +962,11 @@ bool GrContextPriv::readSurfacePixels2(GrSurfaceContext* src, int left, int top,
     ASSERT_OWNED_PROXY_PRIV(src->asSurfaceProxy());
     GR_CREATE_TRACE_MARKER_CONTEXT("GrContextPriv", "readSurfacePixels2", fContext);
 
+    SkASSERT(!(flags & kDontFlush_PixelOpsFlag));
+    if (flags & kDontFlush_PixelOpsFlag) {
+        return false;
+    }
+
     // MDB TODO: delay this instantiation until later in the method
     if (!src->asSurfaceProxy()->instantiate(this->resourceProvider())) {
         return false;
diff --git a/src/gpu/GrSurfaceContext.cpp b/src/gpu/GrSurfaceContext.cpp
index ab6c05a375..37d0b71fb0 100644
--- a/src/gpu/GrSurfaceContext.cpp
+++ b/src/gpu/GrSurfaceContext.cpp
@@ -49,7 +49,8 @@ bool GrSurfaceContext::readPixels(const SkImageInfo& dstInfo, void* dstBuffer,
     GR_AUDIT_TRAIL_AUTO_FRAME(fAuditTrail, "GrSurfaceContext::readPixels");
 
     // TODO: this seems to duplicate code in SkImage_Gpu::onReadPixels
-    if (kUnpremul_SkAlphaType == dstInfo.alphaType()) {
+    if (kUnpremul_SkAlphaType == dstInfo.alphaType() &&
+        !GrPixelConfigIsOpaque(this->asSurfaceProxy()->config())) {
         flags |= GrContextPriv::kUnpremul_PixelOpsFlag;
     }
     auto colorType = SkColorTypeToGrColorType(dstInfo.colorType());
diff --git a/src/gpu/SkGr.cpp b/src/gpu/SkGr.cpp
index 27c9b39f5e..09155a05b3 100644
--- a/src/gpu/SkGr.cpp
+++ b/src/gpu/SkGr.cpp
@@ -264,7 +264,7 @@ GrPixelConfig SkImageInfo2GrPixelConfig(const SkColorType type, SkColorSpace* cs
         // TODO: We're checking for srgbSupport, but we can then end up picking sBGRA as our pixel
         // config (which may not be supported). We need a better test here.
         case kRGB_888x_SkColorType:
-            return kUnknown_GrPixelConfig;
+            return kRGB_888_GrPixelConfig;
         case kBGRA_8888_SkColorType:
             return (caps.srgbSupport() && cs && cs->gammaCloseToSRGB())
                    ? kSBGRA_8888_GrPixelConfig : kBGRA_8888_GrPixelConfig;
@@ -286,49 +286,14 @@ GrPixelConfig SkImageInfo2GrPixelConfig(const SkImageInfo& info, const GrCaps& c
 }
 
 bool GrPixelConfigToColorType(GrPixelConfig config, SkColorType* ctOut) {
-    SkColorType ct;
-    switch (config) {
-        case kAlpha_8_GrPixelConfig: // fall through
-        case kAlpha_8_as_Alpha_GrPixelConfig: // fall through
-        case kAlpha_8_as_Red_GrPixelConfig:
-            ct = kAlpha_8_SkColorType;
-            break;
-        case kGray_8_GrPixelConfig: // fall through
-        case kGray_8_as_Lum_GrPixelConfig: // fall through
-        case kGray_8_as_Red_GrPixelConfig:
-            ct = kGray_8_SkColorType;
-            break;
-        case kRGB_565_GrPixelConfig:
-            ct = kRGB_565_SkColorType;
-            break;
-        case kRGBA_4444_GrPixelConfig:
-            ct = kARGB_4444_SkColorType;
-            break;
-        case kRGBA_8888_GrPixelConfig:
-            ct = kRGBA_8888_SkColorType;
-            break;
-        case kBGRA_8888_GrPixelConfig:
-            ct = kBGRA_8888_SkColorType;
-            break;
-        case kSRGBA_8888_GrPixelConfig:
-            ct = kRGBA_8888_SkColorType;
-            break;
-        case kSBGRA_8888_GrPixelConfig:
-            ct = kBGRA_8888_SkColorType;
-            break;
-        case kRGBA_1010102_GrPixelConfig:
-            ct = kRGBA_1010102_SkColorType;
-            break;
-        case kRGBA_half_GrPixelConfig:
-            ct = kRGBA_F16_SkColorType;
-            break;
-        default:
-            return false;
-    }
-    if (ctOut) {
-        *ctOut = ct;
+    SkColorType ct = GrColorTypeToSkColorType(GrPixelConfigToColorType(config));
+    if (kUnknown_SkColorType != ct) {
+        if (ctOut) {
+            *ctOut = ct;
+        }
+        return true;
     }
-    return true;
+    return false;
 }
 
 GrPixelConfig GrRenderableConfigForColorSpace(const SkColorSpace* colorSpace) {
diff --git a/src/gpu/effects/GrDitherEffect.fp b/src/gpu/effects/GrDitherEffect.fp
index 7cf07da311..c51865e38b 100644
--- a/src/gpu/effects/GrDitherEffect.fp
+++ b/src/gpu/effects/GrDitherEffect.fp
@@ -16,6 +16,7 @@ layout(key) in int rangeType;
             case kGray_8_as_Lum_GrPixelConfig:
             case kGray_8_as_Red_GrPixelConfig:
             case kRGBA_8888_GrPixelConfig:
+            case kRGB_888_GrPixelConfig:
             case kBGRA_8888_GrPixelConfig:
             case kSRGBA_8888_GrPixelConfig:
             case kSBGRA_8888_GrPixelConfig:
diff --git a/src/gpu/effects/GrDitherEffect.h b/src/gpu/effects/GrDitherEffect.h
index 2e650a68f4..d1bc72bf4e 100644
--- a/src/gpu/effects/GrDitherEffect.h
+++ b/src/gpu/effects/GrDitherEffect.h
@@ -25,6 +25,7 @@ public:
             case kGray_8_as_Lum_GrPixelConfig:
             case kGray_8_as_Red_GrPixelConfig:
             case kRGBA_8888_GrPixelConfig:
+            case kRGB_888_GrPixelConfig:
             case kBGRA_8888_GrPixelConfig:
             case kSRGBA_8888_GrPixelConfig:
             case kSBGRA_8888_GrPixelConfig:
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index e05a4bb970..9af54b1dec 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -1167,8 +1167,8 @@ void GrGLCaps::onDumpJSON(SkJSONWriter* writer) const {
         writer->appendHexU32("flags", fConfigTable[i].fFlags);
         writer->appendHexU32("b_internal", fConfigTable[i].fFormats.fBaseInternalFormat);
         writer->appendHexU32("s_internal", fConfigTable[i].fFormats.fSizedInternalFormat);
-        writer->appendHexU32("e_format",
-                             fConfigTable[i].fFormats.fExternalFormat[kOther_ExternalFormatUsage]);
+        writer->appendHexU32("e_format_read_pixels",
+                             fConfigTable[i].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage]);
         writer->appendHexU32(
                 "e_format_teximage",
                 fConfigTable[i].fFormats.fExternalFormat[kTexImage_ExternalFormatUsage]);
@@ -1200,7 +1200,7 @@ bool GrGLCaps::getTexImageFormats(GrPixelConfig surfaceConfig, GrPixelConfig ext
 
 bool GrGLCaps::getReadPixelsFormat(GrPixelConfig surfaceConfig, GrPixelConfig externalConfig,
                                    GrGLenum* externalFormat, GrGLenum* externalType) const {
-    if (!this->getExternalFormat(surfaceConfig, externalConfig, kOther_ExternalFormatUsage,
+    if (!this->getExternalFormat(surfaceConfig, externalConfig, kReadPixels_ExternalFormatUsage,
                                  externalFormat, externalType)) {
         return false;
     }
@@ -1322,6 +1322,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     bool disableSRGBWriteControlForAdreno4xx = false;
     bool disableR8TexStorageForANGLEGL = false;
     bool disableSRGBRenderWithMSAAForMacAMD = false;
+    bool disableRGB8ForMali400 = false;
 
     if (!contextOptions.fDisableDriverCorrectnessWorkarounds) {
         // ARB_texture_rg is part of OpenGL 3.0, but osmesa doesn't support GL_RED
@@ -1348,6 +1349,8 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
 #if defined(SK_BUILD_FOR_MAC)
         disableSRGBRenderWithMSAAForMacAMD = kATI_GrGLVendor == ctxInfo.vendor();
 #endif
+        // Mali-400 fails ReadPixels tests, mostly with non-0xFF alpha values when read as GL_RGBA8.
+        disableRGB8ForMali400 = kMali4xx_GrGLRenderer == ctxInfo.renderer();
     }
 
     uint32_t nonMSAARenderFlags = ConfigInfo::kRenderable_Flag |
@@ -1386,14 +1389,14 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
 
     fConfigTable[kUnknown_GrPixelConfig].fFormats.fBaseInternalFormat = 0;
     fConfigTable[kUnknown_GrPixelConfig].fFormats.fSizedInternalFormat = 0;
-    fConfigTable[kUnknown_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = 0;
+    fConfigTable[kUnknown_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = 0;
     fConfigTable[kUnknown_GrPixelConfig].fFormats.fExternalType = 0;
     fConfigTable[kUnknown_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType;
     fConfigTable[kUnknown_GrPixelConfig].fSwizzle = GrSwizzle::RGBA();
 
     fConfigTable[kRGBA_8888_GrPixelConfig].fFormats.fBaseInternalFormat = GR_GL_RGBA;
     fConfigTable[kRGBA_8888_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGBA8;
-    fConfigTable[kRGBA_8888_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] =
+    fConfigTable[kRGBA_8888_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] =
         GR_GL_RGBA;
     fConfigTable[kRGBA_8888_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_BYTE;
     fConfigTable[kRGBA_8888_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType;
@@ -1415,7 +1418,41 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     }
     fConfigTable[kRGBA_8888_GrPixelConfig].fSwizzle = GrSwizzle::RGBA();
 
-    fConfigTable[kBGRA_8888_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] =
+    fConfigTable[kRGB_888_GrPixelConfig].fFormats.fBaseInternalFormat = GR_GL_RGB;
+    fConfigTable[kRGB_888_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGB8;
+    // Our external RGB data always has a byte where alpha would be. When calling read pixels we
+    // want to read to kRGB_888x color type and ensure that gets 0xFF written. Using GL_RGB would
+    // read back unaligned 24bit RGB color values. Note that this all a bit moot as we don't
+    // currently expect to ever read back GrColorType::kRGB_888x because our implementation of
+    // supportedReadPixelsColorType never returns it.
+    fConfigTable[kRGB_888_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RGBA;
+    fConfigTable[kRGB_888_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_BYTE;
+    fConfigTable[kRGB_888_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType;
+    fConfigTable[kRGB_888_GrPixelConfig].fFlags = ConfigInfo::kTextureable_Flag;
+    if (kGL_GrGLStandard == standard) {
+        // Even in OpenGL 4.6 GL_RGB8 is required to be color renderable but not required to be a
+        // supported render buffer format. Since we usually use render buffers for MSAA on non-ES GL
+        // we don't support MSAA for GL_RGB8. On 4.2+ we could check using
+        // glGetInternalFormativ(GL_RENDERBUFFER, GL_RGB8, GL_INTERNALFORMAT_SUPPORTED, ...) if this
+        // becomes an issue.
+        // This also would probably work in mixed-samples mode where there is no MSAA color buffer
+        // but we don't support that just for simplicity's sake.
+        fConfigTable[kRGB_888_GrPixelConfig].fFlags |= nonMSAARenderFlags;
+    } else {
+        // 3.0 and the extension support this as a render buffer format.
+        if (version >= GR_GL_VER(3, 0) || ctxInfo.hasExtension("GL_OES_rgb8_rgba8")) {
+            fConfigTable[kRGB_888_GrPixelConfig].fFlags |= allRenderFlags;
+        }
+    }
+    if (texStorageSupported) {
+        fConfigTable[kRGB_888_GrPixelConfig].fFlags |= ConfigInfo::kCanUseTexStorage_Flag;
+    }
+    fConfigTable[kRGB_888_GrPixelConfig].fSwizzle = GrSwizzle::RGBA();
+    if (disableRGB8ForMali400) {
+        fConfigTable[kRGB_888_GrPixelConfig].fFlags = 0;
+    }
+
+    fConfigTable[kBGRA_8888_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] =
         GR_GL_BGRA;
     fConfigTable[kBGRA_8888_GrPixelConfig].fFormats.fExternalType  = GR_GL_UNSIGNED_BYTE;
     fConfigTable[kBGRA_8888_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType;
@@ -1527,7 +1564,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     fConfigTable[kSRGBA_8888_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_SRGB8_ALPHA8;
     // GL does not do srgb<->rgb conversions when transferring between cpu and gpu. Thus, the
     // external format is GL_RGBA. See below for note about ES2.0 and glTex[Sub]Image.
-    fConfigTable[kSRGBA_8888_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] =
+    fConfigTable[kSRGBA_8888_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] =
         GR_GL_RGBA;
     fConfigTable[kSRGBA_8888_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_BYTE;
     fConfigTable[kSRGBA_8888_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType;
@@ -1547,7 +1584,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     fConfigTable[kSBGRA_8888_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_SRGB8_ALPHA8;
     // GL does not do srgb<->rgb conversions when transferring between cpu and gpu. Thus, the
     // external format is GL_BGRA.
-    fConfigTable[kSBGRA_8888_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] =
+    fConfigTable[kSBGRA_8888_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] =
         GR_GL_BGRA;
     fConfigTable[kSBGRA_8888_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_BYTE;
     fConfigTable[kSBGRA_8888_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType;
@@ -1567,7 +1604,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     } else {
         fConfigTable[kRGB_565_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGB5;
     }
-    fConfigTable[kRGB_565_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] =
+    fConfigTable[kRGB_565_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] =
         GR_GL_RGB;
     fConfigTable[kRGB_565_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_SHORT_5_6_5;
     fConfigTable[kRGB_565_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType;
@@ -1593,7 +1630,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
 
     fConfigTable[kRGBA_4444_GrPixelConfig].fFormats.fBaseInternalFormat = GR_GL_RGBA;
     fConfigTable[kRGBA_4444_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGBA4;
-    fConfigTable[kRGBA_4444_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] =
+    fConfigTable[kRGBA_4444_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] =
         GR_GL_RGBA;
     fConfigTable[kRGBA_4444_GrPixelConfig].fFormats.fExternalType = GR_GL_UNSIGNED_SHORT_4_4_4_4;
     fConfigTable[kRGBA_4444_GrPixelConfig].fFormatType = kNormalizedFixedPoint_FormatType;
@@ -1612,7 +1649,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
 
     fConfigTable[kRGBA_1010102_GrPixelConfig].fFormats.fBaseInternalFormat = GR_GL_RGBA;
     fConfigTable[kRGBA_1010102_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGB10_A2;
-    fConfigTable[kRGBA_1010102_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] =
+    fConfigTable[kRGBA_1010102_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] =
         GR_GL_RGBA;
     fConfigTable[kRGBA_1010102_GrPixelConfig].fFormats.fExternalType =
         GR_GL_UNSIGNED_INT_2_10_10_10_REV;
@@ -1640,7 +1677,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     }
     alphaInfo.fFormats.fBaseInternalFormat = GR_GL_ALPHA;
     alphaInfo.fFormats.fSizedInternalFormat = GR_GL_ALPHA8;
-    alphaInfo.fFormats.fExternalFormat[kOther_ExternalFormatUsage] = GR_GL_ALPHA;
+    alphaInfo.fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_ALPHA;
     alphaInfo.fSwizzle = GrSwizzle::AAAA();
     if (fAlpha8IsRenderable && alpha8IsValidForGL) {
         alphaInfo.fFlags |= allRenderFlags;
@@ -1651,7 +1688,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     redInfo.fFormatType = kNormalizedFixedPoint_FormatType;
     redInfo.fFormats.fBaseInternalFormat = GR_GL_RED;
     redInfo.fFormats.fSizedInternalFormat = GR_GL_R8;
-    redInfo.fFormats.fExternalFormat[kOther_ExternalFormatUsage] = GR_GL_RED;
+    redInfo.fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RED;
     redInfo.fSwizzle = GrSwizzle::RRRR();
 
     // ES2 Command Buffer does not allow TexStorage with R8_EXT (so Alpha_8 and Gray_8)
@@ -1680,7 +1717,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     grayLumInfo.fFormatType = kNormalizedFixedPoint_FormatType;
     grayLumInfo.fFormats.fBaseInternalFormat = GR_GL_LUMINANCE;
     grayLumInfo.fFormats.fSizedInternalFormat = GR_GL_LUMINANCE8;
-    grayLumInfo.fFormats.fExternalFormat[kOther_ExternalFormatUsage] = GR_GL_LUMINANCE;
+    grayLumInfo.fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_LUMINANCE;
     grayLumInfo.fSwizzle = GrSwizzle::RGBA();
     if ((standard == kGL_GrGLStandard && version <= GR_GL_VER(3, 0)) ||
         (standard == kGLES_GrGLStandard && version < GR_GL_VER(3, 0))) {
@@ -1692,7 +1729,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     grayRedInfo.fFormatType = kNormalizedFixedPoint_FormatType;
     grayRedInfo.fFormats.fBaseInternalFormat = GR_GL_RED;
     grayRedInfo.fFormats.fSizedInternalFormat = GR_GL_R8;
-    grayRedInfo.fFormats.fExternalFormat[kOther_ExternalFormatUsage] = GR_GL_RED;
+    grayRedInfo.fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RED;
     grayRedInfo.fSwizzle = GrSwizzle::RRRA();
     grayRedInfo.fFlags = ConfigInfo::kTextureable_Flag;
 
@@ -1762,7 +1799,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
         fConfigTable[fpconfig].fFormats.fBaseInternalFormat = format;
         fConfigTable[fpconfig].fFormats.fSizedInternalFormat =
             kRGBA_float_GrPixelConfig == fpconfig ? GR_GL_RGBA32F : GR_GL_RG32F;
-        fConfigTable[fpconfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] = format;
+        fConfigTable[fpconfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = format;
         fConfigTable[fpconfig].fFormats.fExternalType = GR_GL_FLOAT;
         fConfigTable[fpconfig].fFormatType = kFloat_FormatType;
         if (hasFPTextures) {
@@ -1794,7 +1831,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     redHalf.fFormatType = kFloat_FormatType;
     redHalf.fFormats.fBaseInternalFormat = GR_GL_RED;
     redHalf.fFormats.fSizedInternalFormat = GR_GL_R16F;
-    redHalf.fFormats.fExternalFormat[kOther_ExternalFormatUsage] = GR_GL_RED;
+    redHalf.fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] = GR_GL_RED;
     redHalf.fSwizzle = GrSwizzle::RRRR();
     if (textureRedSupport && hasHalfFPTextures) {
         redHalf.fFlags = ConfigInfo::kTextureable_Flag;
@@ -1816,7 +1853,7 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
 
     fConfigTable[kRGBA_half_GrPixelConfig].fFormats.fBaseInternalFormat = GR_GL_RGBA;
     fConfigTable[kRGBA_half_GrPixelConfig].fFormats.fSizedInternalFormat = GR_GL_RGBA16F;
-    fConfigTable[kRGBA_half_GrPixelConfig].fFormats.fExternalFormat[kOther_ExternalFormatUsage] =
+    fConfigTable[kRGBA_half_GrPixelConfig].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage] =
         GR_GL_RGBA;
     if (kGL_GrGLStandard == ctxInfo.standard() || ctxInfo.version() >= GR_GL_VER(3, 0)) {
         fConfigTable[kRGBA_half_GrPixelConfig].fFormats.fExternalType = GR_GL_HALF_FLOAT;
@@ -1850,10 +1887,10 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
     bool useSizedRbFormats = kGLES_GrGLStandard == ctxInfo.standard();
 
     for (int i = 0; i < kGrPixelConfigCnt; ++i) {
-        // Almost always we want to pass fExternalFormat[kOther_ExternalFormatUsage] as the <format>
-        // param to glTex[Sub]Image.
+        // Almost always we want to pass fExternalFormat[kReadPixels_ExternalFormatUsage] as the
+        // <format> param to glTex[Sub]Image.
         fConfigTable[i].fFormats.fExternalFormat[kTexImage_ExternalFormatUsage] =
-            fConfigTable[i].fFormats.fExternalFormat[kOther_ExternalFormatUsage];
+            fConfigTable[i].fFormats.fExternalFormat[kReadPixels_ExternalFormatUsage];
         fConfigTable[i].fFormats.fInternalFormatTexImage = useSizedTexFormats ?
             fConfigTable[i].fFormats.fSizedInternalFormat :
             fConfigTable[i].fFormats.fBaseInternalFormat;
@@ -1888,6 +1925,12 @@ void GrGLCaps::initConfigTable(const GrContextOptions& contextOptions,
         // unsupported. (If we have no sRGB support at all, this will get overwritten below).
         fConfigTable[kSBGRA_8888_GrPixelConfig].fFlags = 0;
     }
+    // On ES 2.0 we have to use GL_RGB with glTexImage as the internal/external formats must
+    // be the same. Moreover, if we write kRGB_888x data to a texture format on non-ES2 we want to
+    // be sure that we write 1 for alpha not whatever happens to be in the client provided the 'x'
+    // slot.
+    fConfigTable[kRGB_888_GrPixelConfig].fFormats.fExternalFormat[kTexImage_ExternalFormatUsage] =
+        GR_GL_RGB;
 
     // If BGRA is supported as an internal format it must always be specified to glTex[Sub]Image
     // as a base format.
@@ -2561,7 +2604,10 @@ bool validate_sized_format(GrGLenum format, SkColorType ct, GrPixelConfig* confi
             }
             break;
         case kRGB_888x_SkColorType:
-            return false;
+            if (GR_GL_RGB8 == format) {
+                *config = kRGB_888_GrPixelConfig;
+            }
+            break;
         case kBGRA_8888_SkColorType:
             if (GR_GL_RGBA8 == format) {
                 if (kGL_GrGLStandard == standard) {
diff --git a/src/gpu/gl/GrGLCaps.h b/src/gpu/gl/GrGLCaps.h
index eed3ab3a17..19d443cc1c 100644
--- a/src/gpu/gl/GrGLCaps.h
+++ b/src/gpu/gl/GrGLCaps.h
@@ -419,9 +419,9 @@ public:
 private:
     enum ExternalFormatUsage {
         kTexImage_ExternalFormatUsage,
-        kOther_ExternalFormatUsage,
+        kReadPixels_ExternalFormatUsage,
 
-        kLast_ExternalFormatUsage = kOther_ExternalFormatUsage
+        kLast_ExternalFormatUsage = kReadPixels_ExternalFormatUsage
     };
     static const int kExternalFormatUsageCnt = kLast_ExternalFormatUsage + 1;
     bool getExternalFormat(GrPixelConfig surfaceConfig, GrPixelConfig memoryConfig,
diff --git a/src/gpu/gl/GrGLGpu.cpp b/src/gpu/gl/GrGLGpu.cpp
index ce81adf95f..c6e2d2537f 100644
--- a/src/gpu/gl/GrGLGpu.cpp
+++ b/src/gpu/gl/GrGLGpu.cpp
@@ -776,7 +776,7 @@ bool GrGLGpu::onWritePixels(GrSurface* surface, GrSurfaceOrigin origin, int left
                                srcAsConfig, texels, mipLevelCount);
 }
 
-// For GL_[UN]PACK_ALIGNMENT.
+// For GL_[UN]PACK_ALIGNMENT. TODO: This really wants to be GrColorType.
 static inline GrGLint config_alignment(GrPixelConfig config) {
     switch (config) {
         case kAlpha_8_GrPixelConfig:
@@ -793,6 +793,7 @@ static inline GrGLint config_alignment(GrPixelConfig config) {
         case kRGBA_half_GrPixelConfig:
             return 2;
         case kRGBA_8888_GrPixelConfig:
+        case kRGB_888_GrPixelConfig:  // We're really talking about GrColorType::kRGB_888x here.
         case kBGRA_8888_GrPixelConfig:
         case kSRGBA_8888_GrPixelConfig:
         case kSBGRA_8888_GrPixelConfig:
diff --git a/src/gpu/gl/GrGLUtil.cpp b/src/gpu/gl/GrGLUtil.cpp
index 6b44402c1b..3de894db82 100644
--- a/src/gpu/gl/GrGLUtil.cpp
+++ b/src/gpu/gl/GrGLUtil.cpp
@@ -380,6 +380,11 @@ GrGLRenderer GrGLGetRendererFromString(const char* rendererString) {
         if (0 == strncmp(rendererString, kMaliTStr, SK_ARRAY_COUNT(kMaliTStr) - 1)) {
             return kMaliT_GrGLRenderer;
         }
+        int mali400Num;
+        if (1 == sscanf(rendererString, "Mali-%d", &mali400Num) && mali400Num >= 400 &&
+            mali400Num < 500) {
+            return kMali4xx_GrGLRenderer;
+        }
         if (is_renderer_angle(rendererString)) {
             return kANGLE_GrGLRenderer;
         }
@@ -494,6 +499,8 @@ GrPixelConfig GrGLSizedFormatToPixelConfig(GrGLenum sizedFormat) {
             return kAlpha_8_as_Alpha_GrPixelConfig;
         case GR_GL_RGBA8:
             return kRGBA_8888_GrPixelConfig;
+        case GR_GL_RGB8:
+            return kRGB_888_GrPixelConfig;
         case GR_GL_BGRA8:
             return kBGRA_8888_GrPixelConfig;
         case GR_GL_SRGB8_ALPHA8:
diff --git a/src/gpu/gl/GrGLUtil.h b/src/gpu/gl/GrGLUtil.h
index 9905d75ef1..416538a749 100644
--- a/src/gpu/gl/GrGLUtil.h
+++ b/src/gpu/gl/GrGLUtil.h
@@ -60,12 +60,13 @@ enum GrGLRenderer {
     /** Either HD 6xxx or Iris 6xxx */
     kIntel6xxx_GrGLRenderer,
     kGalliumLLVM_GrGLRenderer,
+    kMali4xx_GrGLRenderer,
     /** T-6xx, T-7xx, or T-8xx */
     kMaliT_GrGLRenderer,
     kANGLE_GrGLRenderer,
 
-    kAMDRadeonHD7xxx_GrGLRenderer, // AMD Radeon HD 7000 Series
-    kAMDRadeonR9M4xx_GrGLRenderer, // AMD Radeon R9 M400 Series
+    kAMDRadeonHD7xxx_GrGLRenderer,  // AMD Radeon HD 7000 Series
+    kAMDRadeonR9M4xx_GrGLRenderer,  // AMD Radeon R9 M400 Series
 
     kOther_GrGLRenderer
 };
diff --git a/src/gpu/mtl/GrMtlUtil.mm b/src/gpu/mtl/GrMtlUtil.mm
index 383cc98264..e8a736c8f1 100644
--- a/src/gpu/mtl/GrMtlUtil.mm
+++ b/src/gpu/mtl/GrMtlUtil.mm
@@ -21,6 +21,9 @@ bool GrPixelConfigToMTLFormat(GrPixelConfig config, MTLPixelFormat* format) {
         case kRGBA_8888_GrPixelConfig:
             *format = MTLPixelFormatRGBA8Unorm;
             return true;
+        case kRGB_888_GrPixelConfig:
+            // TODO: MTLPixelFormatRGB8Unorm
+            return false;
         case kBGRA_8888_GrPixelConfig:
             *format = MTLPixelFormatBGRA8Unorm;
             return true;
diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp
index 0d75cbc56e..83546da5c4 100644
--- a/src/gpu/vk/GrVkCaps.cpp
+++ b/src/gpu/vk/GrVkCaps.cpp
@@ -466,6 +466,7 @@ bool validate_image_info(VkFormat format, SkColorType ct, GrPixelConfig* config)
             }
             break;
         case kRGB_888x_SkColorType:
+            // TODO: VK_FORMAT_R8G8B8_UNORM
             return false;
         case kBGRA_8888_SkColorType:
             if (VK_FORMAT_B8G8R8A8_UNORM == format) {
diff --git a/src/gpu/vk/GrVkUtil.cpp b/src/gpu/vk/GrVkUtil.cpp
index 4f0acdb8f4..e61d7bca17 100644
--- a/src/gpu/vk/GrVkUtil.cpp
+++ b/src/gpu/vk/GrVkUtil.cpp
@@ -22,6 +22,9 @@ bool GrPixelConfigToVkFormat(GrPixelConfig config, VkFormat* format) {
         case kRGBA_8888_GrPixelConfig:
             *format = VK_FORMAT_R8G8B8A8_UNORM;
             return true;
+        case kRGB_888_GrPixelConfig:
+            // TODO: VK_FORMAT_R8G8B8_UNORM
+            return false;
         case kBGRA_8888_GrPixelConfig:
             *format = VK_FORMAT_B8G8R8A8_UNORM;
             return true;