Avoid redundant bindings/states based on Metal profiler feedback. (#2006

)
maplibre · Jan 22, 2024 · 5c30461 · 5c30461
1 parent cd3de6d
commit 5c30461
Show file tree

Hide file tree

Showing 9 changed files with 90 additions and 35 deletions.
diff --git a/include/mbgl/mtl/render_pass.hpp b/include/mbgl/mtl/render_pass.hpp
@@ -27,6 +27,19 @@ class RenderPass final : public gfx::RenderPass {
     const MTLRenderCommandEncoderPtr& getMetalEncoder() const { return encoder; }
     const gfx::RenderPassDescriptor& getDescriptor() const { return descriptor; }
 
+    /// Apply the given depth/stencil state, if different from the current value
+    /// The state may be null, restoring the default state.
+    void setDepthStencilState(const MTLDepthStencilStatePtr&);
+
+    /// Apply the given stencil reference value, if different from the current value
+    void setStencilReference(int32_t referenceValue);
+
+    /// Bind a texture to the fragment location
+    void setFragmentTexture(const MTLTexturePtr&, int32_t location);
+
+    /// Set the sampler for a texture binding
+    void setFragmentSamplerState(const MTLSamplerStatePtr&, int32_t location);
+
     void endEncoding();
 
     void addDebugSignpost(const char* name) override;
@@ -43,16 +56,22 @@ class RenderPass final : public gfx::RenderPass {
     gfx::RenderPassDescriptor descriptor;
     mtl::CommandEncoder& commandEncoder;
     MTLRenderCommandEncoderPtr encoder;
+    MTLDepthStencilStatePtr currentDepthStencilState;
+    int32_t currentStencilReferenceValue = 0;
     std::vector<gfx::DebugGroup<gfx::RenderPass>> debugGroups;
 
     struct BindInfo {
         const BufferResource* buf = nullptr;
+        NS::UInteger size = 0;
         NS::UInteger offset = 0;
         std::uint16_t version = 0;
     };
     static constexpr auto maxBinds = 32;
     std::array<std::optional<BindInfo>, maxBinds> vertexBinds;
     std::array<std::optional<BindInfo>, maxBinds> fragmentBinds;
+
+    std::array<MTLTexturePtr, maxBinds> fragmentTextureBindings;
+    std::array<MTLSamplerStatePtr, maxBinds> fragmentSamplerStates;
 };
 
 } // namespace mtl

