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>
This commit is contained in:
Greg Daniel 2019-03-25 12:30:45 -04:00 committed by Skia Commit-Bot
parent c6142d855c
commit a51e93c9a0
3 changed files with 114 additions and 49 deletions

View File

@ -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;

View File

@ -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;

View File

@ -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);