From ec44099979acd3e83ad93a15dbd9301856a90572 Mon Sep 17 00:00:00 2001 From: egdaniel Date: Tue, 13 Sep 2016 09:54:11 -0700 Subject: [PATCH] Lots of little cleanup improvements to Vulkan BUG=skia: GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2336763002 Review-Url: https://codereview.chromium.org/2336763002 --- src/gpu/vk/GrVkCommandBuffer.cpp | 2 +- src/gpu/vk/GrVkGpu.cpp | 1 + src/gpu/vk/GrVkGpuCommandBuffer.cpp | 11 ++++- src/gpu/vk/GrVkGpuCommandBuffer.h | 1 + src/gpu/vk/GrVkPipeline.cpp | 76 +++++++++++++---------------- 5 files changed, 46 insertions(+), 45 deletions(-) diff --git a/src/gpu/vk/GrVkCommandBuffer.cpp b/src/gpu/vk/GrVkCommandBuffer.cpp index 06b3bb1050..317b735391 100644 --- a/src/gpu/vk/GrVkCommandBuffer.cpp +++ b/src/gpu/vk/GrVkCommandBuffer.cpp @@ -186,7 +186,7 @@ void GrVkCommandBuffer::bindPipeline(const GrVkGpu* gpu, const GrVkPipeline* pip GR_VK_CALL(gpu->vkInterface(), CmdBindPipeline(fCmdBuffer, VK_PIPELINE_BIND_POINT_GRAPHICS, pipeline->pipeline())); - addResource(pipeline); + this->addResource(pipeline); } void GrVkCommandBuffer::drawIndexed(const GrVkGpu* gpu, diff --git a/src/gpu/vk/GrVkGpu.cpp b/src/gpu/vk/GrVkGpu.cpp index ed165c52a1..a3393777bb 100644 --- a/src/gpu/vk/GrVkGpu.cpp +++ b/src/gpu/vk/GrVkGpu.cpp @@ -1570,6 +1570,7 @@ bool GrVkGpu::onCopySurface(GrSurface* dst, const SkIPoint& dstPoint) { if (can_copy_as_resolve(dst, src, this)) { this->copySurfaceAsResolve(dst, src, srcRect, dstPoint); + return true; } GrVkImage* dstImage; diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.cpp b/src/gpu/vk/GrVkGpuCommandBuffer.cpp index e3c7cb77cc..a9d8e68d00 100644 --- a/src/gpu/vk/GrVkGpuCommandBuffer.cpp +++ b/src/gpu/vk/GrVkGpuCommandBuffer.cpp @@ -57,7 +57,8 @@ GrVkGpuCommandBuffer::GrVkGpuCommandBuffer(GrVkGpu* gpu, const LoadAndStoreInfo& stencilInfo) : fGpu(gpu) , fRenderTarget(target) - , fIsEmpty(true) { + , fIsEmpty(true) + , fStartsWithClear(false) { VkAttachmentLoadOp vkLoadOp; VkAttachmentStoreOp vkStoreOp; @@ -101,6 +102,12 @@ void GrVkGpuCommandBuffer::end() { } void GrVkGpuCommandBuffer::onSubmit(const SkIRect& bounds) { + 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; + } + // 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() @@ -155,6 +162,7 @@ void GrVkGpuCommandBuffer::discard(GrRenderTarget* target) { SkASSERT(fRenderPass->isCompatible(*oldRP)); oldRP->unref(fGpu); + fStartsWithClear = false; } } @@ -252,6 +260,7 @@ void GrVkGpuCommandBuffer::onClear(GrRenderTarget* target, const GrFixedClip& cl oldRP->unref(fGpu); GrColorToRGBAFloat(color, fColorClearValue.color.float32); + fStartsWithClear = true; return; } diff --git a/src/gpu/vk/GrVkGpuCommandBuffer.h b/src/gpu/vk/GrVkGpuCommandBuffer.h index 3092ffd99d..d91271e8c8 100644 --- a/src/gpu/vk/GrVkGpuCommandBuffer.h +++ b/src/gpu/vk/GrVkGpuCommandBuffer.h @@ -63,6 +63,7 @@ private: VkClearValue fColorClearValue; bool fIsEmpty; + bool fStartsWithClear; typedef GrGpuCommandBuffer INHERITED; }; diff --git a/src/gpu/vk/GrVkPipeline.cpp b/src/gpu/vk/GrVkPipeline.cpp index c7e99d2ee4..910398183b 100644 --- a/src/gpu/vk/GrVkPipeline.cpp +++ b/src/gpu/vk/GrVkPipeline.cpp @@ -95,7 +95,7 @@ static void setup_input_assembly_state(GrPrimitiveType primitiveType, } -VkStencilOp stencil_op_to_vk_stencil_op(GrStencilOp op) { +static VkStencilOp stencil_op_to_vk_stencil_op(GrStencilOp op) { static const VkStencilOp gTable[] = { VK_STENCIL_OP_KEEP, // kKeep VK_STENCIL_OP_ZERO, // kZero @@ -119,7 +119,7 @@ VkStencilOp stencil_op_to_vk_stencil_op(GrStencilOp op) { return gTable[(int)op]; } -VkCompareOp stencil_func_to_vk_compare_op(GrStencilTest test) { +static VkCompareOp stencil_func_to_vk_compare_op(GrStencilTest test) { static const VkCompareOp gTable[] = { VK_COMPARE_OP_ALWAYS, // kAlways VK_COMPARE_OP_NEVER, // kNever @@ -144,9 +144,8 @@ VkCompareOp stencil_func_to_vk_compare_op(GrStencilTest test) { return gTable[(int)test]; } -void setup_depth_stencil_state(const GrVkGpu* gpu, - const GrStencilSettings& stencilSettings, - VkPipelineDepthStencilStateCreateInfo* stencilInfo) { +static void setup_depth_stencil_state(const GrStencilSettings& stencilSettings, + VkPipelineDepthStencilStateCreateInfo* stencilInfo) { memset(stencilInfo, 0, sizeof(VkPipelineDepthStencilStateCreateInfo)); stencilInfo->sType = VK_STRUCTURE_TYPE_PIPELINE_DEPTH_STENCIL_STATE_CREATE_INFO; stencilInfo->pNext = nullptr; @@ -186,10 +185,7 @@ void setup_depth_stencil_state(const GrVkGpu* gpu, stencilInfo->maxDepthBounds = 1.0f; } -void setup_viewport_scissor_state(const GrVkGpu* gpu, - const GrPipeline& pipeline, - const GrVkRenderTarget* vkRT, - VkPipelineViewportStateCreateInfo* viewportInfo) { +static void setup_viewport_scissor_state(VkPipelineViewportStateCreateInfo* viewportInfo) { memset(viewportInfo, 0, sizeof(VkPipelineViewportStateCreateInfo)); viewportInfo->sType = VK_STRUCTURE_TYPE_PIPELINE_VIEWPORT_STATE_CREATE_INFO; viewportInfo->pNext = nullptr; @@ -204,10 +200,10 @@ void setup_viewport_scissor_state(const GrVkGpu* gpu, SkASSERT(viewportInfo->viewportCount == viewportInfo->scissorCount); } -void setup_multisample_state(const GrPipeline& pipeline, - const GrPrimitiveProcessor& primProc, - const GrCaps* caps, - VkPipelineMultisampleStateCreateInfo* multisampleInfo) { +static void setup_multisample_state(const GrPipeline& pipeline, + const GrPrimitiveProcessor& primProc, + const GrCaps* caps, + VkPipelineMultisampleStateCreateInfo* multisampleInfo) { memset(multisampleInfo, 0, sizeof(VkPipelineMultisampleStateCreateInfo)); multisampleInfo->sType = VK_STRUCTURE_TYPE_PIPELINE_MULTISAMPLE_STATE_CREATE_INFO; multisampleInfo->pNext = nullptr; @@ -286,7 +282,7 @@ static VkBlendOp blend_equation_to_vk_blend_op(GrBlendEquation equation) { return gTable[equation]; } -bool blend_coeff_refs_constant(GrBlendCoeff coeff) { +static bool blend_coeff_refs_constant(GrBlendCoeff coeff) { static const bool gCoeffReferencesBlendConst[] = { false, false, @@ -314,10 +310,9 @@ bool blend_coeff_refs_constant(GrBlendCoeff coeff) { // Individual enum asserts already made in blend_coeff_to_vk_blend } -void setup_color_blend_state(const GrVkGpu* gpu, - const GrPipeline& pipeline, - VkPipelineColorBlendStateCreateInfo* colorBlendInfo, - VkPipelineColorBlendAttachmentState* attachmentState) { +static void setup_color_blend_state(const GrPipeline& pipeline, + VkPipelineColorBlendStateCreateInfo* colorBlendInfo, + VkPipelineColorBlendAttachmentState* attachmentState) { GrXferProcessor::BlendInfo blendInfo; pipeline.getXferProcessor().getBlendInfo(&blendInfo); @@ -355,7 +350,7 @@ void setup_color_blend_state(const GrVkGpu* gpu, // colorBlendInfo->blendConstants is set dynamically } -VkCullModeFlags draw_face_to_vk_cull_mode(GrDrawFace drawFace) { +static VkCullModeFlags draw_face_to_vk_cull_mode(GrDrawFace drawFace) { // Assumes that we've set the front face to be ccw static const VkCullModeFlags gTable[] = { VK_CULL_MODE_NONE, // kBoth_DrawFace @@ -370,9 +365,8 @@ VkCullModeFlags draw_face_to_vk_cull_mode(GrDrawFace drawFace) { return gTable[(int)drawFace]; } -void setup_raster_state(const GrVkGpu* gpu, - const GrPipeline& pipeline, - VkPipelineRasterizationStateCreateInfo* rasterInfo) { +static void setup_raster_state(const GrPipeline& pipeline, + VkPipelineRasterizationStateCreateInfo* rasterInfo) { memset(rasterInfo, 0, sizeof(VkPipelineRasterizationStateCreateInfo)); rasterInfo->sType = VK_STRUCTURE_TYPE_PIPELINE_RASTERIZATION_STATE_CREATE_INFO; rasterInfo->pNext = nullptr; @@ -389,10 +383,8 @@ void setup_raster_state(const GrVkGpu* gpu, rasterInfo->lineWidth = 1.0f; } -void setup_dynamic_state(const GrVkGpu* gpu, - const GrPipeline& pipeline, - VkPipelineDynamicStateCreateInfo* dynamicInfo, - VkDynamicState* dynamicStates) { +static void setup_dynamic_state(VkPipelineDynamicStateCreateInfo* dynamicInfo, + VkDynamicState* dynamicStates) { memset(dynamicInfo, 0, sizeof(VkPipelineDynamicStateCreateInfo)); dynamicInfo->sType = VK_STRUCTURE_TYPE_PIPELINE_DYNAMIC_STATE_CREATE_INFO; dynamicInfo->pNext = VK_NULL_HANDLE; @@ -423,12 +415,10 @@ GrVkPipeline* GrVkPipeline::Create(GrVkGpu* gpu, const GrPipeline& pipeline, setup_input_assembly_state(primitiveType, &inputAssemblyInfo); VkPipelineDepthStencilStateCreateInfo depthStencilInfo; - setup_depth_stencil_state(gpu, pipeline.getStencil(), &depthStencilInfo); + setup_depth_stencil_state(pipeline.getStencil(), &depthStencilInfo); - GrRenderTarget* rt = pipeline.getRenderTarget(); - GrVkRenderTarget* vkRT = static_cast(rt); VkPipelineViewportStateCreateInfo viewportInfo; - setup_viewport_scissor_state(gpu, pipeline, vkRT, &viewportInfo); + setup_viewport_scissor_state(&viewportInfo); VkPipelineMultisampleStateCreateInfo multisampleInfo; setup_multisample_state(pipeline, primProc, gpu->caps(), &multisampleInfo); @@ -436,14 +426,14 @@ GrVkPipeline* GrVkPipeline::Create(GrVkGpu* gpu, const GrPipeline& pipeline, // We will only have one color attachment per pipeline. VkPipelineColorBlendAttachmentState attachmentStates[1]; VkPipelineColorBlendStateCreateInfo colorBlendInfo; - setup_color_blend_state(gpu, pipeline, &colorBlendInfo, attachmentStates); + setup_color_blend_state(pipeline, &colorBlendInfo, attachmentStates); VkPipelineRasterizationStateCreateInfo rasterInfo; - setup_raster_state(gpu, pipeline, &rasterInfo); + setup_raster_state(pipeline, &rasterInfo); VkDynamicState dynamicStates[3]; VkPipelineDynamicStateCreateInfo dynamicInfo; - setup_dynamic_state(gpu, pipeline, &dynamicInfo, dynamicStates); + setup_dynamic_state(&dynamicInfo, dynamicStates); VkGraphicsPipelineCreateInfo pipelineCreateInfo; memset(&pipelineCreateInfo, 0, sizeof(VkGraphicsPipelineCreateInfo)); @@ -483,10 +473,10 @@ void GrVkPipeline::freeGPUData(const GrVkGpu* gpu) const { GR_VK_CALL(gpu->vkInterface(), DestroyPipeline(gpu->device(), fPipeline, nullptr)); } -void set_dynamic_scissor_state(GrVkGpu* gpu, - GrVkCommandBuffer* cmdBuffer, - const GrPipeline& pipeline, - const GrRenderTarget& target) { +static void set_dynamic_scissor_state(GrVkGpu* gpu, + GrVkCommandBuffer* cmdBuffer, + const GrPipeline& pipeline, + const GrRenderTarget& target) { // We always use one scissor and if it is disabled we just make it the size of the RT const GrScissorState& scissorState = pipeline.getScissorState(); VkRect2D scissor; @@ -516,9 +506,9 @@ void set_dynamic_scissor_state(GrVkGpu* gpu, cmdBuffer->setScissor(gpu, 0, 1, &scissor); } -void set_dynamic_viewport_state(GrVkGpu* gpu, - GrVkCommandBuffer* cmdBuffer, - const GrRenderTarget& target) { +static void set_dynamic_viewport_state(GrVkGpu* gpu, + GrVkCommandBuffer* cmdBuffer, + const GrRenderTarget& target) { // We always use one viewport the size of the RT VkViewport viewport; viewport.x = 0.0f; @@ -530,9 +520,9 @@ void set_dynamic_viewport_state(GrVkGpu* gpu, cmdBuffer->setViewport(gpu, 0, 1, &viewport); } -void set_dynamic_blend_constant_state(GrVkGpu* gpu, - GrVkCommandBuffer* cmdBuffer, - const GrPipeline& pipeline) { +static void set_dynamic_blend_constant_state(GrVkGpu* gpu, + GrVkCommandBuffer* cmdBuffer, + const GrPipeline& pipeline) { GrXferProcessor::BlendInfo blendInfo; pipeline.getXferProcessor().getBlendInfo(&blendInfo); GrBlendCoeff srcCoeff = blendInfo.fSrcBlend;