Fix up readPixels, writePixels, and copies when dealing with ycbcr textures in vulkan.

In general this makes sure we never directly read or write pixels or use one of
the vk image copy functions if we are dealing with a VkImage that requires a Ycbcr texture.

Bug: b/129037735
Change-Id: Ib3bbac8bcddb643942e08d2b2ae3381fda6758a7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/203388
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Greg Daniel <egdaniel@google.com>
(cherry picked from commit a51e93c9a0a52a90fe31c8eaebceedd6ea4f3635)
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/203701

3 files changed, 114 insertions, 49 deletions

diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp
index e388a6a5f0..c9d20c172e 100644
--- a/src/gpu/vk/GrVkCaps.cpp
+++ b/src/gpu/vk/GrVkCaps.cpp
@@ -11,6 +11,7 @@
 #include "GrRenderTarget.h"
 #include "GrShaderCaps.h"
 #include "GrVkInterface.h"
+#include "GrVkTexture.h"
 #include "GrVkUtil.h"
 #include "SkGr.h"
 #include "vk/GrVkBackendContext.h"
@@ -75,12 +76,16 @@ bool GrVkCaps::initDescForDstCopy(const GrRenderTargetProxy* src, GrSurfaceDesc*
 }
 
 bool GrVkCaps::canCopyImage(GrPixelConfig dstConfig, int dstSampleCnt, GrSurfaceOrigin dstOrigin,
-                            GrPixelConfig srcConfig, int srcSampleCnt,
-                            GrSurfaceOrigin srcOrigin) const {
+                            bool dstHasYcbcr, GrPixelConfig srcConfig, int srcSampleCnt,
+                            GrSurfaceOrigin srcOrigin, bool srcHasYcbcr) const {
     if ((dstSampleCnt > 1 || srcSampleCnt > 1) && dstSampleCnt != srcSampleCnt) {
         return false;
     }
 
+    if (dstHasYcbcr || srcHasYcbcr) {
+        return false;
+    }
+
     // We require that all vulkan GrSurfaces have been created with transfer_dst and transfer_src
     // as image usage flags.
     if (srcOrigin != dstOrigin || GrBytesPerPixel(srcConfig) != GrBytesPerPixel(dstConfig)) {
@@ -96,7 +101,8 @@ bool GrVkCaps::canCopyImage(GrPixelConfig dstConfig, int dstSampleCnt, GrSurface
 }
 
 bool GrVkCaps::canCopyAsBlit(GrPixelConfig dstConfig, int dstSampleCnt, bool dstIsLinear,
-                             GrPixelConfig srcConfig, int srcSampleCnt, bool srcIsLinear) const {
+                             bool dstHasYcbcr, GrPixelConfig srcConfig, int srcSampleCnt,
+                             bool srcIsLinear, bool srcHasYcbcr) const {
     // We require that all vulkan GrSurfaces have been created with transfer_dst and transfer_src
     // as image usage flags.
     if (!this->configCanBeDstofBlit(dstConfig, dstIsLinear) ||
@@ -115,12 +121,17 @@ bool GrVkCaps::canCopyAsBlit(GrPixelConfig dstConfig, int dstSampleCnt, bool dst
         return false;
     }
 
+    if (dstHasYcbcr || srcHasYcbcr) {
+        return false;
+    }
+
     return true;
 }
 
 bool GrVkCaps::canCopyAsResolve(GrPixelConfig dstConfig, int dstSampleCnt,
-                                GrSurfaceOrigin dstOrigin, GrPixelConfig srcConfig,
-                                int srcSampleCnt, GrSurfaceOrigin srcOrigin) const {
+                                GrSurfaceOrigin dstOrigin, bool dstHasYcbcr,
+                                GrPixelConfig srcConfig, int srcSampleCnt,
+                                GrSurfaceOrigin srcOrigin, bool srcHasYcbcr) const {
     // The src surface must be multisampled.
     if (srcSampleCnt <= 1) {
         return false;
@@ -141,11 +152,16 @@ bool GrVkCaps::canCopyAsResolve(GrPixelConfig dstConfig, int dstSampleCnt,
         return false;
     }
 
+    if (dstHasYcbcr || srcHasYcbcr) {
+        return false;
+    }
+
     return true;
 }
 
-bool GrVkCaps::canCopyAsDraw(GrPixelConfig dstConfig, bool dstIsRenderable,
-                             GrPixelConfig srcConfig, bool srcIsTextureable) const {
+bool GrVkCaps::canCopyAsDraw(GrPixelConfig dstConfig, bool dstIsRenderable, bool dstHasYcbcr,
+                             GrPixelConfig srcConfig, bool srcIsTextureable,
+                             bool srcHasYcbcr) const {
     // TODO: Make copySurfaceAsDraw handle the swizzle
     if (this->shaderCaps()->configOutputSwizzle(srcConfig) !=
         this->shaderCaps()->configOutputSwizzle(dstConfig)) {
@@ -157,6 +173,10 @@ bool GrVkCaps::canCopyAsDraw(GrPixelConfig dstConfig, bool dstIsRenderable,
         return false;
     }
 
+    if (dstHasYcbcr) {
+        return false;
+    }
+
     return true;
 }
 
@@ -198,14 +218,28 @@ bool GrVkCaps::onCanCopySurface(const GrSurfaceProxy* dst, const GrSurfaceProxy*
     SkASSERT((dstSampleCnt > 0) == SkToBool(dst->asRenderTargetProxy()));
     SkASSERT((srcSampleCnt > 0) == SkToBool(src->asRenderTargetProxy()));
 
-    return this->canCopyImage(dstConfig, dstSampleCnt, dstOrigin,
-                              srcConfig, srcSampleCnt, srcOrigin) ||
-           this->canCopyAsBlit(dstConfig, dstSampleCnt, dstIsLinear,
-                               srcConfig, srcSampleCnt, srcIsLinear) ||
-           this->canCopyAsResolve(dstConfig, dstSampleCnt, dstOrigin,
-                                  srcConfig, srcSampleCnt, srcOrigin) ||
-           this->canCopyAsDraw(dstConfig, dstSampleCnt > 0,
-                               srcConfig, SkToBool(src->asTextureProxy()));
+    bool dstHasYcbcr = false;
+    if (auto ycbcr = dst->backendFormat().getVkYcbcrConversionInfo()) {
+        if (ycbcr->isValid()) {
+            dstHasYcbcr = true;
+        }
+    }
+
+    bool srcHasYcbcr = false;
+    if (auto ycbcr = src->backendFormat().getVkYcbcrConversionInfo()) {
+        if (ycbcr->isValid()) {
+            srcHasYcbcr = true;
+        }
+    }
+
+    return this->canCopyImage(dstConfig, dstSampleCnt, dstOrigin, dstHasYcbcr,
+                              srcConfig, srcSampleCnt, srcOrigin, srcHasYcbcr) ||
+           this->canCopyAsBlit(dstConfig, dstSampleCnt, dstIsLinear, dstHasYcbcr,
+                               srcConfig, srcSampleCnt, srcIsLinear, srcHasYcbcr) ||
+           this->canCopyAsResolve(dstConfig, dstSampleCnt, dstOrigin, dstHasYcbcr,
+                                  srcConfig, srcSampleCnt, srcOrigin, srcHasYcbcr) ||
+           this->canCopyAsDraw(dstConfig, dstSampleCnt > 0, dstHasYcbcr,
+                               srcConfig, SkToBool(src->asTextureProxy()), srcHasYcbcr);
 }
 
 template<typename T> T* get_extension_feature_struct(const VkPhysicalDeviceFeatures2& features,
@@ -735,10 +769,27 @@ int GrVkCaps::maxRenderTargetSampleCount(GrPixelConfig config) const {
     return table[table.count() - 1];
 }
 
+bool GrVkCaps::surfaceSupportsReadPixels(const GrSurface* surface) const {
+    if (auto tex = static_cast<const GrVkTexture*>(surface->asTexture())) {
+        // We can't directly read from a VkImage that has a ycbcr sampler.
+        if (tex->ycbcrConversionInfo().isValid()) {
+            return false;
+        }
+    }
+    return true;
+}
+
 bool GrVkCaps::onSurfaceSupportsWritePixels(const GrSurface* surface) const {
     if (auto rt = surface->asRenderTarget()) {
         return rt->numColorSamples() <= 1 && SkToBool(surface->asTexture());
     }
+    // We can't write to a texture that has a ycbcr sampler.
+    if (auto tex = static_cast<const GrVkTexture*>(surface->asTexture())) {
+        // We can't directly read from a VkImage that has a ycbcr sampler.
+        if (tex->ycbcrConversionInfo().isValid()) {
+            return false;
+        }
+    }
     return true;
 }
 
@@ -748,8 +799,8 @@ static GrPixelConfig validate_image_info(VkFormat format, SkColorType ct, bool h
         // we have a valid VkYcbcrConversion.
         if (hasYcbcrConversion) {
             // We don't actually care what the color type or config are since we won't use those
-            // values for external textures, but since our code requires setting a config here
-            // just default it to RGBA.
+            // values for external textures. However, for read pixels we will draw to a non ycbcr
+            // texture of this config so we set RGBA here for that.
             return kRGBA_8888_GrPixelConfig;
         } else {
             return kUnknown_GrPixelConfig;
diff --git a/src/gpu/vk/GrVkCaps.h b/src/gpu/vk/GrVkCaps.h
index 337c8ae7f1..24e3e6d05f 100644
--- a/src/gpu/vk/GrVkCaps.h
+++ b/src/gpu/vk/GrVkCaps.h
@@ -43,7 +43,7 @@ public:
     int getRenderTargetSampleCount(int requestedCount, GrPixelConfig config) const override;
     int maxRenderTargetSampleCount(GrPixelConfig config) const override;
 
-    bool surfaceSupportsReadPixels(const GrSurface*) const override { return true; }
+    bool surfaceSupportsReadPixels(const GrSurface*) const override;
 
     bool isConfigTexturableLinearly(GrPixelConfig config) const {
         return SkToBool(ConfigInfo::kTextureable_Flag & fConfigTable[config].fLinearFlags);
@@ -142,17 +142,19 @@ public:
      * target.
      */
     bool canCopyImage(GrPixelConfig dstConfig, int dstSampleCnt, GrSurfaceOrigin dstOrigin,
-                      GrPixelConfig srcConfig, int srcSamplecnt, GrSurfaceOrigin srcOrigin) const;
+                      bool dstHasYcbcr, GrPixelConfig srcConfig, int srcSamplecnt,
+                      GrSurfaceOrigin srcOrigin, bool srcHasYcbcr) const;
 
     bool canCopyAsBlit(GrPixelConfig dstConfig, int dstSampleCnt, bool dstIsLinear,
-                       GrPixelConfig srcConfig, int srcSampleCnt, bool srcIsLinear) const;
+                       bool dstHasYcbcr, GrPixelConfig srcConfig, int srcSampleCnt,
+                       bool srcIsLinear, bool srcHasYcbcr) const;
 
     bool canCopyAsResolve(GrPixelConfig dstConfig, int dstSampleCnt, GrSurfaceOrigin dstOrigin,
-                          GrPixelConfig srcConfig, int srcSamplecnt,
-                          GrSurfaceOrigin srcOrigin) const;
+                          bool dstHasYcbcr, GrPixelConfig srcConfig, int srcSamplecnt,
+                          GrSurfaceOrigin srcOrigin, bool srcHasYcbcr) const;
 
-    bool canCopyAsDraw(GrPixelConfig dstConfig, bool dstIsRenderable,
-                       GrPixelConfig srcConfig, bool srcIsTextureable) const;
+    bool canCopyAsDraw(GrPixelConfig dstConfig, bool dstIsRenderable, bool dstHasYcbcr,
+                       GrPixelConfig srcConfig, bool srcIsTextureable, bool srcHasYcbcr) const;
 
     bool initDescForDstCopy(const GrRenderTargetProxy* src, GrSurfaceDesc* desc, GrSurfaceOrigin*,
                             bool* rectsMustMatch, bool* disallowSubrect) const override;
diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp
index ee9270b7d3..89e942024c 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -1842,8 +1842,10 @@ void GrVkGpu::copySurfaceAsCopyImage(GrSurface* dst, GrSurfaceOrigin dstOrigin,
 #ifdef SK_DEBUG
     int dstSampleCnt = get_surface_sample_cnt(dst);
     int srcSampleCnt = get_surface_sample_cnt(src);
-    SkASSERT(this->vkCaps().canCopyImage(dst->config(), dstSampleCnt, dstOrigin,
-                                         src->config(), srcSampleCnt, srcOrigin));
+    bool dstHasYcbcr = dstImage->ycbcrConversionInfo().isValid();
+    bool srcHasYcbcr = srcImage->ycbcrConversionInfo().isValid();
+    SkASSERT(this->vkCaps().canCopyImage(dst->config(), dstSampleCnt, dstOrigin, dstHasYcbcr,
+                                         src->config(), srcSampleCnt, srcOrigin, srcHasYcbcr));
 
 #endif
 
@@ -1902,8 +1904,11 @@ void GrVkGpu::copySurfaceAsBlit(GrSurface* dst, GrSurfaceOrigin dstOrigin,
 #ifdef SK_DEBUG
     int dstSampleCnt = get_surface_sample_cnt(dst);
     int srcSampleCnt = get_surface_sample_cnt(src);
+    bool dstHasYcbcr = dstImage->ycbcrConversionInfo().isValid();
+    bool srcHasYcbcr = srcImage->ycbcrConversionInfo().isValid();
     SkASSERT(this->vkCaps().canCopyAsBlit(dst->config(), dstSampleCnt, dstImage->isLinearTiled(),
-                                          src->config(), srcSampleCnt, srcImage->isLinearTiled()));
+                                          dstHasYcbcr, src->config(), srcSampleCnt,
+                                          srcImage->isLinearTiled(), srcHasYcbcr));
 
 #endif
     dstImage->setImageLayout(this,
@@ -2005,21 +2010,6 @@ bool GrVkGpu::onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
     int dstSampleCnt = get_surface_sample_cnt(dst);
     int srcSampleCnt = get_surface_sample_cnt(src);
 
-    if (this->vkCaps().canCopyAsResolve(dstConfig, dstSampleCnt, dstOrigin,
-                                        srcConfig, srcSampleCnt, srcOrigin)) {
-        this->copySurfaceAsResolve(dst, dstOrigin, src, srcOrigin, srcRect, dstPoint);
-        return true;
-    }
-
-    if (this->vkCaps().canCopyAsDraw(dstConfig, SkToBool(dst->asRenderTarget()),
-                                     srcConfig, SkToBool(src->asTexture()))) {
-        SkAssertResult(fCopyManager.copySurfaceAsDraw(this, dst, dstOrigin, src, srcOrigin, srcRect,
-                                                      dstPoint, canDiscardOutsideDstRect));
-        auto dstRect = srcRect.makeOffset(dstPoint.fX, dstPoint.fY);
-        this->didWriteToSurface(dst, dstOrigin, &dstRect);
-        return true;
-    }
-
     GrVkImage* dstImage;
     GrVkImage* srcImage;
     GrRenderTarget* dstRT = dst->asRenderTarget();
@@ -2042,15 +2032,34 @@ bool GrVkGpu::onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
         srcImage = static_cast<GrVkTexture*>(src->asTexture());
     }
 
-    if (this->vkCaps().canCopyImage(dstConfig, dstSampleCnt, dstOrigin,
-                                    srcConfig, srcSampleCnt, srcOrigin)) {
+    bool dstHasYcbcr = dstImage->ycbcrConversionInfo().isValid();
+    bool srcHasYcbcr = srcImage->ycbcrConversionInfo().isValid();
+
+    if (this->vkCaps().canCopyAsResolve(dstConfig, dstSampleCnt, dstOrigin, dstHasYcbcr,
+                                        srcConfig, srcSampleCnt, srcOrigin, srcHasYcbcr)) {
+        this->copySurfaceAsResolve(dst, dstOrigin, src, srcOrigin, srcRect, dstPoint);
+        return true;
+    }
+
+    if (this->vkCaps().canCopyAsDraw(dstConfig, SkToBool(dst->asRenderTarget()), dstHasYcbcr,
+                                     srcConfig, SkToBool(src->asTexture()), srcHasYcbcr)) {
+        SkAssertResult(fCopyManager.copySurfaceAsDraw(this, dst, dstOrigin, src, srcOrigin, srcRect,
+                                                      dstPoint, canDiscardOutsideDstRect));
+        auto dstRect = srcRect.makeOffset(dstPoint.fX, dstPoint.fY);
+        this->didWriteToSurface(dst, dstOrigin, &dstRect);
+        return true;
+    }
+
+    if (this->vkCaps().canCopyImage(dstConfig, dstSampleCnt, dstOrigin, dstHasYcbcr,
+                                    srcConfig, srcSampleCnt, srcOrigin, srcHasYcbcr)) {
         this->copySurfaceAsCopyImage(dst, dstOrigin, src, srcOrigin, dstImage, srcImage,
                                      srcRect, dstPoint);
         return true;
     }
 
     if (this->vkCaps().canCopyAsBlit(dstConfig, dstSampleCnt, dstImage->isLinearTiled(),
-                                     srcConfig, srcSampleCnt, srcImage->isLinearTiled())) {
+                                     dstHasYcbcr, srcConfig, srcSampleCnt,
+                                     srcImage->isLinearTiled(), srcHasYcbcr)) {
         this->copySurfaceAsBlit(dst, dstOrigin, src, srcOrigin, dstImage, srcImage,
                                 srcRect, dstPoint);
         return true;
@@ -2136,11 +2145,14 @@ bool GrVkGpu::onReadPixels(GrSurface* surface, int left, int top, int width, int
         if (rt) {
             srcSampleCount = rt->numColorSamples();
         }
+        bool srcHasYcbcr = image->ycbcrConversionInfo().isValid();
         static const GrSurfaceOrigin kOrigin = kTopLeft_GrSurfaceOrigin;
-        if (!this->vkCaps().canCopyAsBlit(copySurface->config(), 1, kOrigin,
-                                          surface->config(), srcSampleCount, kOrigin) &&
-            !this->vkCaps().canCopyAsDraw(copySurface->config(), false,
-                                          surface->config(), SkToBool(surface->asTexture()))) {
+        if (!this->vkCaps().canCopyAsBlit(copySurface->config(), 1, kOrigin, false,
+                                          surface->config(), srcSampleCount, kOrigin,
+                                          srcHasYcbcr) &&
+            !this->vkCaps().canCopyAsDraw(copySurface->config(), false, false,
+                                          surface->config(), SkToBool(surface->asTexture()),
+                                          srcHasYcbcr)) {
             return false;
         }
         SkIRect srcRect = SkIRect::MakeXYWH(left, top, width, height);