Merge pull request #5857 from smoogipoo/renderer-cleanup

Cleanup Veldrid shader resource layouts and add header guard
ppy · Jun 21, 2023 · 86c9972 · 86c9972
2 parents d01ae5c + 4f37530
commit 86c9972
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 29 deletions.
diff --git a/osu.Framework/Graphics/OpenGL/Shaders/GLShader.cs b/osu.Framework/Graphics/OpenGL/Shaders/GLShader.cs
@@ -181,7 +181,11 @@ private protected virtual bool CompileInternal()
 
  if (layout.Elements.Any(e => e.Kind == ResourceKind.TextureReadOnly || e.Kind == ResourceKind.TextureReadWrite))
  {
- var textureElement = layout.Elements.First(e => e.Kind == ResourceKind.TextureReadOnly || e.Kind == ResourceKind.TextureReadWrite);
+ ResourceLayoutElementDescription textureElement = layout.Elements.First(e => e.Kind == ResourceKind.TextureReadOnly || e.Kind == ResourceKind.TextureReadWrite);
+
+ if (layout.Elements.All(e => e.Kind != ResourceKind.Sampler))
+ throw new ProgramLinkingFailedException(name, $"Texture {textureElement.Name} has no associated sampler.");
+
  textureUniforms.Add(new Uniform<int>(renderer, this, textureElement.Name, GL.GetUniformLocation(this, textureElement.Name))
  {
  Value = textureIndex++

diff --git a/osu.Framework/Graphics/Veldrid/Shaders/VeldridShader.cs b/osu.Framework/Graphics/Veldrid/Shaders/VeldridShader.cs
@@ -181,41 +181,22 @@ private void compile()
 
  if (layout.Elements.Any(e => e.Kind == ResourceKind.TextureReadOnly || e.Kind == ResourceKind.TextureReadWrite))
  {
- // Todo: We should enforce that a texture set contains both a texture and a sampler.
- var textureElement = layout.Elements.First(e => e.Kind == ResourceKind.TextureReadOnly || e.Kind == ResourceKind.TextureReadWrite);
- var samplerElement = layout.Elements.First(e => e.Kind == ResourceKind.Sampler);
-
- textureLayouts.Add(new VeldridUniformLayout(
- set,
- renderer.Factory.CreateResourceLayout(
- new ResourceLayoutDescription(
- new ResourceLayoutElementDescription(
- textureElement.Name,
- ResourceKind.TextureReadOnly,
- ShaderStages.Fragment),
- new ResourceLayoutElementDescription(
- samplerElement.Name,
- ResourceKind.Sampler,
- ShaderStages.Fragment)))));
+ ResourceLayoutElementDescription textureElement = layout.Elements.First(e => e.Kind == ResourceKind.TextureReadOnly || e.Kind == ResourceKind.TextureReadWrite);
+
+ if (layout.Elements.All(e => e.Kind != ResourceKind.Sampler))
+ throw new InvalidOperationException($"Texture {textureElement.Name} has no associated sampler.");
+
+ textureLayouts.Add(new VeldridUniformLayout(set, renderer.Factory.CreateResourceLayout(layout)));
  }
  else if (layout.Elements[0].Kind == ResourceKind.UniformBuffer)
- {
- uniformLayouts[layout.Elements[0].Name] = new VeldridUniformLayout(
- set,
- renderer.Factory.CreateResourceLayout(
- new ResourceLayoutDescription(
- new ResourceLayoutElementDescription(
- layout.Elements[0].Name,
- ResourceKind.UniformBuffer,
- ShaderStages.Fragment | ShaderStages.Vertex))));
- }
+ uniformLayouts[layout.Elements[0].Name] = new VeldridUniformLayout(set, renderer.Factory.CreateResourceLayout(layout));
  }
 
  Logger.Log(cached
  ? $"🖍️ Shader {name} loaded from cache!"
  : $"🖍️ Shader {name} compiled!");
  }
- catch (SpirvCompilationException e)
+ catch (Exception e)
  {
  Logger.Error(e, $"🖍️ Failed to initialise shader {name}");
  throw;

diff --git a/osu.Framework/Resources/Shaders/Internal/sh_Fragment_Output.h b/osu.Framework/Resources/Shaders/Internal/sh_Fragment_Output.h
@@ -1,5 +1,8 @@
 // Automatically included for every fragment shader.
 
+#ifndef INTERNAL_FRAGMENT_OUTPUT_H
+#define INTERNAL_FRAGMENT_OUTPUT_H
+
 {{ fragment_output_layout }}
 
 void main()
@@ -8,4 +11,6 @@ void main()
 
  // Ensure no fragment input is culled out from the shader by passing them in the output.
  {{ fragment_output_assignment }}
-}
+}
+
+#endif