From 1f2ebc5c25c0dd2a5a6211dd78c3081a1a785167 Mon Sep 17 00:00:00 2001 From: mwilsnd Date: Tue, 28 May 2024 16:44:09 -0400 Subject: [PATCH] wip --- include/mbgl/actor/actor.hpp | 11 +++++ include/mbgl/actor/scheduler.hpp | 12 +++-- include/mbgl/gfx/renderer_backend.hpp | 4 +- include/mbgl/gl/renderer_backend.hpp | 2 +- include/mbgl/mtl/context.hpp | 3 +- include/mbgl/mtl/renderer_backend.hpp | 2 +- src/mbgl/gfx/renderer_backend.cpp | 5 ++- src/mbgl/gl/context.cpp | 9 ++-- src/mbgl/gl/context.hpp | 3 +- src/mbgl/gl/renderer_backend.cpp | 7 +-- src/mbgl/mtl/context.cpp | 9 ++-- src/mbgl/mtl/renderer_backend.cpp | 7 +-- src/mbgl/tile/geometry_tile.cpp | 9 ++-- src/mbgl/tile/geometry_tile.hpp | 4 +- src/mbgl/tile/geometry_tile_worker.cpp | 4 +- src/mbgl/tile/geometry_tile_worker.hpp | 2 + src/mbgl/util/thread_pool.hpp | 59 ++++++++++++++++++++----- test/api/custom_drawable_layer.test.cpp | 2 +- 18 files changed, 111 insertions(+), 43 deletions(-) diff --git a/include/mbgl/actor/actor.hpp b/include/mbgl/actor/actor.hpp index af42bec7b97..2a02d966ded 100644 --- a/include/mbgl/actor/actor.hpp +++ b/include/mbgl/actor/actor.hpp @@ -14,6 +14,8 @@ namespace mbgl { +class TaggedScheduler; + /** An `Actor` is an owning reference to an asynchronous object of type `O`: an "actor". Communication with an actor happens via message passing: you send @@ -66,11 +68,20 @@ class Actor { : retainer(std::move(scheduler)), target(*retainer, parent, std::forward(args)...) {} + template + Actor(TaggedScheduler& scheduler, Args&&... args) + : schedulerTag(scheduler.tag()), + retainer(scheduler.get()), + target(*retainer, parent, std::forward(args)...) {} + + + Actor(const Actor&) = delete; ActorRef> self() { return parent.self(); } private: + const void* schedulerTag = nullptr; const std::shared_ptr retainer; AspiringActor parent; EstablishedActor target; diff --git a/include/mbgl/actor/scheduler.hpp b/include/mbgl/actor/scheduler.hpp index 000235d4aca..fb193b3fc40 100644 --- a/include/mbgl/actor/scheduler.hpp +++ b/include/mbgl/actor/scheduler.hpp @@ -43,10 +43,11 @@ class Scheduler { /// Makes a weak pointer to this Scheduler. virtual mapbox::base::WeakPtr makeWeakPtr() = 0; - /// Enqueues a function for execution on the render thread. - virtual void runOnRenderThread(std::function&&) {}; - virtual void runRenderJobs() {} - + /// Enqueues a function for execution on the render thread owned by the given tag. + virtual void runOnRenderThread(const void*, std::function&&) {} + /// Run render thread jobs for the given tag address + /// @param closeQueue Runs all render jobs and then removes the internal queue. + virtual void runRenderJobs(const void*, bool closeQueue = false) {} /// Returns a closure wrapping the given one. /// /// When the returned closure is invoked for the first time, it schedules @@ -130,8 +131,11 @@ class TaggedScheduler { /// @brief Get the wrapped scheduler /// @return const std::shared_ptr& get() const noexcept { return scheduler; } + const void* tag() const noexcept { return tagAddr; } void schedule(std::function&& fn) { scheduler->schedule(tagAddr, std::move(fn)); } + void runOnRenderThread(std::function&& fn) { scheduler->runOnRenderThread(tagAddr, std::move(fn)); } + void runRenderJobs(bool closeQueue = false) { scheduler->runRenderJobs(tagAddr, closeQueue); } void waitForEmpty() const noexcept { scheduler->waitForEmpty(tagAddr); } private: diff --git a/include/mbgl/gfx/renderer_backend.hpp b/include/mbgl/gfx/renderer_backend.hpp index 5d8b74f00d0..b777e686a19 100644 --- a/include/mbgl/gfx/renderer_backend.hpp +++ b/include/mbgl/gfx/renderer_backend.hpp @@ -8,6 +8,7 @@ namespace mbgl { class ProgramParameters; +class Map; namespace gfx { @@ -24,7 +25,7 @@ enum class ContextMode : bool { class RendererBackend { protected: - explicit RendererBackend(ContextMode); + explicit RendererBackend(ContextMode, mbgl::Map*); public: virtual ~RendererBackend(); @@ -69,6 +70,7 @@ class RendererBackend { protected: std::unique_ptr context; const ContextMode contextMode; + const mbgl::Map* owner; std::once_flag initialized; friend class BackendScope; diff --git a/include/mbgl/gl/renderer_backend.hpp b/include/mbgl/gl/renderer_backend.hpp index 0d64a585be9..a23ccfc103d 100644 --- a/include/mbgl/gl/renderer_backend.hpp +++ b/include/mbgl/gl/renderer_backend.hpp @@ -16,7 +16,7 @@ using FramebufferID = uint32_t; class RendererBackend : public gfx::RendererBackend { public: - RendererBackend(gfx::ContextMode); + RendererBackend(gfx::ContextMode, mbgl::Map*); ~RendererBackend() override; /// Called prior to rendering to update the internally assumed OpenGL state. diff --git a/include/mbgl/mtl/context.hpp b/include/mbgl/mtl/context.hpp index ce95fc45f5e..8c75cab615a 100644 --- a/include/mbgl/mtl/context.hpp +++ b/include/mbgl/mtl/context.hpp @@ -44,7 +44,7 @@ using UniqueVertexBufferResource = std::unique_ptr; class Context final : public gfx::Context { public: - Context(RendererBackend&); + Context(RendererBackend&, TaggedScheduler); ~Context() noexcept override; Context(const Context&) = delete; Context& operator=(const Context& other) = delete; @@ -159,6 +159,7 @@ class Context final : public gfx::Context { private: RendererBackend& backend; + TaggedScheduler& scheduler; bool cleanupOnDestruction = true; std::optional emptyBuffer; diff --git a/include/mbgl/mtl/renderer_backend.hpp b/include/mbgl/mtl/renderer_backend.hpp index b519bb69d48..b4d22cddc67 100644 --- a/include/mbgl/mtl/renderer_backend.hpp +++ b/include/mbgl/mtl/renderer_backend.hpp @@ -21,7 +21,7 @@ using FramebufferID = uint32_t; class RendererBackend : public gfx::RendererBackend { public: - RendererBackend(gfx::ContextMode); + RendererBackend(gfx::ContextMode, mbgl::Map*); ~RendererBackend() override; /// Called prior to rendering to update the internally assumed MetalMetal state. diff --git a/src/mbgl/gfx/renderer_backend.cpp b/src/mbgl/gfx/renderer_backend.cpp index b24a66a27be..257b1ab3bad 100644 --- a/src/mbgl/gfx/renderer_backend.cpp +++ b/src/mbgl/gfx/renderer_backend.cpp @@ -5,8 +5,9 @@ namespace mbgl { namespace gfx { -RendererBackend::RendererBackend(const ContextMode contextMode_) - : contextMode(contextMode_) {} +RendererBackend::RendererBackend(const ContextMode contextMode_, mbgl::Map* map) + : contextMode(contextMode_), + owner(map) {} RendererBackend::~RendererBackend() = default; gfx::Context& RendererBackend::getContext() { diff --git a/src/mbgl/gl/context.cpp b/src/mbgl/gl/context.cpp index 8632134d25a..ef8f573d180 100644 --- a/src/mbgl/gl/context.cpp +++ b/src/mbgl/gl/context.cpp @@ -72,9 +72,10 @@ GLint getMaxVertexAttribs() { } } // namespace -Context::Context(RendererBackend& backend_) +Context::Context(RendererBackend& backend_, TaggedScheduler scheduler_) : gfx::Context(/*maximumVertexBindingCount=*/getMaxVertexAttribs()), - backend(backend_) { + backend(backend_), + scheduler(std::move(scheduler_)) { #if MLN_DRAWABLE_RENDERER uboAllocator = std::make_unique(); #endif @@ -82,7 +83,7 @@ Context::Context(RendererBackend& backend_) Context::~Context() noexcept { if (cleanupOnDestruction) { - Scheduler::GetBackground()->runRenderJobs(); + scheduler.runRenderJobs(true /* closeQueue */); reset(); #if !defined(NDEBUG) @@ -93,7 +94,7 @@ Context::~Context() noexcept { } void Context::beginFrame() { - Scheduler::GetBackground()->runRenderJobs(); + scheduler.runRenderJobs(); #if MLN_DRAWABLE_RENDERER frameInFlightFence = std::make_shared(); diff --git a/src/mbgl/gl/context.hpp b/src/mbgl/gl/context.hpp index 5e8a0696842..32799f5b786 100644 --- a/src/mbgl/gl/context.hpp +++ b/src/mbgl/gl/context.hpp @@ -40,7 +40,7 @@ class Debugging; class Context final : public gfx::Context { public: - Context(RendererBackend&); + Context(RendererBackend&, TaggedScheduler); ~Context() noexcept override; Context(const Context&) = delete; Context& operator=(const Context& other) = delete; @@ -153,6 +153,7 @@ class Context final : public gfx::Context { private: RendererBackend& backend; + TaggedScheduler scheduler; bool cleanupOnDestruction = true; std::unique_ptr debugging; diff --git a/src/mbgl/gl/renderer_backend.cpp b/src/mbgl/gl/renderer_backend.cpp index c1b61afc1b0..ddb7933aa7e 100644 --- a/src/mbgl/gl/renderer_backend.cpp +++ b/src/mbgl/gl/renderer_backend.cpp @@ -15,11 +15,12 @@ namespace mbgl { namespace gl { -RendererBackend::RendererBackend(const gfx::ContextMode contextMode_) - : gfx::RendererBackend(contextMode_) {} +RendererBackend::RendererBackend(const gfx::ContextMode contextMode_, mbgl::Map* map) + : gfx::RendererBackend(contextMode_, map) {} std::unique_ptr RendererBackend::createContext() { - auto result = std::make_unique(*this); + auto result = std::make_unique(*this, + TaggedScheduler{Scheduler::GetBackground(), static_cast(owner)}); result->enableDebugging(); result->initializeExtensions(std::bind(&RendererBackend::getExtensionFunctionPointer, this, std::placeholders::_1)); return result; diff --git a/src/mbgl/mtl/context.cpp b/src/mbgl/mtl/context.cpp index b6470ec1335..b61c18789d3 100644 --- a/src/mbgl/mtl/context.cpp +++ b/src/mbgl/mtl/context.cpp @@ -38,13 +38,14 @@ namespace mtl { // 31 for Apple2-8, Mac2, per https://developer.apple.com/metal/Metal-Feature-Set-Tables.pdf constexpr uint32_t maximumVertexBindingCount = 31; -Context::Context(RendererBackend& backend_) +Context::Context(RendererBackend& backend_, TaggedScheduler scheduler_) : gfx::Context(mtl::maximumVertexBindingCount), - backend(backend_) {} + backend(backend_), + scheduler(std::move(scheduler_)) {} Context::~Context() noexcept { if (cleanupOnDestruction) { - Scheduler::GetBackground()->runRenderJobs(); + scheduler.runRenderJobs(true /* closeQueue */); performCleanup(); emptyVertexBuffer.reset(); @@ -68,7 +69,7 @@ Context::~Context() noexcept { } void Context::beginFrame() { - Scheduler::GetBackground()->runRenderJobs(); + scheduler.runRenderJobs(); } void Context::endFrame() {} diff --git a/src/mbgl/mtl/renderer_backend.cpp b/src/mbgl/mtl/renderer_backend.cpp index 6af61d9dc09..83b4b9dfc76 100644 --- a/src/mbgl/mtl/renderer_backend.cpp +++ b/src/mbgl/mtl/renderer_backend.cpp @@ -40,8 +40,8 @@ namespace mbgl { namespace mtl { -RendererBackend::RendererBackend(const gfx::ContextMode contextMode_) - : gfx::RendererBackend(contextMode_), +RendererBackend::RendererBackend(const gfx::ContextMode contextMode_, mbgl::Map* map) + : gfx::RendererBackend(contextMode_, map), device(NS::TransferPtr(MTL::CreateSystemDefaultDevice())), commandQueue(NS::TransferPtr(device->newCommandQueue())) { assert(device); @@ -51,7 +51,8 @@ RendererBackend::RendererBackend(const gfx::ContextMode contextMode_) RendererBackend::~RendererBackend() = default; std::unique_ptr RendererBackend::createContext() { - return std::make_unique(*this); + return std::make_unique(*this, + TaggedScheduler{Scheduler::GetBackground(), static_cast(owner)}); } PremultipliedImage RendererBackend::readFramebuffer(const Size& size) { diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index 1649b69f926..59530636fae 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -156,14 +156,15 @@ const LayerRenderData* GeometryTileRenderData::getLayerRenderData(const style::L that could flag the tile as non-pending too early. */ -GeometryTile::GeometryTile(const OverscaledTileID& id_, std::string sourceID_, const TileParameters& parameters) +GeometryTile::GeometryTile(const OverscaledTileID& id_, std::string sourceID_, const TileParameters& parameters, TaggedScheduler& scheduler) : Tile(Kind::Geometry, id_), ImageRequestor(parameters.imageManager), sourceID(std::move(sourceID_)), - threadPool(Scheduler::GetBackground()), + threadPool(scheduler), mailbox(std::make_shared(*Scheduler::GetCurrent())), - worker(threadPool, + worker(threadPool, // TaggedScheduler reference for the Actor retainer ActorRef(*this, mailbox), + threadPool, // TaggedScheduler reference for the worker id_, sourceID, obsolete, @@ -183,7 +184,7 @@ GeometryTile::~GeometryTile() { imageManager->removeRequestor(*this); if (layoutResult) { - threadPool->runOnRenderThread( + threadPool.runOnRenderThread( [layoutResult_{std::move(layoutResult)}, atlasTextures_{std::move(atlasTextures)}]() {}); } } diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index 31aa70b668f..ced49eb2c92 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -28,7 +28,7 @@ class TileAtlasTextures; class GeometryTile : public Tile, public GlyphRequestor, public ImageRequestor { public: - GeometryTile(const OverscaledTileID&, std::string sourceID, const TileParameters&); + GeometryTile(const OverscaledTileID&, std::string sourceID, const TileParameters&, TaggedScheduler& scheduler); ~GeometryTile() override; @@ -106,7 +106,7 @@ class GeometryTile : public Tile, public GlyphRequestor, public ImageRequestor { // Used to signal the worker that it should abandon parsing this tile as soon as possible. std::atomic obsolete{false}; - const std::shared_ptr threadPool; + TaggedScheduler threadPool; const std::shared_ptr mailbox; Actor worker; diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index ec1c444f57f..4983c35835f 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -30,6 +30,7 @@ using namespace style; GeometryTileWorker::GeometryTileWorker(ActorRef self_, ActorRef parent_, + TaggedScheduler& scheduler_, OverscaledTileID id_, std::string sourceID_, const std::atomic& obsolete_, @@ -38,6 +39,7 @@ GeometryTileWorker::GeometryTileWorker(ActorRef self_, const bool showCollisionBoxes_) : self(std::move(self_)), parent(std::move(parent_)), + scheduler(scheduler_), id(id_), sourceID(std::move(sourceID_)), obsolete(obsolete_), @@ -46,7 +48,7 @@ GeometryTileWorker::GeometryTileWorker(ActorRef self_, showCollisionBoxes(showCollisionBoxes_) {} GeometryTileWorker::~GeometryTileWorker() { - Scheduler::GetBackground()->runOnRenderThread([renderData_{std::move(renderData)}]() {}); + scheduler.runOnRenderThread([renderData_{std::move(renderData)}]() {}); } /* diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp index bb169a0e846..b2d836cbef3 100644 --- a/src/mbgl/tile/geometry_tile_worker.hpp +++ b/src/mbgl/tile/geometry_tile_worker.hpp @@ -30,6 +30,7 @@ class GeometryTileWorker { public: GeometryTileWorker(ActorRef self, ActorRef parent, + TaggedScheduler& scheduler_, OverscaledTileID, std::string, const std::atomic&, @@ -71,6 +72,7 @@ class GeometryTileWorker { ActorRef self; ActorRef parent; + TaggedScheduler scheduler; const OverscaledTileID id; const std::string sourceID; diff --git a/src/mbgl/util/thread_pool.hpp b/src/mbgl/util/thread_pool.hpp index f24438aceff..11d4f4605cf 100644 --- a/src/mbgl/util/thread_pool.hpp +++ b/src/mbgl/util/thread_pool.hpp @@ -86,20 +86,55 @@ class ThreadedScheduler : public ThreadedSchedulerBase { } } - void runOnRenderThread(std::function&& fn) override { - std::lock_guard lock(renderMutex); - renderThreadQueue.push(std::move(fn)); + void runOnRenderThread(const void* tag, std::function&& fn) override { + std::shared_ptr queue; + { + std::lock_guard lock(taggedRenderQueueLock); + auto it = taggedRenderQueue.find(tag); + if (it != taggedRenderQueue.end()) { + queue = it->second; + } else { + queue = std::make_shared(); + taggedRenderQueue.insert({tag, queue}); + } + } + + std::lock_guard lock(queue->mutex); + queue->queue.push(std::move(fn)); } - void runRenderJobs() override { - std::lock_guard lock(renderMutex); - while (renderThreadQueue.size()) { - auto fn = std::move(renderThreadQueue.front()); - renderThreadQueue.pop(); + void runRenderJobs(const void* tag, bool closeQueue = false) override { + std::shared_ptr queue; + std::unique_lock lock(taggedRenderQueueLock); + + { + auto it = taggedRenderQueue.find(tag); + if (it != taggedRenderQueue.end()) { + queue = it->second; + } + + if (!closeQueue) { + lock.unlock(); + } + } + + if (!queue) { + return; + } + + std::lock_guard taskLock(queue->mutex); + while (queue->queue.size()) { + auto fn = std::move(queue->queue.front()); + queue->queue.pop(); if (fn) { fn(); } } + + if (closeQueue) { + // We hold both locks and can safely remove the queue entry + taggedRenderQueue.erase(tag); + } } mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } @@ -108,8 +143,12 @@ class ThreadedScheduler : public ThreadedSchedulerBase { std::vector threads; mapbox::base::WeakPtrFactory weakFactory{this}; - std::queue> renderThreadQueue; - std::mutex renderMutex; + struct RenderQueue { + std::queue> queue; + std::mutex mutex; + }; + mbgl::unordered_map> taggedRenderQueue; + std::mutex taggedRenderQueueLock; }; class SequencedScheduler : public ThreadedScheduler { diff --git a/test/api/custom_drawable_layer.test.cpp b/test/api/custom_drawable_layer.test.cpp index 8b69b3bd46e..33d36cd38f4 100644 --- a/test/api/custom_drawable_layer.test.cpp +++ b/test/api/custom_drawable_layer.test.cpp @@ -109,7 +109,7 @@ class LineTestDrawableLayer : public mbgl::style::CustomDrawableLayerHost { GeometryCoordinates polyline; for (auto ipoint{0}; ipoint < numPoints; ++ipoint) { polyline.emplace_back(ipoint * util::EXTENT / numPoints, - std::sin(ipoint * 2 * M_PI / numPoints) * util::EXTENT / numLines / 2.f); + static_cast(std::sin(ipoint * 2 * M_PI / numPoints) * util::EXTENT / numLines / 2.f)); } for (auto index{0}; index < numLines; ++index) {