From 3e749a94ce5085cccb5b7cbae57f35f813fb4e33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Mon, 13 Nov 2017 11:16:20 +0100 Subject: [PATCH] Vulkan: Fix bug where we ended up creating duplicate pipelines even if vertices decoded to the same format, if they were created from different formats. This can cut down the number of pipelines to a third or less in some games. However, benefit is likely smaller since Vulkan drivers will deduplicate shaders inside each vkPipelineCache object. Helps #10106 while not actually implementing any of the suggestions inside. --- GPU/Common/VertexDecoderCommon.cpp | 8 ++++++-- GPU/Common/VertexDecoderCommon.h | 21 ++++----------------- GPU/D3D11/DrawEngineD3D11.cpp | 2 -- GPU/Directx9/DrawEngineDX9.cpp | 3 --- GPU/GLES/DrawEngineGLES.cpp | 2 -- GPU/Vulkan/DrawEngineVulkan.cpp | 4 ++-- GPU/Vulkan/PipelineManagerVulkan.cpp | 17 +++++++---------- GPU/Vulkan/PipelineManagerVulkan.h | 4 ++-- 8 files changed, 21 insertions(+), 40 deletions(-) diff --git a/GPU/Common/VertexDecoderCommon.cpp b/GPU/Common/VertexDecoderCommon.cpp index 5ddcc18347..4cd6f9e40d 100644 --- a/GPU/Common/VertexDecoderCommon.cpp +++ b/GPU/Common/VertexDecoderCommon.cpp @@ -73,13 +73,15 @@ int DecFmtSize(u8 fmt) { case DEC_U16_2: return 4; case DEC_U16_3: return 8; case DEC_U16_4: return 8; - case DEC_U8A_2: return 4; - case DEC_U16A_2: return 4; default: return 0; } } +void DecVtxFormat::ComputeID() { + id = w0fmt | (w1fmt << 4) | (uvfmt << 8) | (c0fmt << 12) | (c1fmt << 16) | (nrmfmt << 20) | (posfmt << 24); +} + void GetIndexBounds(const void *inds, int count, u32 vertType, u16 *indexLowerBound, u16 *indexUpperBound) { // Find index bounds. Could cache this in display lists. // Also, this could be greatly sped up with SSE2/NEON, although rarely a bottleneck. @@ -1152,6 +1154,8 @@ void VertexDecoder::SetVertexType(u32 fmt, const VertexDecoderOptions &options, decFmt.stride = decOff; + decFmt.ComputeID(); + size = align(size, biggest); onesize_ = size; size *= morphcount; diff --git a/GPU/Common/VertexDecoderCommon.h b/GPU/Common/VertexDecoderCommon.h index 9222090480..fbe60c1756 100644 --- a/GPU/Common/VertexDecoderCommon.h +++ b/GPU/Common/VertexDecoderCommon.h @@ -43,6 +43,7 @@ // Can write code to easily bind these using OpenGL, or read these manually. // No morph support, that is taken care of by the VertexDecoder. +// Keep this in 4 bits. enum { DEC_NONE, DEC_FLOAT_1, @@ -59,8 +60,6 @@ enum { DEC_U16_2, DEC_U16_3, DEC_U16_4, - DEC_U8A_2, - DEC_U16A_2, }; int DecFmtSize(u8 fmt); @@ -74,6 +73,9 @@ struct DecVtxFormat { u8 nrmfmt; u8 nrmoff; u8 posfmt; u8 posoff; short stride; + + uint32_t id; + void ComputeID(); }; struct TransformedVertex @@ -297,21 +299,6 @@ public: } break; - case DEC_U8A_2: - { - const u8 *b = (const u8 *)(data_ + decFmt_.uvoff); - uv[0] = (float)b[0]; - uv[1] = (float)b[1]; - } - break; - - case DEC_U16A_2: - { - const u16 *p = (const u16 *)(data_ + decFmt_.uvoff); - uv[0] = (float)p[0]; - uv[1] = (float)p[1]; - } - break; default: ERROR_LOG_REPORT_ONCE(fmtuv, G3D, "Reader: Unsupported UV Format %d", decFmt_.uvfmt); memset(uv, 0, sizeof(float) * 2); diff --git a/GPU/D3D11/DrawEngineD3D11.cpp b/GPU/D3D11/DrawEngineD3D11.cpp index e8e6ba8580..1a3ad5f9c2 100644 --- a/GPU/D3D11/DrawEngineD3D11.cpp +++ b/GPU/D3D11/DrawEngineD3D11.cpp @@ -175,8 +175,6 @@ static const DeclTypeInfo VComp[] = { { DXGI_FORMAT_UNKNOWN, "UNUSED_DEC_U16_2" }, // DEC_U16_2, { DXGI_FORMAT_R16G16B16A16_UNORM ,"D3DDECLTYPE_USHORT4N "}, // DEC_U16_3, { DXGI_FORMAT_R16G16B16A16_UNORM ,"D3DDECLTYPE_USHORT4N "}, // DEC_U16_4, - { DXGI_FORMAT_UNKNOWN, "UNUSED_DEC_U8A_2"}, // DEC_U8A_2, - { DXGI_FORMAT_UNKNOWN, "UNUSED_DEC_U16A_2" }, // DEC_U16A_2, }; static void VertexAttribSetup(D3D11_INPUT_ELEMENT_DESC * VertexElement, u8 fmt, u8 offset, const char *semantic, u8 semantic_index = 0) { diff --git a/GPU/Directx9/DrawEngineDX9.cpp b/GPU/Directx9/DrawEngineDX9.cpp index 68d5d837fb..f452c945c0 100644 --- a/GPU/Directx9/DrawEngineDX9.cpp +++ b/GPU/Directx9/DrawEngineDX9.cpp @@ -155,9 +155,6 @@ static const DeclTypeInfo VComp[] = { {0, "UNUSED_DEC_U16_2" }, // DEC_U16_2, {D3DDECLTYPE_USHORT4N ,"D3DDECLTYPE_USHORT4N "}, // DEC_U16_3, {D3DDECLTYPE_USHORT4N ,"D3DDECLTYPE_USHORT4N "}, // DEC_U16_4, - // Not supported in regular DX9 so faking, will cause graphics bugs until worked around - {0,"UNUSED_DEC_U8A_2"}, // DEC_U8A_2, - {0,"UNUSED_DEC_U16A_2" }, // DEC_U16A_2, }; static void VertexAttribSetup(D3DVERTEXELEMENT9 * VertexElement, u8 fmt, u8 offset, u8 usage, u8 usage_index = 0) { diff --git a/GPU/GLES/DrawEngineGLES.cpp b/GPU/GLES/DrawEngineGLES.cpp index fe1f1e0653..6034d491c3 100644 --- a/GPU/GLES/DrawEngineGLES.cpp +++ b/GPU/GLES/DrawEngineGLES.cpp @@ -228,8 +228,6 @@ static const GlTypeInfo GLComp[] = { {GL_UNSIGNED_SHORT, 2, GL_TRUE},// DEC_U16_2, {GL_UNSIGNED_SHORT, 3, GL_TRUE},// DEC_U16_3, {GL_UNSIGNED_SHORT, 4, GL_TRUE},// DEC_U16_4, - {GL_UNSIGNED_BYTE, 2, GL_FALSE},// DEC_U8A_2, - {GL_UNSIGNED_SHORT, 2, GL_FALSE},// DEC_U16A_2, }; static inline void VertexAttribSetup(int attrib, int fmt, int stride, u8 *ptr) { diff --git a/GPU/Vulkan/DrawEngineVulkan.cpp b/GPU/Vulkan/DrawEngineVulkan.cpp index 7682e84e21..addf18655e 100644 --- a/GPU/Vulkan/DrawEngineVulkan.cpp +++ b/GPU/Vulkan/DrawEngineVulkan.cpp @@ -887,7 +887,7 @@ void DrawEngineVulkan::DoFlush() { } Draw::NativeObject object = g_Config.iRenderingMode != 0 ? Draw::NativeObject::FRAMEBUFFER_RENDERPASS : Draw::NativeObject::BACKBUFFER_RENDERPASS; VkRenderPass renderPass = (VkRenderPass)draw_->GetNativeObject(object); - VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(pipelineLayout_, renderPass, pipelineKey_, dec_, vshader, fshader, true); + VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(pipelineLayout_, renderPass, pipelineKey_, &dec_->decFmt, vshader, fshader, true); if (!pipeline) { // Already logged, let's bail out. return; @@ -985,7 +985,7 @@ void DrawEngineVulkan::DoFlush() { } Draw::NativeObject object = g_Config.iRenderingMode != 0 ? Draw::NativeObject::FRAMEBUFFER_RENDERPASS : Draw::NativeObject::BACKBUFFER_RENDERPASS; VkRenderPass renderPass = (VkRenderPass)draw_->GetNativeObject(object); - VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(pipelineLayout_, renderPass, pipelineKey_, dec_, vshader, fshader, false); + VulkanPipeline *pipeline = pipelineManager_->GetOrCreatePipeline(pipelineLayout_, renderPass, pipelineKey_, &dec_->decFmt, vshader, fshader, false); if (!pipeline) { // Already logged, let's bail out. return; diff --git a/GPU/Vulkan/PipelineManagerVulkan.cpp b/GPU/Vulkan/PipelineManagerVulkan.cpp index 940254e7d1..928a4398b9 100644 --- a/GPU/Vulkan/PipelineManagerVulkan.cpp +++ b/GPU/Vulkan/PipelineManagerVulkan.cpp @@ -66,9 +66,6 @@ static const DeclTypeInfo VComp[] = { { VK_FORMAT_R16G16_UNORM, "R16G16_UNORM" }, // DEC_U16_2, { VK_FORMAT_R16G16B16A16_UNORM, "R16G16B16A16_UNORM " }, // DEC_U16_3, { VK_FORMAT_R16G16B16A16_UNORM, "R16G16B16A16_UNORM " }, // DEC_U16_4, - - { VK_FORMAT_R8G8_UINT, "R8G8_UINT" }, // DEC_U8A_2, - { VK_FORMAT_R16G16_UINT, "R16G16_UINT" }, // DEC_U16A_2, }; static void VertexAttribSetup(VkVertexInputAttributeDescription *attr, int fmt, int offset, PspAttributeLocation location) { @@ -121,7 +118,7 @@ static bool UsesBlendConstant(int factor) { static VulkanPipeline *CreateVulkanPipeline(VkDevice device, VkPipelineCache pipelineCache, VkPipelineLayout layout, VkRenderPass renderPass, const VulkanPipelineRasterStateKey &key, - const VertexDecoder *vtxDec, VulkanVertexShader *vs, VulkanFragmentShader *fs, bool useHwTransform, float lineWidth) { + const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, bool useHwTransform, float lineWidth) { bool useBlendConstant = false; VkPipelineColorBlendAttachmentState blend0 = {}; @@ -229,10 +226,10 @@ static VulkanPipeline *CreateVulkanPipeline(VkDevice device, VkPipelineCache pip VkVertexInputAttributeDescription attrs[8]; int attributeCount; if (useHwTransform) { - attributeCount = SetupVertexAttribs(attrs, vtxDec->decFmt); - vertexStride = vtxDec->decFmt.stride; + attributeCount = SetupVertexAttribs(attrs, *decFmt); + vertexStride = decFmt->stride; } else { - attributeCount = SetupVertexAttribsPretransformed(attrs, vtxDec->decFmt); + attributeCount = SetupVertexAttribsPretransformed(attrs, *decFmt); vertexStride = 36; } @@ -301,7 +298,7 @@ static VulkanPipeline *CreateVulkanPipeline(VkDevice device, VkPipelineCache pip return vulkanPipeline; } -VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VkPipelineLayout layout, VkRenderPass renderPass, const VulkanPipelineRasterStateKey &rasterKey, const VertexDecoder *vtxDec, VulkanVertexShader *vs, VulkanFragmentShader *fs, bool useHwTransform) { +VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VkPipelineLayout layout, VkRenderPass renderPass, const VulkanPipelineRasterStateKey &rasterKey, const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, bool useHwTransform) { VulkanPipelineKey key; if (!renderPass) Crash(); @@ -311,7 +308,7 @@ VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VkPipelineLayout layo key.useHWTransform = useHwTransform; key.vShader = vs->GetModule(); key.fShader = fs->GetModule(); - key.vtxDec = useHwTransform ? vtxDec : nullptr; + key.vtxDecId = useHwTransform ? decFmt->id : 0; auto iter = pipelines_.Get(key); if (iter) @@ -321,7 +318,7 @@ VulkanPipeline *PipelineManagerVulkan::GetOrCreatePipeline(VkPipelineLayout layo VulkanPipeline *pipeline = CreateVulkanPipeline( vulkan_->GetDevice(), pipelineCache_, layout, renderPass, - rasterKey, vtxDec, vs, fs, useHwTransform, lineWidth_); + rasterKey, decFmt, vs, fs, useHwTransform, lineWidth_); // Even if the result is nullptr, insert it so we don't try to create it repeatedly. pipelines_.Insert(key, pipeline); return pipeline; diff --git a/GPU/Vulkan/PipelineManagerVulkan.h b/GPU/Vulkan/PipelineManagerVulkan.h index 9387ed43fe..1099278be6 100644 --- a/GPU/Vulkan/PipelineManagerVulkan.h +++ b/GPU/Vulkan/PipelineManagerVulkan.h @@ -41,7 +41,7 @@ struct VulkanPipelineKey { VulkanPipelineRasterStateKey raster; // prim is included here VkRenderPass renderPass; bool useHWTransform; - const VertexDecoder *vtxDec; + uint32_t vtxDecId; VkShaderModule vShader; VkShaderModule fShader; @@ -77,7 +77,7 @@ public: PipelineManagerVulkan(VulkanContext *ctx); ~PipelineManagerVulkan(); - VulkanPipeline *GetOrCreatePipeline(VkPipelineLayout layout, VkRenderPass renderPass, const VulkanPipelineRasterStateKey &rasterKey, const VertexDecoder *vtxDec, VulkanVertexShader *vs, VulkanFragmentShader *fs, bool useHwTransform); + VulkanPipeline *GetOrCreatePipeline(VkPipelineLayout layout, VkRenderPass renderPass, const VulkanPipelineRasterStateKey &rasterKey, const DecVtxFormat *decFmt, VulkanVertexShader *vs, VulkanFragmentShader *fs, bool useHwTransform); int GetNumPipelines() const { return (int)pipelines_.size(); } void Clear();