Only call glGetUniformLocation/glGetAttribLocation once (#2970)

These values never change after a program is linked, so it is safe to
cache the uniform and attribute locations. In addition to the obvious
benefit (fewer function calls), calling glGet-type functions inhibits
OpenGL driver optimizations: if there is a mixture of setting/getting
commands, drivers cannot do batch optimizations where they accumulate
a bunch of commands and process them all of them at once, since it is
possible one command could affect the output of a glGet-type function,
whose results are needed immediately.

src/video/gl/gl33core_context.cpp

@@ -68,12 +68,12 @@ GL33CoreContext::bind()
   if (back_renderer->is_rendering() || !back_renderer->get_texture())
   {
     texture = m_black_texture.get();
-    glUniform1f(m_program->get_uniform_location("backbuffer"), 0.0f);
+    glUniform1f(m_program->get_backbuffer_location(), 0.0f);
   }
   else
   {
     texture = static_cast<GLTexture*>(back_renderer->get_texture().get());
-    glUniform1f(m_program->get_uniform_location("backbuffer"), 1.0f);
+    glUniform1f(m_program->get_backbuffer_location(), 1.0f);
   }
 
   glActiveTexture(GL_TEXTURE2);
@@ -99,14 +99,14 @@ GL33CoreContext::bind()
     0.0, sy, 0,
     tx, ty, 1.0,
   };
-  glUniformMatrix3fv(m_program->get_uniform_location("fragcoord2uv"),
+  glUniformMatrix3fv(m_program->get_fragcoord2uv_location(),
                      1, false, matrix);
 
-  glUniform1i(m_program->get_uniform_location("diffuse_texture"), 0);
-  glUniform1i(m_program->get_uniform_location("displacement_texture"), 1);
-  glUniform1i(m_program->get_uniform_location("framebuffer_texture"), 2);
+  glUniform1i(m_program->get_diffuse_texture_location(), 0);
+  glUniform1i(m_program->get_displacement_texture_location(), 1);
+  glUniform1i(m_program->get_framebuffer_texture_location(), 2);
 
-  glUniform1f(m_program->get_uniform_location("game_time"), g_game_time);
+  glUniform1f(m_program->get_game_time_location(), g_game_time);
 
   assert_gl();
 }
@@ -128,7 +128,7 @@ GL33CoreContext::ortho(float width, float height, bool vflip)
     0, 0, 1
   };
 
-  const GLint mvp_loc = m_program->get_uniform_location("modelviewprojection");
+  const GLint mvp_loc = m_program->get_modelviewprojection_location();
   glUniformMatrix3fv(mvp_loc, 1, false, mvp_matrix);
 
   assert_gl();
@@ -196,7 +196,7 @@ GL33CoreContext::bind_texture(const Texture& texture, const Texture* displacemen
     animate.x /= static_cast<float>(texture.get_image_width());
     animate.y /= static_cast<float>(texture.get_image_height());
 
-    glUniform2f(m_program->get_uniform_location("animate"), animate.x, animate.y);
+    glUniform2f(m_program->get_animate_location(), animate.x, animate.y);
   }
 
   if (displacement_texture)
