[core] Use gl::Program to resolve some rough edges in the GL binding types

* Extract `ignore` util to separate header.
* `Segment` now tracks offset and length of indices, rather than primitives. This is more natural.
* Introduce `VertexVector` and `IndexVector` types. These types carry information about the intended draw mode (`Triangles`, `LineStrip`, etc.), and ensure that elements are always appended in a group size appropriate for that draw mode, for both indexed and unindexed rendering.
* `Program`, rather than `Drawable`, is now the unifying object for draw calls. `Program` is the best place to type check the draw call, because it is typed to carry information about the intended primitive, vertex type, attributes, and uniforms.
* Use the debug shaders for debug tile rendering, like gl-js.
* Fix the draw mode for background. It was drawing triangle strips with a triangles array. Surprised this didn’t cause issues. Now it’s type checked.

11 files changed, 210 insertions, 182 deletions

diff --git a/src/mbgl/gl/attribute.hpp b/src/mbgl/gl/attribute.hpp
index 126da38de5..a2b165fa21 100644
--- a/src/mbgl/gl/attribute.hpp
+++ b/src/mbgl/gl/attribute.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include <mbgl/gl/types.hpp>
+#include <mbgl/util/ignore.hpp>
 
 #include <cstddef>
 #include <functional>
@@ -157,17 +158,14 @@ private:
     template <std::size_t... Is>
     static std::function<void (std::size_t)> binder(const State& state, std::index_sequence<Is...>) {
         return [&state] (std::size_t vertexOffset) {
-            noop((bindAttribute(std::get<Is>(state).location,
-                                std::get<Is>(state).count,
-                                std::get<Is>(state).type,
-                                sizeof(Vertex),
-                                vertexOffset,
-                                Vertex::attributeOffsets[Is]), 0)...);
+            ignore((bindAttribute(std::get<Is>(state).location,
+                                  std::get<Is>(state).count,
+                                  std::get<Is>(state).type,
+                                  sizeof(Vertex),
+                                  vertexOffset,
+                                  Vertex::attributeOffsets[Is]), 0)...);
         };
     }
-
-    // This exists only to provide a varags context for unpacking the assignments in `binder`.
-    template <int...> static void noop(int...) {}
 };
 
 } // namespace gl
diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp
index b12c77c1a1..c4485ad0fd 100644
--- a/src/mbgl/gl/context.cpp
+++ b/src/mbgl/gl/context.cpp
@@ -533,9 +533,9 @@ void Context::draw(const Drawable& drawable) {
         if (drawable.indexBuffer) {
             MBGL_CHECK_ERROR(glDrawElements(
                 static_cast<GLenum>(primitiveType),
-                static_cast<GLsizei>(drawable.primitiveSize / sizeof(uint16_t) * segment.primitiveLength),
+                static_cast<GLsizei>(segment.indexLength),
                 GL_UNSIGNED_SHORT,
-                reinterpret_cast<GLvoid*>(drawable.primitiveSize * segment.primitiveOffset)));
+                reinterpret_cast<GLvoid*>(sizeof(uint16_t) * segment.indexOffset)));
         } else {
             MBGL_CHECK_ERROR(glDrawArrays(
                 static_cast<GLenum>(primitiveType),
diff --git a/src/mbgl/gl/context.hpp b/src/mbgl/gl/context.hpp
index 2a3fbea33d..154921a1b9 100644
--- a/src/mbgl/gl/context.hpp
+++ b/src/mbgl/gl/context.hpp
@@ -8,9 +8,16 @@
 #include <mbgl/gl/framebuffer.hpp>
 #include <mbgl/gl/vertex_buffer.hpp>
 #include <mbgl/gl/index_buffer.hpp>
-#include <mbgl/gl/drawable.hpp>
+#include <mbgl/gl/types.hpp>
+#include <mbgl/gl/draw_mode.hpp>
+#include <mbgl/gl/depth_mode.hpp>
+#include <mbgl/gl/stencil_mode.hpp>
+#include <mbgl/gl/color_mode.hpp>
+#include <mbgl/gl/segment.hpp>
 #include <mbgl/util/noncopyable.hpp>
 
+
+#include <functional>
 #include <memory>
 #include <vector>
 #include <array>
@@ -33,18 +40,18 @@ public:
     UniqueProgram createProgram(ShaderID vertexShader, ShaderID fragmentShader);
     UniqueTexture createTexture();
 
-    template <class V>
-    VertexBuffer<V> createVertexBuffer(std::vector<V>&& v) {
-        return VertexBuffer<V> {
-            v.size(),
-            createVertexBuffer(v.data(), v.size() * sizeof(V))
+    template <class Vertex, class DrawMode>
+    VertexBuffer<Vertex, DrawMode> createVertexBuffer(VertexVector<Vertex, DrawMode>&& v) {
+        return VertexBuffer<Vertex, DrawMode> {
+            v.vertexSize(),
+            createVertexBuffer(v.data(), v.byteSize())
         };
     }
 
-    template <class P>
-    IndexBuffer<P> createIndexBuffer(std::vector<P>&& v) {
-        return IndexBuffer<P> {
-            createIndexBuffer(v.data(), v.size() * sizeof(P))
+    template <class DrawMode>
+    IndexBuffer<DrawMode> createIndexBuffer(IndexVector<DrawMode>&& v) {
+        return IndexBuffer<DrawMode> {
+            createIndexBuffer(v.data(), v.byteSize())
         };
     }
 
@@ -111,6 +118,19 @@ public:
                optional<float> depth,
                optional<int32_t> stencil);
 
+    struct Drawable {
+        DrawMode drawMode;
+        DepthMode depthMode;
+        StencilMode stencilMode;
+        ColorMode colorMode;
+        gl::ProgramID program;
+        gl::BufferID vertexBuffer;
+        gl::BufferID indexBuffer;
+        const std::vector<Segment>& segments;
+        std::function<void ()> bindUniforms;
+        std::function<void (std::size_t)> bindAttributes;
+    };
+
     void draw(const Drawable&);
 
     void setDepthMode(const DepthMode&);
diff --git a/src/mbgl/gl/draw_mode.hpp b/src/mbgl/gl/draw_mode.hpp
index a379162210..8f7db0a3fa 100644
--- a/src/mbgl/gl/draw_mode.hpp
+++ b/src/mbgl/gl/draw_mode.hpp
@@ -1,5 +1,6 @@
 #pragma once
 
+#include <mbgl/gl/primitives.hpp>
 #include <mbgl/util/variant.hpp>
 
 namespace mbgl {
@@ -7,21 +8,50 @@ namespace gl {
 
 class Points {
 public:
+    using Primitive = Point;
+    static constexpr std::size_t bufferGroupSize = 1;
+
     float pointSize;
 };
 
 class Lines {
 public:
+    using Primitive = Line;
+    static constexpr std::size_t bufferGroupSize = 2;
+
     float lineWidth;
 };
 
 class LineStrip {
 public:
+    // LineStrip is a form of "Line" rendering, but the element buffer
+    // cannot be grouped into logical elements beyond a single Point.
+    using Primitive = Line;
+    static constexpr std::size_t bufferGroupSize = 1;
+
     float lineWidth;
 };
 
-class Triangles {};
-class TriangleStrip {};
+class Triangles {
+public:
+    using Primitive = Triangle;
+    static constexpr std::size_t bufferGroupSize = 3;
+};
+
+class TriangleStrip {
+public:
+    // TriangleStrip is a form of "Triangle" rendering, but the element buffer
+    // cannot be grouped into logical elements beyond a single Point.
+    using Primitive = Triangle;
+    static constexpr std::size_t bufferGroupSize = 1;
+};
+
+// Special draw mode for use with VertexVector<Indexed, Vertex>, in which
+// case the true draw mode is denoted by the IndexVector type.
+class Indexed {
+public:
+    static constexpr std::size_t bufferGroupSize = 1;
+};
 
 using DrawMode = variant<
     Points,
diff --git a/src/mbgl/gl/drawable.hpp b/src/mbgl/gl/drawable.hpp
deleted file mode 100644
index 747d8facf0..0000000000
--- a/src/mbgl/gl/drawable.hpp
+++ /dev/null
@@ -1,124 +0,0 @@
-#pragma once
-
-#include <mbgl/gl/types.hpp>
-#include <mbgl/gl/draw_mode.hpp>
-#include <mbgl/gl/depth_mode.hpp>
-#include <mbgl/gl/stencil_mode.hpp>
-#include <mbgl/gl/color_mode.hpp>
-#include <mbgl/gl/vertex_buffer.hpp>
-#include <mbgl/gl/index_buffer.hpp>
-#include <mbgl/gl/attribute.hpp>
-#include <mbgl/gl/uniform.hpp>
-#include <mbgl/gl/segment.hpp>
-
-#include <cstddef>
-#include <functional>
-#include <limits>
-#include <vector>
-
-namespace mbgl {
-namespace gl {
-
-template <class Vertex>
-class UnindexedVertices {
-public:
-    using VertexType = Vertex;
-
-    UnindexedVertices(DrawMode drawMode_, const VertexBuffer<Vertex>& vertexBuffer_)
-        : drawMode(std::move(drawMode_)),
-          vertexBuffer(vertexBuffer_.buffer),
-          segments({{ 0, 0, vertexBuffer_.vertexCount, 0 }}) {}
-
-    DrawMode drawMode;
-    gl::BufferID vertexBuffer;
-    static constexpr std::size_t vertexSize = sizeof(Vertex);
-    static constexpr gl::BufferID indexBuffer = 0;
-    static constexpr std::size_t primitiveSize = 0;
-    std::vector<Segment> segments;
-};
-
-template <class DrawMode, class Vertex, class...Args>
-auto Unindexed(const VertexBuffer<Vertex>& vertexBuffer,
-               Args&&... drawModeArguments) {
-    return UnindexedVertices<Vertex>(
-        DrawMode { std::forward<Args>(drawModeArguments)... },
-        vertexBuffer);
-}
-
-template <class Vertex, class Primitive>
-class SegmentedVertices {
-public:
-    using VertexType = Vertex;
-
-    SegmentedVertices(DrawMode drawMode_,
-                      const VertexBuffer<Vertex>& vertexBuffer_,
-                      const IndexBuffer<Primitive>& indexBuffer_,
-                      const std::vector<Segment>& segments_)
-        : drawMode(std::move(drawMode_)),
-          vertexBuffer(vertexBuffer_.buffer),
-          indexBuffer(indexBuffer_.buffer),
-          segments(segments_) {}
-
-    DrawMode drawMode;
-    gl::BufferID vertexBuffer;
-    static constexpr std::size_t vertexSize = sizeof(Vertex);
-    gl::BufferID indexBuffer;
-    static constexpr std::size_t primitiveSize = sizeof(Primitive);
-    const std::vector<Segment>& segments;
-};
-
-template <class DrawMode, class Vertex, class Primitive, class...Args>
-auto Segmented(const VertexBuffer<Vertex>& vertexBuffer,
-               const IndexBuffer<Primitive>& indexBuffer,
-               const std::vector<Segment>& segments,
-               Args&&... drawModeArguments) {
-    static_assert(std::is_same<typename Primitive::DrawMode, DrawMode>::value, "primitive mode mismatch");
-    return SegmentedVertices<Vertex, Primitive>(
-        DrawMode { std::forward<Args>(drawModeArguments)... },
-        vertexBuffer,
-        indexBuffer,
-        segments);
-}
-
-class Drawable {
-public:
-    template <class Program, class Subject>
-    Drawable(DepthMode depthMode_,
-             StencilMode stencilMode_,
-             ColorMode colorMode_,
-             Program& program_,
-             typename Program::UniformValues&& uniformValues,
-             const Subject& subject)
-        : drawMode(subject.drawMode),
-          depthMode(std::move(depthMode_)),
-          stencilMode(std::move(stencilMode_)),
-          colorMode(std::move(colorMode_)),
-          program(program_.program),
-          vertexBuffer(subject.vertexBuffer),
-          vertexSize(subject.vertexSize),
-          indexBuffer(subject.indexBuffer),
-          primitiveSize(subject.primitiveSize),
-          segments(subject.segments),
-          bindUniforms(Program::Uniforms::binder(program_.uniformsState, std::move(uniformValues))),
-          bindAttributes(Program::Attributes::binder(program_.attributesState))
-    {
-        static_assert(std::is_standard_layout<typename Program::Vertex>::value, "vertex type must use standard layout");
-        static_assert(std::is_same<typename Program::Vertex, typename Subject::VertexType>::value, "vertex type mismatch");
-    }
-
-    DrawMode drawMode;
-    DepthMode depthMode;
-    StencilMode stencilMode;
-    ColorMode colorMode;
-    gl::ProgramID program;
-    gl::BufferID vertexBuffer;
-    std::size_t vertexSize;
-    gl::BufferID indexBuffer;
-    std::size_t primitiveSize;
-    const std::vector<Segment>& segments;
-    std::function<void ()> bindUniforms;
-    std::function<void (std::size_t)> bindAttributes;
-};
-
-} // namespace gl
-} // namespace mbgl
diff --git a/src/mbgl/gl/index_buffer.hpp b/src/mbgl/gl/index_buffer.hpp
index 5d8630b902..8b40bf68c3 100644
--- a/src/mbgl/gl/index_buffer.hpp
+++ b/src/mbgl/gl/index_buffer.hpp
@@ -2,39 +2,37 @@
 
 #include <mbgl/gl/object.hpp>
 #include <mbgl/gl/draw_mode.hpp>
+#include <mbgl/util/ignore.hpp>
+
+#include <vector>
 
 namespace mbgl {
 namespace gl {
 
-class Line {
+template <class DrawMode>
+class IndexVector {
 public:
-    using DrawMode = Lines;
-
-    Line(uint16_t a_, uint16_t b_)
-        : a(a_), b(b_) {}
+    static constexpr std::size_t groupSize = DrawMode::bufferGroupSize;
 
-    uint16_t a;
-    uint16_t b;
-};
+    template <class... Args>
+    void emplace_back(Args&&... args) {
+        static_assert(sizeof...(args) == groupSize, "wrong buffer element count");
+        ignore({(v.emplace_back(std::forward<Args>(args)), 0)...});
+    }
 
-class Triangle {
-public:
-    using DrawMode = Triangles;
+    std::size_t indexSize() const { return v.size(); }
+    std::size_t byteSize() const { return v.size() * sizeof(uint16_t); }
 
-    Triangle(uint16_t a_, uint16_t b_, uint16_t c_)
-        : a(a_), b(b_), c(c_) {}
+    bool empty() const { return v.empty(); }
+    const uint16_t* data() const { return v.data(); }
 
-    uint16_t a;
-    uint16_t b;
-    uint16_t c;
+private:
+    std::vector<uint16_t> v;
 };
 
-template <class Primitive>
+template <class DrawMode>
 class IndexBuffer {
 public:
-    static_assert(std::is_same<Primitive, Line>::value || std::is_same<Primitive, Triangle>::value,
-                  "primitive must be Line or Triangle");
-    static constexpr std::size_t primitiveSize = sizeof(Primitive);
     UniqueBuffer buffer;
 };
 
diff --git a/src/mbgl/gl/primitives.hpp b/src/mbgl/gl/primitives.hpp
new file mode 100644
index 0000000000..fe6b1b2e5b
--- /dev/null
+++ b/src/mbgl/gl/primitives.hpp
@@ -0,0 +1,22 @@
+#pragma once
+
+namespace mbgl {
+namespace gl {
+
+class Point {
+public:
+    static constexpr std::size_t vertexCount = 1;
+};
+
+class Line {
+public:
+    static constexpr std::size_t vertexCount = 2;
+};
+
+class Triangle {
+public:
+    static constexpr std::size_t vertexCount = 3;
+};
+
+} // namespace gl
+} // namespace mbgl
diff --git a/src/mbgl/gl/program.hpp b/src/mbgl/gl/program.hpp
index ea4dbcc1df..929237037b 100644
--- a/src/mbgl/gl/program.hpp
+++ b/src/mbgl/gl/program.hpp
@@ -3,21 +3,28 @@
 #include <mbgl/gl/types.hpp>
 #include <mbgl/gl/object.hpp>
 #include <mbgl/gl/context.hpp>
+#include <mbgl/gl/vertex_buffer.hpp>
+#include <mbgl/gl/index_buffer.hpp>
+#include <mbgl/gl/attribute.hpp>
+#include <mbgl/gl/uniform.hpp>
 
 #include <string>
 
 namespace mbgl {
 namespace gl {
 
-template <class As, class Us>
+template <class P, class As, class Us>
 class Program {
 public:
+    using Primitive = P;
     using Attributes = As;
-    using Vertex = typename Attributes::Vertex;
-
     using Uniforms = Us;
+
+    using Vertex = typename Attributes::Vertex;
     using UniformValues = typename Uniforms::Values;
 
+    static_assert(std::is_standard_layout<Vertex>::value, "vertex type must use standard layout");
+
     Program(Context& context, const std::string& vertexSource, const std::string& fragmentSource)
         : vertexShader(context.createShader(ShaderType::Vertex, vertexSource)),
           fragmentShader(context.createShader(ShaderType::Fragment, fragmentSource)),
@@ -25,6 +32,57 @@ public:
           attributesState(Attributes::state(program)),
           uniformsState(Uniforms::state(program)) {}
 
+    // Indexed drawing.
+    template <class DrawMode>
+    void draw(Context& context,
+              DrawMode drawMode,
+              DepthMode depthMode,
+              StencilMode stencilMode,
+              ColorMode colorMode,
+              UniformValues&& uniformValues,
+              const VertexBuffer<Vertex>& vertexBuffer,
+              const IndexBuffer<DrawMode>& indexBuffer,
+              const std::vector<Segment>& segments) {
+        static_assert(std::is_same<Primitive, typename DrawMode::Primitive>::value, "incompatible draw mode");
+        context.draw({
+            std::move(drawMode),
+            std::move(depthMode),
+            std::move(stencilMode),
+            std::move(colorMode),
+            program,
+            vertexBuffer.buffer,
+            indexBuffer.buffer,
+            segments,
+            Uniforms::binder(uniformsState, std::move(uniformValues)),
+            Attributes::binder(attributesState)
+        });
+    }
+
+    // Unindexed drawing.
+    template <class DrawMode>
+    void draw(Context& context,
+              DrawMode drawMode,
+              DepthMode depthMode,
+              StencilMode stencilMode,
+              ColorMode colorMode,
+              UniformValues&& uniformValues,
+              const VertexBuffer<Vertex, DrawMode>& vertexBuffer) {
+        static_assert(std::is_same<Primitive, typename DrawMode::Primitive>::value, "incompatible draw mode");
+        context.draw({
+            std::move(drawMode),
+            std::move(depthMode),
+            std::move(stencilMode),
+            std::move(colorMode),
+            program,
+            vertexBuffer.buffer,
+            0,
+            {{ 0, 0, vertexBuffer.vertexCount, 0 }},
+            Uniforms::binder(uniformsState, std::move(uniformValues)),
+            Attributes::binder(attributesState)
+        });
+    }
+
+private:
     UniqueShader vertexShader;
     UniqueShader fragmentShader;
     UniqueProgram program;
diff --git a/src/mbgl/gl/segment.hpp b/src/mbgl/gl/segment.hpp
index 648d02e902..f65f2a5f7e 100644
--- a/src/mbgl/gl/segment.hpp
+++ b/src/mbgl/gl/segment.hpp
@@ -8,19 +8,19 @@ namespace gl {
 class Segment {
 public:
     Segment(std::size_t vertexOffset_,
-            std::size_t primitiveOffset_,
+            std::size_t indexOffset_,
             std::size_t vertexLength_ = 0,
-            std::size_t primitiveLength_ = 0)
+            std::size_t indexLength_ = 0)
         : vertexOffset(vertexOffset_),
-          primitiveOffset(primitiveOffset_),
+          indexOffset(indexOffset_),
           vertexLength(vertexLength_),
-          primitiveLength(primitiveLength_) {}
+          indexLength(indexLength_) {}
 
     const std::size_t vertexOffset;
-    const std::size_t primitiveOffset;
+    const std::size_t indexOffset;
 
     std::size_t vertexLength;
-    std::size_t primitiveLength;
+    std::size_t indexLength;
 };
 
 } // namespace gl
diff --git a/src/mbgl/gl/uniform.hpp b/src/mbgl/gl/uniform.hpp
index 90ca16e46f..df4de0c75a 100644
--- a/src/mbgl/gl/uniform.hpp
+++ b/src/mbgl/gl/uniform.hpp
@@ -2,6 +2,7 @@
 
 #include <mbgl/gl/types.hpp>
 #include <mbgl/util/optional.hpp>
+#include <mbgl/util/ignore.hpp>
 
 #include <array>
 #include <functional>
@@ -71,13 +72,9 @@ public:
 
     static std::function<void ()> binder(State& state, Values&& values_) {
         return [&state, values = std::move(values_)] () mutable {
-            noop((std::get<typename Us::State>(state) = std::get<typename Us::Value>(values), 0)...);
+            ignore((std::get<typename Us::State>(state) = std::get<typename Us::Value>(values), 0)...);
         };
     }
-
-private:
-    // This exists only to provide a varags context for unpacking the assignments in `binder`.
-    template <int...> static void noop(int...) {}
 };
 
 } // namespace gl
diff --git a/src/mbgl/gl/vertex_buffer.hpp b/src/mbgl/gl/vertex_buffer.hpp
index c77a9a4213..9d5532beab 100644
--- a/src/mbgl/gl/vertex_buffer.hpp
+++ b/src/mbgl/gl/vertex_buffer.hpp
@@ -1,14 +1,43 @@
 #pragma once
 
 #include <mbgl/gl/object.hpp>
+#include <mbgl/gl/primitives.hpp>
+#include <mbgl/gl/draw_mode.hpp>
+#include <mbgl/util/ignore.hpp>
+
+#include <vector>
 
 namespace mbgl {
 namespace gl {
 
-template <class Vertex>
+template <class V, class DrawMode = Indexed>
+class VertexVector {
+public:
+    using Vertex = V;
+    static constexpr std::size_t groupSize = DrawMode::bufferGroupSize;
+
+    template <class... Args>
+    void emplace_back(Args&&... args) {
+        static_assert(sizeof...(args) == groupSize, "wrong buffer element count");
+        ignore({(v.emplace_back(std::forward<Args>(args)), 0)...});
+    }
+
+    std::size_t vertexSize() const { return v.size(); }
+    std::size_t byteSize() const { return v.size() * sizeof(Vertex); }
+
+    bool empty() const { return v.empty(); }
+    const Vertex* data() const { return v.data(); }
+
+private:
+    std::vector<Vertex> v;
+};
+
+template <class V, class DrawMode = Indexed>
 class VertexBuffer {
 public:
+    using Vertex = V;
     static constexpr std::size_t vertexSize = sizeof(Vertex);
+
     std::size_t vertexCount;
     UniqueBuffer buffer;
 };