From 805c62200d6cdb6449ac4a4bf6a7ca7e510e053c Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Tue, 20 Apr 2021 12:44:48 -0400 Subject: [PATCH] Move cached render passes onto GrVkFramebuffer. This moves them off of GrVkRenderTarget so that they can be directly accessed with just a GrVkFramebuffer. Bug: skia:11809 Change-Id: I5e5024779dc106642de9035400df2b04d35ad753 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/398657 Commit-Queue: Greg Daniel Reviewed-by: Jim Van Verth --- src/gpu/vk/GrVkCaps.cpp | 4 +- src/gpu/vk/GrVkFramebuffer.cpp | 17 +++-- src/gpu/vk/GrVkFramebuffer.h | 18 +++-- src/gpu/vk/GrVkRenderTarget.cpp | 127 +++++++++++++------------------- src/gpu/vk/GrVkRenderTarget.h | 28 +++---- 5 files changed, 88 insertions(+), 106 deletions(-) diff --git a/src/gpu/vk/GrVkCaps.cpp b/src/gpu/vk/GrVkCaps.cpp index ad5b863e8a..661be6532e 100644 --- a/src/gpu/vk/GrVkCaps.cpp +++ b/src/gpu/vk/GrVkCaps.cpp @@ -1842,8 +1842,8 @@ GrProgramDesc GrVkCaps::makeDesc(GrRenderTarget* rt, bool needsStencil = programInfo.numStencilSamples() || programInfo.isStencilEnabled(); // TODO: support failure in getSimpleRenderPass - auto[rp, compatibleHandle] = vkRT->getSimpleRenderPass(needsResolve, needsStencil, - selfDepFlags, loadFromResolve); + auto rp = vkRT->getSimpleRenderPass(needsResolve, needsStencil, selfDepFlags, + loadFromResolve); SkASSERT(rp); rp->genKey(&b); diff --git a/src/gpu/vk/GrVkFramebuffer.cpp b/src/gpu/vk/GrVkFramebuffer.cpp index 5dffcc82e4..dfbe9d26c5 100644 --- a/src/gpu/vk/GrVkFramebuffer.cpp +++ b/src/gpu/vk/GrVkFramebuffer.cpp @@ -13,16 +13,16 @@ #include "src/gpu/vk/GrVkImageView.h" #include "src/gpu/vk/GrVkRenderPass.h" -GrVkFramebuffer* GrVkFramebuffer::Create( +sk_sp GrVkFramebuffer::Make( GrVkGpu* gpu, SkISize dimensions, - const GrVkRenderPass* renderPass, + sk_sp compatibleRenderPass, GrVkAttachment* colorAttachment, GrVkAttachment* resolveAttachment, GrVkAttachment* stencilAttachment, GrVkResourceProvider::CompatibleRPHandle compatibleRenderPassHandle) { // At the very least we need a renderPass and a colorAttachment - SkASSERT(renderPass); + SkASSERT(compatibleRenderPass); SkASSERT(colorAttachment); VkImageView attachments[3]; @@ -40,7 +40,7 @@ GrVkFramebuffer* GrVkFramebuffer::Create( createInfo.sType = VK_STRUCTURE_TYPE_FRAMEBUFFER_CREATE_INFO; createInfo.pNext = nullptr; createInfo.flags = 0; - createInfo.renderPass = renderPass->vkRenderPass(); + createInfo.renderPass = compatibleRenderPass->vkRenderPass(); createInfo.attachmentCount = numAttachments; createInfo.pAttachments = attachments; createInfo.width = dimensions.width(); @@ -55,9 +55,10 @@ GrVkFramebuffer* GrVkFramebuffer::Create( return nullptr; } - return new GrVkFramebuffer(gpu, framebuffer, sk_ref_sp(colorAttachment), - sk_ref_sp(resolveAttachment), sk_ref_sp(stencilAttachment), - compatibleRenderPassHandle); + auto fb = new GrVkFramebuffer(gpu, framebuffer, sk_ref_sp(colorAttachment), + sk_ref_sp(resolveAttachment), sk_ref_sp(stencilAttachment), + std::move(compatibleRenderPass), compatibleRenderPassHandle); + return sk_sp(fb); } GrVkFramebuffer::GrVkFramebuffer(const GrVkGpu* gpu, @@ -65,12 +66,14 @@ GrVkFramebuffer::GrVkFramebuffer(const GrVkGpu* gpu, sk_sp colorAttachment, sk_sp resolveAttachment, sk_sp stencilAttachment, + sk_sp compatibleRenderPass, GrVkResourceProvider::CompatibleRPHandle compatibleRPHandle) : GrVkManagedResource(gpu) , fFramebuffer(framebuffer) , fColorAttachment(std::move(colorAttachment)) , fResolveAttachment(std::move(resolveAttachment)) , fStencilAttachment(std::move(stencilAttachment)) + , fCompatibleRenderPass(std::move(compatibleRenderPass)) , fCompatibleRenderPassHandle(compatibleRPHandle) { SkASSERT(fCompatibleRenderPassHandle.isValid()); } diff --git a/src/gpu/vk/GrVkFramebuffer.h b/src/gpu/vk/GrVkFramebuffer.h index 98df382aeb..81b69899af 100644 --- a/src/gpu/vk/GrVkFramebuffer.h +++ b/src/gpu/vk/GrVkFramebuffer.h @@ -20,13 +20,13 @@ class GrVkRenderPass; class GrVkFramebuffer : public GrVkManagedResource { public: - static GrVkFramebuffer* Create(GrVkGpu* gpu, - SkISize dimensions, - const GrVkRenderPass* renderPass, - GrVkAttachment* colorAttachment, - GrVkAttachment* resolveAttachment, - GrVkAttachment* stencilAttachment, - GrVkResourceProvider::CompatibleRPHandle); + static sk_sp Make(GrVkGpu* gpu, + SkISize dimensions, + sk_sp compatibleRenderPass, + GrVkAttachment* colorAttachment, + GrVkAttachment* resolveAttachment, + GrVkAttachment* stencilAttachment, + GrVkResourceProvider::CompatibleRPHandle); // Used for wrapped external secondary command buffers GrVkFramebuffer(const GrVkGpu* gpu, @@ -61,6 +61,8 @@ public: } #endif + const GrVkRenderPass* compatibleRenderPass() const { return fCompatibleRenderPass.get(); } + GrVkResourceProvider::CompatibleRPHandle compatibleRenderPassHandle() const { return fCompatibleRenderPassHandle; } @@ -75,6 +77,7 @@ private: sk_sp colorAttachment, sk_sp resolveAttachment, sk_sp stencilAttachment, + sk_sp compatibleRenderPass, GrVkResourceProvider::CompatibleRPHandle); ~GrVkFramebuffer() override; @@ -88,6 +91,7 @@ private: sk_sp fResolveAttachment; sk_sp fStencilAttachment; + sk_sp fCompatibleRenderPass; GrVkResourceProvider::CompatibleRPHandle fCompatibleRenderPassHandle; sk_sp fExternalRenderPass; diff --git a/src/gpu/vk/GrVkRenderTarget.cpp b/src/gpu/vk/GrVkRenderTarget.cpp index 548832f89c..f7efbf71d3 100644 --- a/src/gpu/vk/GrVkRenderTarget.cpp +++ b/src/gpu/vk/GrVkRenderTarget.cpp @@ -61,8 +61,7 @@ GrVkRenderTarget::GrVkRenderTarget(GrVkGpu* gpu, colorAttachment->isProtected() ? GrProtected::kYes : GrProtected::kNo) , fColorAttachment(std::move(colorAttachment)) , fResolveAttachment(std::move(resolveAttachment)) - , fCachedFramebuffers() - , fCachedRenderPasses() { + , fCachedFramebuffers() { SkASSERT(fColorAttachment); SkASSERT(!resolveAttachment || (fResolveAttachment->isProtected() == fColorAttachment->isProtected())); @@ -83,7 +82,6 @@ GrVkRenderTarget::GrVkRenderTarget(GrVkGpu* gpu, externalFramebuffer->colorAttachment()->isProtected() ? GrProtected::kYes : GrProtected::kNo) , fCachedFramebuffers() - , fCachedRenderPasses() , fExternalFramebuffer(externalFramebuffer) { SkASSERT(fExternalFramebuffer); SkASSERT(!fColorAttachment); @@ -245,66 +243,47 @@ GrVkResourceProvider::CompatibleRPHandle GrVkRenderTarget::compatibleRenderPassH LoadFromResolve loadFromResolve) { SkASSERT(!this->wrapsSecondaryCommandBuffer()); - int cacheIndex = - renderpass_features_to_index(withResolve, withStencil, selfDepFlags, loadFromResolve); - SkASSERT(cacheIndex < GrVkRenderTarget::kNumCachedRenderPasses); - - GrVkResourceProvider::CompatibleRPHandle* pRPHandle; - pRPHandle = &fCompatibleRPHandles[cacheIndex]; - - if (!pRPHandle->isValid()) { - this->createSimpleRenderPass(withResolve, withStencil, selfDepFlags, loadFromResolve); + const GrVkFramebuffer* fb = + this->getFramebuffer(withResolve, withStencil, selfDepFlags, loadFromResolve); + if (!fb) { + return {}; } -#ifdef SK_DEBUG - const GrVkRenderPass* rp = fCachedRenderPasses[cacheIndex]; - SkASSERT(pRPHandle->isValid() == SkToBool(rp)); - if (rp) { - SkASSERT(selfDepFlags == rp->selfDependencyFlags()); - } -#endif + return fb->compatibleRenderPassHandle(); +} - return *pRPHandle; +const GrVkRenderPass* GrVkRenderTarget::getSimpleRenderPass(bool withResolve, + bool withStencil, + SelfDependencyFlags selfDepFlags, + LoadFromResolve loadFromResolve) { + if (this->wrapsSecondaryCommandBuffer()) { + return fExternalFramebuffer->externalRenderPass(); + } + + const GrVkFramebuffer* fb = + this->getFramebuffer(withResolve, withStencil, selfDepFlags, loadFromResolve); + if (!fb) { + return nullptr; + } + + return fb->compatibleRenderPass(); } std::pair - GrVkRenderTarget::getSimpleRenderPass(bool withResolve, - bool withStencil, - SelfDependencyFlags selfDepFlags, - LoadFromResolve loadFromResolve) { - if (this->wrapsSecondaryCommandBuffer()) { - // The compatible handle is invalid for external render passes used in wrapped secondary - // command buffers. However, this should not be called by code using external render passes - // that needs to use the handle. - return {fExternalFramebuffer->externalRenderPass(), - GrVkResourceProvider::CompatibleRPHandle()}; - } - int cacheIndex = - renderpass_features_to_index(withResolve, withStencil, selfDepFlags, loadFromResolve); - SkASSERT(cacheIndex < GrVkRenderTarget::kNumCachedRenderPasses); - const GrVkRenderPass* rp = fCachedRenderPasses[cacheIndex]; - if (!rp) { - rp = this->createSimpleRenderPass(withResolve, withStencil, selfDepFlags, loadFromResolve); - } - SkASSERT(!rp || fCompatibleRPHandles[cacheIndex].isValid()); - return {rp, fCompatibleRPHandles[cacheIndex]}; -} - -const GrVkRenderPass* GrVkRenderTarget::createSimpleRenderPass(bool withResolve, - bool withStencil, - SelfDependencyFlags selfDepFlags, - LoadFromResolve loadFromResolve) { +GrVkRenderTarget::createSimpleRenderPass(bool withResolve, + bool withStencil, + SelfDependencyFlags selfDepFlags, + LoadFromResolve loadFromResolve) { SkASSERT(!this->wrapsSecondaryCommandBuffer()); GrVkResourceProvider& rp = this->getVkGpu()->resourceProvider(); - int cacheIndex = renderpass_features_to_index(withResolve, withStencil, selfDepFlags, - loadFromResolve); - SkASSERT(cacheIndex < GrVkRenderTarget::kNumCachedRenderPasses); - SkASSERT(!fCachedRenderPasses[cacheIndex]); - fCachedRenderPasses[cacheIndex] = rp.findCompatibleRenderPass( - this, &fCompatibleRPHandles[cacheIndex], withResolve, withStencil, selfDepFlags, + + GrVkResourceProvider::CompatibleRPHandle handle; + const GrVkRenderPass* renderPass = rp.findCompatibleRenderPass( + this, &handle, withResolve, withStencil, selfDepFlags, loadFromResolve); - return fCachedRenderPasses[cacheIndex]; + SkASSERT(!renderPass || handle.isValid()); + return {renderPass, handle}; } const GrVkFramebuffer* GrVkRenderTarget::getFramebuffer(bool withResolve, @@ -313,31 +292,32 @@ const GrVkFramebuffer* GrVkRenderTarget::getFramebuffer(bool withResolve, LoadFromResolve loadFromResolve) { int cacheIndex = renderpass_features_to_index(withResolve, withStencil, selfDepFlags, loadFromResolve); - SkASSERT(cacheIndex < GrVkRenderTarget::kNumCachedRenderPasses); + SkASSERT(cacheIndex < GrVkRenderTarget::kNumCachedFramebuffers); if (auto fb = fCachedFramebuffers[cacheIndex]) { - return fb; + return fb.get(); } - return this->createFramebuffer(withResolve, withStencil, selfDepFlags, loadFromResolve); + this->createFramebuffer(withResolve, withStencil, selfDepFlags, loadFromResolve); + return fCachedFramebuffers[cacheIndex].get(); } -const GrVkFramebuffer* GrVkRenderTarget::createFramebuffer(bool withResolve, - bool withStencil, - SelfDependencyFlags selfDepFlags, - LoadFromResolve loadFromResolve) { +void GrVkRenderTarget::createFramebuffer(bool withResolve, + bool withStencil, + SelfDependencyFlags selfDepFlags, + LoadFromResolve loadFromResolve) { SkASSERT(!this->wrapsSecondaryCommandBuffer()); GrVkGpu* gpu = this->getVkGpu(); auto[renderPass, compatibleHandle] = - this->getSimpleRenderPass(withResolve, withStencil, selfDepFlags, loadFromResolve); + this->createSimpleRenderPass(withResolve, withStencil, selfDepFlags, loadFromResolve); if (!renderPass) { - return nullptr; + return; } SkASSERT(compatibleHandle.isValid()); int cacheIndex = renderpass_features_to_index(withResolve, withStencil, selfDepFlags, loadFromResolve); - SkASSERT(cacheIndex < GrVkRenderTarget::kNumCachedRenderPasses); + SkASSERT(cacheIndex < GrVkRenderTarget::kNumCachedFramebuffers); GrVkAttachment* resolve = withResolve ? this->resolveAttachment() : nullptr; GrVkAttachment* colorAttachment = @@ -348,10 +328,9 @@ const GrVkFramebuffer* GrVkRenderTarget::createFramebuffer(bool withResolve, withStencil ? static_cast(this->getStencilAttachment()) : nullptr; fCachedFramebuffers[cacheIndex] = - GrVkFramebuffer::Create(gpu, this->dimensions(), renderPass, - colorAttachment, resolve, stencil, compatibleHandle); - - return fCachedFramebuffers[cacheIndex]; + GrVkFramebuffer::Make(gpu, this->dimensions(), + sk_sp(renderPass), + colorAttachment, resolve, stencil, compatibleHandle); } void GrVkRenderTarget::getAttachmentsDescriptor(GrVkRenderPass::AttachmentsDescriptor* desc, @@ -474,10 +453,10 @@ GrVkRenderTarget::~GrVkRenderTarget() { // either release or abandon should have been called by the owner of this object. SkASSERT(!fColorAttachment); SkASSERT(!fResolveAttachment); + SkASSERT(!fDynamicMSAAAttachment); - for (int i = 0; i < kNumCachedRenderPasses; ++i) { + for (int i = 0; i < kNumCachedFramebuffers; ++i) { SkASSERT(!fCachedFramebuffers[i]); - SkASSERT(!fCachedRenderPasses[i]); } SkASSERT(!fCachedInputDescriptorSet); @@ -486,15 +465,11 @@ GrVkRenderTarget::~GrVkRenderTarget() { void GrVkRenderTarget::releaseInternalObjects() { fColorAttachment.reset(); fResolveAttachment.reset(); + fDynamicMSAAAttachment.reset(); - for (int i = 0; i < kNumCachedRenderPasses; ++i) { + for (int i = 0; i < kNumCachedFramebuffers; ++i) { if (fCachedFramebuffers[i]) { - fCachedFramebuffers[i]->unref(); - fCachedFramebuffers[i] = nullptr; - } - if (fCachedRenderPasses[i]) { - fCachedRenderPasses[i]->unref(); - fCachedRenderPasses[i] = nullptr; + fCachedFramebuffers[i].reset(); } } diff --git a/src/gpu/vk/GrVkRenderTarget.h b/src/gpu/vk/GrVkRenderTarget.h index 056cbc7803..94a0277a54 100644 --- a/src/gpu/vk/GrVkRenderTarget.h +++ b/src/gpu/vk/GrVkRenderTarget.h @@ -86,7 +86,7 @@ public: return fResolveAttachment ? fResolveAttachment.get() : fColorAttachment.get(); } - std::pair getSimpleRenderPass( + const GrVkRenderPass* getSimpleRenderPass( bool withResolve, bool withStencil, SelfDependencyFlags selfDepFlags, @@ -158,14 +158,15 @@ private: GrVkAttachment* dynamicMSAAAttachment(); GrVkAttachment* msaaAttachment(); - const GrVkRenderPass* createSimpleRenderPass(bool withResolve, - bool withStencil, - SelfDependencyFlags selfDepFlags, - LoadFromResolve); - const GrVkFramebuffer* createFramebuffer(bool withResolve, - bool withStencil, - SelfDependencyFlags selfDepFlags, - LoadFromResolve); + std::pair + createSimpleRenderPass(bool withResolve, + bool withStencil, + SelfDependencyFlags selfDepFlags, + LoadFromResolve); + void createFramebuffer(bool withResolve, + bool withStencil, + SelfDependencyFlags selfDepFlags, + LoadFromResolve); bool completeStencilAttachment() override; @@ -187,12 +188,11 @@ private: // We can have a renderpass with and without resolve attachment, stencil attachment, // input attachment dependency, advanced blend dependency, and loading from resolve. All 5 of - // these being completely orthogonal. Thus we have a total of 32 types of render passes. - static constexpr int kNumCachedRenderPasses = 32; + // these being completely orthogonal. Thus we have a total of 32 types of render passes. We then + // cache a framebuffer for each type of these render passes. + static constexpr int kNumCachedFramebuffers = 32; - const GrVkFramebuffer* fCachedFramebuffers[kNumCachedRenderPasses]; - const GrVkRenderPass* fCachedRenderPasses[kNumCachedRenderPasses]; - GrVkResourceProvider::CompatibleRPHandle fCompatibleRPHandles[kNumCachedRenderPasses]; + sk_sp fCachedFramebuffers[kNumCachedFramebuffers]; const GrVkDescriptorSet* fCachedInputDescriptorSet = nullptr;