From c61041b44cd9181641db2351f4657cc356300226 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Mon, 3 Jul 2017 14:05:12 -0700 Subject: [PATCH] [core] Rework attribute binding (again) These changes are necessary for programs whose set of active attributes is not fixed at compile time by a template parameter pack, but rather varies based on the generated shader text at runtime. In such cases, the attribute location of a given named attribute may vary between instances of the same Program. Previously, attribute bindings were implicitly associated with a location based on template parameter order, and -1 was used to indicate an inactive attribute. This left us unable to disable the appropriate attribute when it went from active to inactive. Now, the state tracker for bindings explicitly associates locations and state, and an empty optional is used to indicate an inactive attribute. In addition, a gl::VertexArray class is now exposed, allowing more flexibility in the relationship between Programs, Segments, and attribute bindings. In this commit, that relationship does not change, but the subsequent commit adjusts it to match gl-js, reduce rebinds, and work around buggy VAO implementations. VertexArray uses a pimpl idiom in order to support implementations that lack the VAO extension. In that case, all VertexArrays share global binding state, reflecting the platform reality in the absence of VAOs, while still providing a uniform API. --- cmake/core-files.cmake | 8 +- src/mbgl/gl/attribute.cpp | 63 +------- src/mbgl/gl/attribute.hpp | 148 +++++++++--------- src/mbgl/gl/context.cpp | 31 ++-- src/mbgl/gl/context.hpp | 12 +- src/mbgl/gl/object.cpp | 4 +- src/mbgl/gl/program.hpp | 26 +-- src/mbgl/gl/segment.hpp | 73 --------- src/mbgl/gl/types.hpp | 2 +- src/mbgl/gl/uniform.hpp | 2 +- src/mbgl/gl/value.cpp | 18 +++ src/mbgl/gl/value.hpp | 7 + src/mbgl/gl/vertex_array.cpp | 18 +++ src/mbgl/gl/vertex_array.hpp | 58 +++++++ src/mbgl/programs/binary_program.cpp | 4 +- src/mbgl/programs/binary_program.hpp | 4 +- src/mbgl/programs/program.hpp | 42 +++-- src/mbgl/{gl => programs}/segment.cpp | 2 +- src/mbgl/programs/segment.hpp | 40 +++++ src/mbgl/programs/symbol_program.hpp | 46 ++++-- src/mbgl/renderer/buckets/circle_bucket.hpp | 4 +- src/mbgl/renderer/buckets/debug_bucket.hpp | 2 +- src/mbgl/renderer/buckets/fill_bucket.hpp | 6 +- .../buckets/fill_extrusion_bucket.hpp | 4 +- src/mbgl/renderer/buckets/line_bucket.hpp | 4 +- src/mbgl/renderer/buckets/raster_bucket.hpp | 2 +- src/mbgl/renderer/buckets/symbol_bucket.hpp | 8 +- .../renderer/layers/render_custom_layer.cpp | 2 +- src/mbgl/renderer/paint_property_binder.hpp | 15 +- src/mbgl/renderer/painter.cpp | 2 +- src/mbgl/renderer/painter.hpp | 8 +- test/programs/binary_program.test.cpp | 16 +- 32 files changed, 369 insertions(+), 312 deletions(-) delete mode 100644 src/mbgl/gl/segment.hpp create mode 100644 src/mbgl/gl/vertex_array.cpp create mode 100644 src/mbgl/gl/vertex_array.hpp rename src/mbgl/{gl => programs}/segment.cpp (66%) create mode 100644 src/mbgl/programs/segment.hpp diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake index d912392e1..4ebb2a878 100644 --- a/cmake/core-files.cmake +++ b/cmake/core-files.cmake @@ -73,8 +73,6 @@ set(MBGL_CORE_FILES src/mbgl/gl/program.hpp src/mbgl/gl/program_binary_extension.hpp src/mbgl/gl/renderbuffer.hpp - src/mbgl/gl/segment.cpp - src/mbgl/gl/segment.hpp src/mbgl/gl/state.hpp src/mbgl/gl/stencil_mode.cpp src/mbgl/gl/stencil_mode.hpp @@ -84,6 +82,8 @@ set(MBGL_CORE_FILES src/mbgl/gl/uniform.hpp src/mbgl/gl/value.cpp src/mbgl/gl/value.hpp + src/mbgl/gl/vertex_array.cpp + src/mbgl/gl/vertex_array.hpp src/mbgl/gl/vertex_array_extension.hpp src/mbgl/gl/vertex_buffer.hpp @@ -154,6 +154,8 @@ set(MBGL_CORE_FILES src/mbgl/programs/programs.hpp src/mbgl/programs/raster_program.cpp src/mbgl/programs/raster_program.hpp + src/mbgl/programs/segment.cpp + src/mbgl/programs/segment.hpp src/mbgl/programs/symbol_program.cpp src/mbgl/programs/symbol_program.hpp src/mbgl/programs/uniforms.hpp @@ -611,8 +613,8 @@ set(MBGL_CORE_FILES src/mbgl/util/stopwatch.cpp src/mbgl/util/stopwatch.hpp src/mbgl/util/string.cpp - src/mbgl/util/thread_local.hpp src/mbgl/util/thread.hpp + src/mbgl/util/thread_local.hpp src/mbgl/util/throttler.cpp src/mbgl/util/throttler.hpp src/mbgl/util/tile_coordinate.hpp diff --git a/src/mbgl/gl/attribute.cpp b/src/mbgl/gl/attribute.cpp index 5583cfd91..bb5b2ddc3 100644 --- a/src/mbgl/gl/attribute.cpp +++ b/src/mbgl/gl/attribute.cpp @@ -1,16 +1,14 @@ #include -#include #include -#include - namespace mbgl { namespace gl { -AttributeLocation bindAttributeLocation(ProgramID id, AttributeLocation location, const char* name) { - assert(location < 8); +void bindAttributeLocation(ProgramID id, AttributeLocation location, const char* name) { + if (location >= MAX_ATTRIBUTES) { + throw gl::Error("too many vertex attributes"); + } MBGL_CHECK_ERROR(glBindAttribLocation(id, location, name)); - return location; } std::set getActiveAttributes(ProgramID id) { @@ -37,58 +35,5 @@ std::set getActiveAttributes(ProgramID id) { return activeAttributes; } -void DisabledAttribute::bind(Context&, AttributeLocation location, std::size_t) const { - MBGL_CHECK_ERROR(glDisableVertexAttribArray(location)); -} - -template -constexpr DataType DataTypeOf() { - return std::is_same::value ? DataType::Byte : - std::is_same::value ? DataType::UnsignedByte : - std::is_same::value ? DataType::Short : - std::is_same::value ? DataType::UnsignedShort : - std::is_same::value ? DataType::Integer : - std::is_same::value ? DataType::UnsignedInteger : - std::is_same::value ? DataType::Float : static_cast(0); -} - -template -void AttributeBinding::bind(Context& context, AttributeLocation location, std::size_t vertexOffset) const { - // FillProgram will attempt to bind at location -1 because it includes - // a fill-outline-color paint property but does not use it or have an - // a_outline_color shader attribute - in this case, we have nothing to bind. - if (location == -1) return; - - context.vertexBuffer = vertexBuffer; - MBGL_CHECK_ERROR(glEnableVertexAttribArray(location)); - MBGL_CHECK_ERROR(glVertexAttribPointer( - location, - static_cast(attributeSize), - static_cast(DataTypeOf()), - static_cast(false), - static_cast(vertexSize), - reinterpret_cast(attributeOffset + (vertexSize * vertexOffset)))); -} - -template class AttributeBinding; -template class AttributeBinding; -template class AttributeBinding; -template class AttributeBinding; - -template class AttributeBinding; -template class AttributeBinding; -template class AttributeBinding; -template class AttributeBinding; - -template class AttributeBinding; -template class AttributeBinding; -template class AttributeBinding; -template class AttributeBinding; - -template class AttributeBinding; -template class AttributeBinding; -template class AttributeBinding; -template class AttributeBinding; - } // namespace gl } // namespace mbgl diff --git a/src/mbgl/gl/attribute.hpp b/src/mbgl/gl/attribute.hpp index bc02b54bd..1f60c8c98 100644 --- a/src/mbgl/gl/attribute.hpp +++ b/src/mbgl/gl/attribute.hpp @@ -1,59 +1,51 @@ #pragma once #include -#include +#include #include #include -#include +#include #include #include #include #include +#include +#include namespace mbgl { namespace gl { -class DisabledAttribute { -public: - void bind(Context&, AttributeLocation, std::size_t vertexOffset) const; +static constexpr std::size_t MAX_ATTRIBUTES = 8; - friend bool operator==(const DisabledAttribute&, - const DisabledAttribute&) { - return true; - } -}; +template struct DataTypeOf; +template <> struct DataTypeOf< int8_t> : std::integral_constant {}; +template <> struct DataTypeOf : std::integral_constant {}; +template <> struct DataTypeOf< int16_t> : std::integral_constant {}; +template <> struct DataTypeOf : std::integral_constant {}; +template <> struct DataTypeOf< int32_t> : std::integral_constant {}; +template <> struct DataTypeOf : std::integral_constant {}; +template <> struct DataTypeOf : std::integral_constant {}; -template class AttributeBinding { public: - AttributeBinding(BufferID vertexBuffer_, - std::size_t vertexSize_, - std::size_t attributeOffset_, - std::size_t attributeSize_ = N) - : vertexBuffer(vertexBuffer_), - vertexSize(vertexSize_), - attributeOffset(attributeOffset_), - attributeSize(attributeSize_) - {} + DataType attributeType; + std::size_t attributeSize; + std::size_t attributeOffset; - void bind(Context&, AttributeLocation, std::size_t vertexOffset) const; + BufferID vertexBuffer; + std::size_t vertexSize; + std::size_t vertexOffset; friend bool operator==(const AttributeBinding& lhs, const AttributeBinding& rhs) { - return lhs.vertexBuffer == rhs.vertexBuffer - && lhs.vertexSize == rhs.vertexSize - && lhs.attributeOffset == rhs.attributeOffset - && lhs.attributeSize == rhs.attributeSize; + return std::tie(lhs.attributeType, lhs.attributeSize, lhs.attributeOffset, lhs.vertexBuffer, lhs.vertexSize, lhs.vertexOffset) + == std::tie(rhs.attributeType, rhs.attributeSize, rhs.attributeOffset, rhs.vertexBuffer, rhs.vertexSize, rhs.vertexOffset); } - -private: - BufferID vertexBuffer; - std::size_t vertexSize; - std::size_t attributeOffset; - std::size_t attributeSize; }; +using AttributeBindingArray = std::array, MAX_ATTRIBUTES>; + /* gl::Attribute manages the binding of a vertex buffer to a GL program attribute. - T is the underlying primitive type (exposed as Attribute::ValueType) @@ -67,10 +59,7 @@ public: using Value = std::array; using Location = AttributeLocation; - - using Binding = variant< - DisabledAttribute, - AttributeBinding>; + using Binding = AttributeBinding; /* Create a binding for this attribute. The `attributeSize` parameter may be used to @@ -82,28 +71,24 @@ public: std::size_t attributeIndex, std::size_t attributeSize = N) { static_assert(std::is_standard_layout::value, "vertex type must use standard layout"); - return AttributeBinding { + return AttributeBinding { + DataTypeOf::value, + attributeSize, + Vertex::attributeOffsets[attributeIndex], buffer.buffer, sizeof(Vertex), - Vertex::attributeOffsets[attributeIndex], - attributeSize + 0, }; } - static void bind(Context& context, - const Location& location, - Binding& oldBinding, - const Binding& newBinding, - std::size_t vertexOffset) { - if (oldBinding == newBinding) { - return; + static optional offsetBinding(const optional& binding, std::size_t vertexOffset) { + if (binding) { + AttributeBinding result = *binding; + result.vertexOffset = vertexOffset; + return result; + } else { + return binding; } - - Binding::visit(newBinding, [&] (const auto& binding) { - binding.bind(context, location, vertexOffset); - }); - - oldBinding = newBinding; } }; @@ -223,7 +208,7 @@ const std::size_t Vertex::attributeOffsets[5] = { } // namespace detail -AttributeLocation bindAttributeLocation(ProgramID, AttributeLocation, const char * name); +void bindAttributeLocation(ProgramID, AttributeLocation, const char * name); std::set getActiveAttributes(ProgramID); template @@ -232,10 +217,10 @@ public: using Types = TypeList; using Locations = IndexedTuple< TypeList, - TypeList>; + TypeList...>>; using Bindings = IndexedTuple< TypeList, - TypeList>; + TypeList...>>; using NamedLocations = std::vector>; using Vertex = detail::Vertex; @@ -243,13 +228,17 @@ public: static Locations bindLocations(const ProgramID& id) { std::set activeAttributes = getActiveAttributes(id); - AttributeLocation location = -1; - auto bindAndIncrement = [&](const char* name) { - location++; - return bindAttributeLocation(id, location, name); + AttributeLocation location = 0; + auto maybeBindLocation = [&](const char* name) -> optional { + if (activeAttributes.count(name)) { + bindAttributeLocation(id, location, name); + return location++; + } else { + return {}; + } }; - return Locations{ (activeAttributes.count(As::name()) ? bindAndIncrement(As::name()) - : -1)... }; + + return Locations { maybeBindLocation(As::name())... }; } template @@ -258,7 +247,17 @@ public: } static NamedLocations getNamedLocations(const Locations& locations) { - return NamedLocations{ { As::name(), locations.template get() }... }; + NamedLocations result; + + auto maybeAddLocation = [&] (const std::string& name, const optional& location) { + if (location) { + result.emplace_back(name, *location); + } + }; + + util::ignore({ (maybeAddLocation(As::name(), locations.template get()), 0)... }); + + return result; } template @@ -266,16 +265,23 @@ public: return Bindings { As::Type::binding(buffer, TypeIndex::value)... }; } - static void bind(Context& context, - const Locations& locations, - Bindings& oldBindings, - const Bindings& newBindings, - std::size_t vertexOffset) { - util::ignore({ (As::Type::bind(context, - locations.template get(), - oldBindings.template get(), - newBindings.template get(), - vertexOffset), 0)... }); + static Bindings offsetBindings(const Bindings& bindings, std::size_t vertexOffset) { + return Bindings { As::Type::offsetBinding(bindings.template get(), vertexOffset)... }; + } + + static AttributeBindingArray toBindingArray(const Locations& locations, const Bindings& bindings) { + AttributeBindingArray result; + + auto maybeAddBinding = [&] (const optional& location, + const optional& binding) { + if (location) { + result.at(*location) = binding; + } + }; + + util::ignore({ (maybeAddBinding(locations.template get(), bindings.template get()), 0)... }); + + return result; } }; diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp index 86ca9c060..f7d210d52 100644 --- a/src/mbgl/gl/context.cpp +++ b/src/mbgl/gl/context.cpp @@ -229,8 +229,8 @@ UniqueBuffer Context::createIndexBuffer(const void* data, std::size_t size) { BufferID id = 0; MBGL_CHECK_ERROR(glGenBuffers(1, &id)); UniqueBuffer result { std::move(id), { this } }; - vertexArrayObject = 0; - elementBuffer = result; + bindVertexArray = 0; + globalVertexArrayState.indexBuffer = result; MBGL_CHECK_ERROR(glBufferData(GL_ELEMENT_ARRAY_BUFFER, size, data, GL_STATIC_DRAW)); return result; } @@ -281,11 +281,16 @@ optional> Context::getBinaryProgram( } #endif -UniqueVertexArray Context::createVertexArray() { - assert(supportsVertexArrays()); - VertexArrayID id = 0; - MBGL_CHECK_ERROR(vertexArray->genVertexArrays(1, &id)); - return UniqueVertexArray(std::move(id), { this }); +VertexArray Context::createVertexArray() { + if (supportsVertexArrays()) { + VertexArrayID id = 0; + MBGL_CHECK_ERROR(vertexArray->genVertexArrays(1, &id)); + return { std::make_unique(UniqueVertexArray(std::move(id), { this }), *this) }; + } 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. + return { UniqueVertexArrayState(&globalVertexArrayState, [] (VertexArrayState*) {}) }; + } } UniqueFramebuffer Context::createFramebuffer() { @@ -553,8 +558,8 @@ void Context::setDirtyState() { tex.setDirty(); } vertexBuffer.setDirty(); - elementBuffer.setDirty(); - vertexArrayObject.setDirty(); + bindVertexArray.setDirty(); + globalVertexArrayState.setDirty(); } void Context::clear(optional color, @@ -673,8 +678,8 @@ void Context::performCleanup() { for (const auto id : abandonedBuffers) { if (vertexBuffer == id) { vertexBuffer.setDirty(); - } else if (elementBuffer == id) { - elementBuffer.setDirty(); + } else if (globalVertexArrayState.indexBuffer == id) { + globalVertexArrayState.indexBuffer.setDirty(); } } MBGL_CHECK_ERROR(glDeleteBuffers(int(abandonedBuffers.size()), abandonedBuffers.data())); @@ -694,8 +699,8 @@ void Context::performCleanup() { if (!abandonedVertexArrays.empty()) { assert(supportsVertexArrays()); for (const auto id : abandonedVertexArrays) { - if (vertexArrayObject == id) { - vertexArrayObject.setDirty(); + if (bindVertexArray == id) { + bindVertexArray.setDirty(); } } MBGL_CHECK_ERROR(vertexArray->deleteVertexArrays(int(abandonedVertexArrays.size()), diff --git a/src/mbgl/gl/context.hpp b/src/mbgl/gl/context.hpp index 2e594618d..0dd67d2ed 100644 --- a/src/mbgl/gl/context.hpp +++ b/src/mbgl/gl/context.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -53,9 +54,7 @@ public: void verifyProgramLinkage(ProgramID); void linkProgram(ProgramID); UniqueTexture createTexture(); - - bool supportsVertexArrays() const; - UniqueVertexArray createVertexArray(); + VertexArray createVertexArray(); #if MBGL_HAS_BINARY_PROGRAMS bool supportsProgramBinaries() const; @@ -207,10 +206,11 @@ public: State viewport; State scissorTest; std::array, 2> texture; - State vertexArrayObject { *this }; State program; State vertexBuffer; - State elementBuffer; + + State bindVertexArray { *this }; + VertexArrayState globalVertexArrayState { UniqueVertexArray(0, { this }), *this }; State pixelStorePack; State pixelStoreUnpack; @@ -257,6 +257,8 @@ private: void drawPixels(Size size, const void* data, TextureFormat); #endif // MBGL_USE_GLES2 + bool supportsVertexArrays() const; + friend detail::ProgramDeleter; friend detail::ShaderDeleter; friend detail::BufferDeleter; diff --git a/src/mbgl/gl/object.cpp b/src/mbgl/gl/object.cpp index e2d476e0c..2c5f1bca1 100644 --- a/src/mbgl/gl/object.cpp +++ b/src/mbgl/gl/object.cpp @@ -33,7 +33,9 @@ void TextureDeleter::operator()(TextureID id) const { void VertexArrayDeleter::operator()(VertexArrayID id) const { assert(context); - context->abandonedVertexArrays.push_back(id); + if (id != 0) { + context->abandonedVertexArrays.push_back(id); + } } void FramebufferDeleter::operator()(FramebufferID id) const { diff --git a/src/mbgl/gl/program.hpp b/src/mbgl/gl/program.hpp index ce2b5a335..9d8b0a5b0 100644 --- a/src/mbgl/gl/program.hpp +++ b/src/mbgl/gl/program.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -116,10 +117,12 @@ public: DepthMode depthMode, StencilMode stencilMode, ColorMode colorMode, - UniformValues&& uniformValues, - AttributeBindings&& attributeBindings, + const UniformValues& uniformValues, + VertexArray& vertexArray, + const AttributeBindings& attributeBindings, const IndexBuffer& indexBuffer, - const SegmentVector& segments) { + std::size_t indexOffset, + std::size_t indexLength) { static_assert(std::is_same::value, "incompatible draw mode"); context.setDrawMode(drawMode); @@ -129,18 +132,15 @@ public: context.program = program; - Uniforms::bind(uniformsState, std::move(uniformValues)); + Uniforms::bind(uniformsState, uniformValues); - for (const auto& segment : segments) { - segment.bind(context, - indexBuffer.buffer, - attributeLocations, - attributeBindings); + vertexArray.bind(context, + indexBuffer.buffer, + Attributes::toBindingArray(attributeLocations, attributeBindings)); - context.draw(drawMode.primitiveType, - segment.indexOffset, - segment.indexLength); - } + context.draw(drawMode.primitiveType, + indexOffset, + indexLength); } private: diff --git a/src/mbgl/gl/segment.hpp b/src/mbgl/gl/segment.hpp deleted file mode 100644 index fe0658bf8..000000000 --- a/src/mbgl/gl/segment.hpp +++ /dev/null @@ -1,73 +0,0 @@ -#pragma once - -#include -#include -#include -#include - -#include -#include - -namespace mbgl { -namespace gl { - -template -class Segment { -public: - Segment(std::size_t vertexOffset_, - std::size_t indexOffset_, - std::size_t vertexLength_ = 0, - std::size_t indexLength_ = 0) - : vertexOffset(vertexOffset_), - indexOffset(indexOffset_), - vertexLength(vertexLength_), - indexLength(indexLength_) {} - - const std::size_t vertexOffset; - const std::size_t indexOffset; - - std::size_t vertexLength; - std::size_t indexLength; - - void bind(Context& context, - BufferID indexBuffer_, - const typename Attributes::Locations& attributeLocations, - const typename Attributes::Bindings& attributeBindings_) const { - if (context.supportsVertexArrays()) { - if (!vao) { - vao = context.createVertexArray(); - context.vertexBuffer.setDirty(); - } - context.vertexArrayObject = *vao; - if (indexBuffer != indexBuffer_) { - indexBuffer = indexBuffer_; - context.elementBuffer.setDirty(); - context.elementBuffer = indexBuffer_; - } - } else { - // No VAO support. Force attributes to be rebound. - context.elementBuffer = indexBuffer_; - attributeBindings = {}; - } - - Attributes::bind(context, - attributeLocations, - attributeBindings, - attributeBindings_, - vertexOffset); - } - -private: - mutable optional vao; - mutable optional indexBuffer; - mutable typename Attributes::Bindings attributeBindings; -}; - -template -class SegmentVector : public std::vector> { -public: - SegmentVector() = default; -}; - -} // namespace gl -} // namespace mbgl diff --git a/src/mbgl/gl/types.hpp b/src/mbgl/gl/types.hpp index 1fce878c6..058bc226b 100644 --- a/src/mbgl/gl/types.hpp +++ b/src/mbgl/gl/types.hpp @@ -15,7 +15,7 @@ using VertexArrayID = uint32_t; using FramebufferID = uint32_t; using RenderbufferID = uint32_t; -using AttributeLocation = int32_t; +using AttributeLocation = uint32_t; using UniformLocation = int32_t; using TextureUnit = uint8_t; diff --git a/src/mbgl/gl/uniform.hpp b/src/mbgl/gl/uniform.hpp index 4ed241976..5a78068fc 100644 --- a/src/mbgl/gl/uniform.hpp +++ b/src/mbgl/gl/uniform.hpp @@ -115,7 +115,7 @@ public: return NamedLocations{ { Us::name(), state.template get().location }... }; } - static void bind(State& state, Values&& values) { + static void bind(State& state, const Values& values) { util::ignore({ (state.template get() = values.template get(), 0)... }); } }; diff --git a/src/mbgl/gl/value.cpp b/src/mbgl/gl/value.cpp index 2b825fb2b..89014fe6b 100644 --- a/src/mbgl/gl/value.cpp +++ b/src/mbgl/gl/value.cpp @@ -365,6 +365,24 @@ BindVertexArray::Type BindVertexArray::Get(const Context& context) { return binding; } +const optional VertexAttribute::Default {}; + +void VertexAttribute::Set(const optional& binding, Context& context, AttributeLocation location) { + if (binding) { + context.vertexBuffer = binding->vertexBuffer; + MBGL_CHECK_ERROR(glEnableVertexAttribArray(location)); + MBGL_CHECK_ERROR(glVertexAttribPointer( + location, + static_cast(binding->attributeSize), + static_cast(binding->attributeType), + static_cast(false), + static_cast(binding->vertexSize), + reinterpret_cast(binding->attributeOffset + (binding->vertexSize * binding->vertexOffset)))); + } else { + MBGL_CHECK_ERROR(glDisableVertexAttribArray(location)); + } +} + const constexpr PixelStorePack::Type PixelStorePack::Default; void PixelStorePack::Set(const Type& value) { diff --git a/src/mbgl/gl/value.hpp b/src/mbgl/gl/value.hpp index d4c7b5cdc..19e9af194 100644 --- a/src/mbgl/gl/value.hpp +++ b/src/mbgl/gl/value.hpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -239,6 +240,12 @@ struct BindVertexArray { static Type Get(const Context&); }; +struct VertexAttribute { + using Type = optional; + static const Type Default; + static void Set(const Type&, Context&, AttributeLocation); +}; + struct PixelStorePack { using Type = PixelStorageType; static const constexpr Type Default = { 4 }; diff --git a/src/mbgl/gl/vertex_array.cpp b/src/mbgl/gl/vertex_array.cpp new file mode 100644 index 000000000..68a500ac4 --- /dev/null +++ b/src/mbgl/gl/vertex_array.cpp @@ -0,0 +1,18 @@ +#include +#include +#include + +namespace mbgl { +namespace gl { + +void VertexArray::bind(Context& context, BufferID indexBuffer, const AttributeBindingArray& bindings) { + context.bindVertexArray = state->vertexArray; + state->indexBuffer = indexBuffer; + + for (AttributeLocation location = 0; location < MAX_ATTRIBUTES; ++location) { + state->bindings[location] = bindings[location]; + } +} + +} // namespace gl +} // namespace mbgl diff --git a/src/mbgl/gl/vertex_array.hpp b/src/mbgl/gl/vertex_array.hpp new file mode 100644 index 000000000..9ccf48d9b --- /dev/null +++ b/src/mbgl/gl/vertex_array.hpp @@ -0,0 +1,58 @@ +#pragma once + +#include +#include +#include +#include + +#include +#include + +namespace mbgl { +namespace gl { + +class Context; + +class VertexArrayState { +public: + VertexArrayState(UniqueVertexArray vertexArray_, Context& context) + : vertexArray(std::move(vertexArray_)), + bindings(makeBindings(context, std::make_index_sequence())) { + } + + void setDirty() { + indexBuffer.setDirty(); + for (auto& binding : bindings) { + binding.setDirty(); + } + } + + UniqueVertexArray vertexArray; + State indexBuffer; + + using AttributeState = State; + std::array bindings; + +private: + template + std::array makeBindings(Context& context, std::index_sequence) { + return {{ AttributeState { context, I }... }}; + } +}; + +using UniqueVertexArrayState = std::unique_ptr>; + +class VertexArray { +public: + VertexArray(UniqueVertexArrayState state_) + : state(std::move(state_)) { + } + + void bind(Context&, BufferID indexBuffer, const AttributeBindingArray&); + +private: + UniqueVertexArrayState state; +}; + +} // namespace gl +} // namespace mbgl diff --git a/src/mbgl/programs/binary_program.cpp b/src/mbgl/programs/binary_program.cpp index 57f2cc0d2..da629194b 100644 --- a/src/mbgl/programs/binary_program.cpp +++ b/src/mbgl/programs/binary_program.cpp @@ -98,7 +98,7 @@ std::string BinaryProgram::serialize() const { return data; } -gl::AttributeLocation BinaryProgram::attributeLocation(const std::string& name) const { +optional BinaryProgram::attributeLocation(const std::string& name) const { for (const auto& pair : attributes) { if (pair.first == name) { return pair.second; @@ -113,7 +113,7 @@ gl::UniformLocation BinaryProgram::uniformLocation(const std::string& name) cons return pair.second; } } - return {}; + return -1; } } // namespace mbgl diff --git a/src/mbgl/programs/binary_program.hpp b/src/mbgl/programs/binary_program.hpp index b77cf1a51..8690f3fd6 100644 --- a/src/mbgl/programs/binary_program.hpp +++ b/src/mbgl/programs/binary_program.hpp @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include @@ -29,7 +30,8 @@ public: const std::string& identifier() const { return binaryIdentifier; } - gl::AttributeLocation attributeLocation(const std::string& name) const; + + optional attributeLocation(const std::string& name) const; gl::UniformLocation uniformLocation(const std::string& name) const; private: diff --git a/src/mbgl/programs/program.hpp b/src/mbgl/programs/program.hpp index 3a38f30a8..0199752b0 100644 --- a/src/mbgl/programs/program.hpp +++ b/src/mbgl/programs/program.hpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -51,26 +52,37 @@ public: gl::DepthMode depthMode, gl::StencilMode stencilMode, gl::ColorMode colorMode, - UniformValues&& uniformValues, + const UniformValues& uniformValues, const gl::VertexBuffer& layoutVertexBuffer, const gl::IndexBuffer& indexBuffer, - const gl::SegmentVector& segments, + const SegmentVector& segments, const PaintPropertyBinders& paintPropertyBinders, const typename PaintProperties::PossiblyEvaluated& currentProperties, float currentZoom) { - program.draw( - context, - std::move(drawMode), - std::move(depthMode), - std::move(stencilMode), - std::move(colorMode), - uniformValues - .concat(paintPropertyBinders.uniformValues(currentZoom, currentProperties)), - LayoutAttributes::bindings(layoutVertexBuffer) - .concat(paintPropertyBinders.attributeBindings(currentProperties)), - indexBuffer, - segments - ); + 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) { + if (!segment.vertexArray) { + segment.vertexArray = context.createVertexArray(); + } + + program.draw( + context, + std::move(drawMode), + std::move(depthMode), + std::move(stencilMode), + std::move(colorMode), + allUniformValues, + *segment.vertexArray, + Attributes::offsetBindings(allAttributeBindings, segment.vertexOffset), + indexBuffer, + segment.indexOffset, + segment.indexLength); + } } }; diff --git a/src/mbgl/gl/segment.cpp b/src/mbgl/programs/segment.cpp similarity index 66% rename from src/mbgl/gl/segment.cpp rename to src/mbgl/programs/segment.cpp index aabdc83cd..bb09843e2 100644 --- a/src/mbgl/gl/segment.cpp +++ b/src/mbgl/programs/segment.cpp @@ -1,4 +1,4 @@ -#include +#include namespace mbgl { namespace gl { diff --git a/src/mbgl/programs/segment.hpp b/src/mbgl/programs/segment.hpp new file mode 100644 index 000000000..d8cc9679d --- /dev/null +++ b/src/mbgl/programs/segment.hpp @@ -0,0 +1,40 @@ +#pragma once + +#include +#include +#include + +#include +#include +#include + +namespace mbgl { + +template +class Segment { +public: + Segment(std::size_t vertexOffset_, + std::size_t indexOffset_, + std::size_t vertexLength_ = 0, + std::size_t indexLength_ = 0) + : vertexOffset(vertexOffset_), + indexOffset(indexOffset_), + vertexLength(vertexLength_), + indexLength(indexLength_) {} + + const std::size_t vertexOffset; + const std::size_t indexOffset; + + std::size_t vertexLength; + std::size_t indexLength; + + mutable optional vertexArray; +}; + +template +class SegmentVector : public std::vector> { +public: + SegmentVector() = default; +}; + +} // namespace mbgl diff --git a/src/mbgl/programs/symbol_program.hpp b/src/mbgl/programs/symbol_program.hpp index c74837b12..8c59cfd5a 100644 --- a/src/mbgl/programs/symbol_program.hpp +++ b/src/mbgl/programs/symbol_program.hpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -306,30 +307,41 @@ public: gl::DepthMode depthMode, gl::StencilMode stencilMode, gl::ColorMode colorMode, - UniformValues&& uniformValues, + const UniformValues& uniformValues, const gl::VertexBuffer& layoutVertexBuffer, const gl::VertexBuffer& dynamicLayoutVertexBuffer, const SymbolSizeBinder& symbolSizeBinder, const gl::IndexBuffer& indexBuffer, - const gl::SegmentVector& segments, + const SegmentVector& segments, const PaintPropertyBinders& paintPropertyBinders, const typename PaintProperties::PossiblyEvaluated& currentProperties, float currentZoom) { - program.draw( - context, - std::move(drawMode), - std::move(depthMode), - std::move(stencilMode), - std::move(colorMode), - uniformValues - .concat(symbolSizeBinder.uniformValues(currentZoom)) - .concat(paintPropertyBinders.uniformValues(currentZoom, currentProperties)), - LayoutAttributes::bindings(layoutVertexBuffer) - .concat(SymbolDynamicLayoutAttributes::bindings(dynamicLayoutVertexBuffer)) - .concat(paintPropertyBinders.attributeBindings(currentProperties)), - indexBuffer, - segments - ); + 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(paintPropertyBinders.attributeBindings(currentProperties)); + + for (auto& segment : segments) { + if (!segment.vertexArray) { + segment.vertexArray = context.createVertexArray(); + } + + program.draw( + context, + std::move(drawMode), + std::move(depthMode), + std::move(stencilMode), + std::move(colorMode), + allUniformValues, + *segment.vertexArray, + Attributes::offsetBindings(allAttributeBindings, segment.vertexOffset), + indexBuffer, + segment.indexOffset, + segment.indexLength); + } } }; diff --git a/src/mbgl/renderer/buckets/circle_bucket.hpp b/src/mbgl/renderer/buckets/circle_bucket.hpp index b048fd767..0f27e2a7e 100644 --- a/src/mbgl/renderer/buckets/circle_bucket.hpp +++ b/src/mbgl/renderer/buckets/circle_bucket.hpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #include @@ -29,7 +29,7 @@ public: gl::VertexVector vertices; gl::IndexVector triangles; - gl::SegmentVector segments; + SegmentVector segments; optional> vertexBuffer; optional> indexBuffer; diff --git a/src/mbgl/renderer/buckets/debug_bucket.hpp b/src/mbgl/renderer/buckets/debug_bucket.hpp index 756e58a6d..fc3128e94 100644 --- a/src/mbgl/renderer/buckets/debug_bucket.hpp +++ b/src/mbgl/renderer/buckets/debug_bucket.hpp @@ -33,7 +33,7 @@ public: const optional expires; const MapDebugOptions debugMode; - gl::SegmentVector segments; + SegmentVector segments; optional> vertexBuffer; optional> indexBuffer; }; diff --git a/src/mbgl/renderer/buckets/fill_bucket.hpp b/src/mbgl/renderer/buckets/fill_bucket.hpp index 421d8b332..d3cd92d45 100644 --- a/src/mbgl/renderer/buckets/fill_bucket.hpp +++ b/src/mbgl/renderer/buckets/fill_bucket.hpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include @@ -30,8 +30,8 @@ public: gl::VertexVector vertices; gl::IndexVector lines; gl::IndexVector triangles; - gl::SegmentVector lineSegments; - gl::SegmentVector triangleSegments; + SegmentVector lineSegments; + SegmentVector triangleSegments; optional> vertexBuffer; optional> lineIndexBuffer; diff --git a/src/mbgl/renderer/buckets/fill_extrusion_bucket.hpp b/src/mbgl/renderer/buckets/fill_extrusion_bucket.hpp index c54805d74..d1e695c5a 100644 --- a/src/mbgl/renderer/buckets/fill_extrusion_bucket.hpp +++ b/src/mbgl/renderer/buckets/fill_extrusion_bucket.hpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include @@ -27,7 +27,7 @@ public: gl::VertexVector vertices; gl::IndexVector triangles; - gl::SegmentVector triangleSegments; + SegmentVector triangleSegments; optional> vertexBuffer; optional> indexBuffer; diff --git a/src/mbgl/renderer/buckets/line_bucket.hpp b/src/mbgl/renderer/buckets/line_bucket.hpp index 34d893595..c0a0614d3 100644 --- a/src/mbgl/renderer/buckets/line_bucket.hpp +++ b/src/mbgl/renderer/buckets/line_bucket.hpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include @@ -34,7 +34,7 @@ public: gl::VertexVector vertices; gl::IndexVector triangles; - gl::SegmentVector segments; + SegmentVector segments; optional> vertexBuffer; optional> indexBuffer; diff --git a/src/mbgl/renderer/buckets/raster_bucket.hpp b/src/mbgl/renderer/buckets/raster_bucket.hpp index b5cf7997d..44b9111b8 100644 --- a/src/mbgl/renderer/buckets/raster_bucket.hpp +++ b/src/mbgl/renderer/buckets/raster_bucket.hpp @@ -31,7 +31,7 @@ public: // Raster Tile Sources use the default buffers from Painter gl::VertexVector vertices; gl::IndexVector indices; - gl::SegmentVector segments; + SegmentVector segments; optional> vertexBuffer; optional> indexBuffer; diff --git a/src/mbgl/renderer/buckets/symbol_bucket.hpp b/src/mbgl/renderer/buckets/symbol_bucket.hpp index ffa22e902..424c9d0f7 100644 --- a/src/mbgl/renderer/buckets/symbol_bucket.hpp +++ b/src/mbgl/renderer/buckets/symbol_bucket.hpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include #include @@ -63,7 +63,7 @@ public: gl::VertexVector vertices; gl::VertexVector dynamicVertices; gl::IndexVector triangles; - gl::SegmentVector segments; + SegmentVector segments; std::vector placedSymbols; optional> vertexBuffer; @@ -77,7 +77,7 @@ public: gl::VertexVector vertices; gl::VertexVector dynamicVertices; gl::IndexVector triangles; - gl::SegmentVector segments; + SegmentVector segments; std::vector placedSymbols; PremultipliedImage atlasImage; @@ -89,7 +89,7 @@ public: struct CollisionBoxBuffer { gl::VertexVector vertices; gl::IndexVector lines; - gl::SegmentVector segments; + SegmentVector segments; optional> vertexBuffer; optional> dynamicVertexBuffer; diff --git a/src/mbgl/renderer/layers/render_custom_layer.cpp b/src/mbgl/renderer/layers/render_custom_layer.cpp index 4d6084075..81b8ac301 100644 --- a/src/mbgl/renderer/layers/render_custom_layer.cpp +++ b/src/mbgl/renderer/layers/render_custom_layer.cpp @@ -48,7 +48,7 @@ void RenderCustomLayer::render(Painter& painter, PaintParameters& paintParameter const TransformState& state = painter.state; // Reset GL state to a known state so the CustomLayer always has a clean slate. - context.vertexArrayObject = 0; + context.bindVertexArray = 0; context.setDepthMode(painter.depthModeForSublayer(0, gl::DepthMode::ReadOnly)); context.setStencilMode(gl::StencilMode::disabled()); context.setColorMode(painter.colorModeForRenderPass()); diff --git a/src/mbgl/renderer/paint_property_binder.hpp b/src/mbgl/renderer/paint_property_binder.hpp index f78147fc8..652948c8d 100644 --- a/src/mbgl/renderer/paint_property_binder.hpp +++ b/src/mbgl/renderer/paint_property_binder.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -81,7 +82,7 @@ public: virtual void populateVertexVector(const GeometryTileFeature& feature, std::size_t length) = 0; virtual void upload(gl::Context& context) = 0; - virtual AttributeBinding attributeBinding(const PossiblyEvaluatedPropertyValue& currentValue) const = 0; + virtual optional attributeBinding(const PossiblyEvaluatedPropertyValue& currentValue) const = 0; virtual float interpolationFactor(float currentZoom) const = 0; virtual T uniformValue(const PossiblyEvaluatedPropertyValue& currentValue) const = 0; @@ -103,8 +104,8 @@ public: void populateVertexVector(const GeometryTileFeature&, std::size_t) override {} void upload(gl::Context&) override {} - AttributeBinding attributeBinding(const PossiblyEvaluatedPropertyValue&) const override { - return gl::DisabledAttribute(); + optional attributeBinding(const PossiblyEvaluatedPropertyValue&) const override { + return {}; } float interpolationFactor(float) const override { @@ -147,9 +148,9 @@ public: vertexBuffer = context.createVertexBuffer(std::move(vertexVector)); } - AttributeBinding attributeBinding(const PossiblyEvaluatedPropertyValue& currentValue) const override { + optional attributeBinding(const PossiblyEvaluatedPropertyValue& currentValue) const override { if (currentValue.isConstant()) { - return gl::DisabledAttribute(); + return {}; } else { return Attribute::binding(*vertexBuffer, 0, BaseAttribute::Dimensions); } @@ -208,9 +209,9 @@ public: vertexBuffer = context.createVertexBuffer(std::move(vertexVector)); } - AttributeBinding attributeBinding(const PossiblyEvaluatedPropertyValue& currentValue) const override { + optional attributeBinding(const PossiblyEvaluatedPropertyValue& currentValue) const override { if (currentValue.isConstant()) { - return gl::DisabledAttribute(); + return {}; } else { return Attribute::binding(*vertexBuffer, 0); } diff --git a/src/mbgl/renderer/painter.cpp b/src/mbgl/renderer/painter.cpp index a3decdb60..35d03e7cb 100644 --- a/src/mbgl/renderer/painter.cpp +++ b/src/mbgl/renderer/painter.cpp @@ -262,7 +262,7 @@ void Painter::render(RenderStyle& style, const FrameData& frame_, View& view) { context.activeTexture = 0; context.texture[0] = 0; - context.vertexArrayObject = 0; + context.bindVertexArray = 0; } } diff --git a/src/mbgl/renderer/painter.hpp b/src/mbgl/renderer/painter.hpp index 7e966adef..0b6ee3497 100644 --- a/src/mbgl/renderer/painter.hpp +++ b/src/mbgl/renderer/painter.hpp @@ -172,10 +172,10 @@ public: gl::IndexBuffer quadTriangleIndexBuffer; gl::IndexBuffer tileBorderIndexBuffer; - gl::SegmentVector tileTriangleSegments; - gl::SegmentVector tileBorderSegments; - gl::SegmentVector rasterSegments; - gl::SegmentVector extrusionTextureSegments; + SegmentVector tileTriangleSegments; + SegmentVector tileBorderSegments; + SegmentVector rasterSegments; + SegmentVector extrusionTextureSegments; }; } // namespace mbgl diff --git a/test/programs/binary_program.test.cpp b/test/programs/binary_program.test.cpp index ce544e765..a5cf7b6e3 100644 --- a/test/programs/binary_program.test.cpp +++ b/test/programs/binary_program.test.cpp @@ -14,12 +14,12 @@ TEST(BinaryProgram, ObtainValues) { EXPECT_EQ(42u, binaryProgram.format()); EXPECT_EQ("binary code", binaryProgram.code()); EXPECT_EQ("identifier", binaryProgram.identifier()); - EXPECT_EQ(1, binaryProgram.attributeLocation("a_pos")); - EXPECT_EQ(0, binaryProgram.attributeLocation("u_world")); - EXPECT_EQ(4, binaryProgram.attributeLocation("a_data")); + EXPECT_EQ(1u, binaryProgram.attributeLocation("a_pos")); + EXPECT_FALSE(binaryProgram.attributeLocation("u_world")); + EXPECT_EQ(4u, binaryProgram.attributeLocation("a_data")); EXPECT_EQ(1, binaryProgram.uniformLocation("u_world")); EXPECT_EQ(3, binaryProgram.uniformLocation("u_ratio")); - EXPECT_EQ(0, binaryProgram.uniformLocation("a_data")); + EXPECT_EQ(-1, binaryProgram.uniformLocation("a_data")); auto serialized = binaryProgram.serialize(); @@ -28,12 +28,12 @@ TEST(BinaryProgram, ObtainValues) { EXPECT_EQ(42u, binaryProgram2.format()); EXPECT_EQ("binary code", binaryProgram2.code()); EXPECT_EQ("identifier", binaryProgram2.identifier()); - EXPECT_EQ(1, binaryProgram2.attributeLocation("a_pos")); - EXPECT_EQ(0, binaryProgram2.attributeLocation("u_world")); - EXPECT_EQ(4, binaryProgram2.attributeLocation("a_data")); + EXPECT_EQ(1u, binaryProgram2.attributeLocation("a_pos")); + EXPECT_FALSE(binaryProgram2.attributeLocation("u_world")); + EXPECT_EQ(4u, binaryProgram2.attributeLocation("a_data")); EXPECT_EQ(1, binaryProgram2.uniformLocation("u_world")); EXPECT_EQ(3, binaryProgram2.uniformLocation("u_ratio")); - EXPECT_EQ(0, binaryProgram2.uniformLocation("a_data")); + EXPECT_EQ(-1, binaryProgram2.uniformLocation("a_data")); EXPECT_THROW(BinaryProgram(""), std::runtime_error); }