Fixed memory leak when deleting Effect

GlitchyEngine/src/Content/Loaders/ShaderLoader.bf

@@ -34,6 +34,7 @@ class ShaderLoader : IProcessedAssetLoader
 			Try!(stream.TryRead(vsData));
 
 			vertexShader = (VertexShader)Try!(Shader.CreateFromBlob(vsData, .Vertex));
+			effect.[Friend]VertexShader = vertexShader;
 		}
 
 		if (psDataSize > 0)
@@ -42,8 +43,9 @@ class ShaderLoader : IProcessedAssetLoader
 			Try!(stream.TryRead(psData));
 
 			pixelShader = (PixelShader)Try!(Shader.CreateFromBlob(psData, .Pixel));
+			effect.[Friend]PixelShader = pixelShader;
 		}
-		
+
 		uint16 textureCount = Try!(stream.Read<uint16>());
 
 		for (int i < textureCount)
@@ -77,33 +79,40 @@ class ShaderLoader : IProcessedAssetLoader
 
 		for (int i < bufferCount)
 		{
-			int64 bufferSize = Try!(stream.Read<int64>());
+			Try!(LoadBuffer(stream, effect));
+		}
 
-			int32 vertexShaderBindPoint = Try!(stream.Read<int32>());
-			int32 pixelShaderBindPoint = Try!(stream.Read<int32>());
+		return effect;
+	}
 
-			int16 bufferNameLength = Try!(stream.Read<int16>());
-			String bufferName = scope String(bufferNameLength);
-			stream.ReadStrSized32(bufferNameLength, bufferName);
+	private static Result<void> LoadBuffer(Stream stream, Effect effect)
+	{
+		int64 bufferSize = Try!(stream.Read<int64>());
 
-			int16 engineBufferNameLength = Try!(stream.Read<int16>());
-			String engineBufferName = null;
+		int32 vertexShaderBindPoint = Try!(stream.Read<int32>());
+		int32 pixelShaderBindPoint = Try!(stream.Read<int32>());
 
-			if (engineBufferNameLength > 0)
-			{
-				scope String(engineBufferNameLength);
-				stream.ReadStrSized32(engineBufferNameLength, engineBufferName);
+		int16 bufferNameLength = Try!(stream.Read<int16>());
+		String bufferName = scope String(bufferNameLength);
+		stream.ReadStrSized32(bufferNameLength, bufferName);
 
-				// TODO: Engine buffers currently do nothing. The bind points for each engine buffer are hardcoded.
-				// It only marks the buffer as engine buffer, preventing the variables from becomming accessible.
-				//effect.[Friend]_engineBuffers.Add()
-			}
-			
+		int16 engineBufferNameLength = Try!(stream.Read<int16>());
+		String engineBufferName = null;
+
+		if (engineBufferNameLength > 0)
+		{
+			scope String(engineBufferNameLength);
+			stream.ReadStrSized32(engineBufferNameLength, engineBufferName);
+
+			// TODO: Engine buffers currently do nothing. The bind points for each engine buffer are hardcoded.
+			// It only marks the buffer as engine buffer, preventing the variables from becomming accessible.
+			//effect.[Friend]_engineBuffers.Add()
+		}
+
+		using (ConstantBuffer buffer = new ConstantBuffer(bufferName, bufferSize))
+		{
 			uint16 variableCount = Try!(stream.Read<uint16>());
 
-			ConstantBuffer buffer = new ConstantBuffer(bufferName, bufferSize);
-			defer buffer.ReleaseRef();
-
 			for (int v < variableCount)
 			{
 				uint64 variableOffset = Try!(stream.Read<uint64>());
@@ -113,7 +122,7 @@ class ShaderLoader : IProcessedAssetLoader
 				uint8 rows = Try!(stream.Read<uint8>());
 				uint8 columns = Try!(stream.Read<uint8>());
 				uint64 arraySize = Try!(stream.Read<uint64>());
-	
+
 				int16 variableNameLength = Try!(stream.Read<int16>());
 				String variableName = scope String(variableNameLength);
 				stream.ReadStrSized32(variableNameLength, variableName);
@@ -126,10 +135,10 @@ class ShaderLoader : IProcessedAssetLoader
 			Try!(buffer.Update());
 
 			if (vertexShaderBindPoint != -1)
-				vertexShader.Buffers.Add(vertexShaderBindPoint, buffer.Name, buffer);
-			
+				effect.VertexShader.Buffers.Add(vertexShaderBindPoint, buffer.Name, buffer);
+
 			if (pixelShaderBindPoint != -1)
-				pixelShader.Buffers.Add(pixelShaderBindPoint, buffer.Name, buffer);
+				effect.PixelShader.Buffers.Add(pixelShaderBindPoint, buffer.Name, buffer);
 
 			// TODO: Allow binding buffers to different indices? Does this theoretically work with textures?
 			let tempBindPoint = (vertexShaderBindPoint != -1) ? vertexShaderBindPoint : pixelShaderBindPoint;
@@ -141,9 +150,6 @@ class ShaderLoader : IProcessedAssetLoader
 			}
 		}
 
-		effect.[Friend]VertexShader = vertexShader;
-		effect.[Friend]PixelShader = pixelShader;
-
-		return effect;
+		return .Ok;
 	}
 }

