Correctly discard or load RT when doing copies as draws in Vulkan

This fixes all the copy as draw issues we've had with certain devices and
the cap is no longer needed.

Bug: skia:
Change-Id: Id0b750849c4c920beae2d8cb3eda5f402018f194
Reviewed-on: https://skia-review.googlesource.com/114860
Commit-Queue: Greg Daniel <egdaniel@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
This commit is contained in:
Greg Daniel 2018-03-16 16:13:10 -04:00 committed by Skia Commit-Bot
parent 041e7ced7c
commit 55fa647596
16 changed files with 51 additions and 48 deletions

View File

@ -963,7 +963,14 @@ sk_sp<GrSurfaceContext> GrContextPriv::makeDeferredSurfaceContext(const GrSurfac
return nullptr;
}
return this->makeWrappedSurfaceContext(std::move(proxy), std::move(colorSpace), props);
sk_sp<GrSurfaceContext> sContext = this->makeWrappedSurfaceContext(std::move(proxy),
std::move(colorSpace),
props);
if (sContext && sContext->asRenderTargetContext()) {
sContext->asRenderTargetContext()->discard();
}
return sContext;
}
sk_sp<GrTextureContext> GrContextPriv::makeBackendTextureContext(const GrBackendTexture& tex,

View File

@ -185,11 +185,13 @@ GrBuffer* GrGpu::createBuffer(size_t size, GrBufferType intendedType,
bool GrGpu::copySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
GrSurface* src, GrSurfaceOrigin srcOrigin,
const SkIRect& srcRect, const SkIPoint& dstPoint) {
const SkIRect& srcRect, const SkIPoint& dstPoint,
bool canDiscardOutsideDstRect) {
GR_CREATE_TRACE_MARKER_CONTEXT("GrGpu", "copySurface", fContext);
SkASSERT(dst && src);
this->handleDirtyContext();
return this->onCopySurface(dst, dstOrigin, src, srcOrigin, srcRect, dstPoint);
return this->onCopySurface(dst, dstOrigin, src, srcOrigin, srcRect, dstPoint,
canDiscardOutsideDstRect);
}
bool GrGpu::getReadPixelsInfo(GrSurface* srcSurface, GrSurfaceOrigin srcOrigin, int width,

View File

@ -345,11 +345,13 @@ public:
// Called to perform a surface to surface copy. Fallbacks to issuing a draw from the src to dst
// take place at the GrOpList level and this function implement faster copy paths. The rect
// and point are pre-clipped. The src rect and implied dst rect are guaranteed to be within the
// src/dst bounds and non-empty.
// src/dst bounds and non-empty. If canDiscardOutsideDstRect is set to true then we don't need
// to preserve any data on the dst surface outside of the copy.
bool copySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
GrSurface* src, GrSurfaceOrigin srcOrigin,
const SkIRect& srcRect,
const SkIPoint& dstPoint);
const SkIPoint& dstPoint,
bool canDiscardOutsideDstRect = false);
// Creates a GrGpuRTCommandBuffer which GrOpLists send draw commands to instead of directly
// to the Gpu object.
@ -599,7 +601,8 @@ private:
// overridden by backend specific derived class to perform the copy surface
virtual bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
GrSurface* src, GrSurfaceOrigin srcOrigin,
const SkIRect& srcRect, const SkIPoint& dstPoint) = 0;
const SkIRect& srcRect, const SkIPoint& dstPoint,
bool canDiscardOutsideDstRect) = 0;
virtual void onFinishFlush(bool insertedSemaphores) = 0;

View File

@ -3449,7 +3449,8 @@ void GrGLGpu::unbindTextureFBOForPixelOps(GrGLenum fboTarget, GrSurface* surface
bool GrGLGpu::onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
GrSurface* src, GrSurfaceOrigin srcOrigin,
const SkIRect& srcRect, const SkIPoint& dstPoint) {
const SkIRect& srcRect, const SkIPoint& dstPoint,
bool canDiscardOutsideDstRect) {
// None of our copy methods can handle a swizzle. TODO: Make copySurfaceAsDraw handle the
// swizzle.
if (this->caps()->shaderCaps()->configOutputSwizzle(src->config()) !=

View File

@ -256,7 +256,8 @@ private:
bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
GrSurface* src, GrSurfaceOrigin srcOrigin,
const SkIRect& srcRect, const SkIPoint& dstPoint) override;
const SkIRect& srcRect, const SkIPoint& dstPoint,
bool canDiscardOutsideDstRect) override;
// binds texture unit in GL
void setTextureUnit(int unitIdx);

View File

@ -100,7 +100,7 @@ private:
bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin, GrSurface* src,
GrSurfaceOrigin srcOrigin, const SkIRect& srcRect,
const SkIPoint& dstPoint) override {
const SkIPoint& dstPoint, bool canDiscardOutsideDstRect) override {
return true;
}

View File

