diff options
author | Konstantin Käfer <mail@kkaefer.com> | 2018-04-11 13:27:21 +0200 |
---|---|---|
committer | Konstantin Käfer <mail@kkaefer.com> | 2018-04-13 18:27:14 +0200 |
commit | ec12bab1acce951976a152e2fd9f2110a327285e (patch) | |
tree | 20edb5dc546af978a377d38377c808142229154c | |
parent | 8543ae6e11c21b477c42bc10e73aedba190df592 (diff) | |
download | qtlocation-mapboxgl-upstream/attribute-binding-crash.tar.gz |
[core] warn about using too many data driven properties instead of crashingupstream/attribute-binding-crash
Instead, we're printing a warning message and won't bind the attributes beyond the limit. This will cause rendering errors, but it's still better than crashing.
-rw-r--r-- | src/mbgl/gl/attribute.cpp | 14 | ||||
-rw-r--r-- | src/mbgl/gl/attribute.hpp | 18 | ||||
-rw-r--r-- | src/mbgl/gl/context.cpp | 10 | ||||
-rw-r--r-- | src/mbgl/gl/context.hpp | 4 | ||||
-rw-r--r-- | src/mbgl/gl/program.hpp | 2 | ||||
-rw-r--r-- | src/mbgl/gl/vertex_array.cpp | 6 | ||||
-rw-r--r-- | src/mbgl/gl/vertex_array.hpp | 13 | ||||
-rw-r--r-- | src/mbgl/programs/program.hpp | 4 | ||||
-rw-r--r-- | src/mbgl/programs/symbol_program.hpp | 4 | ||||
-rw-r--r-- | src/mbgl/renderer/layers/render_background_layer.cpp | 2 | ||||
-rw-r--r-- | src/mbgl/renderer/layers/render_circle_layer.cpp | 2 | ||||
-rw-r--r-- | src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp | 4 | ||||
-rw-r--r-- | src/mbgl/renderer/layers/render_fill_layer.cpp | 4 | ||||
-rw-r--r-- | src/mbgl/renderer/layers/render_heatmap_layer.cpp | 4 | ||||
-rw-r--r-- | src/mbgl/renderer/layers/render_hillshade_layer.cpp | 4 | ||||
-rw-r--r-- | src/mbgl/renderer/layers/render_line_layer.cpp | 2 | ||||
-rw-r--r-- | src/mbgl/renderer/layers/render_raster_layer.cpp | 2 | ||||
-rw-r--r-- | src/mbgl/renderer/layers/render_symbol_layer.cpp | 2 | ||||
-rw-r--r-- | src/mbgl/renderer/render_layer.cpp | 29 | ||||
-rw-r--r-- | src/mbgl/renderer/render_layer.hpp | 11 |
20 files changed, 116 insertions, 25 deletions
diff --git a/src/mbgl/gl/attribute.cpp b/src/mbgl/gl/attribute.cpp index bb5b2ddc34..b2d05fe665 100644 --- a/src/mbgl/gl/attribute.cpp +++ b/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) { diff --git a/src/mbgl/gl/attribute.hpp b/src/mbgl/gl/attribute.hpp index fa6c2ddeab..3763f0a583 100644 --- a/src/mbgl/gl/attribute.hpp +++ b/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 @@ public: } }; -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 @@ public: 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 @@ public: 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 @@ public: 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 { diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp index ba44adb42b..c6352d3e84 100644 --- a/src/mbgl/gl/context.cpp +++ b/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. diff --git a/src/mbgl/gl/context.hpp b/src/mbgl/gl/context.hpp index 67624288e2..d95311115e 100644 --- a/src/mbgl/gl/context.hpp +++ b/src/mbgl/gl/context.hpp @@ -226,7 +226,7 @@ public: 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 @@ public: #endif // MBGL_USE_GLES2 bool supportsHalfFloatTextures = false; + const uint32_t maximumVertexBindingCount; + static constexpr const uint32_t minimumRequiredVertexBindingCount = 8; private: State<value::StencilFunc> stencilFunc; diff --git a/src/mbgl/gl/program.hpp b/src/mbgl/gl/program.hpp index af02ad3d54..f33501cd11 100644 --- a/src/mbgl/gl/program.hpp +++ b/src/mbgl/gl/program.hpp @@ -35,7 +35,7 @@ public: 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); diff --git a/src/mbgl/gl/vertex_array.cpp b/src/mbgl/gl/vertex_array.cpp index 68a500ac45..d552a8292e 100644 --- a/src/mbgl/gl/vertex_array.cpp +++ b/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]; } } diff --git a/src/mbgl/gl/vertex_array.hpp b/src/mbgl/gl/vertex_array.hpp index 46c67017bb..604754f672 100644 --- a/src/mbgl/gl/vertex_array.hpp +++ b/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 @@ public: 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 { diff --git a/src/mbgl/programs/program.hpp b/src/mbgl/programs/program.hpp index 2c98d4b481..f26cb135bc 100644 --- a/src/mbgl/programs/program.hpp +++ b/src/mbgl/programs/program.hpp @@ -63,6 +63,10 @@ public: .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, diff --git a/src/mbgl/programs/symbol_program.hpp b/src/mbgl/programs/symbol_program.hpp index a460d2018a..bc318dc897 100644 --- a/src/mbgl/programs/symbol_program.hpp +++ b/src/mbgl/programs/symbol_program.hpp @@ -302,6 +302,10 @@ public: .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, diff --git a/src/mbgl/renderer/layers/render_background_layer.cpp b/src/mbgl/renderer/layers/render_background_layer.cpp index 1a9ff8fe8c..1a472d951e 100644 --- a/src/mbgl/renderer/layers/render_background_layer.cpp +++ b/src/mbgl/renderer/layers/render_background_layer.cpp @@ -62,6 +62,8 @@ void RenderBackgroundLayer::render(PaintParameters& parameters, RenderSource*) { properties ); + checkRenderability(parameters, program.activeBindingCount(allAttributeBindings)); + program.draw( parameters.context, gl::Triangles(), diff --git a/src/mbgl/renderer/layers/render_circle_layer.cpp b/src/mbgl/renderer/layers/render_circle_layer.cpp index 9928ed80d7..ab83009d98 100644 --- a/src/mbgl/renderer/layers/render_circle_layer.cpp +++ b/src/mbgl/renderer/layers/render_circle_layer.cpp @@ -89,6 +89,8 @@ void RenderCircleLayer::render(PaintParameters& parameters, RenderSource*) { evaluated ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, gl::Triangles(), diff --git a/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp b/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp index e7a0f2b1a6..492a57c9bf 100644 --- a/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp +++ b/src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp @@ -83,6 +83,8 @@ void RenderFillExtrusionLayer::render(PaintParameters& parameters, RenderSource* evaluated ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, gl::Triangles(), @@ -176,6 +178,8 @@ void RenderFillExtrusionLayer::render(PaintParameters& parameters, RenderSource* properties ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, gl::Triangles(), diff --git a/src/mbgl/renderer/layers/render_fill_layer.cpp b/src/mbgl/renderer/layers/render_fill_layer.cpp index b5888ce152..73b4004682 100644 --- a/src/mbgl/renderer/layers/render_fill_layer.cpp +++ b/src/mbgl/renderer/layers/render_fill_layer.cpp @@ -92,6 +92,8 @@ void RenderFillLayer::render(PaintParameters& parameters, RenderSource*) { evaluated ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, drawMode, @@ -179,6 +181,8 @@ void RenderFillLayer::render(PaintParameters& parameters, RenderSource*) { evaluated ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, drawMode, diff --git a/src/mbgl/renderer/layers/render_heatmap_layer.cpp b/src/mbgl/renderer/layers/render_heatmap_layer.cpp index 54c9010592..68dd3003d6 100644 --- a/src/mbgl/renderer/layers/render_heatmap_layer.cpp +++ b/src/mbgl/renderer/layers/render_heatmap_layer.cpp @@ -112,6 +112,8 @@ void RenderHeatmapLayer::render(PaintParameters& parameters, RenderSource*) { evaluated ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, gl::Triangles(), @@ -157,6 +159,8 @@ void RenderHeatmapLayer::render(PaintParameters& parameters, RenderSource*) { properties ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, gl::Triangles(), diff --git a/src/mbgl/renderer/layers/render_hillshade_layer.cpp b/src/mbgl/renderer/layers/render_hillshade_layer.cpp index fbe42ad242..d1c2045e9c 100644 --- a/src/mbgl/renderer/layers/render_hillshade_layer.cpp +++ b/src/mbgl/renderer/layers/render_hillshade_layer.cpp @@ -93,6 +93,8 @@ void RenderHillshadeLayer::render(PaintParameters& parameters, RenderSource* src evaluated ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, gl::Triangles(), @@ -147,6 +149,8 @@ void RenderHillshadeLayer::render(PaintParameters& parameters, RenderSource* src properties ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, gl::Triangles(), diff --git a/src/mbgl/renderer/layers/render_line_layer.cpp b/src/mbgl/renderer/layers/render_line_layer.cpp index b5336d6e80..1edbbce1e8 100644 --- a/src/mbgl/renderer/layers/render_line_layer.cpp +++ b/src/mbgl/renderer/layers/render_line_layer.cpp @@ -78,6 +78,8 @@ void RenderLineLayer::render(PaintParameters& parameters, RenderSource*) { evaluated ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, gl::Triangles(), diff --git a/src/mbgl/renderer/layers/render_raster_layer.cpp b/src/mbgl/renderer/layers/render_raster_layer.cpp index 48b01cea41..2e90489a6b 100644 --- a/src/mbgl/renderer/layers/render_raster_layer.cpp +++ b/src/mbgl/renderer/layers/render_raster_layer.cpp @@ -107,6 +107,8 @@ void RenderRasterLayer::render(PaintParameters& parameters, RenderSource* source evaluated ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, gl::Triangles(), diff --git a/src/mbgl/renderer/layers/render_symbol_layer.cpp b/src/mbgl/renderer/layers/render_symbol_layer.cpp index d709ff1cb7..0c184ac171 100644 --- a/src/mbgl/renderer/layers/render_symbol_layer.cpp +++ b/src/mbgl/renderer/layers/render_symbol_layer.cpp @@ -105,6 +105,8 @@ void RenderSymbolLayer::render(PaintParameters& parameters, RenderSource*) { paintProperties ); + checkRenderability(parameters, programInstance.activeBindingCount(allAttributeBindings)); + programInstance.draw( parameters.context, gl::Triangles(), diff --git a/src/mbgl/renderer/render_layer.cpp b/src/mbgl/renderer/render_layer.cpp index bcdc175f14..a667d5837e 100644 --- a/src/mbgl/renderer/render_layer.cpp +++ b/src/mbgl/renderer/render_layer.cpp @@ -9,8 +9,10 @@ #include <mbgl/renderer/layers/render_raster_layer.hpp> #include <mbgl/renderer/layers/render_symbol_layer.hpp> #include <mbgl/renderer/layers/render_heatmap_layer.hpp> +#include <mbgl/renderer/paint_parameters.hpp> #include <mbgl/style/types.hpp> #include <mbgl/renderer/render_tile.hpp> +#include <mbgl/util/logging.hpp> namespace mbgl { @@ -73,5 +75,32 @@ void RenderLayer::setRenderTiles(std::vector<std::reference_wrapper<RenderTile>> renderTiles = std::move(tiles); } +void RenderLayer::checkRenderability(const PaintParameters& parameters, + const uint32_t activeBindingCount) { + // Only warn once for every layer. + if (hasRenderFailures) { + return; + } + + if (activeBindingCount > parameters.context.maximumVertexBindingCount) { + Log::Error(Event::OpenGL, + "The layer '%s' uses more data-driven properties than the current device " + "supports, and will have rendering errors. To ensure compatibility with this " + "device, use %d fewer data driven properties in this layer.", + getID().c_str(), + activeBindingCount - parameters.context.minimumRequiredVertexBindingCount); + hasRenderFailures = true; + } else if (activeBindingCount > parameters.context.minimumRequiredVertexBindingCount) { + Log::Error(Event::OpenGL, + "The layer '%s' uses more data-driven properties than some devices may support. " + "Though it will render correctly on this device, it may have rendering errors " + "on other devices. To ensure compatibility with all devices, use %d fewer " + "data-driven properties in this layer.", + getID().c_str(), + activeBindingCount - parameters.context.minimumRequiredVertexBindingCount); + hasRenderFailures = true; + } +} + } //namespace mbgl diff --git a/src/mbgl/renderer/render_layer.hpp b/src/mbgl/renderer/render_layer.hpp index 55831cb72c..5b92ffa4db 100644 --- a/src/mbgl/renderer/render_layer.hpp +++ b/src/mbgl/renderer/render_layer.hpp @@ -82,6 +82,11 @@ public: friend std::string layoutKey(const RenderLayer&); protected: + // Checks whether the current hardware can render this layer. If it can't, we'll show a warning + // in the console to inform the developer. + void checkRenderability(const PaintParameters&, uint32_t activeBindingCount); + +protected: // renderTiles are exposed directly to CrossTileSymbolIndex and Placement so they // can update opacities in the symbol buckets immediately before rendering friend class CrossTileSymbolIndex; @@ -92,6 +97,12 @@ protected: // Stores what render passes this layer is currently enabled for. This depends on the // evaluated StyleProperties object and is updated accordingly. RenderPass passes = RenderPass::None; + +private: + // Some layers may not render correctly on some hardware when the vertex attribute limit of + // that GPU is exceeded. More attributes are used when adding many data driven paint properties + // to a layer. + bool hasRenderFailures = false; }; } // namespace mbgl |