GlitchyEngine/src/Platform/DX11/Renderer/Dx11BufferCollection.bf

@@ -8,18 +8,17 @@ namespace GlitchyEngine.Renderer
 {
 	extension BufferCollection
 	{
+		public static override int MaxBufferSlotCount => DirectX.D3D11.D3D11_COMMONSHADER_CONSTANT_BUFFER_API_SLOT_COUNT;
+
 		internal ID3D11Buffer*[DirectX.D3D11.D3D11_COMMONSHADER_CONSTANT_BUFFER_API_SLOT_COUNT] nativeBuffers;
 
 		internal void PlatformFetchNativeBuffers()
 		{
 			Debug.Profiler.ProfileRendererFunction!();
 
-			// Clear
-			nativeBuffers = .();
-
 			for(let buffer in _buffers)
 			{
-				nativeBuffers[buffer.Index] = buffer.Buffer.nativeBuffer;
+				nativeBuffers[@buffer] = buffer.Buffer?.nativeBuffer;
 			}
 		}
 	}

GlitchyEngine/src/Platform/DX11/Renderer/Dx11GraphicsContext.bf

@@ -334,10 +334,7 @@ namespace GlitchyEngine.Renderer
 				_ps_FirstTexture = _firstTexture;
 				_ps_BoundTextures = _textureCount;
 
-				for (var buffer in shader?.Buffers)
-				{
-					_psBuffers[buffer.Index] = buffer.Buffer.nativeBuffer;
-				}
+				_psBuffers = shader.Buffers.nativeBuffers;
 
 				//NativeContext.PixelShader.SetConstantBuffers(0, shader.Buffers.nativeBuffers.Count, &shader.Buffers.nativeBuffers);
 
@@ -351,11 +348,8 @@ namespace GlitchyEngine.Renderer
 					NativeContext.VertexShader.SetShaderResources(_firstTexture, _textureCount, &_textures[_firstTexture]);
 					NativeContext.VertexShader.SetSamplers(_firstTexture, _textureCount, &_samplers[_firstTexture]);
 				}
-
-				for (var buffer in shader?.Buffers)
-				{
-					_vsBuffers[buffer.Index] = buffer.Buffer.nativeBuffer;
-				}
+				
+				_vsBuffers = shader.Buffers.nativeBuffers;
 
 				//NativeContext.VertexShader.SetConstantBuffers(0, shader.Buffers.nativeBuffers.Count, &shader.Buffers.nativeBuffers);
 

GlitchyEngine/src/Renderer/BufferCollection.bf

