[core] Improve attribute binding API

7 files changed, 98 insertions, 16 deletions

diff --git a/src/mbgl/gl/attribute.hpp b/src/mbgl/gl/attribute.hpp
new file mode 100644
index 0000000000..8bc474e967
--- /dev/null
+++ b/src/mbgl/gl/attribute.hpp
@@ -0,0 +1,50 @@
+#pragma once
+
+#include <mbgl/gl/types.hpp>
+#include <mbgl/gl/shader.hpp>
+
+#include <cstddef>
+#include <limits>
+#include <vector>
+
+namespace mbgl {
+namespace gl {
+
+template <typename T, std::size_t N>
+class Attribute {
+public:
+    Attribute(const char* name, const Shader& shader)
+        : location(shader.getAttributeLocation(name)) {}
+
+    AttributeLocation location;
+};
+
+class AttributeBinding {
+public:
+    template <class Vertex, class T, std::size_t N, std::size_t O>
+    AttributeBinding(const T (Vertex::*)[N], const Attribute<T, N>& attribute, std::integral_constant<std::size_t, O>)
+        : location(attribute.location),
+          type(DataTypeOf<T>::value),
+          count(N),
+          offset(O) {
+        static_assert(std::is_standard_layout<Vertex>::value, "vertex type must use standard layout");
+        static_assert(O % 4 == 0, "vertex attribute must be optimally aligned");
+        static_assert(1 <= N && N <= 4, "count must be 1, 2, 3, or 4");
+        static_assert(sizeof(Vertex) <= std::numeric_limits<int32_t>::max(), "vertex type is too big");
+    }
+
+    AttributeLocation location;
+    DataType type;
+    uint8_t count;
+    std::size_t offset;
+};
+
+#define MBGL_MAKE_ATTRIBUTE_BINDING(Vertex, shader, name) \
+    ::mbgl::gl::AttributeBinding(&Vertex::name, \
+                                 shader.name, \
+                                 std::integral_constant<std::size_t, offsetof(Vertex, name)>())
+
+template <class Shader, class Vertex> struct AttributeBindings;
+
+} // namespace gl
+} // namespace mbgl
diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp
index 5837aa3399..2a04fcc18e 100644
--- a/src/mbgl/gl/context.cpp
+++ b/src/mbgl/gl/context.cpp
@@ -109,6 +109,16 @@ UniqueBuffer Context::createIndexBuffer(const void* data, std::size_t size) {
     return result;
 }
 
+void Context::bindAttribute(const AttributeBinding& binding, std::size_t stride, const int8_t* offset) {
+    MBGL_CHECK_ERROR(glEnableVertexAttribArray(binding.location));
+    MBGL_CHECK_ERROR(glVertexAttribPointer(binding.location,
+                                           binding.count,
+                                           static_cast<GLenum>(binding.type),
+                                           false,
+                                           static_cast<GLsizei>(stride),
+                                           offset + binding.offset));
+}
+
 UniqueTexture Context::createTexture() {
     if (pooledTextures.empty()) {
         pooledTextures.resize(TextureMax);
diff --git a/src/mbgl/gl/context.hpp b/src/mbgl/gl/context.hpp
index e26b35e7c0..6a5d44793a 100644
--- a/src/mbgl/gl/context.hpp
+++ b/src/mbgl/gl/context.hpp
@@ -6,6 +6,7 @@
 #include <mbgl/gl/texture.hpp>
 #include <mbgl/gl/vertex_buffer.hpp>
 #include <mbgl/gl/index_buffer.hpp>
+#include <mbgl/gl/attribute.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
 #include <memory>
@@ -60,6 +61,14 @@ public:
                      TextureFilter = TextureFilter::Nearest,
                      TextureMipMap = TextureMipMap::No);
 
+    template <class Shader, class Vertex>
+    void bindAttributes(const Shader& shader, const VertexBuffer<Vertex>&, const int8_t* offset) {
+        static_assert(std::is_same<typename Shader::VertexType, Vertex>::value, "vertex type mismatch");
+        for (const auto& binding : AttributeBindings<Shader, Vertex>()(shader)) {
+            bindAttribute(binding, sizeof(Vertex), offset);
+        }
+    }
+
     // Actually remove the objects we marked as abandoned with the above methods.
     // Only call this while the OpenGL context is exclusive to this thread.
     void performCleanup();
@@ -115,6 +124,7 @@ private:
     UniqueBuffer createVertexBuffer(const void* data, std::size_t size);
     UniqueBuffer createIndexBuffer(const void* data, std::size_t size);
     UniqueTexture createTexture(uint16_t width, uint16_t height, const void* data, TextureUnit);
+    void bindAttribute(const AttributeBinding&, std::size_t stride, const int8_t* offset);
 
     friend detail::ProgramDeleter;
     friend detail::ShaderDeleter;
diff --git a/src/mbgl/gl/shader.cpp b/src/mbgl/gl/shader.cpp
index 5b3712e80d..d8ee734567 100644
--- a/src/mbgl/gl/shader.cpp
+++ b/src/mbgl/gl/shader.cpp
@@ -48,13 +48,6 @@ Shader::Shader(const char* name_,
     MBGL_CHECK_ERROR(glAttachShader(program.get(), vertexShader.get()));
     MBGL_CHECK_ERROR(glAttachShader(program.get(), fragmentShader.get()));
 
-    // Bind attribute variables
-    MBGL_CHECK_ERROR(glBindAttribLocation(program.get(), a_pos, "a_pos"));
-    MBGL_CHECK_ERROR(glBindAttribLocation(program.get(), a_extrude, "a_extrude"));
-    MBGL_CHECK_ERROR(glBindAttribLocation(program.get(), a_offset, "a_offset"));
-    MBGL_CHECK_ERROR(glBindAttribLocation(program.get(), a_data, "a_data"));
-    MBGL_CHECK_ERROR(glBindAttribLocation(program.get(), a_texture_pos, "a_texture_pos"));
-
     // Link program
     GLint status;
      MBGL_CHECK_ERROR(glLinkProgram(program.get()));
@@ -112,5 +105,9 @@ UniformLocation Shader::getUniformLocation(const char* uniform) const {
     return MBGL_CHECK_ERROR(glGetUniformLocation(program.get(), uniform));
 }
 
+AttributeLocation Shader::getAttributeLocation(const char* attribute) const {
+    return MBGL_CHECK_ERROR(glGetAttribLocation(program.get(), attribute));
+}
+
 } // namespace gl
 } // namespace mbgl
