Revert "Make RGB_888x pixel operations work."

This reverts commit 8ce24b1d11f25a871fa23285cfd29373aa5d1d6a.

Reason for revert: https://ci.chromium.org/raw/build/logs.chromium.org/skia/52bc09f869c58a11/+/annotations

Original change's description:
> Make RGB_888x pixel operations work.
>
> Also remove vulkan-specific conversions in GrVkGpu and rely
> on higher level SurfaceContext to convert correctly.
>
> Bug: skia:8862
> Change-Id: Ib8b0541c8c5831148b7129c4bbed3f925660116e
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392378
> Reviewed-by: Robert Phillips <robertphillips@google.com>
> Commit-Queue: Brian Salomon <bsalomon@google.com>

TBR=bsalomon@google.com,robertphillips@google.com

Change-Id: I5358e0ba4d0f1dc24cc5c37dd86c401a3a6b6076
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:8862
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/392840
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Salomon <bsalomon@google.com>
diff --git a/src/gpu/GrDataUtils.cpp b/src/gpu/GrDataUtils.cpp
index ce8f7bb..6c47c95 100644
--- a/src/gpu/GrDataUtils.cpp
+++ b/src/gpu/GrDataUtils.cpp
@@ -510,6 +510,10 @@
 
 bool GrConvertPixels(const GrPixmap& dst, const GrCPixmap& src, bool flipY) {
     TRACE_EVENT0("skia.gpu", TRACE_FUNC);
+    if (src.colorType() == GrColorType::kRGB_888) {
+        // We don't expect to have to convert from this format.
+        return false;
+    }
     if (src.dimensions().isEmpty() || dst.dimensions().isEmpty()) {
         return false;
     }
@@ -534,27 +538,12 @@
         auto* dRow = reinterpret_cast<char*>(dst.addr());
         for (int y = 0; y < dst.height(); ++y, tRow += temp.rowBytes(), dRow += dst.rowBytes()) {
             for (int x = 0; x < dst.width(); ++x) {
-                auto t = tRow + x*sizeof(uint32_t);
-                auto d = dRow + x*3;
+                auto t = reinterpret_cast<const uint32_t*>(tRow + x * sizeof(uint32_t));
+                auto d = reinterpret_cast<uint32_t*>(dRow + x * 3);
                 memcpy(d, t, 3);
             }
         }
         return true;
-    } else if (src.colorType() == GrColorType::kRGB_888) {
-        // SkRasterPipeline doesn't handle reading from RGB_888. So convert it to RGB_888x and then
-        // do a recursive call if there is any remaining conversion.
-        GrPixmap temp = GrPixmap::Allocate(src.info().makeColorType(GrColorType::kRGB_888x));
-        auto* sRow = reinterpret_cast<const char*>(src.addr());
-        auto* tRow = reinterpret_cast<char*>(temp.addr());
-        for (int y = 0; y < src.height(); ++y, sRow += src.rowBytes(), tRow += temp.rowBytes()) {
-            for (int x = 0; x < src.width(); ++x) {
-                auto s = sRow + x*3;
-                auto t = tRow + x*sizeof(uint32_t);
-                memcpy(t, s, 3);
-                t[3] = static_cast<char>(0xFF);
-            }
-        }
-        return GrConvertPixels(dst, temp, flipY);
     }
 
     size_t srcBpp = src.info().bpp();