@@ -4,14 +4,15 @@ using GlitchyEngine.Core;
 
 namespace GlitchyEngine.Renderer
 {
-	public class BufferCollection : RefCounter, IEnumerable<(String Name, int Index, Buffer Buffer)>
+	public class BufferCollection : RefCounter, IEnumerable<(String Name, Buffer Buffer)>
 	{
-		public typealias BufferEntry = (String Name, int Index, Buffer Buffer);
+		public static extern int MaxBufferSlotCount { get; }
 
-		List<BufferEntry> _buffers ~ DeleteBufferEntries!(_);
+		public typealias BufferEntry = (String Name, Buffer Buffer);
 
-		Dictionary<String, BufferEntry*> _strToBuf ~ delete _; //delete:append _;
-		Dictionary<int, BufferEntry*> _idxToBuf ~ delete _; //delete:append _;
+		BufferEntry[] _buffers ~ DeleteBufferEntries!(_);
+
+		Dictionary<StringView, BufferEntry*> _strToBuf ~ delete _;
 
 		[AllowAppend]
 		public this()
@@ -19,13 +20,11 @@ namespace GlitchyEngine.Renderer
 			Debug.Profiler.ProfileResourceFunction!();
 
 			// Todo: append allocate as soon as it's fixed
-			let buffers = new List<BufferEntry>();
-			let strToBuf = new Dictionary<String, BufferEntry*>();
-			let idxToBuf = new Dictionary<int, BufferEntry*>();
+			let buffers = new BufferEntry[MaxBufferSlotCount];
+			let strToBuf = new Dictionary<StringView, BufferEntry*>();
 
 			_buffers = buffers;
 			_strToBuf = strToBuf;
-			_idxToBuf = idxToBuf;
 		}
 
 		public ~this()
@@ -33,7 +32,7 @@ namespace GlitchyEngine.Renderer
 			Debug.Profiler.ProfileResourceFunction!();
 		}
 		
-		mixin DeleteBufferEntries(List<BufferEntry> entries)
+		mixin DeleteBufferEntries(BufferEntry[] entries)
 		{
 			if(entries == null)
 				return;
@@ -41,13 +40,13 @@ namespace GlitchyEngine.Renderer
 			for(let entry in entries)
 			{
 				delete entry.Name;
-				entry.Buffer.ReleaseRef();
+				entry.Buffer?.ReleaseRef();
 			}
 
 			delete entries;
 		}
 
-		public Buffer this[int idx] => _idxToBuf[idx].Buffer;
+		public Buffer this[int slot] => _buffers[slot].Buffer;
 		public Buffer this[String name] => _strToBuf[name].Buffer;
 		
 		public Buffer TryGetBuffer(String name)
@@ -55,15 +54,13 @@ namespace GlitchyEngine.Renderer
 			return TryGetBufferEntry(name)?.Buffer;
 		}
 		