diff --git a/include/mbgl/mtl/texture2d.hpp b/include/mbgl/mtl/texture2d.hpp
@@ -54,12 +54,12 @@ class Texture2D : public gfx::Texture2D {
     /// @brief Bind this texture to the specified location
     /// @param renderPass Render pass on which the texture will be assign
     /// @param location Location index of texture sampler in a shader
-    void bind(const RenderPass& renderPass, int32_t location) noexcept;
+    void bind(RenderPass& renderPass, int32_t location) noexcept;
 
     /// @brief Unbind the texture, if it was bound
     /// @param renderPass Render pass from which the texture will be removed
     /// @param location Location index of texture sampler in a shader
-    void unbind(const RenderPass& renderPass, int32_t location) noexcept;
+    void unbind(RenderPass& renderPass, int32_t location) noexcept;
 
 private:
     MTL::PixelFormat getMetalPixelFormat() const noexcept;

diff --git a/src/mbgl/mtl/buffer_resource.cpp b/src/mbgl/mtl/buffer_resource.cpp
@@ -147,7 +147,7 @@ void BufferResource::update(const void* newData, std::size_t updateSize, std::si
 bool BufferResource::needReBind(VersionType version_) const noexcept {
     // If we're using a raw buffer, an update means we have to re-bind.
     // For a MTLBuffer, the binding can be left alone.
-    return (!buffer || version != version_);
+    return (version != version_);
 }
 
 void BufferResource::bindVertex(const MTLRenderCommandEncoderPtr& encoder,

diff --git a/src/mbgl/mtl/context.cpp b/src/mbgl/mtl/context.cpp
@@ -324,12 +324,8 @@ bool Context::renderTileClippingMasks(gfx::RenderPass& renderPass,
             clipMaskDepthStencilState = std::move(depthStencilState);
         }
     }
-    if (clipMaskDepthStencilState) {
-        encoder->setDepthStencilState(clipMaskDepthStencilState.get());
-    } else {
-        assert(!"Failed to create depth-stencil state for clip masking");
-        return false;
-    }
+    assert(clipMaskDepthStencilState || !"Failed to create depth-stencil state for clip masking");
+    mtlRenderPass.setDepthStencilState(clipMaskDepthStencilState);
 
     if (!clipMaskPipelineState) {
         // A vertex descriptor tells Metal what's in the vertex buffer

diff --git a/src/mbgl/mtl/drawable.cpp b/src/mbgl/mtl/drawable.cpp
@@ -233,10 +233,8 @@ void Drawable::draw(PaintParameters& parameters) const {
             impl->depthStencilState = context.makeDepthStencilState(depthMode, stencilMode, renderable);
             impl->previousStencilMode = *newStencilMode;
         }
-        if (impl->depthStencilState) {
-            encoder->setDepthStencilState(impl->depthStencilState.get());
-            encoder->setStencilReferenceValue(impl->previousStencilMode.ref);
-        }
+        renderPass.setDepthStencilState(impl->depthStencilState);
+        renderPass.setStencilReference(impl->previousStencilMode.ref);
     }
 
     for (const auto& seg_ : impl->segments) {

diff --git a/src/mbgl/mtl/render_pass.cpp b/src/mbgl/mtl/render_pass.cpp
@@ -63,6 +63,15 @@ void RenderPass::endEncoding() {
         encoder->endEncoding();
         encoder.reset();
     }
+
+    currentDepthStencilState.reset();
+    currentStencilReferenceValue = 0;
+    for (int i = 0; i < maxBinds; ++i) {
+        vertexBinds[i].reset();
+        fragmentBinds[i].reset();
+        fragmentTextureBindings[i].reset();
+        fragmentSamplerStates[i].reset();
+    }
 }
 
 namespace {
@@ -94,6 +103,8 @@ void RenderPass::addDebugSignpost(const char* name) {
 }
 
 void RenderPass::bindVertex(const BufferResource& buf, std::size_t offset, std::size_t index, std::size_t size) {
+    const auto actualSize = size ? size : buf.getSizeInBytes() - offset;
+    assert(actualSize <= buf.getSizeInBytes());
     assert(0 <= index && index < maxBinds);
     if (0 <= index && index < maxBinds) {
         if (auto& bind = vertexBinds[index]) {
@@ -102,18 +113,20 @@ void RenderPass::bindVertex(const BufferResource& buf, std::size_t offset, std::
                 // Yes, but is the offset different?
                 if (bind->offset != offset) {
                     // Yes, update just the offset
-                    buf.updateVertexBindOffset(encoder, offset, index, size);
+                    buf.updateVertexBindOffset(encoder, offset, index, actualSize);
                     bind->offset = offset;
                 }
                 return;
             }
         }
-        vertexBinds[index] = BindInfo{&buf, offset};
+        vertexBinds[index] = BindInfo{&buf, actualSize, offset};
     }
-    buf.bindVertex(encoder, offset, index, size);
+    buf.bindVertex(encoder, offset, index, actualSize);
 }
 
 void RenderPass::bindFragment(const BufferResource& buf, std::size_t offset, std::size_t index, std::size_t size) {
+    const auto actualSize = size ? size : buf.getSizeInBytes() - offset;
+    assert(actualSize <= buf.getSizeInBytes());
     assert(0 <= index && index < maxBinds);
     if (0 <= index && index < maxBinds) {
         if (auto& bind = fragmentBinds[index]) {
@@ -122,15 +135,49 @@ void RenderPass::bindFragment(const BufferResource& buf, std::size_t offset, std
                 // Yes, but is the offset different?
                 if (bind->offset != offset) {
                     // Yes, update just the offset
-                    buf.updateFragmentBindOffset(encoder, offset, index, size);
+                    buf.updateFragmentBindOffset(encoder, offset, index, actualSize);
                     bind->offset = offset;
                 }
                 return;
             }
         }
-        fragmentBinds[index] = BindInfo{&buf, offset};
+        fragmentBinds[index] = BindInfo{&buf, actualSize, offset};
+    }
+    buf.bindFragment(encoder, offset, index, actualSize);
+}
+
+void RenderPass::setDepthStencilState(const MTLDepthStencilStatePtr& state) {
+    if (state != currentDepthStencilState) {
+        currentDepthStencilState = state;
+        encoder->setDepthStencilState(currentDepthStencilState.get());
+    }
+}
+
+void RenderPass::setStencilReference(int32_t referenceValue) {
+    if (referenceValue != currentStencilReferenceValue) {
+        currentStencilReferenceValue = referenceValue;
+        encoder->setStencilReferenceValue(currentStencilReferenceValue);
+    }
+}
+
+void RenderPass::setFragmentTexture(const MTLTexturePtr& texture, int32_t location) {
+    assert(0 <= location && location < maxBinds);
+    if (0 <= location && location < maxBinds) {
+        if (fragmentTextureBindings[location] != texture) {
+            fragmentTextureBindings[location] = texture;
+            encoder->setFragmentTexture(texture.get(), location);
+        }
+    }
+}
+
+void RenderPass::setFragmentSamplerState(const MTLSamplerStatePtr& state, int32_t location) {
+    assert(0 <= location && location < maxBinds);
+    if (0 <= location && location < maxBinds) {
+        if (fragmentSamplerStates[location] != state) {
+            fragmentSamplerStates[location] = state;
+            encoder->setFragmentSamplerState(state.get(), location);
+        }
     }
-    buf.bindFragment(encoder, offset, index, size);
 }
 
 } // namespace mtl

diff --git a/src/mbgl/mtl/texture2d.cpp b/src/mbgl/mtl/texture2d.cpp
@@ -186,22 +186,21 @@ void Texture2D::updateSamplerConfiguration() noexcept {
     metalSamplerState = context.createMetalSamplerState(samplerDescriptor);
 }
 
-void Texture2D::bind(const RenderPass& renderPass, int32_t location) noexcept {
+void Texture2D::bind(RenderPass& renderPass, int32_t location) noexcept {
     assert(!textureDirty);
-    const auto& encoder = renderPass.getMetalEncoder();
 
     // Update the sampler state if it was changed after resource creation
     if (samplerStateDirty) {
         updateSamplerConfiguration();
     }
 
-    encoder->setFragmentTexture(metalTexture.get(), location);
-    encoder->setFragmentSamplerState(metalSamplerState.get(), location);
+    renderPass.setFragmentTexture(metalTexture, location);
+    renderPass.setFragmentSamplerState(metalSamplerState, location);
 
     context.renderingStats().numTextureBindings++;
 }
 
-void Texture2D::unbind(const RenderPass&, int32_t /*location*/) noexcept {
+void Texture2D::unbind(RenderPass&, int32_t /*location*/) noexcept {
     context.renderingStats().numTextureBindings--;
 }
 

diff --git a/src/mbgl/mtl/tile_layer_group.cpp b/src/mbgl/mtl/tile_layer_group.cpp
@@ -43,7 +43,7 @@ void TileLayerGroup::render(RenderOrchestrator&, PaintParameters& parameters) {
     }
 
     auto& context = static_cast<Context&>(parameters.context);
-    const auto& renderPass = static_cast<const mtl::RenderPass&>(*parameters.renderPass);
+    auto& renderPass = static_cast<mtl::RenderPass&>(*parameters.renderPass);
     const auto& encoder = renderPass.getMetalEncoder();
     const auto& renderable = renderPass.getDescriptor().renderable;
 
@@ -101,9 +101,7 @@ void TileLayerGroup::render(RenderOrchestrator&, PaintParameters& parameters) {
         // 2D drawables will set their own stencil mode within `draw`.
         if (features3d) {
             const auto& state = drawable.getEnableStencil() ? stateWithStencil : stateWithoutStencil;
-            if (state) {
-                encoder->setDepthStencilState(state.get());
-            }
+            renderPass.setDepthStencilState(state);
         }
 
         drawable.draw(parameters);

diff --git a/src/mbgl/mtl/upload_pass.cpp b/src/mbgl/mtl/upload_pass.cpp
@@ -25,13 +25,13 @@ UploadPass::UploadPass(gfx::Renderable& renderable, CommandEncoder& commandEncod
 
     if (const auto& buffer_ = resource.getCommandBuffer()) {
         buffer = buffer_;
-        if (auto upd = resource.getUploadPassDescriptor()) {
-            encoder = NS::RetainPtr(buffer->blitCommandEncoder(upd.get()));
-        }
+        // blit encoder is not being used yet
+        // if (auto upd = resource.getUploadPassDescriptor()) {
+        //    encoder = NS::RetainPtr(buffer->blitCommandEncoder(upd.get()));
+        //}
+        // assert(encoder);
     }
 
-    assert(encoder);
-
     // Push the groups already accumulated by the encoder
     commandEncoder.visitDebugGroups([this](const auto& group) {
         debugGroups.emplace_back(gfx::DebugGroup<gfx::UploadPass>{*this, group.c_str()});
@@ -253,14 +253,12 @@ NS::String* toNSString(const char* str) {
 } // namespace
 
 void UploadPass::pushDebugGroup(const char* name) {
-    assert(encoder);
     if (encoder) {
         encoder->pushDebugGroup(toNSString(name));
     }
 }
 
 void UploadPass::popDebugGroup() {
-    assert(encoder);
     if (encoder) {
         encoder->popDebugGroup();
     }