[core] Improve attribute binding API

cmake/core-files.cmake

@@ -56,6 +56,7 @@ set(MBGL_CORE_FILES
 
     # gl
     include/mbgl/gl/gl.hpp
+    src/mbgl/gl/attribute.hpp
     src/mbgl/gl/context.cpp
     src/mbgl/gl/context.hpp
     src/mbgl/gl/debugging.cpp

include/mbgl/gl/gl.hpp

@@ -1,9 +1,6 @@
 #pragma once
 
-#include <mbgl/gl/types.hpp>
-
 #include <stdexcept>
-#include <type_traits>
 #include <limits>
 
 #if __APPLE__
@@ -46,34 +43,5 @@ void checkError(const char *cmd, const char *file, int line);
 #define MBGL_CHECK_ERROR(cmd) (cmd)
 #endif
 
-template <typename T> struct AttributeType;
-
-template <> struct AttributeType<int8_t>   : std::integral_constant<GLenum, GL_BYTE> {};
-template <> struct AttributeType<uint8_t>  : std::integral_constant<GLenum, GL_UNSIGNED_BYTE> {};
-template <> struct AttributeType<int16_t>  : std::integral_constant<GLenum, GL_SHORT> {};
-template <> struct AttributeType<uint16_t> : std::integral_constant<GLenum, GL_UNSIGNED_SHORT> {};
-template <> struct AttributeType<int32_t>  : std::integral_constant<GLenum, GL_INT> {};
-template <> struct AttributeType<uint32_t> : std::integral_constant<GLenum, GL_UNSIGNED_INT> {};
-template <> struct AttributeType<float>    : std::integral_constant<GLenum, GL_FLOAT> {};
-
-template <std::size_t memberOffset, class V, class E, std::size_t N>
-void bindVertexAttribute(AttributeLocation location, const E (V::*)[N], const int8_t* offset) {
-    static_assert(std::is_standard_layout<V>::value, "vertex type must use standard layout");
-    static_assert(memberOffset % 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(V) <= std::numeric_limits<GLsizei>::max(), "vertex type is too big");
-    MBGL_CHECK_ERROR(glEnableVertexAttribArray(location));
-    MBGL_CHECK_ERROR(glVertexAttribPointer(location,
-                                           static_cast<GLint>(N),
-                                           AttributeType<E>::value,
-                                           false,
-                                           static_cast<GLsizei>(sizeof(V)),
-                                           offset + memberOffset));
-}
-
-// This has to be a macro because it uses the offsetof macro, which is the only legal way to get a member offset.
-#define MBGL_BIND_VERTEX_ATTRIBUTE(VertexType, member, offset) \
-    ::mbgl::gl::bindVertexAttribute<offsetof(VertexType, member)>(gl::Shader::member, &VertexType::member, offset)
-
 } // namespace gl
 } // namespace mbgl

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

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);

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 @@ class Context : private util::noncopyable {
                      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 @@ class Context : private util::noncopyable {
     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;

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

src/mbgl/gl/shader.hpp

@@ -18,6 +18,7 @@ class Shader : private util::noncopyable {
         return program.get();
     }
 
+    AttributeLocation getAttributeLocation(const char* uniform) const;
     UniformLocation getUniformLocation(const char* uniform) const;
 
     enum Defines : bool {
@@ -32,13 +33,6 @@ class Shader : private util::noncopyable {
            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);
 

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

src/mbgl/gl/vao.hpp

@@ -24,7 +24,7 @@ class VertexArrayObject : public util::noncopyable {
         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 @@ class VertexArrayObject : public util::noncopyable {
         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);
             }

src/mbgl/shader/circle_shader.cpp

@@ -12,9 +12,4 @@ CircleShader::CircleShader(gl::Context& context, Defines defines)
              context, defines) {
 }
 
-void CircleShader::bind(const gl::VertexBuffer<CircleVertex>&,
-                        const int8_t* offset) {
-    CircleVertex::bind(offset);
-}
-
 } // namespace mbgl

src/mbgl/shader/circle_shader.hpp

@@ -1,23 +1,21 @@
 #pragma once
 
 #include <mbgl/gl/shader.hpp>
+#include <mbgl/gl/attribute.hpp>
 #include <mbgl/gl/uniform.hpp>
 #include <mbgl/util/color.hpp>
 
 namespace mbgl {
 
-namespace gl {
-template <class> class VertexBuffer;
-} // namespace gl
-
 class CircleVertex;
 
 class CircleShader : public gl::Shader {
 public:
     CircleShader(gl::Context&, Defines defines = None);
 
-    void bind(const gl::VertexBuffer<CircleVertex>&,
-              const int8_t* offset);
+    using VertexType = CircleVertex;
+
+    gl::Attribute<int16_t, 2> a_pos = {"a_pos", *this};
 
     gl::UniformMatrix<4>              u_matrix           = {"u_matrix",           *this};
     gl::Uniform<std::array<float, 2>> u_extrude_scale    = {"u_extrude_scale",    *this};

src/mbgl/shader/circle_vertex.cpp

@@ -1,13 +1,7 @@
 #include <mbgl/shader/circle_vertex.hpp>
-#include <mbgl/gl/shader.hpp>
-#include <mbgl/gl/gl.hpp>
 
 namespace mbgl {
 
-void CircleVertex::bind(const int8_t* offset) {
-    static_assert(sizeof(CircleVertex) == 4, "expected CircleVertex size");
-
-    MBGL_BIND_VERTEX_ATTRIBUTE(CircleVertex, a_pos, offset);
-}
+static_assert(sizeof(CircleVertex) == 4, "expected CircleVertex size");
 
 } // namespace mbgl

src/mbgl/shader/circle_vertex.hpp

@@ -1,5 +1,8 @@
 #pragma once
 
+#include <mbgl/gl/attribute.hpp>
+
+#include <array>
 #include <cstdint>
 
 namespace mbgl {
@@ -19,8 +22,18 @@ class CircleVertex {
         } {}
 
     const int16_t a_pos[2];
+};
+
+namespace gl {
 
-    static void bind(const int8_t* offset);
+template <class Shader>
+struct AttributeBindings<Shader, CircleVertex> {
+    std::array<AttributeBinding, 1> operator()(const Shader& shader) {
+        return {{
+            MBGL_MAKE_ATTRIBUTE_BINDING(CircleVertex, shader, a_pos)
+        }};
+    };
 };
 
+} // namespace gl
 } // namespace mbgl

src/mbgl/shader/collision_box_shader.cpp

@@ -12,9 +12,4 @@ CollisionBoxShader::CollisionBoxShader(gl::Context& context)
              context) {
 }
 
-void CollisionBoxShader::bind(const gl::VertexBuffer<CollisionBoxVertex>&,
-                              const int8_t* offset) {
-    CollisionBoxVertex::bind(offset);
-}
-
 } // namespace mbgl

