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>
diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp
index cc6e2be..2c71f72 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::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::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 @@
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 @@
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 @@
return false;
}
+ if (dstHasYcbcr) {
+ return false;
+ }
+
return true;
}
@@ -198,14 +218,28 @@
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 @@
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 @@
// 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 337c8ae..24e3e6d 100644
--- a/src/gpu/vk/GrVkCaps.h
+++ b/src/gpu/vk/GrVkCaps.h
@@ -43,7 +43,7 @@
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 @@
* 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 ee9270b..89e9420 100644
--- a/src/gpu/vk/GrVkGpu.cpp
+++ b/src/gpu/vk/GrVkGpu.cpp
@@ -1842,8 +1842,10 @@
#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 @@
#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 @@
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 @@
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 @@
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);