diff --git a/src/mbgl/gl/shader.hpp b/src/mbgl/gl/shader.hpp
index f73400bc15..f88bd4f867 100644
--- a/src/mbgl/gl/shader.hpp
+++ b/src/mbgl/gl/shader.hpp
@@ -18,6 +18,7 @@ public:
         return program.get();
     }
 
+    AttributeLocation getAttributeLocation(const char* uniform) const;
     UniformLocation getUniformLocation(const char* uniform) const;
 
     enum Defines : bool {
@@ -32,13 +33,6 @@ protected:
            Context&,
            Defines defines = Defines::None);
 
-public:
-    static constexpr AttributeLocation         a_pos = 0;
-    static constexpr AttributeLocation     a_extrude = 1;
-    static constexpr AttributeLocation      a_offset = 2;
-    static constexpr AttributeLocation        a_data = 3;
-    static constexpr AttributeLocation a_texture_pos = 4;
-
 private:
     bool compileShader(UniqueShader&, const char *source);
 
diff --git a/src/mbgl/gl/types.hpp b/src/mbgl/gl/types.hpp
index e9d14e4807..f24674457a 100644
--- a/src/mbgl/gl/types.hpp
+++ b/src/mbgl/gl/types.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <cstdint>
+#include <type_traits>
 
 namespace mbgl {
 namespace gl {
@@ -22,6 +23,26 @@ using DepthValue = double;
 using StencilValue = int32_t;
 using StencilMaskValue = uint32_t;
 
+enum class DataType : uint32_t {
+    Byte = 0x1400,
+    UnsignedByte = 0x1401,
+    Short = 0x1402,
+    UnsignedShort = 0x1403,
+    Integer = 0x1404,
+    UnsignedInteger = 0x1405,
+    Float = 0x1406
+};
+
+template <typename T> struct DataTypeOf;
+
+template <> struct DataTypeOf<int8_t>   : std::integral_constant<DataType, DataType::Byte> {};
+template <> struct DataTypeOf<uint8_t>  : std::integral_constant<DataType, DataType::UnsignedByte> {};
+template <> struct DataTypeOf<int16_t>  : std::integral_constant<DataType, DataType::Short> {};
+template <> struct DataTypeOf<uint16_t> : std::integral_constant<DataType, DataType::UnsignedShort> {};
+template <> struct DataTypeOf<int32_t>  : std::integral_constant<DataType, DataType::Integer> {};
+template <> struct DataTypeOf<uint32_t> : std::integral_constant<DataType, DataType::UnsignedInteger> {};
+template <> struct DataTypeOf<float>    : std::integral_constant<DataType, DataType::Float> {};
+
 enum class BufferType : uint32_t {
     Vertex = 0x8892,
     Element = 0x8893
diff --git a/src/mbgl/gl/vao.hpp b/src/mbgl/gl/vao.hpp
index 3fe307ed4d..6a5e7d0e60 100644
--- a/src/mbgl/gl/vao.hpp
+++ b/src/mbgl/gl/vao.hpp
@@ -24,7 +24,7 @@ public:
         bindVertexArrayObject(context);
         if (bound_shader == 0) {
             context.vertexBuffer = vertexBuffer.buffer;
-            shader.bind(vertexBuffer, offset);
+            context.bindAttributes(shader, vertexBuffer, offset);
             if (vertexArray) {
                 storeBinding(shader, vertexBuffer.buffer, 0, offset);
             }
@@ -43,7 +43,7 @@ public:
         if (bound_shader == 0) {
             context.vertexBuffer = vertexBuffer.buffer;
             context.elementBuffer = indexBuffer.buffer;
-            shader.bind(vertexBuffer, offset);
+            context.bindAttributes(shader, vertexBuffer, offset);
             if (vertexArray) {
                 storeBinding(shader, vertexBuffer.buffer, indexBuffer.buffer, offset);
             }