diff --git a/src/gpu/GrSurfaceContext.cpp b/src/gpu/GrSurfaceContext.cpp
index f98b377..31b6534 100644
--- a/src/gpu/GrSurfaceContext.cpp
+++ b/src/gpu/GrSurfaceContext.cpp
@@ -615,13 +615,13 @@
     pt.fY = flip ? dstSurface->height() - pt.fY - src[0].height() : pt.fY;
 
     if (!dContext->priv().drawingManager()->newWritePixelsTask(
-                sk_ref_sp(dstProxy),
-                SkIRect::MakePtSize(pt, src[0].dimensions()),
-                allowedColorType,
-                this->colorInfo().colorType(),
-                srcLevels.begin(),
-                numLevels,
-                prepForSampling)) {
+            sk_ref_sp(dstProxy),
+            SkIRect::MakePtSize(pt, src[0].dimensions()),
+            this->colorInfo().colorType(),
+            allowedColorType,
+            srcLevels.begin(),
+            numLevels,
+            prepForSampling)) {
         return false;
     }
     if (numLevels > 1) {
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index a962b2f..9337972 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -2388,11 +2388,17 @@
             ctInfo.fExternalIOFormats = std::make_unique<ColorTypeInfo::ExternalIOFormats[]>(
                     ctInfo.fExternalIOFormatCount);
             int ioIdx = 0;
-            // Format: RGB8, Surface: kRGB_888x, Data: kRGB_888
+            // Format: RGB8, Surface: kRGB_888x, Data: kRGB_888x
             {
                 auto& ioFormat = ctInfo.fExternalIOFormats[ioIdx++];
-                ioFormat.fColorType = GrColorType::kRGB_888;
+                ioFormat.fColorType = GrColorType::kRGB_888x;
                 ioFormat.fExternalType = GR_GL_UNSIGNED_BYTE;
+                // This is technically the wrong format to use for this color type since the color
+                // type is 4 bytes but the format is 3. However, we don't currently upload data of
+                // this type so the format is only used when creating an empty texture. If we want
+                // to support uploading data we should add in RGB_888 GrColorType. Additionally, on
+                // the FormatInfo we should have a default format to use when we want to create an
+                // empty texture.
                 ioFormat.fExternalTexImageFormat = GR_GL_RGB;
                 ioFormat.fExternalReadFormat = 0;
             }
diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp
index 7024065..8ffefb0 100644
--- a/src/gpu/vk/GrVkCaps.cpp
+++ b/src/gpu/vk/GrVkCaps.cpp
@@ -868,7 +868,6 @@
                 constexpr GrColorType ct = GrColorType::kRGBA_8888;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
             // Format: VK_FORMAT_R8G8B8A8_UNORM, Surface: kRGB_888x
@@ -876,7 +875,6 @@
                 constexpr GrColorType ct = GrColorType::kRGB_888x;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag;
                 ctInfo.fReadSwizzle = GrSwizzle::RGB1();
             }
@@ -897,7 +895,6 @@
                 constexpr GrColorType ct = GrColorType::kAlpha_8;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
                 ctInfo.fReadSwizzle = GrSwizzle("000r");
                 ctInfo.fWriteSwizzle = GrSwizzle("a000");
@@ -907,7 +904,6 @@
                 constexpr GrColorType ct = GrColorType::kGray_8;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag;
                 ctInfo.fReadSwizzle = GrSwizzle("rrr1");
             }
@@ -927,7 +923,6 @@
                 constexpr GrColorType ct = GrColorType::kBGRA_8888;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -946,7 +941,6 @@
                 constexpr GrColorType ct = GrColorType::kBGR_565;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -965,7 +959,6 @@
                 constexpr GrColorType ct = GrColorType::kRGBA_F16;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
             // Format: VK_FORMAT_R16G16B16A16_SFLOAT, Surface: GrColorType::kRGBA_F16_Clamped
@@ -973,7 +966,6 @@
                 constexpr GrColorType ct = GrColorType::kRGBA_F16_Clamped;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -992,7 +984,6 @@
                 constexpr GrColorType ct = GrColorType::kAlpha_F16;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
                 ctInfo.fReadSwizzle = GrSwizzle("000r");
                 ctInfo.fWriteSwizzle = GrSwizzle("a000");
@@ -1013,8 +1004,6 @@
                 constexpr GrColorType ct = GrColorType::kRGB_888x;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                // The Vulkan format is 3 bpp so we must convert to/from that when transferring.
-                ctInfo.fTransferColorType = GrColorType::kRGB_888;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -1033,7 +1022,6 @@
                 constexpr GrColorType ct = GrColorType::kRG_88;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -1052,7 +1040,6 @@
                 constexpr GrColorType ct = GrColorType::kRGBA_1010102;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -1071,7 +1058,6 @@
                 constexpr GrColorType ct = GrColorType::kBGRA_1010102;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -1090,7 +1076,6 @@
                 constexpr GrColorType ct = GrColorType::kABGR_4444;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
                 ctInfo.fReadSwizzle = GrSwizzle::BGRA();
                 ctInfo.fWriteSwizzle = GrSwizzle::BGRA();
@@ -1112,7 +1097,6 @@
                 constexpr GrColorType ct = GrColorType::kABGR_4444;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -1131,7 +1115,6 @@
                 constexpr GrColorType ct = GrColorType::kRGBA_8888_SRGB;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -1150,7 +1133,6 @@
                 constexpr GrColorType ct = GrColorType::kAlpha_16;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
                 ctInfo.fReadSwizzle = GrSwizzle("000r");
                 ctInfo.fWriteSwizzle = GrSwizzle("a000");
