Migrating to Factory/Wrapper pattern - Program complete

Diamond/GLObject.cs

@@ -11,11 +11,13 @@ namespace Diamond
     public abstract class GLObject : IDisposable
     {
         private static readonly Logger Logger = LogManager.GetCurrentClassLogger();
+
         private bool _disposed;
 
-        public abstract int Id { get; }
+        public string Name { get; protected set; } = "GLObject";
 
-        protected abstract GLWrapper Wrapper { get; }
+        internal abstract GLWrapper Wrapper { get; }
+        public int Id => Wrapper.Id;
 
         protected virtual void Dispose(bool disposing)
         {
@@ -25,7 +27,7 @@ namespace Diamond
             if (disposing)
             {
                 if (GraphicsContext.CurrentContext == null)
-                    Logger.Warn("No graphics context, cannot dispose GLObject: {0}", Wrapper);
+                    Logger.Error("No graphics context, cannot delete {0}", this);
                 else
                     Wrapper.Dispose();
             }

Diamond/GLWrapper.cs

@@ -7,12 +7,12 @@ namespace Diamond
 {
     internal abstract class GLWrapper : IDisposable
     {
-        protected static Logger Logger = LogManager.GetCurrentClassLogger();
-
         public int Id { get; protected set; }
 
         public override string ToString() => $"{GetType().Name} {Id}";
 
+        public static explicit operator int(GLWrapper o) => o.Id;
+
         #region IDisposable
 
         public abstract void GLDelete();
@@ -24,18 +24,26 @@ namespace Diamond
             if (_disposed)
                 return;
 
+            // no managed resources to dispose
+
             GLDelete();
 
+            Id = 0;
+
             _disposed = true;
         }
 
         public void Dispose()
         {
             Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+
+        ~GLWrapper()
+        {
+            Dispose(false);
         }
 
         #endregion
-
-        public static explicit operator int(GLWrapper o) => o.Id;
     }
 }

Diamond/Level/Level.cs

@@ -71,7 +71,8 @@ namespace Diamond.Level
             var texturePaths = levelData["textures"]
                 .Select(path => (string) path)
                 .ToArray();
-            var textures = texturePaths.Select(path => Texture.FromBitmap(new Bitmap(Path.Combine(dir, path)))).ToArray();
+            var textures = texturePaths.Select(path => Texture.FromBitmap(new Bitmap(Path.Combine(dir, path))))
+                .ToArray();
             var textureMap = texturePaths.Select((path, i) => new {path = path, i = i})
                 .ToDictionary(v => v.path, v => v.i);
 
@@ -125,9 +126,17 @@ namespace Diamond.Level
             {
                 var pgm = tileGroup.Program;
                 pgm.Use();
-                GL.Uniform1(pgm.GetUniform("tex"), tileGroup.Texture);
+
+                var loc = pgm.UniformLocation("tex");
+
+                if (!loc.HasValue)
+                    continue;
+
+                GL.Uniform1((int)loc, tileGroup.Texture);
+
                 pgm.SetAttribPointers(_vertexBuffer);
                 pgm.SetAttribPointers(_tileBuffer);
+
                 tileGroup.Mesh.DrawInstanced(tileGroup.Tiles);
             }
         }

Diamond/Shaders/Program.cs

@@ -1,103 +1,82 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
+using System.Linq;
 using System.Text;
 using Diamond.Buffers;
+using NLog;
 using OpenTK.Graphics.OpenGL4;
 
 namespace Diamond.Shaders
 {
-    public class Program : GLWrapper
+    internal class ProgramWrapper : GLWrapper
     {
+        internal ProgramWrapper()
+        {
+            Id = GL.CreateProgram();
+        }
+
+        public int ActiveUniforms => Get(GetProgramParameterName.ActiveUniforms);
+        public int ActiveAttributes => Get(GetProgramParameterName.ActiveAttributes);
+        public bool Linked => Get(GetProgramParameterName.LinkStatus) != 0;
+
+        public int Get(GetProgramParameterName parameter)
+        {
+            GL.GetProgram(Id, parameter, out int res);
+            return res;
+        }
+
+        public void Link() => GL.LinkProgram(Id);
+        public string InfoLog => GL.GetProgramInfoLog(Id).Trim();
+
+        public void Attach(ShaderWrapper shader) => GL.AttachShader(Id, shader.Id);
+
+        public void Use() => GL.UseProgram(Id);
+
+        public string UniformName(int location)
+        {
+            var sb = new StringBuilder(64);
+            GL.GetActiveUniformName(Id, location, sb.Capacity, out int length, sb);
+            return sb.ToString();
+        }
+
+        public string AttributeName(int location)
+        {
+            var sb = new StringBuilder(64);
+            GL.GetActiveAttrib(Id, location, sb.Capacity, out int length, out int size, out ActiveAttribType type, sb);
+            return sb.ToString();
+        }
+
+        public override void GLDelete() => GL.DeleteProgram(Id);
+    }
+
+    public class Program : GLObject
+    {
+        private static readonly Logger Logger = LogManager.GetCurrentClassLogger();
+
+        internal ProgramWrapper _program;
+        internal override GLWrapper Wrapper => _program;
+
         public static Program Current { get; private set; }
 
         private readonly Dictionary<string, int> _uniforms = new Dictionary<string, int>();
         private readonly Dictionary<string, int> _attributes = new Dictionary<string, int>();
 
-        public string InfoLog => GL.GetProgramInfoLog(Id).Trim();
+        private Program(ProgramWrapper program, string name)
-
-        public Program()
         {
-            Id = GL.CreateProgram();
+            _program = program;
+            Name = name;
         }
 
-        protected override void Delete()
+        public int? UniformLocation(string name)
         {
-            GL.DeleteProgram(Id);
+            if (_uniforms.ContainsKey(name)) return _uniforms[name];
+            return null;
         }
 
-        public void Attach(Shader shader)
+        public int? AttributeLocation(string name)
         {
-            GL.AttachShader(Id, shader.Id);
+            if (_attributes.ContainsKey(name)) return _attributes[name];
-        }
+            return null;
-
-        public bool Linked
-        {
-            get
-            {
-                GL.GetProgram(Id, GetProgramParameterName.LinkStatus, out int success);
-                return success != 0;
-            }
-        }
-
-        public bool Link()
-        {
-            _uniforms.Clear();
-            _attributes.Clear();
-
-            GL.LinkProgram(Id);
-
-            if (!Linked)
-            {
-                Logger.Warn("Failed to link Program {0}", Id);
-                Logger.Debug("Program {0} InfoLog\n{1}", Id, InfoLog);
-                return false;
-            }
-
-            Logger.Info("Successfully linked Program {0}", Id);
-
-            GL.GetProgram(Id, GetProgramParameterName.ActiveUniforms, out int uniformcount);
-            for (var i = 0; i < uniformcount; i++)
-            {
-                var sb = new StringBuilder(256);
-                GL.GetActiveUniformName(Id, i, sb.Capacity, out int length, sb);
-                _uniforms[sb.ToString()] = i;
-            }
-
-            GL.GetProgram(Id, GetProgramParameterName.ActiveAttributes, out int attributecount);
-            for (var i = 0; i < attributecount; i++)
-            {
-                var sb = new StringBuilder(256);
-                GL.GetActiveAttrib(Id, i, sb.Capacity, out int length, out int size,
-                    out ActiveAttribType type, sb);
-                _attributes[sb.ToString()] = i;
-            }
-
-            return true;
-        }
-
-        public bool TryGetUniform(string name, out int id)
-        {
-            return _uniforms.TryGetValue(name, out id);
-        }
-
-        public int GetUniform(string name)
-        {
-            if (TryGetUniform(name, out int id)) return id;
-
-            Logger.Warn("Attempted to access uniform {0} on Program {1}", name, Id);
-            throw new ShaderException($"Shader Program {Id} does not contain uniform '{name}'");
-        }
-
-        public bool TryGetAttribute(string name, out int id)
-        {
-            return _attributes.TryGetValue(name, out id);
-        }
-
-        public int GetAttribute(string name)
-        {
-            if (TryGetAttribute(name, out int id)) return id;
-
-            Logger.Warn("Attempted to access attribute {0} on Program {1}", name, Id);
-            throw new ShaderException($"Shader Program {Id} does not contain id '{name}'");
         }
 
         public void SetAttribPointers<T>(GLBuffer<T> buff) where T : struct
@@ -107,9 +86,9 @@ namespace Diamond.Shaders
             buff.Bind();
             foreach (var attr in vdi.Pointers)
             {
-                if (!TryGetAttribute(attr.Name, out int loc)) continue;
+                var loc = AttributeLocation(attr.Name);
-
+                if (loc.HasValue)
-                GL.VertexAttribPointer(loc, attr.Size, attr.Type, attr.Normalized, vdi.Stride, attr.Offset);
+                    GL.VertexAttribPointer((int) loc, attr.Size, attr.Type, attr.Normalized, vdi.Stride, attr.Offset);
             }
         }
 
@@ -119,39 +98,86 @@ namespace Diamond.Shaders
             Current = this;
         }
 
+        //? Could create static Program instance which wraps the default shader
+        // ie Shader.Default.Use()
+        // would also allow sending arrays to the default attribs like gl_Vertex etc.
         public static void UseDefault()
         {
             GL.UseProgram(0);
             Current = null;
         }
 
-        public static Program FromShaders(params Shader[] shaders)
+        private bool Link()
         {
-            var p = new Program();
+            _uniforms.Clear();
+            _attributes.Clear();
 
-            foreach (var shader in shaders)
+            _program.Link();
+
+            if (!_program.Linked)
+                return false;
+
+            for (var i = 0; i < _program.ActiveUniforms; i++)
+                _uniforms[_program.UniformName(i)] = i;
+
+            for (var i = 0; i < _program.ActiveAttributes; i++)
+                _attributes[_program.AttributeName(i)] = i;
+
+            return true;
+        }
+
+        public override string ToString() => $"\'{Name}\' ({Id})";
+
+        #region Factory Methods
+
+        public static Program FromShaders(string name, params Shader[] shaders) => FromShaders(name,
+            (IEnumerable<Shader>) shaders);
+
+        public static Program FromShaders(string name, IEnumerable<Shader>shaders)
+        {
+            if (shaders == null)
             {
-                p.Attach(shader);
+                Logger.Error("Cannot create a program with no shaders.");
+                return null;
             }
 
-            p.Link();
+            var wrapper = new ProgramWrapper();
+            var service = new Program(wrapper, name);
 
-            return p;
+            Logger.Debug("Created {0}", service);
+
+            foreach (var shader in shaders)
+                wrapper.Attach((ShaderWrapper) shader.Wrapper);
+
+            service.Link();
+
+            if (!wrapper.Linked)
+            {
+                Logger.Warn("Failed to link {0}", service);
+                Logger.Debug("InfoLog for {0}", service);
+                wrapper.Dispose();
+                return null;
+            }
+
+            Logger.Debug("Successfully linked {0}", service);
+
+            return service;
         }
 
-        public static Program FromFiles(params string[] files)
+
+        public static Program FromShaders(params Shader[] shaders) => FromShaders((IEnumerable<Shader>) shaders);
+
+        public static Program FromShaders(IEnumerable<Shader> shaders)
         {
-            var shaders = new Shader[files.Length];
+            var shaderList = shaders.ToList(); // prevent multiple enumeration
-
+            var shaderNames = shaderList.Select(s => s.Name);
-            for (var i = 0; i < files.Length; i++)
+            string name = $"[{string.Join(", ", shaderNames)}]";
-                shaders[i] = Shader.FromFile(files[i]);
+            return FromShaders(name, shaderList);
-
-            var p = FromShaders(shaders);
-
-            for (var i = 0; i < shaders.Length; i++)
-                shaders[i]?.Dispose();
-
-            return p;
         }
+
+        public static Program FromFiles(string name, params string[] paths) => FromShaders(name, paths.Select(Shader.FromFile));
+        public static Program FromFiles(params string[] paths) => FromShaders(paths.Select(Shader.FromFile));
+
+        #endregion
     }
 }

