Added attribute name caching to BindBufferToShaderAttribute

[gepbird]

Gl.GetAttribLocation is an expensive call. It usually gets called multiple times every frame. I implemented caching in BindBufferToShaderAttribute, so it won't call Gl.GetAttribLocation every time. The reason why I only do this at GlMethods.BindBufferToShaderAttribute, is because I don't want to modifiy the OpenGL calls in Gl.cs.

[TheAIBot]

The shader program cache should be removed when Gl.DeleteProgram is called as the program id can then be reused by a new shader program.

[gepbird]

The shader program cache should be removed when Gl.DeleteProgram is called as the program id can then be reused by a new shader program.

Shader program cache gets removed when ShaderProgram.Dispose gets called. https://github.com/giawa/opengl4csharp/pull/39/commits/a4e5929d65a12acf3ed5cc9c72c3a3664a9f7a36

[gepbird]

I'm not sure about touching Gl.cs, since it's an auto generated code and most of the code calls OpenGL calls without any changes.

The reason why I only do this at GlMethods.BindBufferToShaderAttribute, is because I don't want to modifiy the OpenGL calls in Gl.cs.

Right now, only Gl.BindBufferToShaderAttribute and ShaderProgram.Dispose calls use the caching. So if someone uses the call Gl.GetAttribLocation or Gl.DeleteShader it won't use caching.

Is it a good idea to move the caching stuff to Gl.GetAttribLocation and Gl.DeleteShader, or leave it as it is now?

[giawa]

Sorry, it has been a while since I have looked at this codebase, so it might take me a while to get into it. I believe the attributes are already cached (along with uniforms, etc). This is done as part of the Shader constructor, and then ShaderProgram calls GetParams to put together a list of them for the entire ShaderProgram.

You can then access that by using the overridden getter on ShaderProgram like so:

var attribute = program["myAttributeName"];

The Location field of attribute should contain an already cached location. So you could probably do something like so:

public static void BindBufferToShaderAttribute<T>(VBO<T> buffer, ShaderProgram program, string attributeName) 
    where T : struct
{
    var cachedAttribute = program[attributeName];
    if (cachedAttribute == null) throw new ArgumentException($"{attributeName} did not exist in the shader.", nameof(attributeName));
    uint location = (uint)cachedAttribute.Location;

    Gl.EnableVertexAttribArray(location);
    Gl.BindBuffer(buffer);
    Gl.VertexAttribPointer(location, buffer.Size, buffer.PointerType, true, Marshal.SizeOf(typeof(T)), IntPtr.Zero);
}

My main concern is that the GLSL lexer may not extract 100% of the attribute names at this time. I think it would be worth spending some time on that code to ensure it catches everything. Instead of using the lexer it would be awesome to go with an approach like this: https://stackoverflow.com/questions/440144/in-opengl-is-there-a-way-to-get-a-list-of-all-uniforms-attribs-used-by-a-shade

[gepbird]

All right, I removed the attribNamesCache and replaced it with ShaderProgram's ProgramParam cache. https://github.com/giawa/opengl4csharp/pull/39/commits/98a90ffe4de48746a9d6e24bcf478b51232b0224

My main concern is that the GLSL lexer may not extract 100% of the attribute names at this time. I think it would be worth spending some time on that code to ensure it catches everything. Instead of using the lexer it would be awesome to go with an approach like this: https://stackoverflow.com/questions/440144/in-opengl-is-there-a-way-to-get-a-list-of-all-uniforms-attribs-used-by-a-shade

When the ShaderProgram gets constructed, GetParams will be called. It uses the same concept as you linked. The ShaderProgram already has the attribute names, so we don't need to get the names again, we can refer to the cache.

[giawa]

Looks good! Thanks for the contribution