src/mbgl/shader/collision_box_shader.hpp

@@ -1,22 +1,22 @@
 #pragma once
 
 #include <mbgl/gl/shader.hpp>
+#include <mbgl/gl/attribute.hpp>
 #include <mbgl/gl/uniform.hpp>
 
 namespace mbgl {
 
-namespace gl {
-template <class> class VertexBuffer;
-} // namespace gl
-
 class CollisionBoxVertex;
 
 class CollisionBoxShader : public gl::Shader {
 public:
     CollisionBoxShader(gl::Context&);
 
-    void bind(const gl::VertexBuffer<CollisionBoxVertex>&,
-              const int8_t* offset);
+    using VertexType = CollisionBoxVertex;
+
+    gl::Attribute<int16_t, 2> a_pos     = {"a_pos",     *this};
+    gl::Attribute<int16_t, 2> a_extrude = {"a_extrude", *this};
+    gl::Attribute<uint8_t, 2> a_data    = {"a_data",    *this};
 
     gl::UniformMatrix<4> u_matrix  = {"u_matrix",  *this};
     gl::Uniform<float>   u_scale   = {"u_scale",   *this};

src/mbgl/shader/collision_box_vertex.cpp

@@ -1,15 +1,7 @@
 #include <mbgl/shader/collision_box_vertex.hpp>
-#include <mbgl/gl/shader.hpp>
-#include <mbgl/gl/gl.hpp>
 
 namespace mbgl {
 
-void CollisionBoxVertex::bind(const int8_t* offset) {
-    static_assert(sizeof(CollisionBoxVertex) == 10, "expected CollisionBoxVertex size");
-
-    MBGL_BIND_VERTEX_ATTRIBUTE(CollisionBoxVertex, a_pos, offset);
-    MBGL_BIND_VERTEX_ATTRIBUTE(CollisionBoxVertex, a_extrude, offset);
-    MBGL_BIND_VERTEX_ATTRIBUTE(CollisionBoxVertex, a_data, offset);
-}
+static_assert(sizeof(CollisionBoxVertex) == 10, "expected CollisionBoxVertex size");
 
 } // namespace mbgl