From 41bbd4e4f7d66465433e370ca024ab0239fcace3 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Tue, 6 Sep 2016 15:01:34 -0700 Subject: [core] Use an actor model for tile worker concurrency --- cmake/core-files.cmake | 18 +- cmake/test-files.cmake | 12 +- include/mbgl/util/run_loop.hpp | 15 +- src/mbgl/actor/actor.hpp | 76 ++++++++ src/mbgl/actor/actor_ref.hpp | 43 +++++ src/mbgl/actor/mailbox.cpp | 55 ++++++ src/mbgl/actor/mailbox.hpp | 31 +++ src/mbgl/actor/message.hpp | 48 +++++ src/mbgl/actor/scheduler.hpp | 38 ++++ src/mbgl/actor/thread_pool.cpp | 48 +++++ src/mbgl/actor/thread_pool.hpp | 28 +++ src/mbgl/annotation/annotation_tile.cpp | 2 +- src/mbgl/map/map.cpp | 6 +- src/mbgl/style/source_impl.cpp | 34 +--- src/mbgl/style/source_impl.hpp | 15 +- src/mbgl/style/source_observer.hpp | 3 +- src/mbgl/style/style.cpp | 32 +-- src/mbgl/style/style.hpp | 10 +- src/mbgl/style/update_parameters.hpp | 11 +- src/mbgl/tile/geojson_tile.cpp | 3 +- src/mbgl/tile/geometry_tile.cpp | 222 ++++++--------------- src/mbgl/tile/geometry_tile.hpp | 63 +++--- src/mbgl/tile/geometry_tile_worker.cpp | 300 +++++++++++++++++++++++++++++ src/mbgl/tile/geometry_tile_worker.hpp | 76 ++++++++ src/mbgl/tile/raster_tile.cpp | 49 ++--- src/mbgl/tile/raster_tile.hpp | 12 +- src/mbgl/tile/raster_tile_worker.cpp | 27 +++ src/mbgl/tile/raster_tile_worker.hpp | 22 +++ src/mbgl/tile/tile.cpp | 2 +- src/mbgl/tile/tile.hpp | 6 +- src/mbgl/tile/tile_observer.hpp | 8 +- src/mbgl/tile/tile_worker.cpp | 179 ----------------- src/mbgl/tile/tile_worker.hpp | 91 --------- src/mbgl/tile/vector_tile.cpp | 3 +- src/mbgl/util/worker.cpp | 129 ------------- src/mbgl/util/worker.hpp | 66 ------- test/actor/actor.cpp | 139 +++++++++++++ test/actor/actor_ref.cpp | 42 ++++ test/src/mbgl/test/stub_style_observer.hpp | 13 +- test/style/source.cpp | 31 ++- test/style/style.cpp | 1 + 41 files changed, 1191 insertions(+), 818 deletions(-) create mode 100644 src/mbgl/actor/actor.hpp create mode 100644 src/mbgl/actor/actor_ref.hpp create mode 100644 src/mbgl/actor/mailbox.cpp create mode 100644 src/mbgl/actor/mailbox.hpp create mode 100644 src/mbgl/actor/message.hpp create mode 100644 src/mbgl/actor/scheduler.hpp create mode 100644 src/mbgl/actor/thread_pool.cpp create mode 100644 src/mbgl/actor/thread_pool.hpp create mode 100644 src/mbgl/tile/geometry_tile_worker.cpp create mode 100644 src/mbgl/tile/geometry_tile_worker.hpp create mode 100644 src/mbgl/tile/raster_tile_worker.cpp create mode 100644 src/mbgl/tile/raster_tile_worker.hpp delete mode 100644 src/mbgl/tile/tile_worker.cpp delete mode 100644 src/mbgl/tile/tile_worker.hpp delete mode 100644 src/mbgl/util/worker.cpp delete mode 100644 src/mbgl/util/worker.hpp create mode 100644 test/actor/actor.cpp create mode 100644 test/actor/actor_ref.cpp diff --git a/cmake/core-files.cmake b/cmake/core-files.cmake index 1a7353e979..11300e444d 100644 --- a/cmake/core-files.cmake +++ b/cmake/core-files.cmake @@ -1,6 +1,16 @@ # Do not edit. Regenerate this with ./scripts/generate-core-files.sh set(MBGL_CORE_FILES + # actor + src/mbgl/actor/actor.hpp + src/mbgl/actor/actor_ref.hpp + src/mbgl/actor/mailbox.cpp + src/mbgl/actor/mailbox.hpp + src/mbgl/actor/message.hpp + src/mbgl/actor/scheduler.hpp + src/mbgl/actor/thread_pool.cpp + src/mbgl/actor/thread_pool.hpp + # algorithm src/mbgl/algorithm/covered_by_children.hpp src/mbgl/algorithm/generate_clip_ids.cpp @@ -365,8 +375,12 @@ set(MBGL_CORE_FILES src/mbgl/tile/geometry_tile.hpp src/mbgl/tile/geometry_tile_data.cpp src/mbgl/tile/geometry_tile_data.hpp + src/mbgl/tile/geometry_tile_worker.cpp + src/mbgl/tile/geometry_tile_worker.hpp src/mbgl/tile/raster_tile.cpp src/mbgl/tile/raster_tile.hpp + src/mbgl/tile/raster_tile_worker.cpp + src/mbgl/tile/raster_tile_worker.hpp src/mbgl/tile/tile.cpp src/mbgl/tile/tile.hpp src/mbgl/tile/tile_cache.cpp @@ -376,8 +390,6 @@ set(MBGL_CORE_FILES src/mbgl/tile/tile_loader.hpp src/mbgl/tile/tile_loader_impl.hpp src/mbgl/tile/tile_observer.hpp - src/mbgl/tile/tile_worker.cpp - src/mbgl/tile/tile_worker.hpp src/mbgl/tile/vector_tile.cpp src/mbgl/tile/vector_tile.hpp @@ -472,6 +484,4 @@ set(MBGL_CORE_FILES src/mbgl/util/work_queue.cpp src/mbgl/util/work_queue.hpp src/mbgl/util/work_request.cpp - src/mbgl/util/worker.cpp - src/mbgl/util/worker.hpp ) diff --git a/cmake/test-files.cmake b/cmake/test-files.cmake index 0a4a6e3c41..84f9bcca63 100644 --- a/cmake/test-files.cmake +++ b/cmake/test-files.cmake @@ -1,6 +1,10 @@ # Do not edit. Regenerate this with ./scripts/generate-test-files.sh set(MBGL_TEST_FILES + # actor + test/actor/actor.cpp + test/actor/actor_ref.cpp + # algorithm test/algorithm/covered_by_children.cpp test/algorithm/generate_clip_ids.cpp @@ -43,10 +47,12 @@ set(MBGL_TEST_FILES # src/mbgl/test test/src/mbgl/test/conversion_stubs.hpp + test/src/mbgl/test/fake_file_source.hpp test/src/mbgl/test/fixture_log_observer.cpp test/src/mbgl/test/fixture_log_observer.hpp test/src/mbgl/test/stub_file_source.cpp test/src/mbgl/test/stub_file_source.hpp + test/src/mbgl/test/stub_layer_observer.hpp test/src/mbgl/test/stub_style_observer.hpp test/src/mbgl/test/test.cpp test/src/mbgl/test/util.cpp @@ -63,6 +69,9 @@ set(MBGL_TEST_FILES test/storage/online_file_source.cpp test/storage/resource.cpp + # style/conversion + test/style/conversion/geojson_options.cpp + # style test/style/filter.cpp test/style/functions.cpp @@ -73,9 +82,6 @@ set(MBGL_TEST_FILES test/style/style_parser.cpp test/style/tile_source.cpp test/style/variant.cpp - - # style conversion - test/style/conversion/geojson_options.cpp # text test/text/quads.cpp diff --git a/include/mbgl/util/run_loop.hpp b/include/mbgl/util/run_loop.hpp index 6559b72ef8..939531d6c3 100644 --- a/include/mbgl/util/run_loop.hpp +++ b/include/mbgl/util/run_loop.hpp @@ -1,5 +1,7 @@ #pragma once +#include +#include #include #include #include @@ -16,7 +18,8 @@ namespace util { typedef void * LOOP_HANDLE; -class RunLoop : private util::noncopyable { +class RunLoop : public Scheduler, + private util::noncopyable { public: enum class Type : uint8_t { Default, @@ -31,7 +34,7 @@ public: }; RunLoop(Type type = Type::Default); - ~RunLoop(); + ~RunLoop() override; static RunLoop* Get(); static LOOP_HANDLE getLoopHandle(); @@ -78,6 +81,14 @@ private: void push(std::shared_ptr); + void schedule(std::weak_ptr mailbox) override { + invoke([mailbox] () { + if (auto locked = mailbox.lock()) { + locked->receive(); + } + }); + } + void withMutex(std::function&& fn) { std::lock_guard lock(mutex); fn(); diff --git a/src/mbgl/actor/actor.hpp b/src/mbgl/actor/actor.hpp new file mode 100644 index 0000000000..281bbdaed1 --- /dev/null +++ b/src/mbgl/actor/actor.hpp @@ -0,0 +1,76 @@ +#pragma once + +#include +#include +#include +#include + +#include + +namespace mbgl { + +/* + 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 a message to the object + (using `invoke`), passing a pointer to the member function to call and arguments which + are then forwarded to the actor. + + The actor receives messages sent to it asynchronously, in a manner defined its `Scheduler`. + To store incoming messages before their receipt, each actor has a `Mailbox`, which acts as + a FIFO queue. Messages sent from actor S to actor R are guaranteed to be processed in the + order sent. However, relative order of messages sent by two *different* actors S1 and S2 + to R is *not* guaranteed (and can't be: S1 and S2 may be acting asynchronously with respect + to each other). + + Construction and destruction of an actor is currently synchronous: the corresponding `O` + object is constructed synchronously by the `Actor` constructor, and destructed synchronously + by the `~Actor` destructor, after ensuring that the `O` is not currently receiving an + asynchronous message. (Construction and destruction may change to be asynchronous in the + future.) + + An `Actor` can be converted to an `ActorRef`, a non-owning value object representing + a (weak) reference to the actor. Messages can be sent via the `Ref` as well. + + It's safe -- and encouraged -- to pass `Ref`s between actors via messages. This is how two-way + communication and other forms of collaboration between multiple actors is accomplished. + + It's safe for a `Ref` to outlive its `Actor` -- the reference is "weak", and does not extend + the lifetime of the owning Actor, and sending a message to a `Ref` whose `Actor` has died is + a no-op. (In the future, a dead-letters queue or log may be implemented.) + + Please don't send messages that contain shared pointers or references. That subverts the + purpose of the actor model: prohibiting direct concurrent access to shared state. +*/ + +template +class Actor : public util::noncopyable { +public: + template + Actor(Scheduler& scheduler, Args&&... args_) + : mailbox(std::make_shared(scheduler)), + object(self(), std::forward(args_)...) { + } + + ~Actor() { + mailbox->close(); + } + + template + void invoke(Fn fn, Args&&... args) { + mailbox->push(actor::makeMessage(object, fn, std::forward(args)...)); + } + + ActorRef> self() { + return ActorRef>(object, mailbox); + } + + operator ActorRef>() { + return self(); + } + +private: + std::shared_ptr mailbox; + Object object; +}; + +} // namespace mbgl diff --git a/src/mbgl/actor/actor_ref.hpp b/src/mbgl/actor/actor_ref.hpp new file mode 100644 index 0000000000..9d858d823f --- /dev/null +++ b/src/mbgl/actor/actor_ref.hpp @@ -0,0 +1,43 @@ +#pragma once + +#include +#include + +#include + +namespace mbgl { + +/* + An `ActorRef` is a *non*-owning, weak reference to an actor of type `O`. You can send it + messages just like an `Actor`. It's a value object: safe to copy and pass between actors + via messages. + + An `ActorRef` does not extend the lifetime of the corresponding `Actor`. That's determined + entirely by whichever object owns the `Actor` -- the actor's "supervisor". + + It's safe for a `Ref` to outlive its `Actor` -- the reference is "weak", and does not extend + the lifetime of the owning Actor, and sending a message to a `Ref` whose `Actor` has died is + a no-op. (In the future, a dead-letters queue or log may be implemented.) +*/ + +template +class ActorRef { +public: + ActorRef(Object& object_, std::weak_ptr weakMailbox_) + : object(object_), + weakMailbox(std::move(weakMailbox_)) { + } + + template + void invoke(Fn fn, Args&&... args) { + if (auto mailbox = weakMailbox.lock()) { + mailbox->push(actor::makeMessage(object, fn, std::forward(args)...)); + } + } + +private: + Object& object; + std::weak_ptr weakMailbox; +}; + +} // namespace mbgl diff --git a/src/mbgl/actor/mailbox.cpp b/src/mbgl/actor/mailbox.cpp new file mode 100644 index 0000000000..ae3c0967af --- /dev/null +++ b/src/mbgl/actor/mailbox.cpp @@ -0,0 +1,55 @@ +#include +#include +#include + +#include + +namespace mbgl { + +Mailbox::Mailbox(Scheduler& scheduler_) + : scheduler(scheduler_) { +} + +void Mailbox::push(std::unique_ptr message) { + assert(!closing); + + std::lock_guard queueLock(queueMutex); + bool wasEmpty = queue.empty(); + queue.push(std::move(message)); + if (wasEmpty) { + scheduler.schedule(shared_from_this()); + } +} + +void Mailbox::close() { + // Block until the scheduler is guaranteed not to be executing receive(). + std::lock_guard closingLock(closingMutex); + closing = true; +} + +void Mailbox::receive() { + std::lock_guard closingLock(closingMutex); + + if (closing) { + return; + } + + std::unique_ptr message; + bool wasEmpty; + + { + std::lock_guard queueLock(queueMutex); + assert(!queue.empty()); + message = std::move(queue.front()); + queue.pop(); + wasEmpty = queue.empty(); + } + + (*message)(); + + if (!wasEmpty) { + scheduler.schedule(shared_from_this()); + } +} + +} // namespace mbgl diff --git a/src/mbgl/actor/mailbox.hpp b/src/mbgl/actor/mailbox.hpp new file mode 100644 index 0000000000..5d5e8cb924 --- /dev/null +++ b/src/mbgl/actor/mailbox.hpp @@ -0,0 +1,31 @@ +#pragma once + +#include +#include +#include + +namespace mbgl { + +class Scheduler; +class Message; + +class Mailbox : public std::enable_shared_from_this { +public: + Mailbox(Scheduler&); + + void push(std::unique_ptr); + + void close(); + void receive(); + +private: + Scheduler& scheduler; + + std::mutex closingMutex; + bool closing { false }; + + std::mutex queueMutex; + std::queue> queue; +}; + +} // namespace mbgl diff --git a/src/mbgl/actor/message.hpp b/src/mbgl/actor/message.hpp new file mode 100644 index 0000000000..cf071d4933 --- /dev/null +++ b/src/mbgl/actor/message.hpp @@ -0,0 +1,48 @@ +#pragma once + +#include + +namespace mbgl { + +// A movable type-erasing function wrapper. This allows to store arbitrary invokable +// things (like std::function<>, or the result of a movable-only std::bind()) in the queue. +// Source: http://stackoverflow.com/a/29642072/331379 +class Message { +public: + virtual ~Message() = default; + virtual void operator()() = 0; +}; + +template +class MessageImpl : public Message { +public: + MessageImpl(Object& object_, MemberFn memberFn_, ArgsTuple argsTuple_) + : object(object_), + memberFn(memberFn_), + argsTuple(std::move(argsTuple_)) { + } + + void operator()() override { + invoke(std::make_index_sequence::value>()); + } + + template + void invoke(std::index_sequence) { + (object.*memberFn)(std::move(std::get(argsTuple))...); + } + + Object& object; + MemberFn memberFn; + ArgsTuple argsTuple; +}; + +namespace actor { + +template +std::unique_ptr makeMessage(Object& object, MemberFn memberFn, Args&&... args) { + auto tuple = std::make_tuple(std::forward(args)...); + return std::make_unique>(object, memberFn, std::move(tuple)); +} + +} // namespace actor +} // namespace mbgl diff --git a/src/mbgl/actor/scheduler.hpp b/src/mbgl/actor/scheduler.hpp new file mode 100644 index 0000000000..83689c3348 --- /dev/null +++ b/src/mbgl/actor/scheduler.hpp @@ -0,0 +1,38 @@ +#pragma once + +#include + +namespace mbgl { + +class Mailbox; + +/* + A `Scheduler` is responsible for coordinating the processing of messages by + one or more actors via their mailboxes. It's an abstract interface. Currently, + the following concrete implementations exist: + + * `ThreadPool` can coordinate an unlimited number of actors over any number of + threads via a pool, preserving the following behaviors: + + - Messages from each individual mailbox are processed in order + - Only a single message from a mailbox is processed at a time; there is no + concurrency within a mailbox + + Subject to these constraints, processing can happen on whatever thread in the + pool is available. + + * `RunLoop` is a `Scheduler` that is typically used to create a mailbox and + `ActorRef` for an object that lives on the main thread and is not itself wrapped + as an `Actor`: + + auto mailbox = std::make_shared(*util::RunLoop::Get()); + Actor worker(threadPool, ActorRef(*this, mailbox)); +*/ + +class Scheduler { +public: + virtual ~Scheduler() = default; + virtual void schedule(std::weak_ptr) = 0; +}; + +} // namespace mbgl diff --git a/src/mbgl/actor/thread_pool.cpp b/src/mbgl/actor/thread_pool.cpp new file mode 100644 index 0000000000..89ca1b72f0 --- /dev/null +++ b/src/mbgl/actor/thread_pool.cpp @@ -0,0 +1,48 @@ +#include +#include + +namespace mbgl { + +ThreadPool::ThreadPool(std::size_t count) { + threads.reserve(count); + for (std::size_t i = 0; i < count; ++i) { + threads.emplace_back([this] () { + while (true) { + std::unique_lock lock(mutex); + + cv.wait(lock, [this] { + return !queue.empty() || terminate.load(); + }); + + if (terminate.load()) { + return; + } + + auto mailbox = queue.front(); + queue.pop(); + lock.unlock(); + + if (auto locked = mailbox.lock()) { + locked->receive(); + } + } + }); + } +} + +ThreadPool::~ThreadPool() { + terminate.store(true); + cv.notify_all(); + + for (auto& thread : threads) { + thread.join(); + } +} + +void ThreadPool::schedule(std::weak_ptr mailbox) { + std::lock_guard lock(mutex); + queue.push(mailbox); + cv.notify_one(); +} + +} // namespace mbgl diff --git a/src/mbgl/actor/thread_pool.hpp b/src/mbgl/actor/thread_pool.hpp new file mode 100644 index 0000000000..7e0b40f4e3 --- /dev/null +++ b/src/mbgl/actor/thread_pool.hpp @@ -0,0 +1,28 @@ +#pragma once + +#include + +#include +#include +#include +#include +#include + +namespace mbgl { + +class ThreadPool : public Scheduler { +public: + ThreadPool(std::size_t count); + ~ThreadPool() override; + + void schedule(std::weak_ptr) override; + +private: + std::vector threads; + std::queue> queue; + std::mutex mutex; + std::condition_variable cv; + std::atomic_bool terminate { false }; +}; + +} // namespace mbgl diff --git a/src/mbgl/annotation/annotation_tile.cpp b/src/mbgl/annotation/annotation_tile.cpp index c9ad9b5214..9d28f67785 100644 --- a/src/mbgl/annotation/annotation_tile.cpp +++ b/src/mbgl/annotation/annotation_tile.cpp @@ -10,7 +10,7 @@ namespace mbgl { AnnotationTile::AnnotationTile(const OverscaledTileID& overscaledTileID, const style::UpdateParameters& parameters) - : GeometryTile(overscaledTileID, AnnotationManager::SourceID, parameters.style, parameters.mode), + : GeometryTile(overscaledTileID, AnnotationManager::SourceID, parameters), annotationManager(parameters.annotationManager) { annotationManager.addTile(*this); } diff --git a/src/mbgl/map/map.cpp b/src/mbgl/map/map.cpp index 56be99d8fc..ad77ac19fe 100644 --- a/src/mbgl/map/map.cpp +++ b/src/mbgl/map/map.cpp @@ -22,6 +22,7 @@ #include #include #include +#include namespace mbgl { @@ -61,6 +62,7 @@ public: gl::ObjectStore store; Update updateFlags = Update::Nothing; util::AsyncTask asyncUpdate; + ThreadPool workerThreadPool; std::unique_ptr annotationManager; std::unique_ptr painter; @@ -99,6 +101,7 @@ Map::Impl::Impl(View& view_, contextMode(contextMode_), pixelRatio(view.getPixelRatio()), asyncUpdate([this] { update(); }), + workerThreadPool(4), annotationManager(std::make_unique(pixelRatio)) { } @@ -227,9 +230,8 @@ void Map::Impl::update() { style::UpdateParameters parameters(pixelRatio, debugOptions, transform.getState(), - style->workers, + workerThreadPool, fileSource, - style->shouldReparsePartialTiles, mode, *annotationManager, *style); diff --git a/src/mbgl/style/source_impl.cpp b/src/mbgl/style/source_impl.cpp index e124cde583..58ae3d9f91 100644 --- a/src/mbgl/style/source_impl.cpp +++ b/src/mbgl/style/source_impl.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -77,7 +78,7 @@ const std::map& Source::Impl::getRenderTiles() cons return renderTiles; } -void Source::Impl::loadTiles(const UpdateParameters& parameters) { +void Source::Impl::updateTiles(const UpdateParameters& parameters) { if (!loaded) { return; } @@ -161,28 +162,17 @@ void Source::Impl::loadTiles(const UpdateParameters& parameters) { ++retainIt; } } -} -bool Source::Impl::parseTiles(const UpdateParameters& parameters) { - bool allTilesUpdated = true; - const PlacementConfig newConfig{ parameters.transformState.getAngle(), - parameters.transformState.getPitch(), - parameters.debugOptions & MapDebugOptions::Collision }; + const PlacementConfig config { parameters.transformState.getAngle(), + parameters.transformState.getPitch(), + parameters.debugOptions & MapDebugOptions::Collision }; + for (auto& pair : tiles) { - auto tile = pair.second.get(); - if (parameters.shouldReparsePartialTiles && tile->isIncomplete()) { - if (!tile->parsePending()) { - allTilesUpdated = false; - } - } else { - tile->redoPlacement(newConfig); - } + pair.second->setPlacementConfig(config); } - - return allTilesUpdated; } -void Source::Impl::reload() { +void Source::Impl::reloadTiles() { cache.clear(); for (auto& pair : tiles) { @@ -258,18 +248,14 @@ void Source::Impl::setObserver(SourceObserver* observer_) { observer = observer_; } -void Source::Impl::onTileLoaded(Tile& tile, TileLoadState loadState) { - observer->onTileLoaded(base, tile.id, loadState); +void Source::Impl::onTileChanged(Tile& tile) { + observer->onTileChanged(base, tile.id); } void Source::Impl::onTileError(Tile& tile, std::exception_ptr error) { observer->onTileError(base, tile.id, error); } -void Source::Impl::onTileUpdated(Tile& tile) { - observer->onTileUpdated(base, tile.id); -} - void Source::Impl::dumpDebugLogs() const { Log::Info(Event::General, "Source::id: %s", base.getID().c_str()); Log::Info(Event::General, "Source::loaded: %d", loaded); diff --git a/src/mbgl/style/source_impl.hpp b/src/mbgl/style/source_impl.hpp index 5a54f4781a..0de3760fc3 100644 --- a/src/mbgl/style/source_impl.hpp +++ b/src/mbgl/style/source_impl.hpp @@ -43,16 +43,14 @@ public: virtual void loadDescription(FileSource&) = 0; bool isLoaded() const; - // Request or parse all the tiles relevant for the "TransformState". This method - // will return true if all the tiles were scheduled for updating of false if - // they were not. shouldReparsePartialTiles must be set to "true" if there is - // new data available that a tile in the "partial" state might be interested at. - void loadTiles(const UpdateParameters&); - bool parseTiles(const UpdateParameters&); + // Called when the camera has changed or icons or glyphs are loaded. May load new + // tiles, unload obsolete tiles, and trigger further parsing of incomplete tiles or + // re-placement of existing complete tiles. + void updateTiles(const UpdateParameters&); // Request that all loaded tiles re-run the layout operation on the existing source // data with fresh style information. - void reload(); + void reloadTiles(); void startRender(algorithm::ClipIDGenerator&, const mat4& projMatrix, @@ -88,9 +86,8 @@ protected: private: // TileObserver implementation. - void onTileLoaded(Tile&, TileLoadState) override; + void onTileChanged(Tile&) override; void onTileError(Tile&, std::exception_ptr) override; - void onTileUpdated(Tile&) override; virtual uint16_t getTileSize() const = 0; virtual Range getZoomRange() = 0; diff --git a/src/mbgl/style/source_observer.hpp b/src/mbgl/style/source_observer.hpp index 6e0b226c3b..26b25b78da 100644 --- a/src/mbgl/style/source_observer.hpp +++ b/src/mbgl/style/source_observer.hpp @@ -19,9 +19,8 @@ public: virtual void onSourceLoaded(Source&) {} virtual void onSourceError(Source&, std::exception_ptr) {} - virtual void onTileLoaded(Source&, const OverscaledTileID&, TileLoadState) {} + virtual void onTileChanged(Source&, const OverscaledTileID&) {} virtual void onTileError(Source&, const OverscaledTileID&, std::exception_ptr) {} - virtual void onTileUpdated(Source&, const OverscaledTileID&) {} }; } // namespace style diff --git a/src/mbgl/style/style.cpp b/src/mbgl/style/style.cpp index 34790683bc..a00c5e4efc 100644 --- a/src/mbgl/style/style.cpp +++ b/src/mbgl/style/style.cpp @@ -43,8 +43,7 @@ Style::Style(FileSource& fileSource_, float pixelRatio) spriteStore(std::make_unique(pixelRatio)), spriteAtlas(std::make_unique(1024, 1024, pixelRatio, *spriteStore)), lineAtlas(std::make_unique(256, 512)), - observer(&nullObserver), - workers(4) { + observer(&nullObserver) { glyphStore->setObserver(this); spriteStore->setObserver(this); } @@ -215,19 +214,8 @@ double Style::getDefaultPitch() const { } void Style::updateTiles(const UpdateParameters& parameters) { - bool allTilesUpdated = true; - for (const auto& source : sources) { - source->baseImpl->loadTiles(parameters); - if (!source->baseImpl->parseTiles(parameters)) { - allTilesUpdated = false; - } - } - - // We can only stop updating "partial" tiles when all of them - // were notified of the arrival of the new resources. - if (allTilesUpdated) { - shouldReparsePartialTiles = false; + source->baseImpl->updateTiles(parameters); } } @@ -235,7 +223,7 @@ void Style::relayout() { for (const auto& sourceID : updateBatch.sourceIDs) { Source* source = getSource(sourceID); if (!source) continue; - source->baseImpl->reload(); + source->baseImpl->reloadTiles(); } updateBatch.sourceIDs.clear(); } @@ -447,7 +435,6 @@ void Style::setObserver(style::Observer* observer_) { } void Style::onGlyphsLoaded(const FontStack& fontStack, const GlyphRange& glyphRange) { - shouldReparsePartialTiles = true; observer->onGlyphsLoaded(fontStack, glyphRange); observer->onUpdate(Update::Repaint); } @@ -473,12 +460,8 @@ void Style::onSourceError(Source& source, std::exception_ptr error) { observer->onResourceError(error); } -void Style::onTileLoaded(Source& source, const OverscaledTileID& tileID, TileLoadState loadState) { - if (loadState == TileLoadState::First) { - shouldReparsePartialTiles = true; - } - - observer->onTileLoaded(source, tileID, loadState); +void Style::onTileChanged(Source& source, const OverscaledTileID& tileID) { + observer->onTileChanged(source, tileID); observer->onUpdate(Update::Repaint); } @@ -490,12 +473,7 @@ void Style::onTileError(Source& source, const OverscaledTileID& tileID, std::exc observer->onResourceError(error); } -void Style::onTileUpdated(Source&, const OverscaledTileID&) { - observer->onUpdate(Update::Repaint); -} - void Style::onSpriteLoaded() { - shouldReparsePartialTiles = true; observer->onSpriteLoaded(); observer->onUpdate(Update::Repaint); } diff --git a/src/mbgl/style/style.hpp b/src/mbgl/style/style.hpp index 8027313ab9..3e678ec060 100644 --- a/src/mbgl/style/style.hpp +++ b/src/mbgl/style/style.hpp @@ -12,25 +12,28 @@ #include #include -#include #include #include #include #include +#include #include #include namespace mbgl { class FileSource; +class GlyphStore; class GlyphAtlas; +class SpriteStore; class SpriteAtlas; class LineAtlas; class RenderData; namespace style { +class Layer; class UpdateParameters; class QueryParameters; @@ -134,9 +137,8 @@ private: // SourceObserver implementation. void onSourceLoaded(Source&) override; void onSourceError(Source&, std::exception_ptr) override; - void onTileLoaded(Source&, const OverscaledTileID&, TileLoadState) override; + void onTileChanged(Source&, const OverscaledTileID&) override; void onTileError(Source&, const OverscaledTileID&, std::exception_ptr) override; - void onTileUpdated(Source&, const OverscaledTileID&) override; // LayerObserver implementation. void onLayerFilterChanged(Layer&) override; @@ -154,9 +156,7 @@ private: bool hasPendingTransitions = false; public: - bool shouldReparsePartialTiles = false; bool loaded = false; - Worker workers; }; } // namespace style diff --git a/src/mbgl/style/update_parameters.hpp b/src/mbgl/style/update_parameters.hpp index 306cddf706..900f4b5183 100644 --- a/src/mbgl/style/update_parameters.hpp +++ b/src/mbgl/style/update_parameters.hpp @@ -5,7 +5,7 @@ namespace mbgl { class TransformState; -class Worker; +class Scheduler; class FileSource; class AnnotationManager; @@ -18,18 +18,16 @@ public: UpdateParameters(float pixelRatio_, MapDebugOptions debugOptions_, const TransformState& transformState_, - Worker& worker_, + Scheduler& workerScheduler_, FileSource& fileSource_, - bool shouldReparsePartialTiles_, const MapMode mode_, AnnotationManager& annotationManager_, Style& style_) : pixelRatio(pixelRatio_), debugOptions(debugOptions_), transformState(transformState_), - worker(worker_), + workerScheduler(workerScheduler_), fileSource(fileSource_), - shouldReparsePartialTiles(shouldReparsePartialTiles_), mode(mode_), annotationManager(annotationManager_), style(style_) {} @@ -37,9 +35,8 @@ public: float pixelRatio; MapDebugOptions debugOptions; const TransformState& transformState; - Worker& worker; + Scheduler& workerScheduler; FileSource& fileSource; - bool shouldReparsePartialTiles; const MapMode mode; AnnotationManager& annotationManager; diff --git a/src/mbgl/tile/geojson_tile.cpp b/src/mbgl/tile/geojson_tile.cpp index 5dc099de69..2b302e14ec 100644 --- a/src/mbgl/tile/geojson_tile.cpp +++ b/src/mbgl/tile/geojson_tile.cpp @@ -1,6 +1,5 @@ #include #include -#include #include #include @@ -80,7 +79,7 @@ GeoJSONTile::GeoJSONTile(const OverscaledTileID& overscaledTileID, std::string sourceID_, const style::UpdateParameters& parameters, const mapbox::geometry::feature_collection& features) - : GeometryTile(overscaledTileID, sourceID_, parameters.style, parameters.mode) { + : GeometryTile(overscaledTileID, sourceID_, parameters) { setData(std::make_unique(features)); } diff --git a/src/mbgl/tile/geometry_tile.cpp b/src/mbgl/tile/geometry_tile.cpp index c432869fa5..ed9d4a551b 100644 --- a/src/mbgl/tile/geometry_tile.cpp +++ b/src/mbgl/tile/geometry_tile.cpp @@ -1,16 +1,17 @@ #include -#include +#include #include +#include +#include #include #include #include -#include -#include #include #include #include #include #include +#include namespace mbgl { @@ -18,75 +19,53 @@ using namespace style; GeometryTile::GeometryTile(const OverscaledTileID& id_, std::string sourceID_, - Style& style_, - const MapMode mode_) + const style::UpdateParameters& parameters) : Tile(id_), sourceID(std::move(sourceID_)), - style(style_), - worker(style_.workers), - tileWorker(id_, - *style_.spriteStore, - *style_.glyphAtlas, - *style_.glyphStore, - obsolete, - mode_) { + style(parameters.style), + mailbox(std::make_shared(*util::RunLoop::Get())), + worker(parameters.workerScheduler, + ActorRef(*this, mailbox), + id_, + *parameters.style.spriteStore, + *parameters.style.glyphAtlas, + *parameters.style.glyphStore, + obsolete, + parameters.mode) { } GeometryTile::~GeometryTile() { cancel(); } -void GeometryTile::setError(std::exception_ptr err) { - observer->onTileError(*this, err); +void GeometryTile::cancel() { + obsolete = true; } -std::vector> GeometryTile::cloneStyleLayers() const { - std::vector> result; - - for (const Layer* layer : style.getLayers()) { - // Avoid cloning and including irrelevant layers. - if (layer->is() || - layer->is() || - layer->baseImpl->source != sourceID || - id.overscaledZ < std::floor(layer->baseImpl->minZoom) || - id.overscaledZ >= std::ceil(layer->baseImpl->maxZoom) || - layer->baseImpl->visibility == VisibilityType::None) { - continue; - } - - result.push_back(layer->baseImpl->clone()); - } - - return result; +void GeometryTile::setError(std::exception_ptr err) { + availableData = DataAvailability::All; + observer->onTileError(*this, err); } void GeometryTile::setData(std::unique_ptr data_) { - if (!data_) { - // This is a 404 response. We're treating these as empty tiles. - workRequest.reset(); - availableData = DataAvailability::All; - buckets.clear(); - featureIndex.reset(); - data.reset(); - redoPlacement(); - observer->onTileLoaded(*this, TileLoadState::First); - return; - } - // Mark the tile as pending again if it was complete before to prevent signaling a complete // state despite pending parse operations. if (availableData == DataAvailability::All) { availableData = DataAvailability::Some; } - // Kick off a fresh parse of this tile. This happens when the tile is new, or - // when tile data changed. Replacing the workdRequest will cancel a pending work - // request in case there is one. - workRequest.reset(); - workRequest = worker.parseGeometryTile(tileWorker, cloneStyleLayers(), std::move(data_), targetConfig, - [this, config = targetConfig] (TileParseResult result) { - tileLoaded(std::move(result), config); - }); + ++correlationID; + worker.invoke(&GeometryTileWorker::setData, std::move(data_), correlationID); + redoLayout(); +} + +void GeometryTile::setPlacementConfig(const PlacementConfig& desiredConfig) { + if (placedConfig == desiredConfig) { + return; + } + + ++correlationID; + worker.invoke(&GeometryTileWorker::setPlacementConfig, desiredConfig, correlationID); } void GeometryTile::redoLayout() { @@ -96,79 +75,44 @@ void GeometryTile::redoLayout() { availableData = DataAvailability::Some; } - workRequest.reset(); - workRequest = worker.redoLayout(tileWorker, cloneStyleLayers(), targetConfig, - [this, config = targetConfig] (TileParseResult result) { - tileLoaded(std::move(result), config); - }); -} - -void GeometryTile::tileLoaded(TileParseResult result, PlacementConfig config) { - workRequest.reset(); + std::vector> copy; - if (result.is()) { - auto& resultBuckets = result.get(); - availableData = resultBuckets.complete ? DataAvailability::All : DataAvailability::Some; + for (const Layer* layer : style.getLayers()) { + // Avoid cloning and including irrelevant layers. + if (layer->is() || + layer->is() || + layer->baseImpl->source != sourceID || + id.overscaledZ < std::floor(layer->baseImpl->minZoom) || + id.overscaledZ >= std::ceil(layer->baseImpl->maxZoom) || + layer->baseImpl->visibility == VisibilityType::None) { + continue; + } - // Persist the configuration we just placed so that we can later check whether we need to - // place again in case the configuration has changed. - placedConfig = config; + copy.push_back(layer->baseImpl->clone()); + } - // Move over all buckets we received in this parse request, potentially overwriting - // existing buckets in case we got a refresh parse. - buckets = std::move(resultBuckets.buckets); + ++correlationID; + worker.invoke(&GeometryTileWorker::setLayers, std::move(copy), correlationID); +} - if (isComplete()) { - featureIndex = std::move(resultBuckets.featureIndex); - data = std::move(resultBuckets.tileData); - } +void GeometryTile::onLayout(LayoutResult result) { + availableData = DataAvailability::Some; + buckets = std::move(result.buckets); + featureIndex = std::move(result.featureIndex); + data = std::move(result.tileData); + observer->onTileChanged(*this); +} - redoPlacement(); - observer->onTileLoaded(*this, TileLoadState::First); - } else { +void GeometryTile::onPlacement(PlacementResult result) { + if (result.correlationID == correlationID) { availableData = DataAvailability::All; - observer->onTileError(*this, result.get()); } -} - -bool GeometryTile::parsePending() { - if (workRequest) { - // There's already parsing or placement going on. - return false; + for (auto& bucket : result.buckets) { + buckets[bucket.first] = std::move(bucket.second); } - - workRequest.reset(); - workRequest = worker.parsePendingGeometryTileLayers(tileWorker, targetConfig, [this, config = targetConfig] (TileParseResult result) { - workRequest.reset(); - - if (result.is()) { - auto& resultBuckets = result.get(); - availableData = resultBuckets.complete ? DataAvailability::All : DataAvailability::Some; - - // Move over all buckets we received in this parse request, potentially overwriting - // existing buckets in case we got a refresh parse. - for (auto& bucket : resultBuckets.buckets) { - buckets[bucket.first] = std::move(bucket.second); - } - - // Persist the configuration we just placed so that we can later check whether we need to - // place again in case the configuration has changed. - placedConfig = config; - - if (isComplete()) { - featureIndex = std::move(resultBuckets.featureIndex); - data = std::move(resultBuckets.tileData); - } - - redoPlacement(); - observer->onTileLoaded(*this, TileLoadState::Subsequent); - } else { - availableData = DataAvailability::All; - observer->onTileError(*this, result.get()); - } - }); - - return true; + featureIndex->setCollisionTile(std::move(result.collisionTile)); + placedConfig = result.placedConfig; + observer->onTileChanged(*this); } Bucket* GeometryTile::getBucket(const Layer& layer) { @@ -181,43 +125,6 @@ Bucket* GeometryTile::getBucket(const Layer& layer) { return it->second.get(); } -void GeometryTile::redoPlacement(const PlacementConfig newConfig) { - targetConfig = newConfig; - redoPlacement(); -} - -void GeometryTile::redoPlacement() { - // Don't start a new placement request when the current one hasn't completed yet, or when - // we are parsing buckets. - if (workRequest || targetConfig == placedConfig) { - return; - } - - workRequest = worker.redoPlacement(tileWorker, targetConfig, [this, config = targetConfig](TilePlacementResult result) { - workRequest.reset(); - - // Persist the configuration we just placed so that we can later check whether we need to - // place again in case the configuration has changed. - placedConfig = config; - - for (auto& bucket : result.buckets) { - buckets[bucket.first] = std::move(bucket.second); - } - - if (featureIndex) { - featureIndex->setCollisionTile(std::move(result.collisionTile)); - } - - // The target configuration could have changed since we started placement. In this case, - // we're starting another placement run. - if (placedConfig != targetConfig) { - redoPlacement(); - } else { - observer->onTileUpdated(*this); - } - }); -} - void GeometryTile::queryRenderedFeatures( std::unordered_map>& result, const GeometryCoordinates& queryGeometry, @@ -237,9 +144,4 @@ void GeometryTile::queryRenderedFeatures( style); } -void GeometryTile::cancel() { - obsolete = true; - workRequest.reset(); -} - } // namespace mbgl diff --git a/src/mbgl/tile/geometry_tile.hpp b/src/mbgl/tile/geometry_tile.hpp index e0db58325c..a644992376 100644 --- a/src/mbgl/tile/geometry_tile.hpp +++ b/src/mbgl/tile/geometry_tile.hpp @@ -1,9 +1,10 @@ #pragma once #include -#include +#include #include #include +#include #include #include @@ -12,34 +13,31 @@ namespace mbgl { -class AsyncRequest; class GeometryTileData; class FeatureIndex; +class CollisionTile; namespace style { class Style; class Layer; +class UpdateParameters; } // namespace style class GeometryTile : public Tile { public: GeometryTile(const OverscaledTileID&, std::string sourceID, - style::Style&, - const MapMode); + const style::UpdateParameters&); ~GeometryTile() override; void setError(std::exception_ptr err); void setData(std::unique_ptr); + void setPlacementConfig(const PlacementConfig&) override; + void redoLayout() override; Bucket* getBucket(const style::Layer&) override; - bool parsePending() override; - - void redoLayout() override; - void redoPlacement(PlacementConfig) override; - void queryRenderedFeatures( std::unordered_map>& result, const GeometryCoordinates& queryGeometry, @@ -48,35 +46,40 @@ public: void cancel() override; -private: - std::vector> cloneStyleLayers() const; - void redoPlacement(); - - void tileLoaded(TileParseResult, PlacementConfig); + class LayoutResult { + public: + std::unordered_map> buckets; + std::unique_ptr featureIndex; + std::unique_ptr tileData; + uint64_t correlationID; + }; + void onLayout(LayoutResult); + + class PlacementResult { + public: + std::unordered_map> buckets; + std::unique_ptr collisionTile; + PlacementConfig placedConfig; + uint64_t correlationID; + }; + void onPlacement(PlacementResult); +private: const std::string sourceID; style::Style& style; - Worker& worker; - TileWorker tileWorker; - std::unique_ptr workRequest; + // Used to signal the worker that it should abandon parsing this tile as soon as possible. + std::atomic obsolete { false }; - // Contains all the Bucket objects for the tile. Buckets are render - // objects and they get added by tile parsing operations. - std::unordered_map> buckets; + std::shared_ptr mailbox; + Actor worker; + + uint64_t correlationID = 0; + optional placedConfig; + std::unordered_map> buckets; std::unique_ptr featureIndex; std::unique_ptr data; - - // Stores the placement configuration of the text that is currently placed on the screen. - PlacementConfig placedConfig; - - // Stores the placement configuration of how the text should be placed. This isn't necessarily - // the one that is being displayed. - PlacementConfig targetConfig; - - // Used to signal the worker that it should abandon parsing this tile as soon as possible. - std::atomic obsolete { false }; }; } // namespace mbgl diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp new file mode 100644 index 0000000000..35d15ae1fa --- /dev/null +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -0,0 +1,300 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +namespace mbgl { + +using namespace style; + +GeometryTileWorker::GeometryTileWorker(ActorRef self_, + ActorRef parent_, + OverscaledTileID id_, + SpriteStore& spriteStore_, + GlyphAtlas& glyphAtlas_, + GlyphStore& glyphStore_, + const std::atomic& obsolete_, + const MapMode mode_) + : self(std::move(self_)), + parent(std::move(parent_)), + id(std::move(id_)), + spriteStore(spriteStore_), + glyphAtlas(glyphAtlas_), + glyphStore(glyphStore_), + obsolete(obsolete_), + mode(mode_) { +} + +GeometryTileWorker::~GeometryTileWorker() { + glyphAtlas.removeGlyphs(reinterpret_cast(this)); +} + +/* + GeometryTileWorker is a state machine. This is its transition diagram. + States are indicated by [state], lines are transitions triggered by + messages, (parentheses) are actions taken on transition. + + [idle] <------------------. + | | + set{Data,Layers,Placement} | + | | + (do layout/placement; self-send "coalesced") | + v | + [coalescing] --- coalesced --. + | | + .-----------------. .---------------. + | | + .--- set{Data,Layers} setPlacement -----. + | | | | + | v v | + .-- [need layout] <-- set{Data,Layers} -- [need placement] --. + | | + coalesced coalesced + | | + v v + (do layout or placement; self-send "coalesced"; goto [coalescing]) + + The idea is that in the [idle] state, layout or placement happens immediately + in response to a "set" message. During this processing, multiple "set" messages + might get queued in the mailbox. At the end of processing, we self-send "coalesced", + read all the queued messages until we get to "coalesced", and then redo either + layout or placement if there were one or more "set"s (with layout taking priority, + since it will trigger placement when complete), or return to the [idle] state if not. +*/ + +void GeometryTileWorker::setData(std::unique_ptr data_, uint64_t correlationID_) { + try { + data = std::move(data_); + correlationID = correlationID_; + + switch (state) { + case Idle: + redoLayout(); + coalesce(); + break; + + case Coalescing: + case NeedLayout: + case NeedPlacement: + state = NeedLayout; + break; + } + } catch (...) { + parent.invoke(&GeometryTile::setError, std::current_exception()); + } +} + +void GeometryTileWorker::setLayers(std::vector> layers_, uint64_t correlationID_) { + try { + layers = std::move(layers_); + correlationID = correlationID_; + + switch (state) { + case Idle: + redoLayout(); + coalesce(); + break; + + case Coalescing: + case NeedPlacement: + state = NeedLayout; + break; + + case NeedLayout: + break; + } + } catch (...) { + parent.invoke(&GeometryTile::setError, std::current_exception()); + } +} + +void GeometryTileWorker::setPlacementConfig(PlacementConfig placementConfig_, uint64_t correlationID_) { + try { + placementConfig = std::move(placementConfig_); + correlationID = correlationID_; + + switch (state) { + case Idle: + attemptPlacement(); + coalesce(); + break; + + case Coalescing: + state = NeedPlacement; + break; + + case NeedPlacement: + case NeedLayout: + break; + } + } catch (...) { + parent.invoke(&GeometryTile::setError, std::current_exception()); + } +} + +void GeometryTileWorker::coalesced() { + try { + switch (state) { + case Idle: + assert(false); + break; + + case Coalescing: + state = Idle; + break; + + case NeedLayout: + redoLayout(); + coalesce(); + break; + + case NeedPlacement: + attemptPlacement(); + coalesce(); + break; + } + } catch (...) { + parent.invoke(&GeometryTile::setError, std::current_exception()); + } +} + +void GeometryTileWorker::coalesce() { + state = Coalescing; + self.invoke(&GeometryTileWorker::coalesced); +} + +void GeometryTileWorker::redoLayout() { + if (!data || !layers) { + return; + } + + // We're doing a fresh parse of the tile, because the underlying data or style has changed. + symbolLayouts.clear(); + + // We're storing a set of bucket names we've parsed to avoid parsing a bucket twice that is + // referenced from more than one layer + std::unordered_set parsed; + std::unordered_map> buckets; + auto featureIndex = std::make_unique(); + + for (auto i = layers->rbegin(); i != layers->rend(); i++) { + if (obsolete) { + return; + } + + const Layer* layer = i->get(); + const std::string& bucketName = layer->baseImpl->bucketName(); + + featureIndex->addBucketLayerName(bucketName, layer->baseImpl->id); + + if (parsed.find(bucketName) != parsed.end()) { + continue; + } + + parsed.emplace(bucketName); + + if (!*data) { + continue; // Tile has no data. + } + + auto geometryLayer = (*data)->getLayer(layer->baseImpl->sourceLayer); + if (!geometryLayer) { + continue; + } + + BucketParameters parameters(id, + *geometryLayer, + obsolete, + reinterpret_cast(this), + spriteStore, + glyphAtlas, + glyphStore, + *featureIndex, + mode); + + if (layer->is()) { + symbolLayouts.push_back(layer->as()->impl->createLayout(parameters)); + } else { + std::unique_ptr bucket = layer->baseImpl->createBucket(parameters); + if (bucket->hasData()) { + buckets.emplace(layer->baseImpl->bucketName(), std::move(bucket)); + } + } + } + + parent.invoke(&GeometryTile::onLayout, GeometryTile::LayoutResult { + std::move(buckets), + std::move(featureIndex), + *data ? (*data)->clone() : nullptr, + correlationID + }); + + attemptPlacement(); +} + +void GeometryTileWorker::attemptPlacement() { + if (!data || !layers || !placementConfig) { + return; + } + + bool canPlace = true; + + // Prepare as many SymbolLayouts as possible. + for (auto& symbolLayout : symbolLayouts) { + if (obsolete) { + return; + } + + if (symbolLayout->state == SymbolLayout::Pending) { + if (symbolLayout->canPrepare(glyphStore, spriteStore)) { + symbolLayout->state = SymbolLayout::Prepared; + symbolLayout->prepare(reinterpret_cast(this), + glyphAtlas, + glyphStore); + } else { + canPlace = false; + } + } + } + + if (!canPlace) { + return; // We'll be notified (via `setPlacementConfig`) when it's time to try again. + } + + auto collisionTile = std::make_unique(*placementConfig); + std::unordered_map> buckets; + + for (auto& symbolLayout : symbolLayouts) { + if (obsolete) { + return; + } + + symbolLayout->state = SymbolLayout::Placed; + if (symbolLayout->hasSymbolInstances()) { + buckets.emplace(symbolLayout->bucketName, + symbolLayout->place(*collisionTile)); + } + } + + parent.invoke(&GeometryTile::onPlacement, GeometryTile::PlacementResult { + std::move(buckets), + std::move(collisionTile), + *placementConfig, + correlationID + }); +} + +} // namespace mbgl diff --git a/src/mbgl/tile/geometry_tile_worker.hpp b/src/mbgl/tile/geometry_tile_worker.hpp new file mode 100644 index 0000000000..cc5e48f9b4 --- /dev/null +++ b/src/mbgl/tile/geometry_tile_worker.hpp @@ -0,0 +1,76 @@ +#pragma once + +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace mbgl { + +class GeometryTile; +class GeometryTileData; +class SpriteStore; +class GlyphAtlas; +class GlyphStore; +class SymbolLayout; + +namespace style { +class Layer; +} // namespace style + +class GeometryTileWorker { +public: + GeometryTileWorker(ActorRef self, + ActorRef parent, + OverscaledTileID, + SpriteStore&, + GlyphAtlas&, + GlyphStore&, + const std::atomic&, + const MapMode); + ~GeometryTileWorker(); + + void setLayers(std::vector>, uint64_t correlationID); + void setData(std::unique_ptr, uint64_t correlationID); + void setPlacementConfig(PlacementConfig, uint64_t correlationID); + +private: + void coalesce(); + void coalesced(); + void redoLayout(); + void attemptPlacement(); + + ActorRef self; + ActorRef parent; + + const OverscaledTileID id; + SpriteStore& spriteStore; + GlyphAtlas& glyphAtlas; + GlyphStore& glyphStore; + const std::atomic& obsolete; + const MapMode mode; + + enum State { + Idle, + Coalescing, + NeedLayout, + NeedPlacement + }; + + State state = Idle; + uint64_t correlationID = 0; + + // Outer optional indicates whether we've received it or not. + optional>> layers; + optional> data; + optional placementConfig; + + std::vector> symbolLayouts; +}; + +} // namespace mbgl diff --git a/src/mbgl/tile/raster_tile.cpp b/src/mbgl/tile/raster_tile.cpp index 9230c3d79d..303212da80 100644 --- a/src/mbgl/tile/raster_tile.cpp +++ b/src/mbgl/tile/raster_tile.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -6,8 +7,8 @@ #include #include #include -#include -#include +#include +#include namespace mbgl { @@ -15,13 +16,20 @@ RasterTile::RasterTile(const OverscaledTileID& id_, const style::UpdateParameters& parameters, const Tileset& tileset) : Tile(id_), - worker(parameters.worker), - loader(*this, id_, parameters, tileset) { + loader(*this, id_, parameters, tileset), + mailbox(std::make_shared(*util::RunLoop::Get())), + worker(parameters.workerScheduler, + ActorRef(*this, mailbox)) { } RasterTile::~RasterTile() = default; +void RasterTile::cancel() { +} + void RasterTile::setError(std::exception_ptr err) { + bucket.reset(); + availableData = DataAvailability::All; observer->onTileError(*this, err); } @@ -30,30 +38,13 @@ void RasterTile::setData(std::shared_ptr data, optional expires_) { modified = modified_; expires = expires_; + worker.invoke(&RasterTileWorker::parse, data); +} - if (!data) { - // This is a 404 response. We're treating these as empty tiles. - workRequest.reset(); - availableData = DataAvailability::All; - bucket.reset(); - observer->onTileLoaded(*this, TileLoadState::First); - return; - } - - workRequest.reset(); - workRequest = worker.parseRasterTile(std::make_unique(), data, [this] (RasterTileParseResult result) { - workRequest.reset(); - - availableData = DataAvailability::All; - - if (result.is>()) { - bucket = std::move(result.get>()); - observer->onTileLoaded(*this, TileLoadState::First); - } else { - bucket.reset(); - observer->onTileError(*this, result.get()); - } - }); +void RasterTile::onParsed(std::unique_ptr result) { + bucket = std::move(result); + availableData = DataAvailability::All; + observer->onTileChanged(*this); } Bucket* RasterTile::getBucket(const style::Layer&) { @@ -64,8 +55,4 @@ void RasterTile::setNecessity(Necessity necessity) { loader.setNecessity(necessity); } -void RasterTile::cancel() { - workRequest.reset(); -} - } // namespace mbgl diff --git a/src/mbgl/tile/raster_tile.hpp b/src/mbgl/tile/raster_tile.hpp index 496edda6b3..2b2e84d463 100644 --- a/src/mbgl/tile/raster_tile.hpp +++ b/src/mbgl/tile/raster_tile.hpp @@ -2,11 +2,11 @@ #include #include -#include +#include +#include namespace mbgl { -class AsyncRequest; class Tileset; namespace style { @@ -32,11 +32,13 @@ public: void cancel() override; Bucket* getBucket(const style::Layer&) override; -private: - Worker& worker; + void onParsed(std::unique_ptr result); +private: TileLoader loader; - std::unique_ptr workRequest; + + std::shared_ptr mailbox; + Actor worker; // Contains the Bucket object for the tile. Buckets are render // objects and they get added by tile parsing operations. diff --git a/src/mbgl/tile/raster_tile_worker.cpp b/src/mbgl/tile/raster_tile_worker.cpp new file mode 100644 index 0000000000..838737d5b8 --- /dev/null +++ b/src/mbgl/tile/raster_tile_worker.cpp @@ -0,0 +1,27 @@ +#include +#include +#include +#include + +namespace mbgl { + +RasterTileWorker::RasterTileWorker(ActorRef, ActorRef parent_) + : parent(std::move(parent_)) { +} + +void RasterTileWorker::parse(std::shared_ptr data) { + if (!data) { + parent.invoke(&RasterTile::onParsed, nullptr); // No data; empty tile. + return; + } + + try { + auto bucket = std::make_unique(); + bucket->setImage(decodeImage(*data)); + parent.invoke(&RasterTile::onParsed, std::move(bucket)); + } catch (...) { + parent.invoke(&RasterTile::setError, std::current_exception()); + } +} + +} // namespace mbgl diff --git a/src/mbgl/tile/raster_tile_worker.hpp b/src/mbgl/tile/raster_tile_worker.hpp new file mode 100644 index 0000000000..44bc37ca5d --- /dev/null +++ b/src/mbgl/tile/raster_tile_worker.hpp @@ -0,0 +1,22 @@ +#pragma once + +#include + +#include +#include + +namespace mbgl { + +class RasterTile; + +class RasterTileWorker { +public: + RasterTileWorker(ActorRef, ActorRef); + + void parse(std::shared_ptr data); + +private: + ActorRef parent; +}; + +} // namespace mbgl diff --git a/src/mbgl/tile/tile.cpp b/src/mbgl/tile/tile.cpp index 632e271093..ec0540b1ad 100644 --- a/src/mbgl/tile/tile.cpp +++ b/src/mbgl/tile/tile.cpp @@ -18,7 +18,7 @@ void Tile::setObserver(TileObserver* observer_) { void Tile::setTriedOptional() { triedOptional = true; - observer->onTileUpdated(*this); + observer->onTileChanged(*this); } void Tile::dumpDebugLogs() const { diff --git a/src/mbgl/tile/tile.hpp b/src/mbgl/tile/tile.hpp index 740ced3898..be03608994 100644 --- a/src/mbgl/tile/tile.hpp +++ b/src/mbgl/tile/tile.hpp @@ -6,7 +6,6 @@ #include #include #include -#include #include #include @@ -17,10 +16,10 @@ namespace mbgl { -class Worker; class DebugBucket; class TransformState; class TileObserver; +class PlacementConfig; namespace style { class Layer; @@ -47,9 +46,8 @@ public: virtual Bucket* getBucket(const style::Layer&) = 0; - virtual bool parsePending() { return true; } + virtual void setPlacementConfig(const PlacementConfig&) {} virtual void redoLayout() {} - virtual void redoPlacement(PlacementConfig) {} virtual void queryRenderedFeatures( std::unordered_map>& result, diff --git a/src/mbgl/tile/tile_observer.hpp b/src/mbgl/tile/tile_observer.hpp index 94243cb1fa..837b47ae0b 100644 --- a/src/mbgl/tile/tile_observer.hpp +++ b/src/mbgl/tile/tile_observer.hpp @@ -6,18 +6,12 @@ namespace mbgl { class Tile; -enum class TileLoadState : bool { - First = true, - Subsequent = false, -}; - class TileObserver { public: virtual ~TileObserver() = default; - virtual void onTileLoaded(Tile&, TileLoadState) {} + virtual void onTileChanged(Tile&) {} virtual void onTileError(Tile&, std::exception_ptr) {} - virtual void onTileUpdated(Tile&) {} }; } // namespace mbgl diff --git a/src/mbgl/tile/tile_worker.cpp b/src/mbgl/tile/tile_worker.cpp deleted file mode 100644 index 97e682d697..0000000000 --- a/src/mbgl/tile/tile_worker.cpp +++ /dev/null @@ -1,179 +0,0 @@ -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -namespace mbgl { - -using namespace style; - -TileWorker::TileWorker(OverscaledTileID id_, - SpriteStore& spriteStore_, - GlyphAtlas& glyphAtlas_, - GlyphStore& glyphStore_, - const std::atomic& obsolete_, - const MapMode mode_) - : id(std::move(id_)), - spriteStore(spriteStore_), - glyphAtlas(glyphAtlas_), - glyphStore(glyphStore_), - obsolete(obsolete_), - mode(mode_) { -} - -TileWorker::~TileWorker() { - glyphAtlas.removeGlyphs(reinterpret_cast(this)); -} - -TileParseResult TileWorker::parseAllLayers(std::vector> layers_, - std::unique_ptr tileData_, - const PlacementConfig& config) { - tileData = std::move(tileData_); - return redoLayout(std::move(layers_), config); -} - -TileParseResult TileWorker::redoLayout(std::vector> layers_, - const PlacementConfig& config) { - layers = std::move(layers_); - - // We're doing a fresh parse of the tile, because the underlying data or style has changed. - featureIndex = std::make_unique(); - symbolLayouts.clear(); - - // We're storing a set of bucket names we've parsed to avoid parsing a bucket twice that is - // referenced from more than one layer - std::unordered_map> buckets; - std::unordered_set parsed; - - for (auto i = layers.rbegin(); i != layers.rend(); i++) { - if (obsolete) { - break; - } - - // Temporary prevention for crashing due to https://github.com/mapbox/mapbox-gl-native/issues/6263. - // Instead, the race condition will produce a blank tile. - if (!tileData) { - break; - } - - const Layer* layer = i->get(); - const std::string& bucketName = layer->baseImpl->bucketName(); - - featureIndex->addBucketLayerName(bucketName, layer->baseImpl->id); - - if (parsed.find(bucketName) != parsed.end()) { - continue; - } - - parsed.emplace(bucketName); - - auto geometryLayer = tileData->getLayer(layer->baseImpl->sourceLayer); - if (!geometryLayer) { - continue; - } - - BucketParameters parameters(id, - *geometryLayer, - obsolete, - reinterpret_cast(this), - spriteStore, - glyphAtlas, - glyphStore, - *featureIndex, - mode); - - if (layer->is()) { - symbolLayouts.push_back(layer->as()->impl->createLayout(parameters)); - } else { - std::unique_ptr bucket = layer->baseImpl->createBucket(parameters); - if (bucket->hasData()) { - buckets.emplace(layer->baseImpl->bucketName(), std::move(bucket)); - } - } - } - - return parsePendingLayers(config, std::move(buckets)); -} - -TileParseResult TileWorker::parsePendingLayers(const PlacementConfig& config) { - return parsePendingLayers(config, std::unordered_map>()); -} - -TileParseResult TileWorker::parsePendingLayers(const PlacementConfig& config, - std::unordered_map> buckets) { - TileParseResultData result; - - result.complete = true; - result.buckets = std::move(buckets); - - // Prepare as many SymbolLayouts as possible. - for (auto& symbolLayout : symbolLayouts) { - if (symbolLayout->state == SymbolLayout::Pending) { - if (symbolLayout->canPrepare(glyphStore, spriteStore)) { - symbolLayout->state = SymbolLayout::Prepared; - symbolLayout->prepare(reinterpret_cast(this), - glyphAtlas, - glyphStore); - } else { - result.complete = false; - } - } - } - - // If all SymbolLayouts are prepared, then perform placement. Otherwise, parsePendingLayers - // will eventually be re-run. - if (result.complete) { - TilePlacementResult placementResult = redoPlacement(config); - - featureIndex->setCollisionTile(std::move(placementResult.collisionTile)); - - for (auto& bucket : placementResult.buckets) { - result.buckets.emplace(std::move(bucket)); - } - - result.featureIndex = std::move(featureIndex); - - if (tileData) { - result.tileData = tileData->clone(); - } - } - - return std::move(result); -} - -TilePlacementResult TileWorker::redoPlacement(const PlacementConfig& config) { - TilePlacementResult result; - - result.collisionTile = std::make_unique(config); - - for (auto& symbolLayout : symbolLayouts) { - if (symbolLayout->state == SymbolLayout::Pending) { - // Can't do placement until all layouts are prepared. - return result; - } - } - - for (auto& symbolLayout : symbolLayouts) { - symbolLayout->state = SymbolLayout::Placed; - if (symbolLayout->hasSymbolInstances()) { - result.buckets.emplace(symbolLayout->bucketName, - symbolLayout->place(*result.collisionTile)); - } - } - - return result; -} - -} // namespace mbgl diff --git a/src/mbgl/tile/tile_worker.hpp b/src/mbgl/tile/tile_worker.hpp deleted file mode 100644 index e64e7dee19..0000000000 --- a/src/mbgl/tile/tile_worker.hpp +++ /dev/null @@ -1,91 +0,0 @@ -#pragma once - -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include -#include - -namespace mbgl { - -class CollisionTile; -class GeometryTileData; -class SpriteStore; -class GlyphAtlas; -class GlyphStore; -class Bucket; -class SymbolLayout; -class PlacementConfig; - -namespace style { -class Layer; -} // namespace style - -// We're using this class to shuttle the resulting buckets from the worker thread to the MapContext -// thread. This class is movable-only because the vector contains movable-only value elements. -class TileParseResultData { -public: - bool complete = false; - std::unordered_map> buckets; - std::unique_ptr featureIndex; - std::unique_ptr tileData; -}; - -using TileParseResult = variant< - TileParseResultData, // success - std::exception_ptr>; // error - -class TilePlacementResult { -public: - std::unordered_map> buckets; - std::unique_ptr collisionTile; -}; - -class TileWorker : public util::noncopyable { -public: - TileWorker(OverscaledTileID, - SpriteStore&, - GlyphAtlas&, - GlyphStore&, - const std::atomic&, - const MapMode); - ~TileWorker(); - - TileParseResult parseAllLayers(std::vector>, - std::unique_ptr, - const PlacementConfig&); - - TileParseResult parsePendingLayers(const PlacementConfig&); - - TileParseResult redoLayout(std::vector>, - const PlacementConfig&); - - TilePlacementResult redoPlacement(const PlacementConfig&); - -private: - TileParseResult parsePendingLayers(const PlacementConfig&, - std::unordered_map>); - - const OverscaledTileID id; - - SpriteStore& spriteStore; - GlyphAtlas& glyphAtlas; - GlyphStore& glyphStore; - const std::atomic& obsolete; - const MapMode mode; - - std::vector> layers; - std::unique_ptr tileData; - - std::unique_ptr featureIndex; - std::vector> symbolLayouts; -}; - -} // namespace mbgl diff --git a/src/mbgl/tile/vector_tile.cpp b/src/mbgl/tile/vector_tile.cpp index bde0b4f63e..a195885415 100644 --- a/src/mbgl/tile/vector_tile.cpp +++ b/src/mbgl/tile/vector_tile.cpp @@ -1,7 +1,6 @@ #include #include #include -#include #include @@ -75,7 +74,7 @@ VectorTile::VectorTile(const OverscaledTileID& id_, std::string sourceID_, const style::UpdateParameters& parameters, const Tileset& tileset) - : GeometryTile(id_, sourceID_, parameters.style, parameters.mode), + : GeometryTile(id_, sourceID_, parameters), loader(*this, id_, parameters, tileset) { } diff --git a/src/mbgl/util/worker.cpp b/src/mbgl/util/worker.cpp deleted file mode 100644 index 8245628e84..0000000000 --- a/src/mbgl/util/worker.cpp +++ /dev/null @@ -1,129 +0,0 @@ -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -namespace mbgl { - -class Worker::Impl { -public: - Impl() = default; - - void parseRasterTile(std::unique_ptr bucket, - std::shared_ptr data, - std::function callback) { - try { - bucket->setImage(decodeImage(*data)); - // Destruct the shared pointer before calling the callback. - data.reset(); - callback(RasterTileParseResult(std::move(bucket))); - } catch (...) { - callback(std::current_exception()); - } - } - - void parseGeometryTile(TileWorker* worker, - std::vector> layers, - std::unique_ptr tileData, - PlacementConfig config, - std::function callback) { - try { - callback(worker->parseAllLayers(std::move(layers), std::move(tileData), config)); - } catch (...) { - callback(std::current_exception()); - } - } - - void parsePendingGeometryTileLayers(TileWorker* worker, - PlacementConfig config, - std::function callback) { - try { - callback(worker->parsePendingLayers(config)); - } catch (...) { - callback(std::current_exception()); - } - } - - void redoLayout(TileWorker* worker, - std::vector> layers, - PlacementConfig config, - std::function callback) { - try { - callback(worker->redoLayout(std::move(layers), config)); - } catch (...) { - callback(std::current_exception()); - } - } - - void redoPlacement(TileWorker* worker, - PlacementConfig config, - std::function callback) { - callback(worker->redoPlacement(config)); - } -}; - -Worker::Worker(std::size_t count) { - util::ThreadContext context = { "Worker", util::ThreadPriority::Low }; - for (std::size_t i = 0; i < count; i++) { - threads.emplace_back(std::make_unique>(context)); - } -} - -Worker::~Worker() = default; - -std::unique_ptr -Worker::parseRasterTile(std::unique_ptr bucket, - const std::shared_ptr data, - std::function callback) { - current = (current + 1) % threads.size(); - return threads[current]->invokeWithCallback(&Worker::Impl::parseRasterTile, std::move(bucket), - data, callback); -} - -std::unique_ptr -Worker::parseGeometryTile(TileWorker& worker, - std::vector> layers, - std::unique_ptr tileData, - PlacementConfig config, - std::function callback) { - current = (current + 1) % threads.size(); - return threads[current]->invokeWithCallback(&Worker::Impl::parseGeometryTile, &worker, - std::move(layers), std::move(tileData), config, callback); -} - -std::unique_ptr -Worker::parsePendingGeometryTileLayers(TileWorker& worker, - PlacementConfig config, - std::function callback) { - current = (current + 1) % threads.size(); - return threads[current]->invokeWithCallback(&Worker::Impl::parsePendingGeometryTileLayers, - &worker, config, callback); -} - -std::unique_ptr -Worker::redoLayout(TileWorker& worker, - std::vector> layers, - PlacementConfig config, - std::function callback) { - current = (current + 1) % threads.size(); - return threads[current]->invokeWithCallback(&Worker::Impl::redoLayout, &worker, - std::move(layers), config, callback); -} - -std::unique_ptr -Worker::redoPlacement(TileWorker& worker, - PlacementConfig config, - std::function callback) { - current = (current + 1) % threads.size(); - return threads[current]->invokeWithCallback(&Worker::Impl::redoPlacement, &worker, - config, callback); -} - -} // end namespace mbgl diff --git a/src/mbgl/util/worker.hpp b/src/mbgl/util/worker.hpp deleted file mode 100644 index 5b2ea06525..0000000000 --- a/src/mbgl/util/worker.hpp +++ /dev/null @@ -1,66 +0,0 @@ -#pragma once - -#include -#include -#include - -#include -#include - -namespace mbgl { - -class AsyncRequest; -class RasterBucket; -class GeometryTileLoader; -class CollisionTile; - -using RasterTileParseResult = variant< - std::unique_ptr, // success - std::exception_ptr>; // error - -class Worker : public mbgl::util::noncopyable { -public: - explicit Worker(std::size_t count); - ~Worker(); - - // Request work be done on a thread pool. Callbacks are executed on the invoking - // thread, which must have a run loop, after the work is complete. - // - // The return value represents the request to perform the work asynchronously. - // Its destructor guarantees that the work function has finished executing, and - // that the after function has either finished executing or will not execute. - // Together, this means that an object may make a work request with lambdas which - // bind references to itself, and if and when those lambdas execute, the references - // will still be valid. - - using Request = std::unique_ptr; - - Request parseRasterTile(std::unique_ptr bucket, - std::shared_ptr data, - std::function callback); - - Request parseGeometryTile(TileWorker&, - std::vector>, - std::unique_ptr, - PlacementConfig, - std::function callback); - - Request parsePendingGeometryTileLayers(TileWorker&, - PlacementConfig config, - std::function callback); - - Request redoLayout(TileWorker&, - std::vector>, - PlacementConfig, - std::function callback); - - Request redoPlacement(TileWorker&, - PlacementConfig config, - std::function callback); - -private: - class Impl; - std::vector>> threads; - std::size_t current = 0; -}; -} // namespace mbgl diff --git a/test/actor/actor.cpp b/test/actor/actor.cpp new file mode 100644 index 0000000000..7bb76784d6 --- /dev/null +++ b/test/actor/actor.cpp @@ -0,0 +1,139 @@ +#include +#include + +#include + +#include +#include +#include + +using namespace mbgl; +using namespace std::chrono_literals; + +TEST(Actor, Construction) { + // Construction is currently synchronous. It may become asynchronous in the future. + + struct Test { + Test(ActorRef, bool& constructed) { + constructed = true; + }; + }; + + ThreadPool pool { 1 }; + bool constructed = false; + Actor test(pool, std::ref(constructed)); + + EXPECT_TRUE(constructed); +} + +TEST(Actor, DestructionClosesMailbox) { + // Destruction blocks until the actor is not receiving. + + struct Test { + std::promise promise; + std::future future; + std::atomic waited; + + Test(ActorRef, std::promise promise_, std::future future_) + : promise(std::move(promise_)), + future(std::move(future_)), + waited(false) { + } + + ~Test() { + EXPECT_TRUE(waited.load()); + } + + void wait() { + promise.set_value(); + future.wait(); + std::this_thread::sleep_for(1ms); + waited = true; + } + }; + + ThreadPool pool { 1 }; + + std::promise enteredPromise; + std::future enteredFuture = enteredPromise.get_future(); + + std::promise exitingPromise; + std::future exitingFuture = exitingPromise.get_future(); + + Actor test(pool, std::move(enteredPromise), std::move(exitingFuture)); + + test.invoke(&Test::wait); + enteredFuture.wait(); + exitingPromise.set_value(); +} + +TEST(Actor, OrderedMailbox) { + // Messages are processed in order. + + struct Test { + int last = 0; + std::promise promise; + + Test(ActorRef, std::promise promise_) + : promise(std::move(promise_)) { + } + + void receive(int i) { + EXPECT_EQ(i, last + 1); + last = i; + } + + void end() { + promise.set_value(); + } + }; + + ThreadPool pool { 1 }; + + std::promise endedPromise; + std::future endedFuture = endedPromise.get_future(); + Actor test(pool, std::move(endedPromise)); + + for (auto i = 1; i <= 10; ++i) { + test.invoke(&Test::receive, i); + } + + test.invoke(&Test::end); + endedFuture.wait(); +} + +TEST(Actor, NonConcurrentMailbox) { + // An individual actor is never itself concurrent. + + struct Test { + int last = 0; + std::promise promise; + + Test(ActorRef, std::promise promise_) + : promise(std::move(promise_)) { + } + + void receive(int i) { + EXPECT_EQ(i, last + 1); + last = i; + std::this_thread::sleep_for(1ms); + } + + void end() { + promise.set_value(); + } + }; + + ThreadPool pool { 10 }; + + std::promise endedPromise; + std::future endedFuture = endedPromise.get_future(); + Actor test(pool, std::move(endedPromise)); + + for (auto i = 1; i <= 10; ++i) { + test.invoke(&Test::receive, i); + } + + test.invoke(&Test::end); + endedFuture.wait(); +} diff --git a/test/actor/actor_ref.cpp b/test/actor/actor_ref.cpp new file mode 100644 index 0000000000..655529035f --- /dev/null +++ b/test/actor/actor_ref.cpp @@ -0,0 +1,42 @@ +#include +#include + +#include + +#include +#include +#include + +using namespace mbgl; +using namespace std::chrono_literals; + +TEST(ActorRef, CanOutliveActor) { + // An ActorRef can outlive its actor. Doing does not extend the actor's lifetime. + // Sending a message to an ActorRef whose actor has died is a no-op. + + struct Test { + bool& died; + + Test(ActorRef, bool& died_) + : died(died_) { + } + + ~Test() { + died = true; + } + + void receive() { + FAIL(); + } + }; + + ThreadPool pool { 1 }; + bool died = false; + + ActorRef test = [&] () { + return Actor(pool, std::ref(died)).self(); + }(); + + EXPECT_TRUE(died); + test.invoke(&Test::receive); +} diff --git a/test/src/mbgl/test/stub_style_observer.hpp b/test/src/mbgl/test/stub_style_observer.hpp index 4de3430fcc..aa780121f5 100644 --- a/test/src/mbgl/test/stub_style_observer.hpp +++ b/test/src/mbgl/test/stub_style_observer.hpp @@ -34,19 +34,15 @@ public: if (sourceError) sourceError(source, error); } - void onTileLoaded(Source& source, const OverscaledTileID& tileID, TileLoadState loadState) override { - if (tileLoaded) tileLoaded(source, tileID, loadState); - } + void onTileChanged(Source& source, const OverscaledTileID& tileID) override { + if (tileChanged) tileChanged(source, tileID); + }; void onTileError(Source& source, const OverscaledTileID& tileID, std::exception_ptr error) override { if (tileError) tileError(source, tileID, error); } - void onTileUpdated(Source& source, const OverscaledTileID& tileID) override { - if (tileUpdated) tileUpdated(source, tileID); - }; - void onResourceError(std::exception_ptr error) override { if (resourceError) resourceError(error); }; @@ -57,8 +53,7 @@ public: std::function spriteError; std::function sourceLoaded; std::function sourceError; - std::function tileLoaded; + std::function tileChanged; std::function tileError; - std::function tileUpdated; std::function resourceError; }; diff --git a/test/style/source.cpp b/test/style/source.cpp index a33467dedb..519ca9288e 100644 --- a/test/style/source.cpp +++ b/test/style/source.cpp @@ -11,10 +11,10 @@ #include #include #include +#include #include #include -#include #include #include #include @@ -31,7 +31,7 @@ public: StubStyleObserver observer; Transform transform; TransformState transformState; - Worker worker { 1 }; + ThreadPool threadPool { 1 }; AnnotationManager annotationManager { 1.0 }; style::Style style { fileSource, 1.0 }; @@ -39,9 +39,8 @@ public: 1.0, MapDebugOptions(), transformState, - worker, + threadPool, fileSource, - true, MapMode::Continuous, annotationManager, style @@ -123,7 +122,7 @@ TEST(Source, RasterTileEmpty) { return response; }; - test.observer.tileLoaded = [&] (Source& source, const OverscaledTileID&, TileLoadState) { + test.observer.tileChanged = [&] (Source& source, const OverscaledTileID&) { EXPECT_EQ("source", source.getID()); test.end(); }; @@ -138,7 +137,7 @@ TEST(Source, RasterTileEmpty) { RasterSource source("source", tileset, 512); source.baseImpl->setObserver(&test.observer); source.baseImpl->loadDescription(test.fileSource); - source.baseImpl->loadTiles(test.updateParameters); + source.baseImpl->updateTiles(test.updateParameters); test.run(); } @@ -152,7 +151,7 @@ TEST(Source, VectorTileEmpty) { return response; }; - test.observer.tileLoaded = [&] (Source& source, const OverscaledTileID&, TileLoadState) { + test.observer.tileChanged = [&] (Source& source, const OverscaledTileID&) { EXPECT_EQ("source", source.getID()); test.end(); }; @@ -167,7 +166,7 @@ TEST(Source, VectorTileEmpty) { VectorSource source("source", tileset); source.baseImpl->setObserver(&test.observer); source.baseImpl->loadDescription(test.fileSource); - source.baseImpl->loadTiles(test.updateParameters); + source.baseImpl->updateTiles(test.updateParameters); test.run(); } @@ -196,7 +195,7 @@ TEST(Source, RasterTileFail) { RasterSource source("source", tileset, 512); source.baseImpl->setObserver(&test.observer); source.baseImpl->loadDescription(test.fileSource); - source.baseImpl->loadTiles(test.updateParameters); + source.baseImpl->updateTiles(test.updateParameters); test.run(); } @@ -225,7 +224,7 @@ TEST(Source, VectorTileFail) { VectorSource source("source", tileset); source.baseImpl->setObserver(&test.observer); source.baseImpl->loadDescription(test.fileSource); - source.baseImpl->loadTiles(test.updateParameters); + source.baseImpl->updateTiles(test.updateParameters); test.run(); } @@ -253,7 +252,7 @@ TEST(Source, RasterTileCorrupt) { RasterSource source("source", tileset, 512); source.baseImpl->setObserver(&test.observer); source.baseImpl->loadDescription(test.fileSource); - source.baseImpl->loadTiles(test.updateParameters); + source.baseImpl->updateTiles(test.updateParameters); test.run(); } @@ -285,7 +284,7 @@ TEST(Source, VectorTileCorrupt) { VectorSource source("source", tileset); source.baseImpl->setObserver(&test.observer); source.baseImpl->loadDescription(test.fileSource); - source.baseImpl->loadTiles(test.updateParameters); + source.baseImpl->updateTiles(test.updateParameters); test.run(); } @@ -298,7 +297,7 @@ TEST(Source, RasterTileCancel) { return optional(); }; - test.observer.tileLoaded = [&] (Source&, const OverscaledTileID&, TileLoadState) { + test.observer.tileChanged = [&] (Source&, const OverscaledTileID&) { FAIL() << "Should never be called"; }; @@ -312,7 +311,7 @@ TEST(Source, RasterTileCancel) { RasterSource source("source", tileset, 512); source.baseImpl->setObserver(&test.observer); source.baseImpl->loadDescription(test.fileSource); - source.baseImpl->loadTiles(test.updateParameters); + source.baseImpl->updateTiles(test.updateParameters); test.run(); } @@ -325,7 +324,7 @@ TEST(Source, VectorTileCancel) { return optional(); }; - test.observer.tileLoaded = [&] (Source&, const OverscaledTileID&, TileLoadState) { + test.observer.tileChanged = [&] (Source&, const OverscaledTileID&) { FAIL() << "Should never be called"; }; @@ -339,7 +338,7 @@ TEST(Source, VectorTileCancel) { VectorSource source("source", tileset); source.baseImpl->setObserver(&test.observer); source.baseImpl->loadDescription(test.fileSource); - source.baseImpl->loadTiles(test.updateParameters); + source.baseImpl->updateTiles(test.updateParameters); test.run(); } diff --git a/test/style/style.cpp b/test/style/style.cpp index 0d04fa5834..ff03e3c2cc 100644 --- a/test/style/style.cpp +++ b/test/style/style.cpp @@ -4,6 +4,7 @@ #include #include #include +#include using namespace mbgl; using namespace mbgl::style; -- cgit v1.2.1