Diamond/Shaders/Shader.cs

@@ -9,6 +9,12 @@ namespace Diamond.Shaders
 {
     internal sealed class ShaderWrapper : GLWrapper
     {
+        internal ShaderWrapper(ShaderType shaderType)
+        {
+            Id = GL.CreateShader(shaderType);
+            ShaderType = shaderType;
+        }
+
         public readonly ShaderType ShaderType;
 
         public string Source
@@ -33,21 +39,9 @@ namespace Diamond.Shaders
 
         public string InfoLog => GL.GetShaderInfoLog(Id).Trim();
 
-        internal ShaderWrapper(ShaderType shaderType)
+        public void Compile() => GL.CompileShader(Id);
-        {
-            Id = GL.CreateShader(shaderType);
-            ShaderType = shaderType;
-        }
 
-        public override void GLDelete()
+        public override void GLDelete() => GL.DeleteShader(Id);
-        {
-            GL.DeleteShader(Id);
-        }
-
-        public void Compile()
-        {
-            GL.CompileShader(Id);
-        }
     }
 
     public class Shader : GLObject
@@ -55,40 +49,52 @@ namespace Diamond.Shaders
         private static readonly Logger Logger = LogManager.GetCurrentClassLogger();
 
         private readonly ShaderWrapper _shader;
+        internal override GLWrapper Wrapper => _shader;
 
-        public override int Id => _shader.Id;
-        protected override GLWrapper Wrapper => _shader;
-
-        public string Name { get; }
         public string Source { get; }
+        public ShaderType Type { get; }
 
-        private Shader(ShaderWrapper shader, string source, string name)
+        private Shader(ShaderWrapper shader, string source, ShaderType type, string name)
         {
             _shader = shader;
-            Name = name;
             Source = source;
+            Type = type;
+            Name = name;
         }
 
+        public override string ToString() => $"{Type} \'{Name}\' ({Id})";
+
         #region Factory Methods
 
+        private static readonly Dictionary<string, ShaderType> _extensions = new Dictionary<string, ShaderType>
+        {
+            [".vs"] = ShaderType.VertexShader,
+            [".vert"] = ShaderType.VertexShader,
+            [".fs"] = ShaderType.VertexShader,
+            [".frag"] = ShaderType.VertexShader,
+        };
+
         public static Shader FromSource(string source, ShaderType type, string name = "Shader")
         {
             var wrapper = new ShaderWrapper(type);
-            Logger.Debug("Created {0} \'{1}\' {2}", type, name, wrapper.Id);
+            var service = new Shader(wrapper, source, type, name);
+
+            Logger.Debug("Created {0}", service);
 
             wrapper.Source = source;
             wrapper.Compile();
 
             if (!wrapper.Compiled)
             {
-                Logger.Warn("Failed to compile {0} \'{1}\' {2}", type, name, wrapper.Id);
+                Logger.Warn("Failed to compile {0}", service);
-                Logger.Debug("InfoLog for {0} \'{1}\' {2}", type, name, wrapper.Id);
+                Logger.Debug("InfoLog for {0}", service);
                 wrapper.Dispose();
                 return null;
             }
 
-            Logger.Debug("Successfully compiled {0} \'{1}\' {2}", type, name, wrapper.Id);
+            Logger.Debug("Successfully compiled {0}", service);
-            return new Shader(wrapper, source, name);
+
+            return service;
         }
 
         public static Shader FromFile(string path, ShaderType type)
@@ -114,25 +120,18 @@ namespace Diamond.Shaders
             var ext = Path.GetExtension(path);
             var name = Path.GetFileNameWithoutExtension(path);
 
-            var extensions = new Dictionary<string, ShaderType>
-            {
-                [".vs"] = ShaderType.VertexShader,
-                [".vert"] = ShaderType.VertexShader,
-                [".fs"] = ShaderType.VertexShader,
-                [".frag"] = ShaderType.VertexShader,
-            };
 
             if (ext != null)
-                if (!extensions.ContainsKey(ext))
+                if (!_extensions.ContainsKey(ext))
                     ext = Path.GetExtension(name);
 
-            if (ext == null || !extensions.ContainsKey(ext))
+            if (ext == null || !_extensions.ContainsKey(ext))
             {
                 Logger.Warn("Could not infer shader type from glsl file name {0}", path);
                 return null;
             }
 
-            var type = extensions[ext];
+            var type = _extensions[ext];
             return FromFile(path, type);
         }