@ -46,7 +46,8 @@ public:
bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
GrSurface* src, GrSurfaceOrigin srcOrigin,
const SkIRect& srcRect,
const SkIPoint& dstPoint) override { return false; }
const SkIPoint& dstPoint,
bool canDiscardOutsideDstRect) override { return false; }
GrGpuRTCommandBuffer* createCommandBuffer(
GrRenderTarget*, GrSurfaceOrigin,

View File

@ -19,7 +19,6 @@ GrVkCaps::GrVkCaps(const GrContextOptions& contextOptions, const GrVkInterface*
: INHERITED(contextOptions) {
fCanUseGLSLForShaderModule = false;
fMustDoCopiesFromOrigin = false;
fSupportsCopiesAsDraws = true;
fMustSubmitCommandsBeforeCopyOp = false;
fMustSleepOnTearDown = false;
fNewCBOnPipelineChange = false;
@ -65,7 +64,7 @@ bool GrVkCaps::initDescForDstCopy(const GrRenderTargetProxy* src, GrSurfaceDesc*
// render target as well.
*origin = src->origin();
desc->fConfig = src->config();
if (src->numColorSamples() > 1 || (src->asTextureProxy() && this->supportsCopiesAsDraws())) {
if (src->numColorSamples() > 1 || src->asTextureProxy()) {
desc->fFlags = kRenderTarget_GrSurfaceFlag;
} else {
// Just going to use CopyImage here
@ -116,13 +115,6 @@ void GrVkCaps::applyDriverCorrectnessWorkarounds(const VkPhysicalDevicePropertie
fMustSubmitCommandsBeforeCopyOp = true;
}
if (kQualcomm_VkVendor == properties.vendorID ||
kARM_VkVendor == properties.vendorID) {
fSupportsCopiesAsDraws = false;
// We require copies as draws to support cross context textures.
fCrossContextTextureSupport = false;
}
#if defined(SK_BUILD_FOR_WIN)
if (kNvidia_VkVendor == properties.vendorID) {
fMustSleepOnTearDown = true;

View File

@ -74,11 +74,6 @@ public:
return fMustDoCopiesFromOrigin;
}
// Check whether we support using draws for copies.
bool supportsCopiesAsDraws() const {
return fSupportsCopiesAsDraws;
}
// On Nvidia there is a current bug where we must the current command buffer before copy
// operations or else the copy will not happen. This includes copies, blits, resolves, and copy
// as draws.
@ -176,8 +171,6 @@ private:
bool fMustDoCopiesFromOrigin;
bool fSupportsCopiesAsDraws;
bool fMustSubmitCommandsBeforeCopyOp;
bool fMustSleepOnTearDown;

View File

@ -142,7 +142,8 @@ bool GrVkCopyManager::createCopyProgram(GrVkGpu* gpu) {
bool GrVkCopyManager::copySurfaceAsDraw(GrVkGpu* gpu,
GrSurface* dst, GrSurfaceOrigin dstOrigin,
GrSurface* src, GrSurfaceOrigin srcOrigin,
const SkIRect& srcRect, const SkIPoint& dstPoint) {
const SkIRect& srcRect, const SkIPoint& dstPoint,
bool canDiscardOutsideDstRect) {
// None of our copy methods can handle a swizzle. TODO: Make copySurfaceAsDraw handle the
// swizzle.
if (gpu->caps()->shaderCaps()->configOutputSwizzle(src->config()) !=
@ -150,10 +151,6 @@ bool GrVkCopyManager::copySurfaceAsDraw(GrVkGpu* gpu,
return false;
}
if (!gpu->vkCaps().supportsCopiesAsDraws()) {
return false;
}
if (gpu->vkCaps().newCBOnPipelineChange()) {
// We bind a new pipeline here for the copy so we must start a new command buffer.
gpu->finishFlush(0, nullptr);
@ -312,13 +309,13 @@ bool GrVkCopyManager::copySurfaceAsDraw(GrVkGpu* gpu,
VK_PIPELINE_STAGE_ALL_GRAPHICS_BIT,
false);
GrVkRenderPass::LoadStoreOps vkColorOps(VK_ATTACHMENT_LOAD_OP_DONT_CARE,
VK_ATTACHMENT_STORE_OP_STORE);
VkAttachmentLoadOp loadOp = canDiscardOutsideDstRect ? VK_ATTACHMENT_LOAD_OP_DONT_CARE
: VK_ATTACHMENT_LOAD_OP_LOAD;
GrVkRenderPass::LoadStoreOps vkColorOps(loadOp, VK_ATTACHMENT_STORE_OP_STORE);
GrVkRenderPass::LoadStoreOps vkStencilOps(VK_ATTACHMENT_LOAD_OP_LOAD,
VK_ATTACHMENT_STORE_OP_STORE);
const GrVkRenderPass* renderPass;
const GrVkResourceProvider::CompatibleRPHandle& rpHandle =
rt->compatibleRenderPassHandle();
const GrVkResourceProvider::CompatibleRPHandle& rpHandle = rt->compatibleRenderPassHandle();
if (rpHandle.isValid()) {
renderPass = gpu->resourceProvider().findRenderPass(rpHandle,
vkColorOps,

View File

@ -30,7 +30,8 @@ public:
bool copySurfaceAsDraw(GrVkGpu* gpu,
GrSurface* dst, GrSurfaceOrigin dstOrigin,
GrSurface* src, GrSurfaceOrigin srcOrigin,
const SkIRect& srcRect, const SkIPoint& dstPoint);
const SkIRect& srcRect, const SkIPoint& dstPoint,
bool canDiscardOutsideDstRect);
void destroyResources(GrVkGpu* gpu);
void abandonResources();

View File

@ -1854,8 +1854,8 @@ void GrVkGpu::copySurfaceAsResolve(GrSurface* dst, GrSurfaceOrigin dstOrigin, Gr
bool GrVkGpu::onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
GrSurface* src, GrSurfaceOrigin srcOrigin,
const SkIRect& srcRect,
const SkIPoint& dstPoint) {
const SkIRect& srcRect, const SkIPoint& dstPoint,
bool canDiscardOutsideDstRect) {
if (can_copy_as_resolve(dst, dstOrigin, src, srcOrigin, this)) {
this->copySurfaceAsResolve(dst, dstOrigin, src, srcOrigin, srcRect, dstPoint);
return true;
@ -1865,7 +1865,8 @@ bool GrVkGpu::onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin,
this->submitCommandBuffer(GrVkGpu::kSkip_SyncQueue);
}
if (fCopyManager.copySurfaceAsDraw(this, dst, dstOrigin, src, srcOrigin, srcRect, dstPoint)) {
if (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;

View File

@ -204,7 +204,7 @@ private:
bool onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin, GrSurface* src,
GrSurfaceOrigin srcOrigin, const SkIRect& srcRect,
const SkIPoint& dstPoint) override;
const SkIPoint& dstPoint, bool canDiscardOutsideDstRect) override;
void onFinishFlush(bool insertedSemaphores) override;

View File

@ -174,7 +174,7 @@ void GrVkGpuRTCommandBuffer::submit() {
for (int j = 0; j < cbInfo.fPreCopies.count(); ++j) {
CopyInfo& copyInfo = cbInfo.fPreCopies[j];
fGpu->copySurface(fRenderTarget, fOrigin, copyInfo.fSrc, copyInfo.fSrcOrigin,
copyInfo.fSrcRect, copyInfo.fDstPoint);
copyInfo.fSrcRect, copyInfo.fDstPoint, copyInfo.fShouldDiscardDst);
}
// Make sure we do the following layout changes after all copies, uploads, or any other pre
@ -476,6 +476,10 @@ void GrVkGpuRTCommandBuffer::copy(GrSurface* src, GrSurfaceOrigin srcOrigin, con
this->addAdditionalRenderPass();
}
fCommandBufferInfos[fCurrentCmdInfo].fPreCopies.emplace_back(
src, srcOrigin, srcRect, dstPoint,
LoadStoreState::kStartsWithDiscard == cbInfo.fLoadStoreState);
if (LoadStoreState::kLoadAndStore != cbInfo.fLoadStoreState) {
// Change the render pass to do a load and store so we don't lose the results of our copy
GrVkRenderPass::LoadStoreOps vkColorOps(VK_ATTACHMENT_LOAD_OP_LOAD,
@ -503,7 +507,6 @@ void GrVkGpuRTCommandBuffer::copy(GrSurface* src, GrSurfaceOrigin srcOrigin, con
cbInfo.fLoadStoreState = LoadStoreState::kLoadAndStore;
}
fCommandBufferInfos[fCurrentCmdInfo].fPreCopies.emplace_back(src, srcOrigin, srcRect, dstPoint);
}
////////////////////////////////////////////////////////////////////////////////

View File

@ -143,13 +143,18 @@ private:
struct CopyInfo {
CopyInfo(GrSurface* src, GrSurfaceOrigin srcOrigin, const SkIRect& srcRect,
const SkIPoint& dstPoint)
: fSrc(src), fSrcOrigin(srcOrigin), fSrcRect(srcRect), fDstPoint(dstPoint) {}
const SkIPoint& dstPoint, bool shouldDiscardDst)
: fSrc(src)
, fSrcOrigin(srcOrigin)
, fSrcRect(srcRect)
, fDstPoint(dstPoint)
, fShouldDiscardDst(shouldDiscardDst) {}
GrSurface* fSrc;
GrSurfaceOrigin fSrcOrigin;
SkIRect fSrcRect;
SkIPoint fDstPoint;
bool fShouldDiscardDst;
};
enum class LoadStoreState {

View File

@ -178,10 +178,6 @@ DEF_GPUTEST_FOR_VULKAN_CONTEXT(VkMakeCopyPipelineTest, reporter, ctxInfo) {
GrContext* context = ctxInfo.grContext();
GrVkGpu* gpu = static_cast<GrVkGpu*>(context->contextPriv().getGpu());
if (!gpu->vkCaps().supportsCopiesAsDraws()) {
return;
}
TestVkCopyProgram copyProgram;
copyProgram.test(gpu, reporter);
}