Fix update time not being set when only drawable indexes are set (#2743)

src/mbgl/gl/drawable_gl.cpp

@@ -158,17 +158,28 @@ void DrawableGL::upload(gfx::UploadPass& uploadPass) {
     }
 #endif
 
-    const bool build = vertexAttributes &&
-                       (vertexAttributes->isModifiedAfter(attributeUpdateTime) ||
-                        std::any_of(impl->segments.begin(), impl->segments.end(), [](const auto& seg) {
-                            return !static_cast<const DrawSegmentGL&>(*seg).getVertexArray().isValid();
-                        }));
+    auto& context = uploadPass.getContext();
+    auto& glContext = static_cast<gl::Context&>(context);
+    constexpr auto usage = gfx::BufferUsageType::StaticDraw;
 
-    if (build) {
+    // Create an index buffer if necessary
+    if (impl->indexes) {
+        impl->indexes->updateModified();
+    }
+    if (impl->indexes && (!impl->indexes->getBuffer() || impl->indexes->isModifiedAfter(attributeUpdateTime))) {
+        MLN_TRACE_ZONE(build indexes);
+        auto indexBufferResource{
+            uploadPass.createIndexBufferResource(impl->indexes->data(), impl->indexes->bytes(), usage)};
+        auto indexBuffer = std::make_unique<gfx::IndexBuffer>(impl->indexes->elements(),
+                                                              std::move(indexBufferResource));
+        auto buffer = std::make_unique<IndexBufferGL>(std::move(indexBuffer));
+        impl->indexes->setBuffer(std::move(buffer));
+    }
+
+    // Build the vertex attributes and bindings, if necessary
+    if (impl->attributeBindings.empty() ||
+        (vertexAttributes && vertexAttributes->isModifiedAfter(attributeUpdateTime))) {
         MLN_TRACE_ZONE(build attributes);
-        auto& context = uploadPass.getContext();
-        auto& glContext = static_cast<gl::Context&>(context);
-        constexpr auto usage = gfx::BufferUsageType::StaticDraw;
 
         // Apply drawable values to shader defaults
         const auto& defaults = shader->getVertexAttributes();
@@ -178,68 +189,54 @@ void DrawableGL::upload(gfx::UploadPass& uploadPass) {
         const auto vertexAttributeIndex = static_cast<std::size_t>(indexAttribute ? indexAttribute->getIndex() : -1);
 
         std::vector<std::unique_ptr<gfx::VertexBufferResource>> vertexBuffers;
-        auto bindings = uploadPass.buildAttributeBindings(impl->vertexCount,
-                                                          impl->vertexType,
-                                                          vertexAttributeIndex,
-                                                          impl->vertexData,
-                                                          defaults,
-                                                          overrides,
-                                                          usage,
-                                                          attributeUpdateTime,
-                                                          vertexBuffers);
+        impl->attributeBindings = uploadPass.buildAttributeBindings(impl->vertexCount,
+                                                                    impl->vertexType,
+                                                                    vertexAttributeIndex,
+                                                                    impl->vertexData,
+                                                                    defaults,
+                                                                    overrides,
+                                                                    usage,
+                                                                    attributeUpdateTime,
+                                                                    vertexBuffers);
 
         impl->attributeBuffers = std::move(vertexBuffers);
-
-        if (impl->indexes) {
-            impl->indexes->updateModified();
-        }
-        if (!impl->indexes->getBuffer() || impl->indexes->isModifiedAfter(attributeUpdateTime)) {
-            MLN_TRACE_ZONE(build indexes);
-            auto indexBufferResource{
-                uploadPass.createIndexBufferResource(impl->indexes->data(), impl->indexes->bytes(), usage)};
-            auto indexBuffer = std::make_unique<gfx::IndexBuffer>(impl->indexes->elements(),
-                                                                  std::move(indexBufferResource));
-            auto buffer = std::make_unique<IndexBufferGL>(std::move(indexBuffer));
-
-            impl->indexes->setBuffer(std::move(buffer));
-        }
-
-        // Create a VAO for each group of vertexes described by a segment
-        for (const auto& seg : impl->segments) {
-            MLN_TRACE_ZONE(segment);
-            auto& glSeg = static_cast<DrawSegmentGL&>(*seg);
-            const auto& mlSeg = glSeg.getSegment();
-
-            if (mlSeg.indexLength == 0) {
-                continue;
-            }
-
-            for (auto& binding : bindings) {
-                if (binding) {
-                    binding->vertexOffset = static_cast<uint32_t>(mlSeg.vertexOffset);
-                }
-            }
-
-            if (!glSeg.getVertexArray().isValid()) {
-                auto vertexArray = glContext.createVertexArray();
-                const auto& indexBuffer = static_cast<IndexBufferGL&>(*impl->indexes->getBuffer());
-                vertexArray.bind(glContext, *indexBuffer.buffer, bindings);
-                assert(vertexArray.isValid());
-                if (vertexArray.isValid()) {
-                    glSeg.setVertexArray(std::move(vertexArray));
-                }
-            }
-        }
-
-        attributeUpdateTime = util::MonotonicTimer::now();
     }
 
-    const bool texturesNeedUpload = std::any_of(
-        textures.begin(), textures.end(), [](const auto& texture) { return texture && texture->needsUpload(); });
+    // Bind a VAO for each group of vertexes described by a segment
+    for (const auto& seg : impl->segments) {
+        MLN_TRACE_ZONE(segment);
+        auto& glSeg = static_cast<DrawSegmentGL&>(*seg);
+        const auto& mlSeg = glSeg.getSegment();
 
-    if (texturesNeedUpload) {
+        if (mlSeg.indexLength == 0) {
+            continue;
+        }
+
+        for (auto& binding : impl->attributeBindings) {
+            if (binding) {
+                binding->vertexOffset = static_cast<uint32_t>(mlSeg.vertexOffset);
+            }
+        }
+
+        if (!glSeg.getVertexArray().isValid() && impl->indexes) {
+            auto vertexArray = glContext.createVertexArray();
+            const auto& indexBuffer = static_cast<IndexBufferGL&>(*impl->indexes->getBuffer());
+            vertexArray.bind(glContext, *indexBuffer.buffer, impl->attributeBindings);
+            assert(vertexArray.isValid());
+            if (vertexArray.isValid()) {
+                glSeg.setVertexArray(std::move(vertexArray));
+            }
+        }
+    }
+
+    const auto needsUpload = [](const auto& texture) {
+        return texture && texture->needsUpload();
+    };
+    if (std::any_of(textures.begin(), textures.end(), needsUpload)) {
         uploadTextures();
     }
+
+    attributeUpdateTime = util::MonotonicTimer::now();
 }
 
 gfx::ColorMode DrawableGL::makeColorMode(PaintParameters& parameters) const {

src/mbgl/gl/drawable_gl_impl.hpp

@@ -38,6 +38,8 @@ public:
     std::size_t vertexCount = 0;
     gfx::AttributeDataType vertexType = static_cast<gfx::AttributeDataType>(-1);
 
+    AttributeBindingArray attributeBindings;
+
     gfx::IndexBuffer indexBuffer = {0, nullptr};
     std::vector<gfx::UniqueVertexBufferResource> attributeBuffers;
 

src/mbgl/gl/upload_pass.cpp

@@ -284,10 +284,10 @@ gfx::AttributeBindingArray UploadPass::buildAttributeBindings(
         vertexStride += static_cast<uint32_t>(stride);
     };
     // This version is called when the attribute is available, but isn't being used by the shader
-    const auto missingAttr = [&](const size_t, auto& missingAttr) -> void {
+    const auto onMissingAttr = [&](const size_t, auto& missingAttr) -> void {
         missingAttr->setDirty(false);
     };
-    defaults.resolve(overrides, resolveAttr, missingAttr);
+    defaults.resolve(overrides, resolveAttr, onMissingAttr);
 
     assert(vertexStride * vertexCount <= allData.size());
 

src/mbgl/mtl/drawable.cpp

@@ -661,9 +661,7 @@ void Drawable::upload(gfx::UploadPass& uploadPass_) {
         uploadTextures(uploadPass);
     }
 
-    if (buildAttribs || buildInstanceBuffer) {
-        attributeUpdateTime = util::MonotonicTimer::now();
-    }
+    attributeUpdateTime = util::MonotonicTimer::now();
 }
 
 } // namespace mtl

src/mbgl/vulkan/drawable.cpp

@@ -187,8 +187,6 @@ void Drawable::upload(gfx::UploadPass& uploadPass_) {
         if (impl->instanceBindings != instanceBindings_) {
             impl->instanceBindings = std::move(instanceBindings_);
         }
-
-        attributeUpdateTime = util::MonotonicTimer::now();
     }
 
     if (buildAttribs || buildInstanceBuffer) {
@@ -201,6 +199,8 @@ void Drawable::upload(gfx::UploadPass& uploadPass_) {
     if (texturesNeedUpload) {
         uploadTextures(uploadPass);
     }
+
+    attributeUpdateTime = util::MonotonicTimer::now();
 }
 
 void Drawable::draw(PaintParameters& parameters) const {