From 77b53f66bacd9a1d1c9df7d879a419b2abe069ba Mon Sep 17 00:00:00 2001 From: Greg Daniel Date: Tue, 18 Oct 2016 11:48:51 -0400 Subject: [PATCH] Support inline uploads in Vulkan BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=3586 Change-Id: I5913c336aa33851d6d2e80d9638df2efa8ac0400 Reviewed-on: https://skia-review.googlesource.com/3586 Reviewed-by: Brian Salomon Commit-Queue: Greg Daniel --- src/gpu/GrBatchFlushState.h | 1 + src/gpu/GrGpuCommandBuffer.h | 15 ++-- src/gpu/GrPipeline.h | 1 - src/gpu/GrResourceProvider.h | 1 + src/gpu/batches/GrDashLinePathRenderer.h | 2 + src/gpu/batches/GrVertexBatch.cpp | 2 +- src/gpu/gl/GrGLGpuCommandBuffer.h | 5 ++ src/gpu/instanced/InstancedRendering.h | 1 + src/gpu/vk/GrVkCommandBuffer.cpp | 1 + src/gpu/vk/GrVkGpuCommandBuffer.cpp | 94 ++++++++++++++++++------ src/gpu/vk/GrVkGpuCommandBuffer.h | 27 +++++-- 11 files changed, 113 insertions(+), 37 deletions(-) diff --git a/src/gpu/GrBatchFlushState.h b/src/gpu/GrBatchFlushState.h index d2d9a4b488..89c5292eea 100644 --- a/src/gpu/GrBatchFlushState.h +++ b/src/gpu/GrBatchFlushState.h @@ -9,6 +9,7 @@ #define GrBatchBuffer_DEFINED #include "GrBufferAllocPool.h" +#include "GrGpu.h" #include "batches/GrVertexBatch.h" class GrGpuCommandBuffer; diff --git a/src/gpu/GrGpuCommandBuffer.h b/src/gpu/GrGpuCommandBuffer.h index 841d311e96..ef4f428c9c 100644 --- a/src/gpu/GrGpuCommandBuffer.h +++ b/src/gpu/GrGpuCommandBuffer.h @@ -9,7 +9,9 @@ #define GrGpuCommandBuffer_DEFINED #include "GrColor.h" +#include "batches/GrDrawBatch.h" +class GrBatchFlushState; class GrFixedClip; class GrGpu; class GrMesh; @@ -64,17 +66,20 @@ public: int meshCount, const SkRect& bounds); + // Performs an upload of vertex data in the middle of a set of a set of draws + virtual void inlineUpload(GrBatchFlushState* state, GrDrawBatch::DeferredUploadFn& upload) = 0; + /** - * Clear the passed in render target. Ignores the draw state and clip. - */ + * Clear the passed in render target. Ignores the draw state and clip. + */ void clear(const GrFixedClip&, GrColor); void clearStencilClip(const GrFixedClip&, bool insideStencilMask); /** - * Discards the contents render target. nullptr indicates that the current render target should - * be discarded. - **/ + * Discards the contents render target. nullptr indicates that the current render target should + * be discarded. + */ // TODO: This should be removed in the future to favor using the load and store ops for discard virtual void discard() = 0; diff --git a/src/gpu/GrPipeline.h b/src/gpu/GrPipeline.h index 6366e8050f..845044f720 100644 --- a/src/gpu/GrPipeline.h +++ b/src/gpu/GrPipeline.h @@ -10,7 +10,6 @@ #include "GrColor.h" #include "GrFragmentProcessor.h" -#include "GrGpu.h" #include "GrNonAtomicRef.h" #include "GrPendingProgramElement.h" #include "GrPrimitiveProcessor.h" diff --git a/src/gpu/GrResourceProvider.h b/src/gpu/GrResourceProvider.h index c0922c0a01..b4d72d5338 100644 --- a/src/gpu/GrResourceProvider.h +++ b/src/gpu/GrResourceProvider.h @@ -10,6 +10,7 @@ #include "GrBatchAtlas.h" #include "GrBuffer.h" +#include "GrGpu.h" #include "GrTextureProvider.h" #include "GrPathRange.h" diff --git a/src/gpu/batches/GrDashLinePathRenderer.h b/src/gpu/batches/GrDashLinePathRenderer.h index d959421776..db2f87fdc3 100644 --- a/src/gpu/batches/GrDashLinePathRenderer.h +++ b/src/gpu/batches/GrDashLinePathRenderer.h @@ -10,6 +10,8 @@ #include "GrPathRenderer.h" +#include "GrGpu.h" + class GrDashLinePathRenderer : public GrPathRenderer { private: bool onCanDrawPath(const CanDrawPathArgs&) const override; diff --git a/src/gpu/batches/GrVertexBatch.cpp b/src/gpu/batches/GrVertexBatch.cpp index 32413b81cb..2286342a94 100644 --- a/src/gpu/batches/GrVertexBatch.cpp +++ b/src/gpu/batches/GrVertexBatch.cpp @@ -72,7 +72,7 @@ void GrVertexBatch::onDraw(GrBatchFlushState* state, const SkRect& bounds) { GrBatchDrawToken drawToken = state->nextTokenToFlush(); while (currUploadIdx < fInlineUploads.count() && fInlineUploads[currUploadIdx].fUploadBeforeToken == drawToken) { - state->doUpload(fInlineUploads[currUploadIdx++].fUpload); + state->commandBuffer()->inlineUpload(state, fInlineUploads[currUploadIdx++].fUpload); } const QueuedDraw &draw = fQueuedDraws[currDrawIdx]; state->commandBuffer()->draw(*this->pipeline(), *draw.fGeometryProcessor.get(), diff --git a/src/gpu/gl/GrGLGpuCommandBuffer.h b/src/gpu/gl/GrGLGpuCommandBuffer.h index 65ad543d49..069eea5f58 100644 --- a/src/gpu/gl/GrGLGpuCommandBuffer.h +++ b/src/gpu/gl/GrGLGpuCommandBuffer.h @@ -10,6 +10,7 @@ #include "GrGpuCommandBuffer.h" +#include "GrBatchFlushState.h" #include "GrGLGpu.h" class GrGLRenderTarget; @@ -29,6 +30,10 @@ public: void discard() override {} + void inlineUpload(GrBatchFlushState* state, GrDrawBatch::DeferredUploadFn& upload) override { + state->doUpload(upload); + } + private: GrGpu* gpu() override { return fGpu; } GrRenderTarget* renderTarget() override { return fRenderTarget; } diff --git a/src/gpu/instanced/InstancedRendering.h b/src/gpu/instanced/InstancedRendering.h index 77dc07e516..b71ca80c39 100644 --- a/src/gpu/instanced/InstancedRendering.h +++ b/src/gpu/instanced/InstancedRendering.h @@ -8,6 +8,7 @@ #ifndef gr_instanced_InstancedRendering_DEFINED #define gr_instanced_InstancedRendering_DEFINED +#include "GrGpu.h" #include "GrMemoryPool.h" #include "SkTInternalLList.h" #include "batches/GrDrawBatch.h" diff --git a/src/gpu/vk/GrVkCommandBuffer.cpp b/src/gpu/vk/GrVkCommandBuffer.cpp index bc6272c0c6..91acf9847f 100644 --- a/src/gpu/vk/GrVkCommandBuffer.cpp +++ b/src/gpu/vk/GrVkCommandBuffer.cpp @@ -387,6 +387,7 @@ void GrVkPrimaryCommandBuffer::endRenderPass(const GrVkGpu* gpu) { void GrVkPrimaryCommandBuffer::executeCommands(const GrVkGpu* gpu, GrVkSecondaryCommandBuffer* buffer) { SkASSERT(fIsActive); + SkASSERT(!buffer->fIsActive); SkASSERT(fActiveRenderPass); SkASSERT(fActiveRenderPass->isCompatible(*buffer->fActiveRenderPass)); diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.cpp b/src/gpu/vk/GrVkGpuCommandBuffer.cpp index 1a9ea1f479..ccfe37b98e 100644 --- a/src/gpu/vk/GrVkGpuCommandBuffer.cpp +++ b/src/gpu/vk/GrVkGpuCommandBuffer.cpp @@ -7,6 +7,7 @@ #include "GrVkGpuCommandBuffer.h" +#include "GrBatchFlushState.h" #include "GrFixedClip.h" #include "GrMesh.h" #include "GrPipeline.h" @@ -57,9 +58,7 @@ GrVkGpuCommandBuffer::GrVkGpuCommandBuffer(GrVkGpu* gpu, const LoadAndStoreInfo& colorInfo, const LoadAndStoreInfo& stencilInfo) : fGpu(gpu) - , fRenderTarget(target) - , fIsEmpty(true) - , fStartsWithClear(false) { + , fRenderTarget(target) { VkAttachmentLoadOp vkLoadOp; VkAttachmentStoreOp vkStoreOp; @@ -86,6 +85,8 @@ GrVkGpuCommandBuffer::GrVkGpuCommandBuffer(GrVkGpu* gpu, GrColorToRGBAFloat(colorInfo.fClearColor, cbInfo.fColorClearValue.color.float32); cbInfo.fBounds.setEmpty(); + cbInfo.fIsEmpty = true; + cbInfo.fStartsWithClear = false; cbInfo.fCommandBuffer = gpu->resourceProvider().findOrCreateSecondaryCommandBuffer(); cbInfo.fCommandBuffer->begin(gpu, target->framebuffer(), cbInfo.fRenderPass); @@ -107,18 +108,6 @@ void GrVkGpuCommandBuffer::end() { } void GrVkGpuCommandBuffer::onSubmit() { - // TODO: We can't add this optimization yet since many things create a scratch texture which - // adds the discard immediately, but then don't draw to it right away. This causes the discard - // to be ignored and we get yelled at for loading uninitialized data. However, once MDP lands, - // the discard will get reordered with the rest of the draw commands and we can re-enable this. -#if 0 - if (fIsEmpty && !fStartsWithClear) { - // We have sumbitted no actual draw commands to the command buffer and we are not using - // the render pass to do a clear so there is no need to submit anything. - return; - } -#endif - // Change layout of our render target so it can be used as the color attachment. Currently // we don't attach the resolve to the framebuffer so no need to change its layout. GrVkImage* targetImage = fRenderTarget->msaaImage() ? fRenderTarget->msaaImage() @@ -145,6 +134,23 @@ void GrVkGpuCommandBuffer::onSubmit() { for (int i = 0; i < fCommandBufferInfos.count(); ++i) { CommandBufferInfo& cbInfo = fCommandBufferInfos[i]; + for (int j = 0; j < cbInfo.fPreDrawUploads.count(); ++j) { + InlineUploadInfo& iuInfo = cbInfo.fPreDrawUploads[j]; + iuInfo.fFlushState->doUpload(iuInfo.fUpload); + } + + // TODO: We can't add this optimization yet since many things create a scratch texture which + // adds the discard immediately, but then don't draw to it right away. This causes the + // discard to be ignored and we get yelled at for loading uninitialized data. However, once + // MDP lands, the discard will get reordered with the rest of the draw commands and we can + // re-enable this. +#if 0 + if (cbInfo.fIsEmpty && !cbInfo.fStartsWithClear) { + // We have sumbitted no actual draw commands to the command buffer and we are not using + // the render pass to do a clear so there is no need to submit anything. + continue; + } +#endif if (cbInfo.fBounds.intersect(0, 0, SkIntToScalar(fRenderTarget->width()), SkIntToScalar(fRenderTarget->height()))) { @@ -158,8 +164,8 @@ void GrVkGpuCommandBuffer::onSubmit() { } void GrVkGpuCommandBuffer::discard() { - if (fIsEmpty) { - CommandBufferInfo& cbInfo = fCommandBufferInfos[fCurrentCmdBuffer]; + CommandBufferInfo& cbInfo = fCommandBufferInfos[fCurrentCmdBuffer]; + if (cbInfo.fIsEmpty) { // We will change the render pass to do a clear load instead GrVkRenderPass::LoadStoreOps vkColorOps(VK_ATTACHMENT_LOAD_OP_DONT_CARE, VK_ATTACHMENT_STORE_OP_STORE); @@ -182,7 +188,7 @@ void GrVkGpuCommandBuffer::discard() { SkASSERT(cbInfo.fRenderPass->isCompatible(*oldRP)); oldRP->unref(fGpu); - fStartsWithClear = false; + cbInfo.fStartsWithClear = false; } } @@ -237,7 +243,7 @@ void GrVkGpuCommandBuffer::onClearStencilClip(const GrFixedClip& clip, attachment.clearValue.depthStencil = vkStencilColor; cbInfo.fCommandBuffer->clearAttachments(fGpu, 1, &attachment, 1, &clearRect); - fIsEmpty = false; + cbInfo.fIsEmpty = false; // Update command buffer bounds if (!clip.scissorEnabled()) { @@ -256,7 +262,7 @@ void GrVkGpuCommandBuffer::onClear(const GrFixedClip& clip, GrColor color) { VkClearColorValue vkColor; GrColorToRGBAFloat(color, vkColor.float32); - if (fIsEmpty && !clip.scissorEnabled()) { + if (cbInfo.fIsEmpty && !clip.scissorEnabled()) { // We will change the render pass to do a clear load instead GrVkRenderPass::LoadStoreOps vkColorOps(VK_ATTACHMENT_LOAD_OP_CLEAR, VK_ATTACHMENT_STORE_OP_STORE); @@ -281,7 +287,7 @@ void GrVkGpuCommandBuffer::onClear(const GrFixedClip& clip, GrColor color) { oldRP->unref(fGpu); GrColorToRGBAFloat(color, cbInfo.fColorClearValue.color.float32); - fStartsWithClear = true; + cbInfo.fStartsWithClear = true; // Update command buffer bounds cbInfo.fBounds.join(fRenderTarget->getBoundsRect()); @@ -315,7 +321,7 @@ void GrVkGpuCommandBuffer::onClear(const GrFixedClip& clip, GrColor color) { attachment.clearValue.color = vkColor; cbInfo.fCommandBuffer->clearAttachments(fGpu, 1, &attachment, 1, &clearRect); - fIsEmpty = false; + cbInfo.fIsEmpty = false; // Update command buffer bounds if (!clip.scissorEnabled()) { @@ -326,6 +332,48 @@ void GrVkGpuCommandBuffer::onClear(const GrFixedClip& clip, GrColor color) { return; } +void GrVkGpuCommandBuffer::addAdditionalCommandBuffer() { + fCommandBufferInfos[fCurrentCmdBuffer].fCommandBuffer->end(fGpu); + + CommandBufferInfo& cbInfo = fCommandBufferInfos.push_back(); + fCurrentCmdBuffer++; + + GrVkRenderPass::LoadStoreOps vkColorOps(VK_ATTACHMENT_LOAD_OP_LOAD, + VK_ATTACHMENT_STORE_OP_STORE); + GrVkRenderPass::LoadStoreOps vkStencilOps(VK_ATTACHMENT_LOAD_OP_LOAD, + VK_ATTACHMENT_STORE_OP_STORE); + + const GrVkResourceProvider::CompatibleRPHandle& rpHandle = + fRenderTarget->compatibleRenderPassHandle(); + if (rpHandle.isValid()) { + cbInfo.fRenderPass = fGpu->resourceProvider().findRenderPass(rpHandle, + vkColorOps, + vkStencilOps); + } else { + cbInfo.fRenderPass = fGpu->resourceProvider().findRenderPass(*fRenderTarget, + vkColorOps, + vkStencilOps); + } + + cbInfo.fCommandBuffer = fGpu->resourceProvider().findOrCreateSecondaryCommandBuffer(); + // It shouldn't matter what we set the clear color to here since we will assume loading of the + // attachment. + memset(&cbInfo.fColorClearValue, 0, sizeof(VkClearValue)); + cbInfo.fBounds.setEmpty(); + cbInfo.fIsEmpty = true; + cbInfo.fStartsWithClear = false; + + cbInfo.fCommandBuffer->begin(fGpu, fRenderTarget->framebuffer(), cbInfo.fRenderPass); +} + +void GrVkGpuCommandBuffer::inlineUpload(GrBatchFlushState* state, + GrDrawBatch::DeferredUploadFn& upload) { + if (!fCommandBufferInfos[fCurrentCmdBuffer].fIsEmpty) { + this->addAdditionalCommandBuffer(); + } + fCommandBufferInfos[fCurrentCmdBuffer].fPreDrawUploads.emplace_back(state, upload); +} + //////////////////////////////////////////////////////////////////////////////// void GrVkGpuCommandBuffer::bindGeometry(const GrPrimitiveProcessor& primProc, @@ -471,7 +519,7 @@ void GrVkGpuCommandBuffer::onDraw(const GrPipeline& pipeline, nonIdxMesh->startVertex(), 0); } - fIsEmpty = false; + cbInfo.fIsEmpty = false; fGpu->stats()->incNumDraws(); } while ((nonIdxMesh = iter.next())); diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.h b/src/gpu/vk/GrVkGpuCommandBuffer.h index 527a1b2f46..160e4ca63f 100644 --- a/src/gpu/vk/GrVkGpuCommandBuffer.h +++ b/src/gpu/vk/GrVkGpuCommandBuffer.h @@ -34,6 +34,8 @@ public: void discard() override; + void inlineUpload(GrBatchFlushState* state, GrDrawBatch::DeferredUploadFn& upload) override; + private: GrGpu* gpu() override; GrRenderTarget* renderTarget() override; @@ -57,11 +59,25 @@ private: void onClearStencilClip(const GrFixedClip&, bool insideStencilMask) override; + void addAdditionalCommandBuffer(); + + struct InlineUploadInfo { + InlineUploadInfo(GrBatchFlushState* state, const GrDrawBatch::DeferredUploadFn& upload) + : fFlushState(state) + , fUpload(upload) {} + + GrBatchFlushState* fFlushState; + GrDrawBatch::DeferredUploadFn fUpload; + }; + struct CommandBufferInfo { - const GrVkRenderPass* fRenderPass; - GrVkSecondaryCommandBuffer* fCommandBuffer; - VkClearValue fColorClearValue; - SkRect fBounds; + const GrVkRenderPass* fRenderPass; + GrVkSecondaryCommandBuffer* fCommandBuffer; + VkClearValue fColorClearValue; + SkRect fBounds; + bool fIsEmpty; + bool fStartsWithClear; + SkTArray fPreDrawUploads; }; SkTArray fCommandBufferInfos; @@ -70,9 +86,6 @@ private: GrVkGpu* fGpu; GrVkRenderTarget* fRenderTarget; - bool fIsEmpty; - bool fStartsWithClear; - typedef GrGpuCommandBuffer INHERITED; };