From 1b38f8606b361f52d7cbbf9e6543889fb65f7108 Mon Sep 17 00:00:00 2001 From: TheSpydog Date: Tue, 8 Nov 2022 19:29:05 +0000 Subject: [PATCH] Added more graphics validation, misc api tweaks (#30) * Refactored render pass attachment validation to avoid copy-pasting the asserts * Added validation for texture usage flags and null textures/samplers * Added an exception for when GraphicsPipeline/ComputePipeline creation fails * Changed TextureSamplerBinding so that it holds references to the Texture and Sampler classes, rather than just their handles * Removed the CommandBuffer.BindVertex/FragmentSamplers overloads that took a length parameter Co-authored-by: Caleb Cornett Reviewed-on: https://gitea.moonside.games/MoonsideGames/MoonWorks/pulls/30 Co-authored-by: TheSpydog Co-committed-by: TheSpydog --- .../Bindings/TextureSamplerBinding.cs | 14 +- src/Graphics/CommandBuffer.cs | 162 ++++++++++-------- src/Graphics/Resources/ComputePipeline.cs | 4 + src/Graphics/Resources/GraphicsPipeline.cs | 4 + 4 files changed, 101 insertions(+), 83 deletions(-) diff --git a/src/Graphics/Bindings/TextureSamplerBinding.cs b/src/Graphics/Bindings/TextureSamplerBinding.cs index fdb249e..06179bf 100644 --- a/src/Graphics/Bindings/TextureSamplerBinding.cs +++ b/src/Graphics/Bindings/TextureSamplerBinding.cs @@ -4,19 +4,13 @@ namespace MoonWorks.Graphics { public struct TextureSamplerBinding { - public IntPtr TextureHandle; - public IntPtr SamplerHandle; + public Texture Texture; + public Sampler Sampler; public TextureSamplerBinding(Texture texture, Sampler sampler) { - TextureHandle = texture.Handle; - SamplerHandle = sampler.Handle; - } - - public TextureSamplerBinding(IntPtr textureHandle, IntPtr samplerHandle) - { - TextureHandle = textureHandle; - SamplerHandle = samplerHandle; + Texture = texture; + Sampler = sampler; } } } diff --git a/src/Graphics/CommandBuffer.cs b/src/Graphics/CommandBuffer.cs index 1918940..a0746d3 100644 --- a/src/Graphics/CommandBuffer.cs +++ b/src/Graphics/CommandBuffer.cs @@ -40,15 +40,7 @@ namespace MoonWorks.Graphics ) { #if DEBUG - if (colorAttachmentInfos.Length == 0) - { - throw new System.ArgumentException("Render pass must contain at least one attachment!"); - } - - if (colorAttachmentInfos.Length > 4) - { - throw new System.ArgumentException("Render pass cannot have more than 4 color attachments!"); - } + AssertValidColorAttachments(colorAttachmentInfos, true); #endif var refreshColorAttachmentInfos = new Refresh.ColorAttachmentInfo[colorAttachmentInfos.Length]; @@ -86,10 +78,8 @@ namespace MoonWorks.Graphics ) { #if DEBUG - if (colorAttachmentInfos.Length > 4) - { - throw new System.ArgumentException("Render pass cannot have more than 4 color attachments!"); - } + AssertValidDepthAttachments(depthStencilAttachmentInfo); + AssertValidColorAttachments(colorAttachmentInfos, false); #endif var refreshColorAttachmentInfos = new Refresh.ColorAttachmentInfo[colorAttachmentInfos.Length]; @@ -129,15 +119,7 @@ namespace MoonWorks.Graphics ) { #if DEBUG - if (colorAttachmentInfos.Length == 0) - { - throw new System.ArgumentException("Render pass must contain at least one attachment!"); - } - - if (colorAttachmentInfos.Length > 4) - { - throw new System.ArgumentException("Render pass cannot have more than 4 color attachments!"); - } + AssertValidColorAttachments(colorAttachmentInfos, true); #endif var refreshColorAttachmentInfos = new Refresh.ColorAttachmentInfo[colorAttachmentInfos.Length]; @@ -177,10 +159,8 @@ namespace MoonWorks.Graphics ) { #if DEBUG - if (colorAttachmentInfos.Length > 4) - { - throw new System.ArgumentException("Render pass cannot have more than 4 color attachments!"); - } + AssertValidDepthAttachments(depthStencilAttachmentInfo); + AssertValidColorAttachments(colorAttachmentInfos, false); #endif var refreshColorAttachmentInfos = new Refresh.ColorAttachmentInfo[colorAttachmentInfos.Length]; @@ -453,11 +433,9 @@ namespace MoonWorks.Graphics /// /// Binds samplers to be used by the vertex shader. /// - /// An array of texture-sampler pairs to bind. - /// The number of texture-sampler pairs from the array to bind. + /// The texture-sampler pairs to bind. public unsafe void BindVertexSamplers( - TextureSamplerBinding[] textureSamplerBindings, - int length + params TextureSamplerBinding[] textureSamplerBindings ) { #if DEBUG @@ -468,7 +446,7 @@ namespace MoonWorks.Graphics throw new System.InvalidOperationException("The vertex shader of the current graphics pipeline does not take any samplers!"); } - if (currentGraphicsPipeline.VertexShaderInfo.SamplerBindingCount < length) + if (currentGraphicsPipeline.VertexShaderInfo.SamplerBindingCount < textureSamplerBindings.Length) { throw new System.InvalidOperationException("Vertex sampler count exceeds the amount used by the vertex shader!"); } @@ -477,10 +455,15 @@ namespace MoonWorks.Graphics var texturePtrs = stackalloc IntPtr[textureSamplerBindings.Length]; var samplerPtrs = stackalloc IntPtr[textureSamplerBindings.Length]; - for (var i = 0; i < length; i += 1) + for (var i = 0; i < textureSamplerBindings.Length; i += 1) { - texturePtrs[i] = textureSamplerBindings[i].TextureHandle; - samplerPtrs[i] = textureSamplerBindings[i].SamplerHandle; +#if DEBUG + AssertTextureSamplerBindingNonNull(textureSamplerBindings[i]); + AssertTextureBindingUsageFlags(textureSamplerBindings[i].Texture); +#endif + + texturePtrs[i] = textureSamplerBindings[i].Texture.Handle; + samplerPtrs[i] = textureSamplerBindings[i].Sampler.Handle; } Refresh.Refresh_BindVertexSamplers( @@ -491,25 +474,12 @@ namespace MoonWorks.Graphics ); } - /// - /// Binds samplers to be used by the vertex shader. - /// - /// The texture-sampler pairs to bind. - public unsafe void BindVertexSamplers( - params TextureSamplerBinding[] textureSamplerBindings - ) - { - BindVertexSamplers(textureSamplerBindings, textureSamplerBindings.Length); - } - /// /// Binds samplers to be used by the fragment shader. /// /// An array of texture-sampler pairs to bind. - /// The number of texture-sampler pairs from the given array to bind. public unsafe void BindFragmentSamplers( - TextureSamplerBinding[] textureSamplerBindings, - int length + params TextureSamplerBinding[] textureSamplerBindings ) { #if DEBUG @@ -520,7 +490,7 @@ namespace MoonWorks.Graphics throw new System.InvalidOperationException("The fragment shader of the current graphics pipeline does not take any samplers!"); } - if (currentGraphicsPipeline.FragmentShaderInfo.SamplerBindingCount < length) + if (currentGraphicsPipeline.FragmentShaderInfo.SamplerBindingCount < textureSamplerBindings.Length) { throw new System.InvalidOperationException("Fragment sampler count exceeds the amount used by the fragment shader!"); } @@ -529,21 +499,15 @@ namespace MoonWorks.Graphics var texturePtrs = stackalloc IntPtr[textureSamplerBindings.Length]; var samplerPtrs = stackalloc IntPtr[textureSamplerBindings.Length]; - for (var i = 0; i < length; i += 1) + for (var i = 0; i < textureSamplerBindings.Length; i += 1) { - #if DEBUG - if (textureSamplerBindings[i].TextureHandle == IntPtr.Zero) - { - throw new NullReferenceException("Texture binding must not be null!"); - } - if (textureSamplerBindings[i].TextureHandle == IntPtr.Zero) - { - throw new NullReferenceException("Sampler binding must not be null!"); - } - #endif +#if DEBUG + AssertTextureSamplerBindingNonNull(textureSamplerBindings[i]); + AssertTextureBindingUsageFlags(textureSamplerBindings[i].Texture); +#endif - texturePtrs[i] = textureSamplerBindings[i].TextureHandle; - samplerPtrs[i] = textureSamplerBindings[i].SamplerHandle; + texturePtrs[i] = textureSamplerBindings[i].Texture.Handle; + samplerPtrs[i] = textureSamplerBindings[i].Sampler.Handle; } Refresh.Refresh_BindFragmentSamplers( @@ -554,17 +518,6 @@ namespace MoonWorks.Graphics ); } - /// - /// Binds samplers to be used by the fragment shader. - /// - /// An array of texture-sampler pairs to bind. - public unsafe void BindFragmentSamplers( - params TextureSamplerBinding[] textureSamplerBindings - ) - { - BindFragmentSamplers(textureSamplerBindings, textureSamplerBindings.Length); - } - /// /// Pushes vertex shader uniforms to the device. /// @@ -581,6 +534,7 @@ namespace MoonWorks.Graphics throw new InvalidOperationException("The current vertex shader does not take a uniform buffer!"); } #endif + fixed (T* ptr = &uniforms[0]) { return Refresh.Refresh_PushVertexShaderUniforms( @@ -1111,6 +1065,68 @@ namespace MoonWorks.Graphics throw new System.InvalidOperationException(message); } } + + private void AssertValidColorAttachments(ColorAttachmentInfo[] colorAttachmentInfos, bool atLeastOneRequired) + { + if (atLeastOneRequired && colorAttachmentInfos.Length == 0) + { + throw new System.ArgumentException("Render pass must contain at least one attachment!"); + } + + if (colorAttachmentInfos.Length > 4) + { + throw new System.ArgumentException("Render pass cannot have more than 4 color attachments!"); + } + + for (int i = 0; i < colorAttachmentInfos.Length; i += 1) + { + if (colorAttachmentInfos[i].Texture == null || + colorAttachmentInfos[i].Texture.Handle == IntPtr.Zero) + { + throw new System.ArgumentException("Render pass color attachment Texture cannot be null!"); + } + + if ((colorAttachmentInfos[i].Texture.UsageFlags & TextureUsageFlags.ColorTarget) == 0) + { + throw new System.ArgumentException("Render pass color attachment UsageFlags must include TextureUsageFlags.ColorTarget!"); + } + } + } + + private void AssertValidDepthAttachments(DepthStencilAttachmentInfo depthStencilAttachmentInfo) + { + if (depthStencilAttachmentInfo.Texture == null || + depthStencilAttachmentInfo.Texture.Handle == IntPtr.Zero) + { + throw new System.ArgumentException("Render pass depth stencil attachment Texture cannot be null!"); + } + + if ((depthStencilAttachmentInfo.Texture.UsageFlags & TextureUsageFlags.DepthStencilTarget) == 0) + { + throw new System.ArgumentException("Render pass depth stencil attachment UsageFlags must include TextureUsageFlags.DepthStencilTarget!"); + } + } + + private void AssertTextureSamplerBindingNonNull(in TextureSamplerBinding binding) + { + if (binding.Texture == null || binding.Texture.Handle == IntPtr.Zero) + { + throw new NullReferenceException("Texture binding must not be null!"); + } + + if (binding.Sampler == null || binding.Sampler.Handle == IntPtr.Zero) + { + throw new NullReferenceException("Sampler binding must not be null!"); + } + } + + private void AssertTextureBindingUsageFlags(Texture texture) + { + if ((texture.UsageFlags & TextureUsageFlags.Sampler) == 0) + { + throw new System.ArgumentException("The bound Texture's UsageFlags must include TextureUsageFlags.Sampler!"); + } + } #endif } } diff --git a/src/Graphics/Resources/ComputePipeline.cs b/src/Graphics/Resources/ComputePipeline.cs index f39d9a0..e654f9b 100644 --- a/src/Graphics/Resources/ComputePipeline.cs +++ b/src/Graphics/Resources/ComputePipeline.cs @@ -28,6 +28,10 @@ namespace MoonWorks.Graphics device.Handle, refreshComputeShaderInfo ); + if (Handle == IntPtr.Zero) + { + throw new Exception("Could not create compute pipeline!"); + } ComputeShaderInfo = computeShaderInfo; } diff --git a/src/Graphics/Resources/GraphicsPipeline.cs b/src/Graphics/Resources/GraphicsPipeline.cs index 67fe632..11026eb 100644 --- a/src/Graphics/Resources/GraphicsPipeline.cs +++ b/src/Graphics/Resources/GraphicsPipeline.cs @@ -102,6 +102,10 @@ namespace MoonWorks.Graphics refreshGraphicsPipelineCreateInfo.attachmentInfo.hasDepthStencilAttachment = Conversions.BoolToByte(attachmentInfo.HasDepthStencilAttachment); Handle = Refresh.Refresh_CreateGraphicsPipeline(device.Handle, refreshGraphicsPipelineCreateInfo); + if (Handle == IntPtr.Zero) + { + throw new Exception("Could not create graphics pipeline!"); + } vertexAttributesHandle.Free(); vertexBindingsHandle.Free();