@@ -1171,7 +1153,6 @@
                 constexpr GrColorType ct = GrColorType::kRG_1616;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -1190,7 +1171,6 @@
                 constexpr GrColorType ct = GrColorType::kRGBA_16161616;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -1209,7 +1189,6 @@
                 constexpr GrColorType ct = GrColorType::kRG_F16;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kRenderable_Flag;
             }
         }
@@ -1230,7 +1209,6 @@
                 constexpr GrColorType ct = GrColorType::kRGB_888x;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kWrappedOnly_Flag;
             }
         }
@@ -1251,7 +1229,6 @@
                 constexpr GrColorType ct = GrColorType::kRGB_888x;
                 auto& ctInfo = info.fColorTypeInfos[ctIdx++];
                 ctInfo.fColorType = ct;
-                ctInfo.fTransferColorType = ct;
                 ctInfo.fFlags = ColorTypeInfo::kUploadData_Flag | ColorTypeInfo::kWrappedOnly_Flag;
             }
         }
@@ -1552,7 +1529,7 @@
     for (int i = 0; i < info.fColorTypeInfoCount; ++i) {
         const auto& ctInfo = info.fColorTypeInfos[i];
         if (ctInfo.fColorType == surfaceColorType) {
-            return {ctInfo.fTransferColorType, offsetAlignment};
+            return {surfaceColorType, offsetAlignment};
         }
     }
     return {GrColorType::kUnknown, 0};
@@ -1583,16 +1560,6 @@
     return SurfaceReadPixelsSupport::kUnsupported;
 }
 
