From 297f234957fa2aee80d99285768f526524fe1e8f Mon Sep 17 00:00:00 2001 From: TheSpydog Date: Tue, 8 Nov 2022 19:09:21 +0000 Subject: [PATCH] Miscellaneous API changes + more MSAA fixes (#26) **Breaking API Changes** * Removed `REFRESH_SAMPLECOUNT_16/32/64`, since hardware support for these sample counts is generally poor (and completely non-existent with MoltenVK and certain consoles). * Removed unused `sampleCount` parameter from `Refresh_TextureCreateInfo`. * Removed `sampleCount` parameter from `Refresh_ColorAttachmentDescription`. The existence of this parameter meant you had to sync up three different sample count values (render pass, pipeline, and color attachment description) whenever you wanted to use multisampling. However, Vulkan requires that all color attachments in a given pipeline _must_ match the pipeline's sample count anyway, so we can assume all color attachments will use the pipeline's sample count. * Removed the `renderArea` parameter from `Refresh_BeginRenderPass()` since it didn't serve much practical purpose and slightly complicated things on the MoonWorks managed side. **Behavior Changes** * When creating a render pass or graphics pipeline, the requested multisample count will be converted into a sample count that's actually supported by the GPU. For example, if you request 8x MSAA on a device that only supports up to 4x MSAA, it will silently fall back to 4x MSAA. * All color attachments are now forced to have an internal store op of `STORE`, even if `REFRESH_STORE_OP_DONTCARE` is specified. The one exception is internal multisample textures -- if `DONTCARE` is used, those textures will be discarded to save on bandwidth. (Their resolve textures will still be stored.) * The RenderPass hashing logic was updated so that it would still work correctly with the removal of `Refresh_ColorAttachmentDescription.sampleCount`. **Bug Fixes** * Fixed bugs where multisampling logic wasn't kicking in for certain sample counts due to incorrect enum comparisons. Co-authored-by: Caleb Cornett Reviewed-on: https://gitea.moonside.games/MoonsideGames/Refresh/pulls/26 Co-authored-by: TheSpydog Co-committed-by: TheSpydog --- include/Refresh.h | 13 +---- src/Refresh.c | 2 - src/Refresh_Driver.h | 1 - src/Refresh_Driver_Vulkan.c | 106 +++++++++++++++++++++++------------- 4 files changed, 70 insertions(+), 52 deletions(-) diff --git a/include/Refresh.h b/include/Refresh.h index 6822f56..655763c 100644 --- a/include/Refresh.h +++ b/include/Refresh.h @@ -169,10 +169,7 @@ typedef enum Refresh_SampleCount REFRESH_SAMPLECOUNT_1, REFRESH_SAMPLECOUNT_2, REFRESH_SAMPLECOUNT_4, - REFRESH_SAMPLECOUNT_8, - REFRESH_SAMPLECOUNT_16, - REFRESH_SAMPLECOUNT_32, - REFRESH_SAMPLECOUNT_64 + REFRESH_SAMPLECOUNT_8 } Refresh_SampleCount; typedef enum Refresh_CubeMapFace @@ -467,7 +464,6 @@ typedef struct Refresh_TextureCreateInfo uint32_t height; uint32_t depth; uint8_t isCube; - Refresh_SampleCount sampleCount; uint32_t levelCount; Refresh_TextureFormat format; Refresh_TextureUsageFlags usageFlags; @@ -526,7 +522,6 @@ typedef struct Refresh_DepthStencilState typedef struct Refresh_ColorAttachmentDescription { Refresh_TextureFormat format; - Refresh_SampleCount sampleCount; Refresh_ColorAttachmentBlendState blendState; } Refresh_ColorAttachmentDescription; @@ -1000,11 +995,6 @@ REFRESHAPI void Refresh_QueueDestroyGraphicsPipeline( /* Begins a render pass. * This will also set a default viewport and scissor state. * - * renderArea: - * The area affected by the render pass. - * All load, store and resolve operations are restricted - * to the given rectangle. - * If NULL, a sensible default will be chosen. * colorAttachmentInfos: * A pointer to an array of Refresh_ColorAttachmentInfo structures * that contains render targets and clear values. May be NULL. @@ -1014,7 +1004,6 @@ REFRESHAPI void Refresh_QueueDestroyGraphicsPipeline( REFRESHAPI void Refresh_BeginRenderPass( Refresh_Device *device, Refresh_CommandBuffer *commandBuffer, - Refresh_Rect *renderArea, Refresh_ColorAttachmentInfo *colorAttachmentInfos, uint32_t colorAttachmentCount, Refresh_DepthStencilAttachmentInfo *depthStencilAttachmentInfo diff --git a/src/Refresh.c b/src/Refresh.c index 775a6e2..809269e 100644 --- a/src/Refresh.c +++ b/src/Refresh.c @@ -625,7 +625,6 @@ void Refresh_QueueDestroyGraphicsPipeline( void Refresh_BeginRenderPass( Refresh_Device *device, Refresh_CommandBuffer *commandBuffer, - Refresh_Rect *renderArea, Refresh_ColorAttachmentInfo *colorAttachmentInfos, uint32_t colorAttachmentCount, Refresh_DepthStencilAttachmentInfo *depthStencilAttachmentInfo @@ -634,7 +633,6 @@ void Refresh_BeginRenderPass( device->BeginRenderPass( device->driverData, commandBuffer, - renderArea, colorAttachmentInfos, colorAttachmentCount, depthStencilAttachmentInfo diff --git a/src/Refresh_Driver.h b/src/Refresh_Driver.h index e3c83c3..b48378f 100644 --- a/src/Refresh_Driver.h +++ b/src/Refresh_Driver.h @@ -395,7 +395,6 @@ struct Refresh_Device void(*BeginRenderPass)( Refresh_Renderer *driverData, Refresh_CommandBuffer *commandBuffer, - Refresh_Rect *renderArea, Refresh_ColorAttachmentInfo *colorAttachmentInfos, uint32_t colorAttachmentCount, Refresh_DepthStencilAttachmentInfo *depthStencilAttachmentInfo diff --git a/src/Refresh_Driver_Vulkan.c b/src/Refresh_Driver_Vulkan.c index fe9da12..79d8fa6 100644 --- a/src/Refresh_Driver_Vulkan.c +++ b/src/Refresh_Driver_Vulkan.c @@ -971,6 +971,7 @@ typedef struct RenderPassHash RenderPassColorTargetDescription colorTargetDescriptions[MAX_COLOR_TARGET_BINDINGS]; uint32_t colorAttachmentCount; RenderPassDepthStencilTargetDescription depthStencilTargetDescription; + Refresh_SampleCount colorAttachmentSampleCount; } RenderPassHash; typedef struct RenderPassHashMap @@ -997,6 +998,11 @@ static inline uint8_t RenderPassHash_Compare( return 0; } + if (a->colorAttachmentSampleCount != b->colorAttachmentSampleCount) + { + return 0; + } + for (i = 0; i < a->colorAttachmentCount; i += 1) { if (a->colorTargetDescriptions[i].format != b->colorTargetDescriptions[i].format) @@ -2023,6 +2029,29 @@ static inline VkDeviceSize VULKAN_INTERNAL_BytesPerImage( return blocksPerRow * blocksPerColumn * VULKAN_INTERNAL_BytesPerPixel(format); } +static inline Refresh_SampleCount VULKAN_INTERNAL_GetMaxMultiSampleCount( + VulkanRenderer *renderer, + Refresh_SampleCount multiSampleCount +) { + VkSampleCountFlags flags = renderer->physicalDeviceProperties.properties.limits.framebufferColorSampleCounts; + Refresh_SampleCount maxSupported = REFRESH_SAMPLECOUNT_1; + + if (flags & VK_SAMPLE_COUNT_8_BIT) + { + maxSupported = REFRESH_SAMPLECOUNT_8; + } + else if (flags & VK_SAMPLE_COUNT_4_BIT) + { + maxSupported = REFRESH_SAMPLECOUNT_4; + } + else if (flags & VK_SAMPLE_COUNT_2_BIT) + { + maxSupported = REFRESH_SAMPLECOUNT_2; + } + + return SDL_min(multiSampleCount, maxSupported); +} + /* Memory Management */ static inline VkDeviceSize VULKAN_INTERNAL_NextHighestAlignment( @@ -5431,6 +5460,12 @@ static VulkanRenderTarget* VULKAN_INTERNAL_CreateRenderTarget( /* create resolve target for multisample */ if (multisampleCount > REFRESH_SAMPLECOUNT_1) { + /* Find a compatible sample count to use */ + multisampleCount = VULKAN_INTERNAL_GetMaxMultiSampleCount( + renderer, + multisampleCount + ); + renderTarget->multisampleTexture = VULKAN_INTERNAL_CreateTexture( renderer, @@ -5576,8 +5611,12 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( colorAttachmentInfos[i].sampleCount ); - if (renderTarget->multisampleTexture != NULL) + if (renderTarget->multisampleCount > VK_SAMPLE_COUNT_1_BIT) { + multisampling = 1; + + /* Transition the multisample attachment */ + VULKAN_INTERNAL_ImageMemoryBarrier( renderer, commandBuffer->commandBuffer, @@ -5591,11 +5630,6 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( renderTarget->multisampleTexture->image, &renderTarget->multisampleTexture->resourceAccessType ); - } - - if (renderTarget->multisampleCount > VK_SAMPLE_COUNT_1_BIT) - { - multisampling = 1; /* Resolve attachment and multisample attachment */ @@ -5606,9 +5640,8 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( attachmentDescriptions[attachmentDescriptionCount].loadOp = RefreshToVK_LoadOp[ colorAttachmentInfos[i].loadOp ]; - attachmentDescriptions[attachmentDescriptionCount].storeOp = RefreshToVK_StoreOp[ - colorAttachmentInfos[i].storeOp - ]; + attachmentDescriptions[attachmentDescriptionCount].storeOp = + VK_ATTACHMENT_STORE_OP_STORE; /* Always store the resolve texture */ attachmentDescriptions[attachmentDescriptionCount].stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE; attachmentDescriptions[attachmentDescriptionCount].stencilStoreOp = @@ -5661,9 +5694,8 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( attachmentDescriptions[attachmentDescriptionCount].loadOp = RefreshToVK_LoadOp[ colorAttachmentInfos[i].loadOp ]; - attachmentDescriptions[attachmentDescriptionCount].storeOp = RefreshToVK_StoreOp[ - colorAttachmentInfos[i].storeOp - ]; + attachmentDescriptions[attachmentDescriptionCount].storeOp = + VK_ATTACHMENT_STORE_OP_STORE; /* Always store non-MSAA textures */ attachmentDescriptions[attachmentDescriptionCount].stencilLoadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE; attachmentDescriptions[attachmentDescriptionCount].stencilStoreOp = @@ -5778,7 +5810,8 @@ static VkRenderPass VULKAN_INTERNAL_CreateRenderPass( static VkRenderPass VULKAN_INTERNAL_CreateTransientRenderPass( VulkanRenderer *renderer, - Refresh_GraphicsPipelineAttachmentInfo attachmentInfo + Refresh_GraphicsPipelineAttachmentInfo attachmentInfo, + Refresh_SampleCount sampleCount ) { VkAttachmentDescription attachmentDescriptions[2 * MAX_COLOR_TARGET_BINDINGS + 1]; VkAttachmentReference colorAttachmentReferences[MAX_COLOR_TARGET_BINDINGS]; @@ -5800,7 +5833,7 @@ static VkRenderPass VULKAN_INTERNAL_CreateTransientRenderPass( { attachmentDescription = attachmentInfo.colorAttachmentDescriptions[i]; - if (attachmentDescription.sampleCount > REFRESH_SAMPLECOUNT_1) + if (sampleCount > REFRESH_SAMPLECOUNT_1) { multisampling = 1; @@ -5829,7 +5862,7 @@ static VkRenderPass VULKAN_INTERNAL_CreateTransientRenderPass( attachmentDescription.format ]; attachmentDescriptions[attachmentDescriptionCount].samples = RefreshToVK_SampleCount[ - attachmentDescription.sampleCount + sampleCount ]; attachmentDescriptions[attachmentDescriptionCount].loadOp = VK_ATTACHMENT_LOAD_OP_DONT_CARE; attachmentDescriptions[attachmentDescriptionCount].storeOp = VK_ATTACHMENT_STORE_OP_DONT_CARE; @@ -5960,6 +5993,7 @@ static Refresh_GraphicsPipeline* VULKAN_CreateGraphicsPipeline( ) { VkResult vulkanResult; uint32_t i; + Refresh_SampleCount actualSampleCount; VulkanGraphicsPipeline *graphicsPipeline = (VulkanGraphicsPipeline*) SDL_malloc(sizeof(VulkanGraphicsPipeline)); VkGraphicsPipelineCreateInfo vkPipelineCreateInfo; @@ -5997,11 +6031,19 @@ static Refresh_GraphicsPipeline* VULKAN_CreateGraphicsPipeline( VulkanRenderer *renderer = (VulkanRenderer*) driverData; + /* Find a compatible sample count to use */ + + actualSampleCount = VULKAN_INTERNAL_GetMaxMultiSampleCount( + renderer, + pipelineCreateInfo->multisampleState.multisampleCount + ); + /* Create a "compatible" render pass */ VkRenderPass transientRenderPass = VULKAN_INTERNAL_CreateTransientRenderPass( renderer, - pipelineCreateInfo->attachmentInfo + pipelineCreateInfo->attachmentInfo, + actualSampleCount ); /* Dynamic state */ @@ -6132,9 +6174,7 @@ static Refresh_GraphicsPipeline* VULKAN_CreateGraphicsPipeline( multisampleStateCreateInfo.sType = VK_STRUCTURE_TYPE_PIPELINE_MULTISAMPLE_STATE_CREATE_INFO; multisampleStateCreateInfo.pNext = NULL; multisampleStateCreateInfo.flags = 0; - multisampleStateCreateInfo.rasterizationSamples = RefreshToVK_SampleCount[ - pipelineCreateInfo->multisampleState.multisampleCount - ]; + multisampleStateCreateInfo.rasterizationSamples = RefreshToVK_SampleCount[actualSampleCount]; multisampleStateCreateInfo.sampleShadingEnable = VK_FALSE; multisampleStateCreateInfo.minSampleShading = 1.0f; multisampleStateCreateInfo.pSampleMask = @@ -8019,6 +8059,10 @@ static VkRenderPass VULKAN_INTERNAL_FetchRenderPass( hash.colorTargetDescriptions[i].storeOp = colorAttachmentInfos[i].storeOp; } + hash.colorAttachmentSampleCount = (colorAttachmentCount > 0) ? + colorAttachmentInfos[0].sampleCount : + REFRESH_SAMPLECOUNT_1; + hash.colorAttachmentCount = colorAttachmentCount; if (depthStencilAttachmentInfo == NULL) @@ -8316,7 +8360,6 @@ static void VULKAN_SetScissor( static void VULKAN_BeginRenderPass( Refresh_Renderer *driverData, Refresh_CommandBuffer *commandBuffer, - Refresh_Rect *renderArea, Refresh_ColorAttachmentInfo *colorAttachmentInfos, uint32_t colorAttachmentCount, Refresh_DepthStencilAttachmentInfo *depthStencilAttachmentInfo @@ -8417,7 +8460,7 @@ static void VULKAN_BeginRenderPass( &texture->resourceAccessType ); - if (colorAttachmentInfos[i].sampleCount > 1) + if (colorAttachmentInfos[i].sampleCount > REFRESH_SAMPLECOUNT_1) { clearCount += 1; multisampleAttachmentCount += 1; @@ -8467,7 +8510,7 @@ static void VULKAN_BeginRenderPass( clearValues[i].color.float32[2] = colorAttachmentInfos[i].clearColor.z; clearValues[i].color.float32[3] = colorAttachmentInfos[i].clearColor.w; - if (colorAttachmentInfos[i].sampleCount > 0) + if (colorAttachmentInfos[i].sampleCount > REFRESH_SAMPLECOUNT_1) { i += 1; clearValues[i].color.float32[0] = colorAttachmentInfos[i].clearColor.x; @@ -8492,21 +8535,10 @@ static void VULKAN_BeginRenderPass( renderPassBeginInfo.framebuffer = framebuffer->framebuffer; renderPassBeginInfo.pClearValues = clearValues; renderPassBeginInfo.clearValueCount = clearCount; - - if (renderArea != NULL) - { - renderPassBeginInfo.renderArea.extent.width = renderArea->w; - renderPassBeginInfo.renderArea.extent.height = renderArea->h; - renderPassBeginInfo.renderArea.offset.x = renderArea->x; - renderPassBeginInfo.renderArea.offset.y = renderArea->y; - } - else - { - renderPassBeginInfo.renderArea.extent.width = framebufferWidth; - renderPassBeginInfo.renderArea.extent.height = framebufferHeight; - renderPassBeginInfo.renderArea.offset.x = 0; - renderPassBeginInfo.renderArea.offset.y = 0; - } + renderPassBeginInfo.renderArea.extent.width = framebufferWidth; + renderPassBeginInfo.renderArea.extent.height = framebufferHeight; + renderPassBeginInfo.renderArea.offset.x = 0; + renderPassBeginInfo.renderArea.offset.y = 0; renderer->vkCmdBeginRenderPass( vulkanCommandBuffer->commandBuffer,