summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKonstantin Käfer <mail@kkaefer.com>2018-04-11 13:27:21 +0200
committerKonstantin Käfer <mail@kkaefer.com>2018-04-13 18:27:14 +0200
commitec12bab1acce951976a152e2fd9f2110a327285e (patch)
tree20edb5dc546af978a377d38377c808142229154c
parent8543ae6e11c21b477c42bc10e73aedba190df592 (diff)
downloadqtlocation-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.cpp14
-rw-r--r--src/mbgl/gl/attribute.hpp18
-rw-r--r--src/mbgl/gl/context.cpp10
-rw-r--r--src/mbgl/gl/context.hpp4
-rw-r--r--src/mbgl/gl/program.hpp2
-rw-r--r--src/mbgl/gl/vertex_array.cpp6
-rw-r--r--src/mbgl/gl/vertex_array.hpp13
-rw-r--r--src/mbgl/programs/program.hpp4
-rw-r--r--src/mbgl/programs/symbol_program.hpp4
-rw-r--r--src/mbgl/renderer/layers/render_background_layer.cpp2
-rw-r--r--src/mbgl/renderer/layers/render_circle_layer.cpp2
-rw-r--r--src/mbgl/renderer/layers/render_fill_extrusion_layer.cpp4
-rw-r--r--src/mbgl/renderer/layers/render_fill_layer.cpp4
-rw-r--r--src/mbgl/renderer/layers/render_heatmap_layer.cpp4
-rw-r--r--src/mbgl/renderer/layers/render_hillshade_layer.cpp4
-rw-r--r--src/mbgl/renderer/layers/render_line_layer.cpp2
-rw-r--r--src/mbgl/renderer/layers/render_raster_layer.cpp2
-rw-r--r--src/mbgl/renderer/layers/render_symbol_layer.cpp2
-rw-r--r--src/mbgl/renderer/render_layer.cpp29
-rw-r--r--src/mbgl/renderer/render_layer.hpp11
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