@@ -209,7 +209,7 @@ GL33CoreContext::bind_texture(const Texture& texture, const Texture* displacemen
     animate.x /= static_cast<float>(displacement_texture->get_image_width());
     animate.y /= static_cast<float>(displacement_texture->get_image_height());
 
-    glUniform2f(m_program->get_uniform_location("displacement_animate"), animate.x, animate.y);
+    glUniform2f(m_program->get_displacement_animate_location(), animate.x, animate.y);
   }
   else
   {

src/video/gl/gl_program.cpp

@@ -18,13 +18,24 @@
 
 #include <sstream>
 
-#include "util/log.hpp"
 #include "video/glutil.hpp"
 
 GLProgram::GLProgram() :
   m_program(glCreateProgram()),
   m_frag_shader(),
-  m_vert_shader()
+  m_vert_shader(),
+  m_backbuffer_location(-1),
+  m_fragcoord2uv_location(-1),
+  m_diffuse_texture_location(-1),
+  m_displacement_texture_location(-1),
+  m_framebuffer_texture_location(-1),
+  m_game_time_location(-1),
+  m_modelviewprojection_location(-1),
+  m_animate_location(-1),
+  m_displacement_animate_location(-1),
+  m_position_location(-1),
+  m_texcoord_location(-1),
+  m_diffuse_location(-1)
 {
   assert_gl();
 
@@ -49,6 +60,21 @@ GLProgram::GLProgram() :
     throw std::runtime_error(out.str());
   }
 
+  // Any uniform/attribute not in m_program will be given a value of -1,
+  // and an error will be reported if code attempts to use it
+  m_backbuffer_location = glGetUniformLocation(m_program, "backbuffer");
+  m_fragcoord2uv_location = glGetUniformLocation(m_program, "fragcoord2uv");
+  m_diffuse_texture_location = glGetUniformLocation(m_program, "diffuse_texture");
+  m_displacement_texture_location = glGetUniformLocation(m_program, "displacement_texture");
+  m_framebuffer_texture_location = glGetUniformLocation(m_program, "framebuffer_texture");
+  m_game_time_location = glGetUniformLocation(m_program, "game_time");
+  m_modelviewprojection_location = glGetUniformLocation(m_program, "modelviewprojection");
+  m_animate_location = glGetUniformLocation(m_program, "animate");
+  m_displacement_animate_location = glGetUniformLocation(m_program, "displacement_animate");
+  m_position_location = glGetAttribLocation(m_program, "position");
+  m_texcoord_location = glGetAttribLocation(m_program, "texcoord");
+  m_diffuse_location = glGetAttribLocation(m_program, "diffuse");
+
   assert_gl();
 }
 
@@ -86,34 +112,14 @@ GLProgram::validate()
 }
 
 GLint
-GLProgram::get_attrib_location(const char* name) const
+GLProgram::check_valid(GLint loc, const char* name)
 {
-  assert_gl();
-
-  GLint loc = glGetAttribLocation(m_program, name);
   if (loc == -1)
   {
-    log_debug << "GLProgram::get_attrib_location(\"" << name << "\") failed" << std::endl;
-  }
-
-  assert_gl();
-
-  return loc;
-}
-
-GLint
-GLProgram::get_uniform_location(const char* name) const
-{
-  assert_gl();
-
-  GLint loc = glGetUniformLocation(m_program, name);
-  if (loc == -1)
-  {
-    log_debug << "GLProgram::get_uniform_location(\"" << name << "\") failed" << std::endl;
+    std::ostringstream out;
+    out << "Getting uniform or attribute location for \"" << name << "\" failed" << std::endl;
+    throw std::runtime_error(out.str());
   }
-
-  assert_gl();
-
   return loc;
 }
 

src/video/gl/gl_program.hpp

@@ -34,20 +34,44 @@ class GLProgram final
 
   GLuint get_handle() const { return m_program; }
 
-  GLint get_attrib_location(const char* name) const;
-  GLint get_uniform_location(const char* name) const;
+  GLint get_backbuffer_location() const { return check_valid(m_backbuffer_location, "backbuffer"); }
+  GLint get_fragcoord2uv_location() const { return check_valid(m_fragcoord2uv_location, "fragcoord2uv"); }
+  GLint get_diffuse_texture_location() const { return check_valid(m_diffuse_texture_location, "diffuse_texture"); }
+  GLint get_displacement_texture_location() const { return check_valid(m_displacement_texture_location, "displacement_texture"); }
+  GLint get_framebuffer_texture_location() const { return check_valid(m_framebuffer_texture_location, "framebuffer_texture"); }
+  GLint get_game_time_location() const { return check_valid(m_game_time_location, "game_time"); }
+  GLint get_modelviewprojection_location() const { return check_valid(m_modelviewprojection_location, "modelviewprojection"); }
+  GLint get_animate_location() const { return check_valid(m_animate_location, "animate"); }
+  GLint get_displacement_animate_location() const { return check_valid(m_displacement_animate_location, "displacement_animate"); }
+  GLint get_position_location() const { return check_valid(m_position_location, "position"); }
+  GLint get_texcoord_location() const { return check_valid(m_texcoord_location, "texcoord"); }
+  GLint get_diffuse_location() const { return check_valid(m_diffuse_location, "diffuse"); }
 
 private:
   bool get_link_status() const;
   bool get_validate_status() const;
   std::string get_info_log() const;