-		public Buffer TryGetBuffer(int index)
+		public Buffer TryGetBuffer(int slot)
 		{
-			return TryGetBufferEntry(index)?.Buffer;
+			return TryGetBufferEntry(slot)?.Buffer;
 		}
 
 		public BufferEntry* TryGetBufferEntry(String name)
 		{
-			Debug.Profiler.ProfileResourceFunction!();
-
 			if(_strToBuf.TryGetValue(name, let buffer))
 			{
 				return buffer;
@@ -72,16 +69,12 @@ namespace GlitchyEngine.Renderer
 			return null;
 		}
 
-		public BufferEntry* TryGetBufferEntry(int index)
+		public BufferEntry* TryGetBufferEntry(int slot)
 		{
-			Debug.Profiler.ProfileResourceFunction!();
+			if (slot < 0 || slot >= _buffers.Count)
+				return null;
 
-			if(_idxToBuf.TryGetValue(index, let buffer))
-			{
-				return buffer;
-			}
-
-			return null;
+			return &_buffers[slot];
 		}
 
 		/**
@@ -90,18 +83,12 @@ namespace GlitchyEngine.Renderer
 		 * @param buffer The new buffer.
 		 * @returns True, if the buffer was replaced successfully; false, otherwise.
 		 */
-		public bool TryReplaceBuffer(int idx, Buffer buffer)
+		public bool TryReplaceBuffer(int slot, Buffer buffer)
 		{
-			Debug.Profiler.ProfileResourceFunction!();
-
-			if(_idxToBuf.TryGetValue(idx, let bufferEntry))
+			BufferEntry* bufferEntry = TryGetBufferEntry(slot);
+			if(bufferEntry != null)
 			{
-				Log.EngineLogger.Assert(idx == bufferEntry.Index);
-
-				bufferEntry.Buffer.ReleaseRef();
-
-				buffer.AddRef();
-				bufferEntry.Buffer = buffer;
+				SetReference!(bufferEntry.Buffer, buffer);
 
 				return true;
 			}
@@ -119,8 +106,6 @@ namespace GlitchyEngine.Renderer
 		 */
 		public bool TryReplaceBuffer(String name, Buffer buffer)
 		{
-			Debug.Profiler.ProfileResourceFunction!();
-
 			if(_strToBuf.TryGetValue(name, let bufferEntry))
 			{
 				Log.EngineLogger.AssertDebug(name == bufferEntry.Name);
@@ -135,23 +120,14 @@ namespace GlitchyEngine.Renderer
 			}
 		}
 
-		public void Add(int index, String name, Buffer buffer)
+		public void Add(int slot, StringView name, Buffer buffer)
 		{
-			Add((name, index, buffer));
-		}
+			ref BufferEntry bufferEntry = ref _buffers[slot];
 
-		public void Add(BufferEntry entry)
-		{
-			Debug.Profiler.ProfileResourceFunction!();
+			SetReference!(bufferEntry.Buffer, buffer);
+			String.NewOrSet!(bufferEntry.Name, name);
 
-			BufferEntry copy = (new String(entry.Name), entry.Index, entry.Buffer..AddRef());
-
-			_buffers.Add(copy);
-
-			BufferEntry* copyRef = &_buffers.Back;
-
-			_strToBuf.Add(copy.Name, copyRef);
-			_idxToBuf.Add(copy.Index, copyRef);
+			_strToBuf.Add(bufferEntry.Name, &bufferEntry);
 		}
 
 		/**
@@ -196,7 +172,7 @@ namespace GlitchyEngine.Renderer
 			return null;
 		}
 
-		public List<BufferEntry>.Enumerator GetEnumerator()
+		public Span<BufferEntry>.Enumerator GetEnumerator()
 		{
 			return _buffers.GetEnumerator();
 		}

GlitchyEngine/src/Renderer/BufferVariable.bf

@@ -41,7 +41,7 @@ namespace GlitchyEngine.Renderer
 		public this(StringView name, ConstantBuffer constantBuffer, ShaderVariableType type, uint32 columns, uint32 rows, uint32 offset, uint32 sizeInBytes, uint32 elements, bool isUsed)
 		{
 			_name = new String(name);
-			_constantBuffer = constantBuffer..AddRef();
+			_constantBuffer = constantBuffer; // Only hold a weak reference. This variable has to die with the buffer
 			_type = type;
 			_columns = columns;
 			_rows = rows;

GlitchyEngine/src/Renderer/BufferVariableCollection.bf

@@ -0,0 +1,55 @@
+using System;
+using System.Collections;
+
+namespace GlitchyEngine.Renderer;
+
+public class BufferVariableCollection : IEnumerable<BufferVariable>
+{
+	protected bool _ownsVariables = true;
+	protected List<BufferVariable> _variables = new .();
+	protected Dictionary<String, BufferVariable> _nameToVariable = new .() ~ delete _;
+
+	public this(bool ownsVariables = true)
+	{
+		_ownsVariables = ownsVariables;
+	}
+
+	public ~this()
+	{
+		if(_ownsVariables)
+			DeleteContainerAndItems!(_variables);
+		else
+			delete _variables;
+	}
+
+	public void Add(BufferVariable ownVariable)
+	{
+		_variables.Add(ownVariable);
+		_nameToVariable.Add(ownVariable.Name, ownVariable);
+	}
+
+	public bool TryAdd(BufferVariable ownVariable)
+	{
+		if(_nameToVariable.TryAdd(ownVariable.Name, ownVariable))
+		{
+			_variables.Add(ownVariable);
+			return true;
+		}
+		else
+		{
+			return false;
+		}
+	}
+
+	public bool TryGetVariable(String name, out BufferVariable variable)
+	{
+		return _nameToVariable.TryGetValue(name, out variable);
+	}
+
+	public BufferVariable this[String name] => _nameToVariable[name];
+
+	public List<BufferVariable>.Enumerator GetEnumerator()
+	{
+		return _variables.GetEnumerator();
+	}
+}

GlitchyEngine/src/Renderer/ConstantBuffer.bf

@@ -1,63 +1,11 @@
-using DirectX.D3D11Shader;
 using System;
-using System.Collections;
 using GlitchyEngine.Math;
+using DirectX.D3D11Shader;
 
 using internal GlitchyEngine.Renderer;
 
 namespace GlitchyEngine.Renderer
 {
-	public class BufferVariableCollection : IEnumerable<BufferVariable>
-	{
-		protected bool _ownsVariables = true;
-		protected List<BufferVariable> _variables = new .();
-		protected Dictionary<String, BufferVariable> _nameToVariable = new .() ~ delete _;
-
-		public this(bool ownsVariables = true)
-		{
-			_ownsVariables = ownsVariables;
-		}
-
-		public ~this()
-		{
-			if(_ownsVariables)
-				DeleteContainerAndItems!(_variables);
-			else
-				delete _variables;
-		}
-
-		public void Add(BufferVariable ownVariable)
-		{
-			_variables.Add(ownVariable);
-			_nameToVariable.Add(ownVariable.Name, ownVariable);
-		}
-
-		public bool TryAdd(BufferVariable ownVariable)
-		{
-			if(_nameToVariable.TryAdd(ownVariable.Name, ownVariable))
-			{
-				_variables.Add(ownVariable);
-				return true;
-			}
-			else
-			{
-				return false;
-			}
-		}
-
-		public bool TryGetVariable(String name, out BufferVariable variable)
-		{
-			return _nameToVariable.TryGetValue(name, out variable);
-		}
-
-		public BufferVariable this[String name] => _nameToVariable[name];
-
-		public List<BufferVariable>.Enumerator GetEnumerator()
-		{
-			return _variables.GetEnumerator();
-		}
-	}
-
 	public class ConstantBuffer : Buffer
 	{
 		protected String _name ~ delete _;

GlitchyEngine/src/Renderer/Effect.bf

@@ -7,18 +7,6 @@ using GlitchyEngine.Content;
 
 namespace GlitchyEngine.Renderer;
 
-public enum TextureDimension
-{
-	Unknown,
-	Texture1D,
-	Texture2D,
-	Texture3D,
-	TextureCube,
-	Texture1DArray,
-	Texture2DArray,
-	TextureCubeArray
-}
-
 public class Effect : Asset
 {
 	internal VertexShader _vs ~ _?.ReleaseRef();
@@ -43,7 +31,6 @@ public class Effect : Asset
 		delete _;
 	};
 
-	protected Dictionary<String, String> _engineBuffers = new .() ~ DeleteDictionaryAndKeysAndValues!(_);
 
 	BufferCollection _bufferCollection ~ _.ReleaseRef();
 

GlitchyEngine/src/Renderer/Shader.bf

@@ -6,20 +6,6 @@ using GlitchyEngine.Content;
 
 namespace GlitchyEngine.Renderer
 {
-	public struct ShaderDefine
-	{
-		public String Name;
-		public String Definition;
-
-		public this() => this = default;
-
-		public this(String name, String definition)
-		{
-			Name = name;
-			Definition = definition;
-		}
-	}
-
 	public enum ShaderType
 	{
 		Unknown,
@@ -28,8 +14,7 @@ namespace GlitchyEngine.Renderer
 	}
 
 	// TODO: We may not even need the distinction between shader types anymore (maybe just as an enum)
-	public abstract class
-		Shader : RefCounter
+	public abstract class Shader : RefCounter
 	{
 		protected internal BufferCollection _buffers ~ _.ReleaseRef();//:append _;
 

GlitchyEngine/src/Renderer/TextureDimension.bf

@@ -0,0 +1,13 @@
+namespace GlitchyEngine.Renderer;
+
+public enum TextureDimension
+{
+	Unknown,
+	Texture1D,
+	Texture2D,
+	Texture3D,
+	TextureCube,
+	Texture1DArray,
+	Texture2DArray,
+	TextureCubeArray
+}