flyingdevelopmentstudio/MonoGame — pull request #14 by rgba8 (2014-12-09 12:43:21+00:00)
Title: DO NOT MERGE, this is for discussion only.

Fix : ConstantBuffer cache flawed design
These 3 changes address the same issue : uploading a contant buffer of a previous shader to the current shader.

MojoShader groups uniforms per shader (vs/ps) typed constant buffers. A vs or a ps can have 3 distincts contant buffers : vs/ps _uniform_vec4, vs/ps_uniform_ivec4, vs/ps_uniform_bool.

In our shaders we only use vs_uniform_vec4 and ps_uniform_vec4,

Use case where it breaks :
Shader A : vs_uniform_vec4, ps_uniform_vec4
Shader B : vs_uniform_vec4 (no ps uniforms)

Bind and draw with Shader A : we'll upload vs and ps constant buffers.
Bind and draw with Shader B : we'll upload vs constant buffer + the previous ps constant buffer remaining in the cache => MG will try to find the location of the ps_uniform_vec4, triggering unwanted GL.GetUniformLocation each time since it meets another flaw (ConstantBuffer.Opengl.cs don't store failure and try to find the location even if it has previously failed for the same shader and uniform).

We can fix this by :
1) Clear the constant buffer cache after each draw call (GraphicsDevice.OpenGL.cs)
2) Unflag _valid constant buffers when they are uploaded and flag them _valid when set to the collection even if they're equals (We can think it'll trigger extra reupload but since the _valid is never unflaged, they're already always reuploaded).
3) Store when we failed to GL.GetUniformLocation for a shader, so that we don't try again.

I think the best is 2)

PS : found another issue while writing this, the cbuffer cache is marking dirty each contant buffer.
Shader C : vs_uniform_vec4, ps_uniform_vec4
a) Apply Vertex Constant buffers : comparing vs_uniform_vec4 against static _lastContantBufferApplied, apply if changed + storing this cbuffer in _lastConstantBufferApplied.
b) Apply Pixel Constant buffers : comparing ps_uniform_vec4 against static _lastContantBufferApplied (not working)
c) Draw.

It should be a per uniform location, cache