+  static GLint check_valid(GLint location, const char* name);
 
 private:
   GLuint m_program;
 
   std::unique_ptr<GLShader> m_frag_shader;
   std::unique_ptr<GLShader> m_vert_shader;
 
+  GLint m_backbuffer_location;
+  GLint m_fragcoord2uv_location;
+  GLint m_diffuse_texture_location;
+  GLint m_displacement_texture_location;
+  GLint m_framebuffer_texture_location;
+  GLint m_game_time_location;
+  GLint m_modelviewprojection_location;
+  GLint m_animate_location;
+  GLint m_displacement_animate_location;
+  GLint m_position_location;
+  GLint m_texcoord_location;
+  GLint m_diffuse_location;
+
 private:
   GLProgram(const GLProgram&) = delete;
   GLProgram& operator=(const GLProgram&) = delete;

src/video/gl/gl_vertex_arrays.cpp

@@ -65,7 +65,7 @@ GLVertexArrays::set_positions(const float* data, size_t size)
   glBindBuffer(GL_ARRAY_BUFFER, m_positions_buffer);
   glBufferData(GL_ARRAY_BUFFER, size, data, GL_DYNAMIC_DRAW);
 
-  int loc = m_context.get_program().get_attrib_location("position");
+  int loc = m_context.get_program().get_position_location();
   glVertexAttribPointer(loc, 2, GL_FLOAT, GL_FALSE, 0, nullptr);
   glEnableVertexAttribArray(loc);
 
@@ -80,7 +80,7 @@ GLVertexArrays::set_texcoords(const float* data, size_t size)
   glBindBuffer(GL_ARRAY_BUFFER, m_texcoords_buffer);
   glBufferData(GL_ARRAY_BUFFER, size, data, GL_DYNAMIC_DRAW);
 
-  int loc = m_context.get_program().get_attrib_location("texcoord");
+  int loc = m_context.get_program().get_texcoord_location();
   glVertexAttribPointer(loc, 2, GL_FLOAT, GL_FALSE, 0, nullptr);
   glEnableVertexAttribArray(loc);
 
@@ -92,7 +92,7 @@ GLVertexArrays::set_texcoord(float u, float v)
 {
   assert_gl();
 
-  int loc = m_context.get_program().get_attrib_location("texcoord");
+  int loc = m_context.get_program().get_texcoord_location();
   glVertexAttrib2f(loc, u, v);
   glDisableVertexAttribArray(loc);
 
@@ -107,7 +107,7 @@ GLVertexArrays::set_colors(const float* data, size_t size)
   glBindBuffer(GL_ARRAY_BUFFER, m_color_buffer);
   glBufferData(GL_ARRAY_BUFFER, size, data, GL_DYNAMIC_DRAW);
 
-  int loc = m_context.get_program().get_attrib_location("diffuse");
+  int loc = m_context.get_program().get_diffuse_location();
   glVertexAttribPointer(loc, 4, GL_FLOAT, GL_FALSE, 0, nullptr);
   glEnableVertexAttribArray(loc);
 
@@ -119,7 +119,7 @@ GLVertexArrays::set_color(const Color& color)
 {
   assert_gl();
 
-  int loc = m_context.get_program().get_attrib_location("diffuse");
+  int loc = m_context.get_program().get_diffuse_location();
   glVertexAttrib4f(loc, color.red, color.green, color.blue, color.alpha);
   glDisableVertexAttribArray(loc);