Don't crash when attribute count is exceeded

src/mbgl/gl/attribute.cpp

@@ -1,14 +1,20 @@
 #include <mbgl/gl/attribute.hpp>
+#include <mbgl/gl/context.hpp>
 #include <mbgl/gl/gl.hpp>
 
 namespace mbgl {
 namespace gl {
 
-void bindAttributeLocation(ProgramID id, AttributeLocation location, const char* name) {
-    if (location >= MAX_ATTRIBUTES) {
-        throw gl::Error("too many vertex attributes");
+void bindAttributeLocation(Context& context, ProgramID id, AttributeLocation location, const char* name) {
+    // We're using sequentially numberered attribute locations starting with 0. Therefore we can use
+    // the location as a proxy for the number of attributes.
+    if (location >= context.maximumVertexBindingCount) {
+        // Don't bind the location on this hardware since it exceeds the limit (otherwise we'd get
+        // an OpenGL error). This means we'll see rendering errors, and possibly slow rendering due
+        // to unbound attributes.
+    } else {
+        MBGL_CHECK_ERROR(glBindAttribLocation(id, location, name));
     }
-    MBGL_CHECK_ERROR(glBindAttribLocation(id, location, name));
 }
 
 std::set<std::string> getActiveAttributes(ProgramID id) {

src/mbgl/gl/attribute.hpp

@@ -17,8 +17,6 @@
 namespace mbgl {
 namespace gl {
 
-static constexpr std::size_t MAX_ATTRIBUTES = 8;
-
 template <class> struct DataTypeOf;
 template <> struct DataTypeOf< int8_t>  : std::integral_constant<DataType, DataType::Byte> {};
 template <> struct DataTypeOf<uint8_t>  : std::integral_constant<DataType, DataType::UnsignedByte> {};
@@ -45,7 +43,7 @@ class AttributeBinding {
     }
 };
 
-using AttributeBindingArray = std::array<optional<AttributeBinding>, MAX_ATTRIBUTES>;
+using AttributeBindingArray = std::vector<optional<AttributeBinding>>;
 
 /*
     gl::Attribute<T,N> manages the binding of a vertex buffer to a GL program attribute.
@@ -214,7 +212,8 @@ const std::size_t Vertex<A1, A2, A3, A4, A5>::attributeOffsets[5] = {
 
 } // namespace detail
 
-void bindAttributeLocation(ProgramID, AttributeLocation, const char * name);
+class Context;
+void bindAttributeLocation(Context&, ProgramID, AttributeLocation, const char * name);
 std::set<std::string> getActiveAttributes(ProgramID);
 
 template <class... As>
@@ -231,13 +230,13 @@ class Attributes {
 
     using Vertex = detail::Vertex<typename As::Type...>;
 
-    static Locations bindLocations(const ProgramID& id) {
+    static Locations bindLocations(Context& context, const ProgramID& id) {
         std::set<std::string> activeAttributes = getActiveAttributes(id);
 
         AttributeLocation location = 0;
         auto maybeBindLocation = [&](const char* name) -> optional<AttributeLocation> {
             if (activeAttributes.count(name)) {
-                bindAttributeLocation(id, location, name);
+                bindAttributeLocation(context, id, location, name);
                 return location++;
             } else {
                 return {};
@@ -277,6 +276,7 @@ class Attributes {
 
     static AttributeBindingArray toBindingArray(const Locations& locations, const Bindings& bindings) {
         AttributeBindingArray result;
+        result.resize(sizeof...(As));
 
         auto maybeAddBinding = [&] (const optional<AttributeLocation>& location,
                                     const optional<AttributeBinding>& binding) {
@@ -289,6 +289,12 @@ class Attributes {
 
         return result;
     }
+
+    static uint32_t activeBindingCount(const Bindings& bindings) {
+        uint32_t result = 0;
+        util::ignore({ ((result += bool(bindings.template get<As>())), 0)... });
+        return result;
+    }
 };
 
 namespace detail {

src/mbgl/gl/context.cpp

@@ -94,7 +94,13 @@ static_assert(underlying_type(BufferUsage::DynamicDraw) == GL_DYNAMIC_DRAW, "Ope
 
 static_assert(std::is_same<BinaryProgramFormat, GLenum>::value, "OpenGL type mismatch");
 
-Context::Context() = default;
+Context::Context()
+    : maximumVertexBindingCount([] {
+          GLint value;
+          MBGL_CHECK_ERROR(glGetIntegerv(GL_MAX_VERTEX_ATTRIBS, &value));
+          return value;
+      }()) {
+}
 
 Context::~Context() {
     if (cleanupOnDestruction) {
@@ -351,7 +357,7 @@ VertexArray Context::createVertexArray() {
         VertexArrayID id = 0;
         MBGL_CHECK_ERROR(vertexArray->genVertexArrays(1, &id));
         UniqueVertexArray vao(std::move(id), { this });
-        return { UniqueVertexArrayState(new VertexArrayState(std::move(vao), *this), VertexArrayStateDeleter { true })};
+        return { UniqueVertexArrayState(new VertexArrayState(std::move(vao)), VertexArrayStateDeleter { true })};
     } else {
         // On GL implementations which do not support vertex arrays, attribute bindings are global state.
         // So return a VertexArray which shares our global state tracking and whose deleter is a no-op.

src/mbgl/gl/context.hpp

@@ -226,7 +226,7 @@ class Context : private util::noncopyable {
     State<value::BindVertexBuffer> vertexBuffer;
 
     State<value::BindVertexArray, const Context&> bindVertexArray { *this };
-    VertexArrayState globalVertexArrayState { UniqueVertexArray(0, { this }), *this };
+    VertexArrayState globalVertexArrayState { UniqueVertexArray(0, { this }) };
 
     State<value::PixelStorePack> pixelStorePack;
     State<value::PixelStoreUnpack> pixelStoreUnpack;
@@ -239,6 +239,8 @@ class Context : private util::noncopyable {
 #endif // MBGL_USE_GLES2
 
     bool supportsHalfFloatTextures = false;
+    const uint32_t maximumVertexBindingCount;
+    static constexpr const uint32_t minimumRequiredVertexBindingCount = 8;
 
 private:
     State<value::StencilFunc> stencilFunc;

src/mbgl/gl/program.hpp

@@ -35,7 +35,7 @@ class Program {
               context.createProgram(context.createShader(ShaderType::Vertex, vertexSource),
                                     context.createShader(ShaderType::Fragment, fragmentSource))),
           uniformsState((context.linkProgram(program), Uniforms::bindLocations(program))),
-          attributeLocations(Attributes::bindLocations(program)) {
+          attributeLocations(Attributes::bindLocations(context, program)) {
 
         // Re-link program after manually binding only active attributes in Attributes::bindLocations
         context.linkProgram(program);

src/mbgl/gl/vertex_array.cpp

@@ -9,7 +9,11 @@ void VertexArray::bind(Context& context, BufferID indexBuffer, const AttributeBi
     context.bindVertexArray = state->vertexArray;
     state->indexBuffer = indexBuffer;
 
-    for (AttributeLocation location = 0; location < MAX_ATTRIBUTES; ++location) {
+    state->bindings.reserve(bindings.size());
+    for (AttributeLocation location = 0; location < bindings.size(); ++location) {
+        if (state->bindings.size() <= location) {
+            state->bindings.emplace_back(context, AttributeLocation(location));
+        }
         state->bindings[location] = bindings[location];
     }
 }

src/mbgl/gl/vertex_array.hpp

@@ -15,9 +15,8 @@ class Context;
 
 class VertexArrayState {
 public:
-    VertexArrayState(UniqueVertexArray vertexArray_, Context& context)
-        : vertexArray(std::move(vertexArray_)),
-          bindings(makeBindings(context, std::make_index_sequence<MAX_ATTRIBUTES>())) {
+    VertexArrayState(UniqueVertexArray vertexArray_)
+        : vertexArray(std::move(vertexArray_)) {
     }
 
     void setDirty() {
@@ -31,13 +30,7 @@ class VertexArrayState {
     State<value::BindElementBuffer> indexBuffer;
 
     using AttributeState = State<value::VertexAttribute, Context&, AttributeLocation>;
-    std::array<AttributeState, MAX_ATTRIBUTES> bindings;
-
-private:
-    template <std::size_t... I>
-    std::array<AttributeState, MAX_ATTRIBUTES> makeBindings(Context& context, std::index_sequence<I...>) {
-        return {{ AttributeState { context, I }... }};
-    }
+    std::vector<AttributeState> bindings;
 };
 
 class VertexArrayStateDeleter {

src/mbgl/programs/program.hpp

@@ -46,26 +46,38 @@ class Program {
             Shaders::fragmentSource)) {
     }
 
+    static typename AllUniforms::Values computeAllUniformValues(
+        const UniformValues& uniformValues,
+        const PaintPropertyBinders& paintPropertyBinders,
+        const typename PaintProperties::PossiblyEvaluated& currentProperties,
+        float currentZoom) {
+        return uniformValues
+            .concat(paintPropertyBinders.uniformValues(currentZoom, currentProperties));
+    }
+
+    static typename Attributes::Bindings computeAllAttributeBindings(
+        const gl::VertexBuffer<LayoutVertex>& layoutVertexBuffer,
+        const PaintPropertyBinders& paintPropertyBinders,
+        const typename PaintProperties::PossiblyEvaluated& currentProperties) {
+        return LayoutAttributes::bindings(layoutVertexBuffer)
+            .concat(paintPropertyBinders.attributeBindings(currentProperties));
+    }
+
+    static uint32_t activeBindingCount(const typename Attributes::Bindings& allAttributeBindings) {
+        return Attributes::activeBindingCount(allAttributeBindings);
+    }
+
     template <class DrawMode>
     void draw(gl::Context& context,
               DrawMode drawMode,
               gl::DepthMode depthMode,
               gl::StencilMode stencilMode,
               gl::ColorMode colorMode,
-              const UniformValues& uniformValues,
-              const gl::VertexBuffer<LayoutVertex>& layoutVertexBuffer,
               const gl::IndexBuffer<DrawMode>& indexBuffer,
               const SegmentVector<Attributes>& segments,
-              const PaintPropertyBinders& paintPropertyBinders,
-              const typename PaintProperties::PossiblyEvaluated& currentProperties,
-              float currentZoom,
+              const typename AllUniforms::Values& allUniformValues,
+              const typename Attributes::Bindings& allAttributeBindings,
               const std::string& layerID) {
-        typename AllUniforms::Values allUniformValues = uniformValues
-            .concat(paintPropertyBinders.uniformValues(currentZoom, currentProperties));
-
-        typename Attributes::Bindings allAttributeBindings = LayoutAttributes::bindings(layoutVertexBuffer)
-            .concat(paintPropertyBinders.attributeBindings(currentProperties));
-
         for (auto& segment : segments) {
             auto vertexArrayIt = segment.vertexArrays.find(layerID);
 

src/mbgl/programs/symbol_program.hpp

@@ -278,35 +278,45 @@ class SymbolProgram {
             Shaders::fragmentSource)) {
     }
 
+    static typename AllUniforms::Values computeAllUniformValues(
+        const UniformValues& uniformValues,
+        const SymbolSizeBinder& symbolSizeBinder,
+        const PaintPropertyBinders& paintPropertyBinders,
+        const typename PaintProperties::PossiblyEvaluated& currentProperties,
+        float currentZoom) {
+        return uniformValues.concat(symbolSizeBinder.uniformValues(currentZoom))
+            .concat(paintPropertyBinders.uniformValues(currentZoom, currentProperties));
+    }
+
+    static typename Attributes::Bindings computeAllAttributeBindings(
+        const gl::VertexBuffer<LayoutVertex>& layoutVertexBuffer,
+        const gl::VertexBuffer<SymbolDynamicLayoutAttributes::Vertex>& dynamicLayoutVertexBuffer,
+        const gl::VertexBuffer<SymbolOpacityAttributes::Vertex>& opacityVertexBuffer,
+        const PaintPropertyBinders& paintPropertyBinders,
+        const typename PaintProperties::PossiblyEvaluated& currentProperties) {
+        assert(layoutVertexBuffer.vertexCount == dynamicLayoutVertexBuffer.vertexCount &&
+               layoutVertexBuffer.vertexCount == opacityVertexBuffer.vertexCount);
+        return LayoutAttributes::bindings(layoutVertexBuffer)
+            .concat(SymbolDynamicLayoutAttributes::bindings(dynamicLayoutVertexBuffer))
+            .concat(SymbolOpacityAttributes::bindings(opacityVertexBuffer))
+            .concat(paintPropertyBinders.attributeBindings(currentProperties));
+    }
+
+    static uint32_t activeBindingCount(const typename Attributes::Bindings& allAttributeBindings) {
+        return Attributes::activeBindingCount(allAttributeBindings);
+    }
+
     template <class DrawMode>
     void draw(gl::Context& context,
               DrawMode drawMode,
               gl::DepthMode depthMode,
               gl::StencilMode stencilMode,
               gl::ColorMode colorMode,
-              const UniformValues& uniformValues,
-              const gl::VertexBuffer<LayoutVertex>& layoutVertexBuffer,
-              const gl::VertexBuffer<SymbolDynamicLayoutAttributes::Vertex>& dynamicLayoutVertexBuffer,
-              const gl::VertexBuffer<SymbolOpacityAttributes::Vertex>& opacityVertexBuffer,
-              const SymbolSizeBinder& symbolSizeBinder,
               const gl::IndexBuffer<DrawMode>& indexBuffer,
               const SegmentVector<Attributes>& segments,
-              const PaintPropertyBinders& paintPropertyBinders,
-              const typename PaintProperties::PossiblyEvaluated& currentProperties,
-              float currentZoom,
+              const typename AllUniforms::Values& allUniformValues,
+              const typename Attributes::Bindings& allAttributeBindings,
               const std::string& layerID) {
-        typename AllUniforms::Values allUniformValues = uniformValues
-            .concat(symbolSizeBinder.uniformValues(currentZoom))
-            .concat(paintPropertyBinders.uniformValues(currentZoom, currentProperties));
-
-        typename Attributes::Bindings allAttributeBindings = LayoutAttributes::bindings(layoutVertexBuffer)
-            .concat(SymbolDynamicLayoutAttributes::bindings(dynamicLayoutVertexBuffer))
-            .concat(SymbolOpacityAttributes::bindings(opacityVertexBuffer))
-            .concat(paintPropertyBinders.attributeBindings(currentProperties));
-
-        assert(layoutVertexBuffer.vertexCount == dynamicLayoutVertexBuffer.vertexCount &&
-                layoutVertexBuffer.vertexCount == opacityVertexBuffer.vertexCount);
-
         for (auto& segment : segments) {
             auto vertexArrayIt = segment.vertexArrays.find(layerID);
 

src/mbgl/renderer/layers/render_background_layer.cpp

@@ -49,6 +49,35 @@ void RenderBackgroundLayer::render(PaintParameters& parameters, RenderSource*) {
     const Properties<>::PossiblyEvaluated properties;
     const BackgroundProgram::PaintPropertyBinders paintAttributeData(properties, 0);
 
+    auto draw = [&](auto& program, auto&& uniformValues) {
+        const auto allUniformValues = program.computeAllUniformValues(
+            std::move(uniformValues),
+            paintAttributeData,
+            properties,
+            parameters.state.getZoom()
+        );
+        const auto allAttributeBindings = program.computeAllAttributeBindings(
+            parameters.staticData.tileVertexBuffer,
+            paintAttributeData,
+            properties
+        );
+
+        checkRenderability(parameters, program.activeBindingCount(allAttributeBindings));
+
+        program.draw(
+            parameters.context,
+            gl::Triangles(),
+            parameters.depthModeForSublayer(0, gl::DepthMode::ReadOnly),
+            gl::StencilMode::disabled(),
+            parameters.colorModeForRenderPass(),
+            parameters.staticData.quadTriangleIndexBuffer,
+            parameters.staticData.tileTriangleSegments,
+            allUniformValues,
+            allAttributeBindings,
+            getID()
+        );
+    };
+
     if (!evaluated.get<BackgroundPattern>().to.empty()) {
         optional<ImagePosition> imagePosA = parameters.imageManager.getPattern(evaluated.get<BackgroundPattern>().from);
         optional<ImagePosition> imagePosB = parameters.imageManager.getPattern(evaluated.get<BackgroundPattern>().to);
@@ -59,12 +88,8 @@ void RenderBackgroundLayer::render(PaintParameters& parameters, RenderSource*) {
         parameters.imageManager.bind(parameters.context, 0);
 
         for (const auto& tileID : util::tileCover(parameters.state, parameters.state.getIntegerZoom())) {
-            parameters.programs.backgroundPattern.draw(
-                parameters.context,
-                gl::Triangles(),
-                parameters.depthModeForSublayer(0, gl::DepthMode::ReadOnly),
-                gl::StencilMode::disabled(),
-                parameters.colorModeForRenderPass(),
+            draw(
+                parameters.programs.backgroundPattern,
                 BackgroundPatternUniforms::values(
                     parameters.matrixForTile(tileID),
                     evaluated.get<BackgroundOpacity>(),
@@ -74,36 +99,18 @@ void RenderBackgroundLayer::render(PaintParameters& parameters, RenderSource*) {
                     evaluated.get<BackgroundPattern>(),
                     tileID,
                     parameters.state
-                ),
-                parameters.staticData.tileVertexBuffer,
-                parameters.staticData.quadTriangleIndexBuffer,
-                parameters.staticData.tileTriangleSegments,
-                paintAttributeData,
-                properties,
-                parameters.state.getZoom(),
-                getID()
+                )
             );
         }
     } else {
         for (const auto& tileID : util::tileCover(parameters.state, parameters.state.getIntegerZoom())) {
-            parameters.programs.background.draw(
-                parameters.context,
-                gl::Triangles(),
-                parameters.depthModeForSublayer(0, gl::DepthMode::ReadOnly),
-                gl::StencilMode::disabled(),
-                parameters.colorModeForRenderPass(),
+            draw(
+                parameters.programs.background,
                 BackgroundProgram::UniformValues {
                     uniforms::u_matrix::Value{ parameters.matrixForTile(tileID) },
                     uniforms::u_color::Value{ evaluated.get<BackgroundColor>() },
                     uniforms::u_opacity::Value{ evaluated.get<BackgroundOpacity>() },
-                },
-                parameters.staticData.tileVertexBuffer,
-                parameters.staticData.quadTriangleIndexBuffer,
-                parameters.staticData.tileTriangleSegments,
-                paintAttributeData,
-                properties,
-                parameters.state.getZoom(),
-                getID()
+                }
             );
         }
     }