From be7fc466271b225d4f286856eb14a1cadb103d0a Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Thu, 3 Jan 2019 16:40:42 -0500 Subject: [PATCH] Restrict ops that can be executed when we have a wrapped vulkan secondary command buffer. Bug: skia: Change-Id: Ib58ba23053d019988a23cfb489808bad3122d867 Reviewed-on: https://skia-review.googlesource.com/c/178936 Commit-Queue: Greg Daniel Reviewed-by: Robert Phillips --- src/gpu/GrClipStackClip.cpp | 14 +++++++++---- src/gpu/GrPathRenderer.cpp | 1 + src/gpu/GrPathRenderer.h | 3 ++- src/gpu/GrReducedClip.cpp | 1 + src/gpu/GrRenderTargetContext.cpp | 17 ++++++++++++++++ src/gpu/SkGpuDevice.cpp | 20 +++++++++++++++---- src/gpu/ops/GrDefaultPathRenderer.cpp | 3 ++- src/gpu/ops/GrStencilAndCoverPathRenderer.cpp | 1 + src/gpu/vk/GrVkCaps.cpp | 12 +++++++++++ src/gpu/vk/GrVkGpu.cpp | 19 ++++++++++++++++++ 10 files changed, 81 insertions(+), 10 deletions(-) diff --git a/src/gpu/GrClipStackClip.cpp b/src/gpu/GrClipStackClip.cpp index b62f9a1687..7aadaa45dc 100644 --- a/src/gpu/GrClipStackClip.cpp +++ b/src/gpu/GrClipStackClip.cpp @@ -127,6 +127,8 @@ bool GrClipStackClip::PathNeedsSWRenderer(GrContext* context, renderTargetContext->fsaaType(), GrAllowMixedSamples::kYes, *context->contextPriv().caps()); + SkASSERT(!renderTargetContext->wrapsVkSecondaryCB()); + canDrawArgs.fTargetIsWrappedVkSecondaryCB = false; canDrawArgs.fHasUserStencilSettings = hasUserStencilSettings; // the 'false' parameter disallows use of the SW path renderer @@ -156,8 +158,10 @@ bool GrClipStackClip::UseSWOnlyPath(GrContext* context, // a clip gets complex enough it can just be done in SW regardless // of whether it would invoke the GrSoftwarePathRenderer. - // If we're avoiding stencils, always use SW: - if (context->contextPriv().caps()->avoidStencilBuffers()) { + // If we're avoiding stencils, always use SW. This includes drawing into a wrapped vulkan + // secondary command buffer which can't handle stencils. + if (context->contextPriv().caps()->avoidStencilBuffers() || + renderTargetContext->wrapsVkSecondaryCB()) { return true; } @@ -258,7 +262,8 @@ bool GrClipStackClip::applyClipMask(GrContext* context, GrRenderTargetContext* r // If the stencil buffer is multisampled we can use it to do everything. if ((GrFSAAType::kNone == renderTargetContext->fsaaType() && reducedClip.maskRequiresAA()) || - context->contextPriv().caps()->avoidStencilBuffers()) { + context->contextPriv().caps()->avoidStencilBuffers() || + renderTargetContext->wrapsVkSecondaryCB()) { sk_sp result; if (UseSWOnlyPath(context, hasUserStencilSettings, renderTargetContext, reducedClip)) { // The clip geometry is complex enough that it will be more efficient to create it @@ -277,7 +282,8 @@ bool GrClipStackClip::applyClipMask(GrContext* context, GrRenderTargetContext* r // If alpha or software clip mask creation fails, fall through to the stencil code paths, // unless stencils are disallowed. - if (context->contextPriv().caps()->avoidStencilBuffers()) { + if (context->contextPriv().caps()->avoidStencilBuffers() || + renderTargetContext->wrapsVkSecondaryCB()) { SkDebugf("WARNING: Clip mask requires stencil, but stencil unavailable. " "Clip will be ignored.\n"); return false; diff --git a/src/gpu/GrPathRenderer.cpp b/src/gpu/GrPathRenderer.cpp index 71a34ea08a..631f96d78f 100644 --- a/src/gpu/GrPathRenderer.cpp +++ b/src/gpu/GrPathRenderer.cpp @@ -50,6 +50,7 @@ bool GrPathRenderer::drawPath(const DrawPathArgs& args) { canArgs.fViewMatrix = args.fViewMatrix; canArgs.fShape = args.fShape; canArgs.fAAType = args.fAAType; + canArgs.fTargetIsWrappedVkSecondaryCB = args.fRenderTargetContext->wrapsVkSecondaryCB(); canArgs.validate(); canArgs.fHasUserStencilSettings = !args.fUserStencilSettings->isUnused(); diff --git a/src/gpu/GrPathRenderer.h b/src/gpu/GrPathRenderer.h index 5b59563eda..a6c3ed5d2c 100644 --- a/src/gpu/GrPathRenderer.h +++ b/src/gpu/GrPathRenderer.h @@ -80,8 +80,9 @@ public: const SkMatrix* fViewMatrix; const GrShape* fShape; GrAAType fAAType; + bool fTargetIsWrappedVkSecondaryCB; - // These next two are only used by GrStencilAndCoverPathRenderer + // This is only used by GrStencilAndCoverPathRenderer bool fHasUserStencilSettings; #ifdef SK_DEBUG diff --git a/src/gpu/GrReducedClip.cpp b/src/gpu/GrReducedClip.cpp index 97af1de05c..6379cbd124 100644 --- a/src/gpu/GrReducedClip.cpp +++ b/src/gpu/GrReducedClip.cpp @@ -855,6 +855,7 @@ bool GrReducedClip::drawStencilClipMask(GrContext* context, canDrawArgs.fShape = &shape; canDrawArgs.fAAType = aaType; canDrawArgs.fHasUserStencilSettings = false; + canDrawArgs.fTargetIsWrappedVkSecondaryCB = renderTargetContext->wrapsVkSecondaryCB(); GrDrawingManager* dm = context->contextPriv().drawingManager(); pr = dm->getPathRenderer(canDrawArgs, false, GrPathRendererChain::DrawType::kStencil, diff --git a/src/gpu/GrRenderTargetContext.cpp b/src/gpu/GrRenderTargetContext.cpp index 8f865b1658..ec40e4849b 100644 --- a/src/gpu/GrRenderTargetContext.cpp +++ b/src/gpu/GrRenderTargetContext.cpp @@ -241,6 +241,13 @@ void GrRenderTargetContext::drawGlyphRunList( SkDEBUGCODE(this->validate();) GR_CREATE_TRACE_MARKER_CONTEXT("GrRenderTargetContext", "drawGlyphRunList", fContext); + // Drawing text can cause us to do inline uploads. This is not supported for wrapped vulkan + // secondary command buffers because it would require stopping and starting a render pass which + // we don't have access to. + if (this->wrapsVkSecondaryCB()) { + return; + } + GrTextContext* atlasTextContext = this->drawingManager()->getTextContext(); atlasTextContext->drawGlyphRunList(fContext, fTextTarget.get(), clip, viewMatrix, fSurfaceProps, blob); @@ -1577,6 +1584,8 @@ bool GrRenderTargetContextPriv::drawAndStencilPath(const GrHardClip& clip, canDrawArgs.fShape = &shape; canDrawArgs.fClipConservativeBounds = &clipConservativeBounds; canDrawArgs.fAAType = aaType; + SkASSERT(!fRenderTargetContext->wrapsVkSecondaryCB()); + canDrawArgs.fTargetIsWrappedVkSecondaryCB = false; canDrawArgs.fHasUserStencilSettings = hasUserStencilSettings; // Don't allow the SW renderer @@ -1644,6 +1653,7 @@ void GrRenderTargetContext::drawShapeUsingPathRenderer(const GrClip& clip, canDrawArgs.fViewMatrix = &viewMatrix; canDrawArgs.fShape = &originalShape; canDrawArgs.fClipConservativeBounds = &clipConservativeBounds; + canDrawArgs.fTargetIsWrappedVkSecondaryCB = this->wrapsVkSecondaryCB(); canDrawArgs.fHasUserStencilSettings = false; GrPathRenderer* pr; @@ -1777,6 +1787,13 @@ void GrRenderTargetContext::addDrawOp(const GrClip& clip, std::unique_ptrwrapsVkSecondaryCB()) { + return false; + } + if (this->caps()->textureBarrierSupport()) { if (GrTextureProxy* texProxy = rtProxy->asTextureProxy()) { // The render target is a texture, so we can read from it directly in the shader. The XP diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index 1dd1e4f55b..1524b7b79c 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -1230,6 +1230,14 @@ sk_sp SkGpuDevice::makeSpecial(const SkImage* image) { } sk_sp SkGpuDevice::snapSpecial() { + // If we are wrapping a vulkan secondary command buffer, then we can't snap off a special image + // since it would require us to make a copy of the underlying VkImage which we don't have access + // to. Additionaly we can't stop and start the render pass that is used with the secondary + // command buffer. + if (this->accessRenderTargetContext()->wrapsVkSecondaryCB()) { + return nullptr; + } + sk_sp proxy(this->accessRenderTargetContext()->asTextureProxyRef()); if (!proxy) { // When the device doesn't have a texture, we create a temporary texture. @@ -1258,14 +1266,18 @@ sk_sp SkGpuDevice::snapSpecial() { sk_sp SkGpuDevice::snapBackImage(const SkIRect& subset) { GrRenderTargetContext* rtc = this->accessRenderTargetContext(); - if (!rtc) { + + // If we are wrapping a vulkan secondary command buffer, then we can't snap off a special image + // since it would require us to make a copy of the underlying VkImage which we don't have access + // to. Additionaly we can't stop and start the render pass that is used with the secondary + // command buffer. + if (rtc->wrapsVkSecondaryCB()) { return nullptr; } + GrContext* ctx = this->context(); - if (!rtc->asSurfaceProxy()) { - return nullptr; - } + SkASSERT(rtc->asSurfaceProxy()); auto srcProxy = GrSurfaceProxy::Copy(ctx, rtc->asSurfaceProxy(), rtc->mipMapped(), subset, diff --git a/src/gpu/ops/GrDefaultPathRenderer.cpp b/src/gpu/ops/GrDefaultPathRenderer.cpp index 439d8903cf..54e98f16c0 100644 --- a/src/gpu/ops/GrDefaultPathRenderer.cpp +++ b/src/gpu/ops/GrDefaultPathRenderer.cpp @@ -641,7 +641,8 @@ GrPathRenderer::CanDrawPath GrDefaultPathRenderer::onCanDrawPath(const CanDrawPathArgs& args) const { bool isHairline = IsStrokeHairlineOrEquivalent(args.fShape->style(), *args.fViewMatrix, nullptr); // If we aren't a single_pass_shape or hairline, we require stencil buffers. - if (!(single_pass_shape(*args.fShape) || isHairline) && args.fCaps->avoidStencilBuffers()) { + if (!(single_pass_shape(*args.fShape) || isHairline) && + (args.fCaps->avoidStencilBuffers() || args.fTargetIsWrappedVkSecondaryCB)) { return CanDrawPath::kNo; } // This can draw any path with any simple fill style but doesn't do coverage-based antialiasing. diff --git a/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp b/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp index 9a54dfedfb..16db3b3ee3 100644 --- a/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp +++ b/src/gpu/ops/GrStencilAndCoverPathRenderer.cpp @@ -35,6 +35,7 @@ GrStencilAndCoverPathRenderer::GrStencilAndCoverPathRenderer(GrResourceProvider* GrPathRenderer::CanDrawPath GrStencilAndCoverPathRenderer::onCanDrawPath(const CanDrawPathArgs& args) const { + SkASSERT(!args.fTargetIsWrappedVkSecondaryCB); // GrPath doesn't support hairline paths. An arbitrary path effect could produce a hairline // path. if (args.fShape->style().strokeRec().isHairlineStyle() || diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp index 21302fe5de..ba4d1a63bf 100644 --- a/src/gpu/vk/GrVkCaps.cpp +++ b/src/gpu/vk/GrVkCaps.cpp @@ -176,9 +176,21 @@ bool GrVkCaps::onCanCopySurface(const GrSurfaceProxy* dst, const GrSurfaceProxy* int dstSampleCnt = 0; int srcSampleCnt = 0; if (const GrRenderTargetProxy* rtProxy = dst->asRenderTargetProxy()) { + // Copying to or from render targets that wrap a secondary command buffer is not allowed + // since they would require us to know the VkImage, which we don't have, as well as need us + // to stop and start the VkRenderPass which we don't have access to. + if (rtProxy->wrapsVkSecondaryCB()) { + return false; + } dstSampleCnt = rtProxy->numColorSamples(); } if (const GrRenderTargetProxy* rtProxy = src->asRenderTargetProxy()) { + // Copying to or from render targets that wrap a secondary command buffer is not allowed + // since they would require us to know the VkImage, which we don't have, as well as need us + // to stop and start the VkRenderPass which we don't have access to. + if (rtProxy->wrapsVkSecondaryCB()) { + return false; + } srcSampleCnt = rtProxy->numColorSamples(); } SkASSERT((dstSampleCnt > 0) == SkToBool(dst->asRenderTargetProxy())); diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp index 7d71d22c6e..59764ad59e 100644 --- a/src/gpu/vk/GrVkGpu.cpp +++ b/src/gpu/vk/GrVkGpu.cpp @@ -1773,6 +1773,15 @@ bool GrVkGpu::onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin, GrSurface* src, GrSurfaceOrigin srcOrigin, const SkIRect& srcRect, const SkIPoint& dstPoint, bool canDiscardOutsideDstRect) { +#ifdef SK_DEBUG + if (GrVkRenderTarget* srcRT = static_cast(src->asRenderTarget())) { + SkASSERT(!srcRT->wrapsSecondaryCommandBuffer()); + } + if (GrVkRenderTarget* dstRT = static_cast(dst->asRenderTarget())) { + SkASSERT(!dstRT->wrapsSecondaryCommandBuffer()); + } +#endif + GrPixelConfig dstConfig = dst->config(); GrPixelConfig srcConfig = src->config(); @@ -1799,6 +1808,9 @@ bool GrVkGpu::onCopySurface(GrSurface* dst, GrSurfaceOrigin dstOrigin, GrRenderTarget* dstRT = dst->asRenderTarget(); if (dstRT) { GrVkRenderTarget* vkRT = static_cast(dstRT); + if (vkRT->wrapsSecondaryCommandBuffer()) { + return false; + } dstImage = vkRT->numColorSamples() > 1 ? vkRT->msaaImage() : vkRT; } else { SkASSERT(dst->asTexture()); @@ -1839,6 +1851,12 @@ bool GrVkGpu::onReadPixels(GrSurface* surface, int left, int top, int width, int GrVkImage* image = nullptr; GrVkRenderTarget* rt = static_cast(surface->asRenderTarget()); if (rt) { + // Reading from render targets that wrap a secondary command buffer is not allowed since + // it would require us to know the VkImage, which we don't have, as well as need us to + // stop and start the VkRenderPass which we don't have access to. + if (rt->wrapsSecondaryCommandBuffer()) { + return false; + } // resolve the render target if necessary switch (rt->getResolveType()) { case GrVkRenderTarget::kCantResolve_ResolveType: @@ -2039,6 +2057,7 @@ void GrVkGpu::submitSecondaryCommandBuffer(const SkTArraywrapsSecondaryCommandBuffer()); const SkIRect* pBounds = &bounds; SkIRect flippedBounds; if (kBottomLeft_GrSurfaceOrigin == origin) {