-GrColorType GrVkCaps::transferColorType(VkFormat vkFormat, GrColorType surfaceColorType) const {
-    const auto& info = this->getFormatInfo(vkFormat);
-    for (int i = 0; i < info.fColorTypeInfoCount; ++i) {
-        if (info.fColorTypeInfos[i].fColorType == surfaceColorType) {
-            return info.fColorTypeInfos[i].fTransferColorType;
-        }
-    }
-    return GrColorType::kUnknown;
-}
-
 bool GrVkCaps::onSurfaceSupportsWritePixels(const GrSurface* surface) const {
     if (auto rt = surface->asRenderTarget()) {
         return rt->numSamples() <= 1 && SkToBool(surface->asTexture());
@@ -1753,7 +1720,7 @@
     for (int i = 0; i < info.fColorTypeInfoCount; ++i) {
         const auto& ctInfo = info.fColorTypeInfos[i];
         if (ctInfo.fColorType == srcColorType) {
-            return {ctInfo.fTransferColorType, offsetAlignment};
+            return {srcColorType, offsetAlignment};
         }
     }
     return {GrColorType::kUnknown, 0};
diff --git a/src/gpu/vk/GrVkCaps.h b/src/gpu/vk/GrVkCaps.h
index 44c26c2..76cd9c0 100644
--- a/src/gpu/vk/GrVkCaps.h
+++ b/src/gpu/vk/GrVkCaps.h
@@ -75,10 +75,6 @@
         return SkToBool(FormatInfo::kBlitSrc_Flag & flags);
     }
 
-    // Gets the GrColorType that should be used to transfer data in/out of a transfer buffer to
-    // write/read data when using a VkFormat with a specified color type.
-    GrColorType transferColorType(VkFormat, GrColorType surfaceColorType) const;
-
     // On some GPUs (Windows Nvidia and Imagination) calls to QueueWaitIdle return before actually
     // signalling the fences on the command buffers even though they have completed. This causes
     // issues when then deleting the command buffers. Therefore we additionally will call
@@ -298,7 +294,6 @@
     // ColorTypeInfo for a specific format
     struct ColorTypeInfo {
         GrColorType fColorType = GrColorType::kUnknown;
-        GrColorType fTransferColorType = GrColorType::kUnknown;
         enum {
             kUploadData_Flag = 0x1,
             // Does Ganesh itself support rendering to this colorType & format pair. Renderability
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index 6d32816..4cd71cc 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -483,6 +483,9 @@
     if (!this->currentCommandBuffer()) {
         return false;
     }
+    if (surfaceColorType != bufferColorType) {
+        return false;
+    }
 
     size_t bpp = GrColorTypeBytesPerPixel(bufferColorType);
     if (GrBackendFormatBytesPerPixel(texture->backendFormat()) != bpp) {
@@ -498,20 +501,14 @@
         return false;
     }
     GrVkAttachment* vkTex = tex->textureAttachment();
-    VkFormat format = vkTex->imageFormat();
 
     // Can't transfer compressed data
-    SkASSERT(!GrVkFormatIsCompressed(format));
+    SkASSERT(!GrVkFormatIsCompressed(vkTex->imageFormat()));
 
     if (!transferBuffer) {
         return false;
     }
 
-    if (bufferColorType != this->vkCaps().transferColorType(format, surfaceColorType)) {
-        return false;
-    }
-    SkASSERT(GrVkFormatBytesPerBlock(format) == GrColorTypeBytesPerPixel(bufferColorType));
-
     SkDEBUGCODE(
         SkIRect subRect = SkIRect::MakeXYWH(left, top, width, height);
         SkIRect bounds = SkIRect::MakeWH(texture->width(), texture->height());
@@ -561,6 +558,9 @@
     if (fProtectedContext == GrProtected::kYes) {
         return false;
     }
+    if (surfaceColorType != bufferColorType) {
+        return false;
+    }
 
     GrVkImage* srcImage;
     if (GrVkRenderTarget* rt = static_cast<GrVkRenderTarget*>(surface->asRenderTarget())) {
@@ -579,11 +579,10 @@
         srcImage = static_cast<GrVkTexture*>(surface->asTexture())->textureAttachment();
     }
 
-    VkFormat format = srcImage->imageFormat();
-    if (bufferColorType != this->vkCaps().transferColorType(format, surfaceColorType)) {
+    if (GrVkFormatBytesPerBlock(srcImage->imageFormat()) !=
+        GrColorTypeBytesPerPixel(surfaceColorType)) {
         return false;
     }
-    SkASSERT(GrVkFormatBytesPerBlock(format) == GrColorTypeBytesPerPixel(bufferColorType));
 
     // Set up copy region
     VkBufferImageCopy region;
@@ -809,6 +808,22 @@
     }
 
     SkASSERT(this->vkCaps().surfaceSupportsWritePixels(texAttachment));
+    SkASSERT(this->vkCaps().areColorTypeAndFormatCompatible(
+            dataColorType, texAttachment->backendFormat()));
+
+    // For RGB_888x src data we are uploading it first to an RGBA texture and then copying it to the
+    // dst RGB texture. Thus we do not upload mip levels for that.
+    if (dataColorType == GrColorType::kRGB_888x &&
+        texAttachment->imageFormat() == VK_FORMAT_R8G8B8_UNORM) {
+        // First check that we'll be able to do the copy to the to the R8G8B8 image in the end via a
+        // blit or draw.
+        if (!this->vkCaps().formatCanBeDstofBlit(VK_FORMAT_R8G8B8_UNORM,
+                                                 texAttachment->isLinearTiled()) &&
+            !this->vkCaps().isFormatRenderable(VK_FORMAT_R8G8B8_UNORM, 1)) {
+            return false;
+        }
+        mipLevelCount = 1;
+    }
 
     SkASSERT(this->vkCaps().isVkFormatTexturable(texAttachment->imageFormat()));
     size_t bpp = GrColorTypeBytesPerPixel(dataColorType);
@@ -819,19 +834,39 @@
     SkAutoTArray<GrMipLevel> texelsShallowCopy(mipLevelCount);
     std::copy_n(texels, mipLevelCount, texelsShallowCopy.get());
 
-    SkTArray<size_t> individualMipOffsets;
-    size_t combinedBufferSize;
-    if (mipLevelCount > 1) {
-        combinedBufferSize = GrComputeTightCombinedBufferSize(bpp,
-                                                              {width, height},
-                                                              &individualMipOffsets,
-                                                              mipLevelCount);
-    } else {
-        SkASSERT(texelsShallowCopy[0].fPixels && texelsShallowCopy[0].fRowBytes);
-        combinedBufferSize = width*height*bpp;
-        individualMipOffsets.push_back(0);
+    SkTArray<size_t> individualMipOffsets(mipLevelCount);
+    individualMipOffsets.push_back(0);
+    size_t combinedBufferSize = width * bpp * height;
+    int currentWidth = width;
+    int currentHeight = height;
+    if (!texelsShallowCopy[0].fPixels) {
+        combinedBufferSize = 0;
     }
-    SkASSERT(combinedBufferSize);
+
+    // The alignment must be at least 4 bytes and a multiple of the bytes per pixel of the image
+    // config. This works with the assumption that the bytes in pixel config is always a power of 2.
+    SkASSERT((bpp & (bpp - 1)) == 0);
+    const size_t alignmentMask = 0x3 | (bpp - 1);
+    for (int currentMipLevel = 1; currentMipLevel < mipLevelCount; currentMipLevel++) {
+        currentWidth = std::max(1, currentWidth/2);
+        currentHeight = std::max(1, currentHeight/2);
+
+        if (texelsShallowCopy[currentMipLevel].fPixels) {
+            const size_t trimmedSize = currentWidth * bpp * currentHeight;
+            const size_t alignmentDiff = combinedBufferSize & alignmentMask;
+            if (alignmentDiff != 0) {
+                combinedBufferSize += alignmentMask - alignmentDiff + 1;
+            }
+            individualMipOffsets.push_back(combinedBufferSize);
+            combinedBufferSize += trimmedSize;
+        } else {
+            individualMipOffsets.push_back(0);
+        }
+    }
+    if (0 == combinedBufferSize) {
+        // We don't actually have any data to upload so just return success
+        return true;
+    }
 
     // Get a staging buffer slice to hold our mip data.
     // Vulkan requires offsets in the buffer to be aligned to multiple of the texel size and 4
@@ -844,13 +879,37 @@
 
     int uploadLeft = left;
     int uploadTop = top;
+    GrVkAttachment* uploadTexture = texAttachment;
+    // For uploading RGB_888x data to an R8G8B8_UNORM texture we must first upload the data to an
+    // R8G8B8A8_UNORM image and then copy it.
+    sk_sp<GrVkAttachment> copyTexAttachment;
+    if (dataColorType == GrColorType::kRGB_888x &&
+        texAttachment->imageFormat() == VK_FORMAT_R8G8B8_UNORM) {
+        bool dstHasYcbcr = texAttachment->ycbcrConversionInfo().isValid();
+        if (!this->vkCaps().canCopyAsBlit(texAttachment->imageFormat(), 1, false, dstHasYcbcr,
+                                          VK_FORMAT_R8G8B8A8_UNORM, 1, false, false)) {
+            return false;
+        }
+
+        copyTexAttachment = GrVkAttachment::MakeTexture(this, {width, height},
+                                                        VK_FORMAT_R8G8B8A8_UNORM, /*mipLevels=*/1,
+                                                        GrRenderable::kNo, /*numSamples=*/1,
+                                                        SkBudgeted::kYes, GrProtected::kNo);
+        if (!copyTexAttachment) {
+            return false;
+        }
+
+        uploadTexture = copyTexAttachment.get();
+        uploadLeft = 0;
+        uploadTop = 0;
+    }
 
     char* buffer = (char*) slice.fOffsetMapPtr;
     SkTArray<VkBufferImageCopy> regions(mipLevelCount);
 
-    int currentWidth = width;
-    int currentHeight = height;
-    int layerHeight = texAttachment->height();
+    currentWidth = width;
+    currentHeight = height;
+    int layerHeight = uploadTexture->height();
     for (int currentMipLevel = 0; currentMipLevel < mipLevelCount; currentMipLevel++) {
         if (texelsShallowCopy[currentMipLevel].fPixels) {
             SkASSERT(1 == mipLevelCount || currentHeight == layerHeight);
@@ -867,19 +926,17 @@
             region.bufferOffset = slice.fOffset + individualMipOffsets[currentMipLevel];
             region.bufferRowLength = currentWidth;
             region.bufferImageHeight = currentHeight;
-            region.imageSubresource = {VK_IMAGE_ASPECT_COLOR_BIT, SkToU32(currentMipLevel), 0, 1};
+            region.imageSubresource = { VK_IMAGE_ASPECT_COLOR_BIT, SkToU32(currentMipLevel), 0, 1 };
             region.imageOffset = {uploadLeft, uploadTop, 0};
-            region.imageExtent = {(uint32_t)currentWidth, (uint32_t)currentHeight, 1};
+            region.imageExtent = { (uint32_t)currentWidth, (uint32_t)currentHeight, 1 };
         }
-
-        currentWidth  = std::max(1,  currentWidth/2);
+        currentWidth = std::max(1, currentWidth/2);
         currentHeight = std::max(1, currentHeight/2);
-
         layerHeight = currentHeight;
     }
 
     // Change layout of our target so it can be copied to
-    texAttachment->setImageLayout(this,
+    uploadTexture->setImageLayout(this,
                                   VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
                                   VK_ACCESS_TRANSFER_WRITE_BIT,
                                   VK_PIPELINE_STAGE_TRANSFER_BIT,
@@ -893,10 +950,20 @@
     GrVkBuffer* vkBuffer = static_cast<GrVkBuffer*>(slice.fBuffer);
     this->currentCommandBuffer()->copyBufferToImage(this,
                                                     vkBuffer->vkBuffer(),
-                                                    texAttachment,
+                                                    uploadTexture,
                                                     VK_IMAGE_LAYOUT_TRANSFER_DST_OPTIMAL,
                                                     regions.count(),
                                                     regions.begin());
+
+    // If we copied the data into a temporary image first, copy that image into our main texture
+    // now.
+    if (copyTexAttachment) {
+        SkASSERT(dataColorType == GrColorType::kRGB_888x);
+        SkAssertResult(this->copySurface(texAttachment, copyTexAttachment.get(),
+                                         SkIRect::MakeWH(width, height),
+                                         SkIPoint::Make(left, top)));
+    }
+
     return true;
 }
 
@@ -2320,6 +2387,10 @@
         return false;
     }
 
+    if (surfaceColorType != dstColorType) {
+        return false;
+    }
+
     if (!this->currentCommandBuffer()) {
         return false;
     }
@@ -2342,9 +2413,55 @@
         return false;
     }
 
-    if (dstColorType == GrColorType::kUnknown ||
-        dstColorType != this->vkCaps().transferColorType(image->imageFormat(), surfaceColorType)) {
-        return false;
+    // Skia's RGB_888x color type, which we map to the vulkan R8G8B8_UNORM, expects the data to be
+    // 32 bits, but the Vulkan format is only 24. So we first copy the surface into an R8G8B8A8
+    // image and then do the read pixels from that.
+    sk_sp<GrVkAttachment> copySurface;
+    if (dstColorType == GrColorType::kRGB_888x && image->imageFormat() == VK_FORMAT_R8G8B8_UNORM) {
+        int srcSampleCount = 0;
+        if (rt) {
+            srcSampleCount = rt->numSamples();
+        }
+        bool srcHasYcbcr = image->ycbcrConversionInfo().isValid();
+        if (!this->vkCaps().canCopyAsBlit(VK_FORMAT_R8G8B8A8_UNORM, 1, false, false,
+                                          image->imageFormat(), srcSampleCount,
+                                          image->isLinearTiled(), srcHasYcbcr)) {
+            return false;
+        }
+
+        // Make a new surface that is RGBA to copy the RGB surface into.
+        VkImageUsageFlags usageFlags = VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT |
+                                       VK_IMAGE_USAGE_INPUT_ATTACHMENT_BIT |
+                                       VK_IMAGE_USAGE_SAMPLED_BIT |
+                                       VK_IMAGE_USAGE_TRANSFER_SRC_BIT |
+                                       VK_IMAGE_USAGE_TRANSFER_DST_BIT;
+
+        GrVkImage::ImageDesc imageDesc;
+        imageDesc.fImageType = VK_IMAGE_TYPE_2D;
+        imageDesc.fFormat = VK_FORMAT_R8G8B8A8_UNORM;
+        imageDesc.fWidth = width;
+        imageDesc.fHeight = height;
+        imageDesc.fLevels = 1;
+        imageDesc.fSamples = 1;
+        imageDesc.fImageTiling = VK_IMAGE_TILING_OPTIMAL;
+        imageDesc.fUsageFlags = usageFlags;
+        imageDesc.fMemProps = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT;
+
+        copySurface =
+                GrVkAttachment::MakeTexture(this, {width, height}, VK_FORMAT_R8G8B8A8_UNORM,
+                                            /*mipLevels=*/1, GrRenderable::kYes, /*numSamples=*/1,
+                                            SkBudgeted::kYes, GrProtected::kNo);
+        if (!copySurface) {
+            return false;
+        }
+
+        SkIRect srcRect = SkIRect::MakeXYWH(left, top, width, height);
+        SkAssertResult(this->copySurface(copySurface.get(), surface, srcRect, SkIPoint::Make(0,0)));
+
+        top = 0;
+        left = 0;
+        dstColorType = GrColorType::kRGBA_8888;
+        image = copySurface.get();
     }
 
     // Change layout of our target so it can be used as copy