From 60889db1a61ce492657cf5886b7b47200868cf67 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Tue, 19 Nov 2024 11:33:58 -0800 Subject: [PATCH 01/14] Set deferred deletions aside and schedule them once per frame (per source) --- CMakeLists.txt | 1 + bazel/core.bzl | 1 + include/mbgl/util/unique_function.hpp | 71 +++++++++++++++++++++++++++ src/mbgl/renderer/tile_pyramid.cpp | 3 ++ src/mbgl/tile/tile_cache.cpp | 70 +++++++++++++++----------- src/mbgl/tile/tile_cache.hpp | 6 ++- 6 files changed, 123 insertions(+), 29 deletions(-) create mode 100644 include/mbgl/util/unique_function.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 52267c770b0..3fc30bd8917 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -435,6 +435,7 @@ list(APPEND INCLUDE_FILES ${PROJECT_SOURCE_DIR}/include/mbgl/util/traits.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/type_list.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/unitbezier.hpp + ${PROJECT_SOURCE_DIR}/include/mbgl/util/unique_function.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/util.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/variant.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/vectors.hpp diff --git a/bazel/core.bzl b/bazel/core.bzl index 472fde18105..c6781ca3cfd 100644 --- a/bazel/core.bzl +++ b/bazel/core.bzl @@ -861,6 +861,7 @@ MLN_CORE_HEADERS = [ "include/mbgl/util/traits.hpp", "include/mbgl/util/type_list.hpp", "include/mbgl/util/unitbezier.hpp", + "include/mbgl/util/unique_function.hpp", "include/mbgl/util/util.hpp", "include/mbgl/util/variant.hpp", "include/mbgl/util/vectors.hpp", diff --git a/include/mbgl/util/unique_function.hpp b/include/mbgl/util/unique_function.hpp new file mode 100644 index 00000000000..c02efeb631a --- /dev/null +++ b/include/mbgl/util/unique_function.hpp @@ -0,0 +1,71 @@ +#pragma once + +#include + +namespace mbgl { +namespace util { + +// C++20 non-copying lambda capture, pending C++23 `move_only_function` +// Adapted from https://stackoverflow.com/a/52358928/135138 +template +class unique_function : public std::function { + using base = std::function; + +public: + unique_function() noexcept = default; + + unique_function(std::nullptr_t) noexcept + : base(nullptr) {} + + template + requires requires(Fn f) { + { f() } -> std::same_as>; + } + unique_function(Fn &&f) + : base(wrapper>{std::forward>(f)}) {} + + unique_function(unique_function &&) = default; + + unique_function &operator=(unique_function &&) = default; + + template + requires requires(Fn f) { + { f() } -> std::same_as>; + } + unique_function &operator=(Fn &&f) { + base::operator=(wrapper>{std::forward>(f)}); + return *this; + } + + using base::operator(); + +private: + template + struct wrapper; + + // specialization for MoveConstructible-only Fn + template + struct wrapper && std::is_move_constructible_v>> { + Fn fn; + + wrapper(Fn &&fn) + : fn(std::forward(fn)) {} + wrapper(wrapper &&) = default; + wrapper &operator=(wrapper &&) = default; + + // these two functions are instantiated by std::function and are never called + // We can't delete this or `fn` is uninitialized for non-DefaultContructible types + wrapper(const wrapper &rhs) + : fn(const_cast(rhs.fn)) { + throw std::runtime_error{{}}; + } + wrapper &operator=(const wrapper &) = delete; + + template + auto operator()(Args &&...args) { + return fn(std::forward(args)...); + } + }; +}; +} // namespace util +} // namespace mbgl diff --git a/src/mbgl/renderer/tile_pyramid.cpp b/src/mbgl/renderer/tile_pyramid.cpp index 4533e6f7549..3e79657b5bf 100644 --- a/src/mbgl/renderer/tile_pyramid.cpp +++ b/src/mbgl/renderer/tile_pyramid.cpp @@ -82,6 +82,7 @@ void TilePyramid::update(const std::vector>& l tiles.clear(); renderedTiles.clear(); + cache.deferPendingReleases(); return; } @@ -279,6 +280,8 @@ void TilePyramid::update(const std::vector>& l tile.usedByRenderedLayers |= tile.layerPropertiesUpdated(layerProperties); } } + + cache.deferPendingReleases(); } void TilePyramid::handleWrapJump(float lng) { diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp index 2bbde03a82b..512dbe9beaf 100644 --- a/src/mbgl/tile/tile_cache.cpp +++ b/src/mbgl/tile/tile_cache.cpp @@ -1,6 +1,9 @@ #include + #include #include +#include + #include namespace mbgl { @@ -26,16 +29,17 @@ void TileCache::setSize(size_t size_) { } namespace { - -/// This exists solely to prevent a problem where temporary lambda captures -/// are retained for the duration of the scope instead of being destroyed immediately. -template +// Capturing `std::vector>` directly produces an error related to +// copying, but this somehow avoids the same problem. It may be possible to eliminate it. struct CaptureWrapper { - CaptureWrapper(std::unique_ptr&& item_) - : item(std::move(item_)) {} - CaptureWrapper(const CaptureWrapper& other) - : item(other.item) {} - std::shared_ptr item; + std::vector> releases; + + CaptureWrapper(std::vector>&& x) + : releases(std::move(x)) {} + CaptureWrapper(CaptureWrapper&&) = default; + CaptureWrapper& operator=(CaptureWrapper&&) = default; + CaptureWrapper(CaptureWrapper const&) = delete; + CaptureWrapper& operator=(CaptureWrapper const&) = delete; }; } // namespace @@ -43,26 +47,36 @@ void TileCache::deferredRelease(std::unique_ptr&& tile) { MLN_TRACE_FUNC(); tile->cancel(); + pendingReleases.push_back(std::move(tile)); +} + +void TileCache::deferPendingReleases() { + MLN_TRACE_FUNC(); + + constexpr std::size_t scheduleThreshold = 1; + if (pendingReleases.size() < scheduleThreshold) { + return; + } + + // Block destruction until the cleanup task is complete + { + std::lock_guard counterLock{deferredSignalLock}; + deferredDeletionsPending++; + } + + threadPool.schedule( + util::unique_function{[this, wrap_{CaptureWrapper{std::move(pendingReleases)}}]() mutable { + MLN_TRACE_ZONE(deferPendingReleases lambda); + MLN_ZONE_VALUE(wrap_.releases.size()); + + // Run the deletions + wrap_.releases.clear(); - // The `std::function` must be created in a separate statement from the `schedule` call. - // Creating a `std::function` from a lambda involves a copy, which is why we must use - // `shared_ptr` rather than `unique_ptr` for the capture. As a result, a temporary holds - // a reference until the construction is complete and the lambda is destroyed. - // If this temporary outlives the `schedule` call, and the function is executed immediately - // by a waiting thread and is already complete, that temporary reference ends up being the - // last one and the destruction actually occurs here on this thread. - std::function func{[tile_{CaptureWrapper{std::move(tile)}}, this]() mutable { - tile_.item = {}; - - std::lock_guard counterLock(deferredSignalLock); - deferredDeletionsPending--; - deferredSignal.notify_all(); - }}; - - std::unique_lock counterLock(deferredSignalLock); - deferredDeletionsPending++; - - threadPool.schedule(std::move(func)); + // Wake up a waiting destructor + std::lock_guard counterLock{deferredSignalLock}; + deferredDeletionsPending--; + deferredSignal.notify_all(); + }}); } void TileCache::add(const OverscaledTileID& key, std::unique_ptr&& tile) { diff --git a/src/mbgl/tile/tile_cache.hpp b/src/mbgl/tile/tile_cache.hpp index db9f8658c46..72a4ccd15ec 100644 --- a/src/mbgl/tile/tile_cache.hpp +++ b/src/mbgl/tile/tile_cache.hpp @@ -43,13 +43,17 @@ class TileCache { bool has(const OverscaledTileID& key); void clear(); - /// Destroy a tile without blocking + /// Set aside a tile to be destroyed later, without blocking void deferredRelease(std::unique_ptr&&); + /// Schedule any accumulated deferred tiles to be destroyed + void deferPendingReleases(); + private: std::map> tiles; std::list orderedKeys; TaggedScheduler threadPool; + std::vector> pendingReleases; size_t deferredDeletionsPending{0}; std::mutex deferredSignalLock; std::condition_variable deferredSignal; From d31954523bf906e8d9f10bb7c73ff4454ae1ff93 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Tue, 19 Nov 2024 11:53:02 -0800 Subject: [PATCH 02/14] release pending items before waiting --- include/mbgl/util/unique_function.hpp | 4 ++-- src/mbgl/tile/tile_cache.cpp | 12 ++++++++++++ src/mbgl/tile/tile_cache.hpp | 10 +--------- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/include/mbgl/util/unique_function.hpp b/include/mbgl/util/unique_function.hpp index c02efeb631a..880748ad79e 100644 --- a/include/mbgl/util/unique_function.hpp +++ b/include/mbgl/util/unique_function.hpp @@ -48,8 +48,8 @@ class unique_function : public std::function { struct wrapper && std::is_move_constructible_v>> { Fn fn; - wrapper(Fn &&fn) - : fn(std::forward(fn)) {} + wrapper(Fn &&fn_) + : fn(std::forward(fn_)) {} wrapper(wrapper &&) = default; wrapper &operator=(wrapper &&) = default; diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp index 512dbe9beaf..619afd9a1ad 100644 --- a/src/mbgl/tile/tile_cache.cpp +++ b/src/mbgl/tile/tile_cache.cpp @@ -8,6 +8,18 @@ namespace mbgl { +TileCache::~TileCache() { + MLN_TRACE_FUNC(); + + clear(); + pendingReleases.clear(); + + std::unique_lock counterLock(deferredSignalLock); + while (deferredDeletionsPending != 0) { + deferredSignal.wait(counterLock); + } +} + void TileCache::setSize(size_t size_) { MLN_TRACE_FUNC(); diff --git a/src/mbgl/tile/tile_cache.hpp b/src/mbgl/tile/tile_cache.hpp index 72a4ccd15ec..fea185a32ba 100644 --- a/src/mbgl/tile/tile_cache.hpp +++ b/src/mbgl/tile/tile_cache.hpp @@ -18,15 +18,7 @@ class TileCache { TileCache(const TaggedScheduler& threadPool_, size_t size_ = 0) : threadPool(threadPool_), size(size_) {} - - ~TileCache() { - clear(); - - std::unique_lock counterLock(deferredSignalLock); - while (deferredDeletionsPending != 0) { - deferredSignal.wait(counterLock); - } - } + ~TileCache(); /// Change the maximum size of the cache. void setSize(size_t); From 0bf1090f0f47e71144c9e562027c5cca67d3c86c Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Tue, 19 Nov 2024 15:28:24 -0800 Subject: [PATCH 03/14] clear vector to resume a well-defined state --- src/mbgl/tile/tile_cache.cpp | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp index 619afd9a1ad..3bbaa2b083f 100644 --- a/src/mbgl/tile/tile_cache.cpp +++ b/src/mbgl/tile/tile_cache.cpp @@ -76,19 +76,21 @@ void TileCache::deferPendingReleases() { deferredDeletionsPending++; } - threadPool.schedule( - util::unique_function{[this, wrap_{CaptureWrapper{std::move(pendingReleases)}}]() mutable { - MLN_TRACE_ZONE(deferPendingReleases lambda); - MLN_ZONE_VALUE(wrap_.releases.size()); - - // Run the deletions - wrap_.releases.clear(); - - // Wake up a waiting destructor - std::lock_guard counterLock{deferredSignalLock}; - deferredDeletionsPending--; - deferredSignal.notify_all(); - }}); + CaptureWrapper wrap{std::move(pendingReleases)}; + pendingReleases.clear(); + + threadPool.schedule(util::unique_function{[this, wrap_{std::move(wrap)}]() mutable { + MLN_TRACE_ZONE(deferPendingReleases lambda); + MLN_ZONE_VALUE(wrap_.releases.size()); + + // Run the deletions + wrap_.releases.clear(); + + // Wake up a waiting destructor + std::lock_guard counterLock{deferredSignalLock}; + deferredDeletionsPending--; + deferredSignal.notify_all(); + }}); } void TileCache::add(const OverscaledTileID& key, std::unique_ptr&& tile) { From 2e43839a11625e63ae8debcc16fde16ff91bf9b5 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Wed, 20 Nov 2024 13:49:57 -0800 Subject: [PATCH 04/14] back out capture changes --- CMakeLists.txt | 1 - bazel/core.bzl | 1 - include/mbgl/util/unique_function.hpp | 71 --------------------------- src/mbgl/tile/tile_cache.cpp | 38 +++++++------- 4 files changed, 21 insertions(+), 90 deletions(-) delete mode 100644 include/mbgl/util/unique_function.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 3fc30bd8917..52267c770b0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -435,7 +435,6 @@ list(APPEND INCLUDE_FILES ${PROJECT_SOURCE_DIR}/include/mbgl/util/traits.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/type_list.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/unitbezier.hpp - ${PROJECT_SOURCE_DIR}/include/mbgl/util/unique_function.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/util.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/variant.hpp ${PROJECT_SOURCE_DIR}/include/mbgl/util/vectors.hpp diff --git a/bazel/core.bzl b/bazel/core.bzl index c6781ca3cfd..472fde18105 100644 --- a/bazel/core.bzl +++ b/bazel/core.bzl @@ -861,7 +861,6 @@ MLN_CORE_HEADERS = [ "include/mbgl/util/traits.hpp", "include/mbgl/util/type_list.hpp", "include/mbgl/util/unitbezier.hpp", - "include/mbgl/util/unique_function.hpp", "include/mbgl/util/util.hpp", "include/mbgl/util/variant.hpp", "include/mbgl/util/vectors.hpp", diff --git a/include/mbgl/util/unique_function.hpp b/include/mbgl/util/unique_function.hpp deleted file mode 100644 index 880748ad79e..00000000000 --- a/include/mbgl/util/unique_function.hpp +++ /dev/null @@ -1,71 +0,0 @@ -#pragma once - -#include - -namespace mbgl { -namespace util { - -// C++20 non-copying lambda capture, pending C++23 `move_only_function` -// Adapted from https://stackoverflow.com/a/52358928/135138 -template -class unique_function : public std::function { - using base = std::function; - -public: - unique_function() noexcept = default; - - unique_function(std::nullptr_t) noexcept - : base(nullptr) {} - - template - requires requires(Fn f) { - { f() } -> std::same_as>; - } - unique_function(Fn &&f) - : base(wrapper>{std::forward>(f)}) {} - - unique_function(unique_function &&) = default; - - unique_function &operator=(unique_function &&) = default; - - template - requires requires(Fn f) { - { f() } -> std::same_as>; - } - unique_function &operator=(Fn &&f) { - base::operator=(wrapper>{std::forward>(f)}); - return *this; - } - - using base::operator(); - -private: - template - struct wrapper; - - // specialization for MoveConstructible-only Fn - template - struct wrapper && std::is_move_constructible_v>> { - Fn fn; - - wrapper(Fn &&fn_) - : fn(std::forward(fn_)) {} - wrapper(wrapper &&) = default; - wrapper &operator=(wrapper &&) = default; - - // these two functions are instantiated by std::function and are never called - // We can't delete this or `fn` is uninitialized for non-DefaultContructible types - wrapper(const wrapper &rhs) - : fn(const_cast(rhs.fn)) { - throw std::runtime_error{{}}; - } - wrapper &operator=(const wrapper &) = delete; - - template - auto operator()(Args &&...args) { - return fn(std::forward(args)...); - } - }; -}; -} // namespace util -} // namespace mbgl diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp index 3bbaa2b083f..c0c534a2c75 100644 --- a/src/mbgl/tile/tile_cache.cpp +++ b/src/mbgl/tile/tile_cache.cpp @@ -2,7 +2,6 @@ #include #include -#include #include @@ -41,17 +40,17 @@ void TileCache::setSize(size_t size_) { } namespace { -// Capturing `std::vector>` directly produces an error related to -// copying, but this somehow avoids the same problem. It may be possible to eliminate it. +/// This exists solely to prevent a problem where temporary lambda captures +/// are retained for the duration of the scope instead of being destroyed immediately. struct CaptureWrapper { - std::vector> releases; - - CaptureWrapper(std::vector>&& x) - : releases(std::move(x)) {} + CaptureWrapper(std::vector>&& items_) + : items(items_.size()) { + std::ranges::move(items_, items.begin()); + } CaptureWrapper(CaptureWrapper&&) = default; - CaptureWrapper& operator=(CaptureWrapper&&) = default; - CaptureWrapper(CaptureWrapper const&) = delete; - CaptureWrapper& operator=(CaptureWrapper const&) = delete; + CaptureWrapper(const CaptureWrapper& other) + : items(other.items) {} + std::vector> items; }; } // namespace @@ -79,18 +78,23 @@ void TileCache::deferPendingReleases() { CaptureWrapper wrap{std::move(pendingReleases)}; pendingReleases.clear(); - threadPool.schedule(util::unique_function{[this, wrap_{std::move(wrap)}]() mutable { + // The `std::function` must be created in a separate statement from the `schedule` call. + // Creating a `std::function` from a lambda involves a copy, which is why we must use + // `shared_ptr` rather than `unique_ptr` for the capture. As a result, a temporary holds + // a reference until the construction is complete and the lambda is destroyed. + // If this temporary outlives the `schedule` call, and the function is executed immediately + // by a waiting thread and is already complete, that temporary reference ends up being the + // last one and the destruction actually occurs here on this thread. + std::function func{[tile_{CaptureWrapper{std::move(wrap)}}, this]() mutable { MLN_TRACE_ZONE(deferPendingReleases lambda); MLN_ZONE_VALUE(wrap_.releases.size()); + tile_.items.clear(); - // Run the deletions - wrap_.releases.clear(); - - // Wake up a waiting destructor - std::lock_guard counterLock{deferredSignalLock}; + std::lock_guard counterLock(deferredSignalLock); deferredDeletionsPending--; deferredSignal.notify_all(); - }}); + }}; + threadPool.schedule(std::move(func)); } void TileCache::add(const OverscaledTileID& key, std::unique_ptr&& tile) { From f10768bc77a797ca46338ca86a46a9b85fdf6549 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Wed, 20 Nov 2024 13:54:05 -0800 Subject: [PATCH 05/14] normalize syntax --- src/mbgl/tile/tile_cache.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp index c0c534a2c75..9a05cefaaf4 100644 --- a/src/mbgl/tile/tile_cache.cpp +++ b/src/mbgl/tile/tile_cache.cpp @@ -13,10 +13,8 @@ TileCache::~TileCache() { clear(); pendingReleases.clear(); - std::unique_lock counterLock(deferredSignalLock); - while (deferredDeletionsPending != 0) { - deferredSignal.wait(counterLock); - } + std::unique_lock counterLock{deferredSignalLock}; + deferredSignal.wait(counterLock, [&]() { return deferredDeletionsPending == 0; }); } void TileCache::setSize(size_t size_) { @@ -94,6 +92,7 @@ void TileCache::deferPendingReleases() { deferredDeletionsPending--; deferredSignal.notify_all(); }}; + threadPool.schedule(std::move(func)); } From 949ef8bb02af63688e672e0228bd89a50532a4e9 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Mon, 2 Dec 2024 10:14:40 -0800 Subject: [PATCH 06/14] improve coverage, rebase on main --- src/mbgl/tile/tile_cache.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp index 9a05cefaaf4..9985352e022 100644 --- a/src/mbgl/tile/tile_cache.cpp +++ b/src/mbgl/tile/tile_cache.cpp @@ -46,8 +46,13 @@ struct CaptureWrapper { std::ranges::move(items_, items.begin()); } CaptureWrapper(CaptureWrapper&&) = default; + + // coverage:ignore-start + /// This copy constructor is required to build, but doesn't seem to be called. CaptureWrapper(const CaptureWrapper& other) : items(other.items) {} + // coverage:ignore-end + std::vector> items; }; } // namespace From 86420029ab0ffc90e8f580c3f4b8192a51710ff9 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Wed, 20 Nov 2024 10:36:50 -0800 Subject: [PATCH 07/14] use `std23::move_only_function` --- .gitmodules | 4 + CMakeLists.txt | 9 +- include/mbgl/actor/scheduler.hpp | 22 +++-- include/mbgl/renderer/renderer_observer.hpp | 3 +- include/mbgl/storage/database_file_source.hpp | 34 ++++---- include/mbgl/storage/file_source.hpp | 13 ++- include/mbgl/storage/online_file_source.hpp | 2 +- include/mbgl/util/run_loop.hpp | 10 +-- include/mbgl/util/unique_function.hpp | 71 ++++++++++++++++ .../MapLibreAndroid/src/cpp/CMakeLists.txt | 1 + .../src/cpp/android_renderer_frontend.cpp | 9 +- .../src/cpp/asset_manager_file_source.cpp | 5 +- .../src/cpp/asset_manager_file_source.hpp | 2 +- .../MapLibreAndroid/src/cpp/file_source.cpp | 6 +- .../MapLibreAndroid/src/cpp/file_source.hpp | 4 +- .../src/cpp/http_file_source.cpp | 21 +++-- .../MapLibreAndroid/src/cpp/map_renderer.cpp | 2 +- .../MapLibreAndroid/src/cpp/map_renderer.hpp | 4 +- .../src/cpp/map_renderer_runnable.cpp | 2 +- .../src/cpp/map_renderer_runnable.hpp | 4 +- platform/android/android.cmake | 2 + platform/android/src/run_loop.cpp | 2 +- platform/android/src/run_loop_impl.hpp | 2 +- .../mbgl/storage/file_source_request.hpp | 6 +- .../default/src/mbgl/map/map_snapshotter.cpp | 8 +- .../src/mbgl/storage/asset_file_source.cpp | 3 +- .../src/mbgl/storage/database_file_source.cpp | 82 +++++++++---------- .../src/mbgl/storage/file_source_request.cpp | 4 +- .../src/mbgl/storage/local_file_source.cpp | 5 +- .../src/mbgl/storage/main_resource_loader.cpp | 7 +- .../src/mbgl/storage/mbtiles_file_source.cpp | 29 +++---- .../src/mbgl/storage/online_file_source.cpp | 7 +- render-test/file_source.hpp | 2 +- src/mbgl/actor/scheduler.cpp | 2 +- src/mbgl/map/map_impl.cpp | 12 ++- src/mbgl/map/map_impl.hpp | 4 +- src/mbgl/renderer/image_manager.cpp | 8 +- src/mbgl/renderer/image_manager_observer.hpp | 2 +- src/mbgl/renderer/render_orchestrator.cpp | 6 +- src/mbgl/renderer/render_orchestrator.hpp | 2 +- src/mbgl/storage/asset_file_source.hpp | 2 +- src/mbgl/storage/http_file_source.hpp | 2 +- src/mbgl/storage/local_file_source.hpp | 2 +- src/mbgl/storage/main_resource_loader.hpp | 2 +- src/mbgl/storage/mbtiles_file_source.hpp | 2 +- src/mbgl/tile/geometry_tile_worker.cpp | 13 +-- src/mbgl/tile/tile_cache.cpp | 44 +++------- src/mbgl/util/stopwatch.hpp | 6 +- src/mbgl/util/thread_pool.cpp | 6 +- src/mbgl/util/thread_pool.hpp | 16 ++-- test/src/mbgl/test/fake_file_source.hpp | 18 ++-- test/src/mbgl/test/stub_file_source.hpp | 4 +- test/storage/sync_file_source.test.cpp | 3 +- vendor/BUILD.bazel | 9 ++ vendor/nontype_functional | 1 + vendor/nontype_functional.cmake | 21 +++++ 56 files changed, 340 insertions(+), 234 deletions(-) create mode 100644 include/mbgl/util/unique_function.hpp create mode 160000 vendor/nontype_functional create mode 100644 vendor/nontype_functional.cmake diff --git a/.gitmodules b/.gitmodules index 9876bf6711b..d9c320d860a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -55,3 +55,7 @@ [submodule "vendor/glslang"] path = vendor/glslang url = https://github.com/KhronosGroup/glslang.git +[submodule "vendor/nontype_functional"] + path = vendor/nontype_functional + url = https://github.com/zhihaoy/nontype_functional.git + branch = compat diff --git a/CMakeLists.txt b/CMakeLists.txt index 52267c770b0..e736f861f0c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1449,6 +1449,8 @@ include(${PROJECT_SOURCE_DIR}/vendor/csscolorparser.cmake) include(${PROJECT_SOURCE_DIR}/vendor/earcut.hpp.cmake) include(${PROJECT_SOURCE_DIR}/vendor/eternal.cmake) include(${PROJECT_SOURCE_DIR}/vendor/mapbox-base.cmake) +include(${PROJECT_SOURCE_DIR}/vendor/metal-cpp.cmake) +include(${PROJECT_SOURCE_DIR}/vendor/nontype_functional.cmake) include(${PROJECT_SOURCE_DIR}/vendor/parsedate.cmake) include(${PROJECT_SOURCE_DIR}/vendor/polylabel.cmake) include(${PROJECT_SOURCE_DIR}/vendor/protozero.cmake) @@ -1457,7 +1459,6 @@ include(${PROJECT_SOURCE_DIR}/vendor/unique_resource.cmake) include(${PROJECT_SOURCE_DIR}/vendor/unordered_dense.cmake) include(${PROJECT_SOURCE_DIR}/vendor/vector-tile.cmake) include(${PROJECT_SOURCE_DIR}/vendor/wagyu.cmake) -include(${PROJECT_SOURCE_DIR}/vendor/metal-cpp.cmake) target_link_libraries( mbgl-core @@ -1472,13 +1473,14 @@ target_link_libraries( mbgl-vendor-csscolorparser mbgl-vendor-earcut.hpp mbgl-vendor-eternal + $<$:mbgl-vendor-metal-cpp> + mbgl-vendor-nontype_functional mbgl-vendor-parsedate mbgl-vendor-polylabel mbgl-vendor-protozero mbgl-vendor-unique_resource mbgl-vendor-vector-tile mbgl-vendor-wagyu - $<$:mbgl-vendor-metal-cpp> PUBLIC Mapbox::Base Mapbox::Base::Extras::expected-lite @@ -1511,13 +1513,14 @@ export(TARGETS mbgl-vendor-csscolorparser mbgl-vendor-earcut.hpp mbgl-vendor-eternal + mbgl-vendor-metal-cpp + mbgl-vendor-nontype_functional mbgl-vendor-parsedate mbgl-vendor-polylabel mbgl-vendor-protozero mbgl-vendor-unique_resource mbgl-vendor-vector-tile mbgl-vendor-wagyu - mbgl-vendor-metal-cpp unordered_dense FILE MapboxCoreTargets.cmake diff --git a/include/mbgl/actor/scheduler.hpp b/include/mbgl/actor/scheduler.hpp index 8b2f81f22b8..ee87f872ccd 100644 --- a/include/mbgl/actor/scheduler.hpp +++ b/include/mbgl/actor/scheduler.hpp @@ -4,6 +4,8 @@ #include +#include + #include #include #include @@ -36,16 +38,18 @@ class Mailbox; */ class Scheduler { public: + using Task = std23::move_only_function; + virtual ~Scheduler() = default; /// Enqueues a function for execution. - virtual void schedule(std::function&&) = 0; - virtual void schedule(const util::SimpleIdentity, std::function&&) = 0; + virtual void schedule(Task&&) = 0; + virtual void schedule(const util::SimpleIdentity, Task&&) = 0; /// Makes a weak pointer to this Scheduler. virtual mapbox::base::WeakPtr makeWeakPtr() = 0; /// Enqueues a function for execution on the render thread owned by the given tag. - virtual void runOnRenderThread(const util::SimpleIdentity, std::function&&) {} + virtual void runOnRenderThread(const util::SimpleIdentity, Task&&) {} /// Run render thread jobs for the given tag /// @param tag Tag of owner /// @param closeQueue Runs all render jobs and then removes the internal queue. @@ -59,7 +63,7 @@ class Scheduler { /// /// If this scheduler is already deleted by the time the returnded closure /// is first invoked, the call is ignored. - std::function bindOnce(std::function); + Scheduler::Task bindOnce(Task); /// Enqueues the given |task| for execution into this scheduler's task queue /// and then enqueues the |reply| with the captured task result to the @@ -103,10 +107,12 @@ class Scheduler { [[nodiscard]] static std::shared_ptr GetSequenced(); /// Set a function to be called when an exception occurs on a thread controlled by the scheduler - void setExceptionHandler(std::function handler_) { handler = std::move(handler_); } + void setExceptionHandler(std23::move_only_function handler_) { + handler = std::move(handler_); + } protected: - std::function handler; + std23::move_only_function handler; private: template @@ -136,8 +142,8 @@ class TaggedScheduler { /// @brief Get the wrapped scheduler const std::shared_ptr& get() const noexcept { return scheduler; } - void schedule(std::function&& fn) { scheduler->schedule(tag, std::move(fn)); } - void runOnRenderThread(std::function&& fn) { scheduler->runOnRenderThread(tag, std::move(fn)); } + void schedule(Scheduler::Task&& fn) { scheduler->schedule(tag, std::move(fn)); } + void runOnRenderThread(Scheduler::Task&& fn) { scheduler->runOnRenderThread(tag, std::move(fn)); } void runRenderJobs(bool closeQueue = false) { scheduler->runRenderJobs(tag, closeQueue); } void waitForEmpty() const noexcept { scheduler->waitForEmpty(tag); } diff --git a/include/mbgl/renderer/renderer_observer.hpp b/include/mbgl/renderer/renderer_observer.hpp index 5df817afa43..d1d1594c356 100644 --- a/include/mbgl/renderer/renderer_observer.hpp +++ b/include/mbgl/renderer/renderer_observer.hpp @@ -55,8 +55,7 @@ class RendererObserver { virtual void onDidFinishRenderingMap() {} /// Style is missing an image - using StyleImageMissingCallback = std::function; - virtual void onStyleImageMissing(const std::string&, const StyleImageMissingCallback& done) { done(); } + virtual void onStyleImageMissing(const std::string&, Scheduler::Task&& done) { done(); } virtual void onRemoveUnusedStyleImages(const std::vector&) {} // Entry point for custom shader registration diff --git a/include/mbgl/storage/database_file_source.hpp b/include/mbgl/storage/database_file_source.hpp index 3c3bcbeea3e..1b781776ae8 100644 --- a/include/mbgl/storage/database_file_source.hpp +++ b/include/mbgl/storage/database_file_source.hpp @@ -16,8 +16,8 @@ class DatabaseFileSource : public FileSource { ~DatabaseFileSource() override; /// FileSource overrides - std::unique_ptr request(const Resource&, Callback) override; - void forward(const Resource&, const Response&, std::function callback) override; + std::unique_ptr request(const Resource&, CopyableCallback&&) override; + void forward(const Resource&, const Response&, Scheduler::Task&& callback) override; bool canRequest(const Resource&) const override; void setProperty(const std::string&, const mapbox::base::Value&) override; void pause() override; @@ -29,7 +29,7 @@ class DatabaseFileSource : public FileSource { * Sets path of a database to be used by DatabaseFileSource and invokes * provided callback when a database path is set. */ - virtual void setDatabasePath(const std::string&, std::function callback); + virtual void setDatabasePath(const std::string&, Callback<>&& callback); /** * Delete existing database and re-initialize. @@ -38,7 +38,7 @@ class DatabaseFileSource : public FileSource { * will be executed on the database thread; it is the responsibility of the * SDK bindings to re-execute a user-provided callback on the main thread. */ - virtual void resetDatabase(std::function); + virtual void resetDatabase(ExceptionCallback&&); /** * Packs the existing database file into a minimal amount of disk space. @@ -50,7 +50,7 @@ class DatabaseFileSource : public FileSource { * will be executed on the database thread; it is the responsibility of the * SDK bindings to re-execute a user-provided callback on the main thread. */ - virtual void packDatabase(std::function callback); + virtual void packDatabase(ExceptionCallback&& callback); /** * Sets whether packing the database file occurs automatically after an @@ -87,7 +87,7 @@ class DatabaseFileSource : public FileSource { * Resources overlapping with offline regions will not be affected * by this call. */ - virtual void invalidateAmbientCache(std::function); + virtual void invalidateAmbientCache(ExceptionCallback&&); /** * Erase resources from the ambient cache, freeing storage space. @@ -100,7 +100,7 @@ class DatabaseFileSource : public FileSource { * Resources overlapping with offline regions will not be affected * by this call. */ - virtual void clearAmbientCache(std::function); + virtual void clearAmbientCache(ExceptionCallback&&); /** * Sets the maximum size in bytes for the ambient cache. @@ -122,7 +122,7 @@ class DatabaseFileSource : public FileSource { * This method should always be called before using the database, * otherwise the default maximum size will be used. */ - virtual void setMaximumAmbientCacheSize(uint64_t size, std::function callback); + virtual void setMaximumAmbientCacheSize(uint64_t size, ExceptionCallback&& callback); // Offline @@ -134,7 +134,7 @@ class DatabaseFileSource : public FileSource { * responsibility of the SDK bindings to re-execute a user-provided callback * on the main thread. */ - virtual void listOfflineRegions(std::function)>); + virtual void listOfflineRegions(Callback)>&&); /** * Retrieve given region in the offline database. @@ -145,7 +145,7 @@ class DatabaseFileSource : public FileSource { * on the main thread. */ virtual void getOfflineRegion(int64_t regionID, - std::function, std::exception_ptr>)>); + Callback, std::exception_ptr>)>&&); /** * Create an offline region in the database. @@ -161,18 +161,18 @@ class DatabaseFileSource : public FileSource { */ virtual void createOfflineRegion(const OfflineRegionDefinition& definition, const OfflineRegionMetadata& metadata, - std::function)>); + Callback)>&&); /** * Update an offline region metadata in the database. */ virtual void updateOfflineMetadata(int64_t regionID, const OfflineRegionMetadata& metadata, - std::function)>); + Callback)>&&); /** * Register an observer to be notified when the state of the region changes. */ - virtual void setOfflineRegionObserver(const OfflineRegion&, std::unique_ptr); + virtual void setOfflineRegionObserver(const OfflineRegion&, std::unique_ptr&&); /** * Pause or resume downloading of regional resources. @@ -186,7 +186,7 @@ class DatabaseFileSource : public FileSource { * bindings to re-execute a user-provided callback on the main thread. */ virtual void getOfflineRegionStatus(const OfflineRegion&, - std::function)>) const; + Callback)>&&) const; /** * Merge offline regions from a secondary database into the main offline database. @@ -209,7 +209,7 @@ class DatabaseFileSource : public FileSource { * does not contain all the tiles or resources required by the region definition. */ virtual void mergeOfflineRegions(const std::string& sideDatabasePath, - std::function)>); + Callback)>&&); /** * Remove an offline region from the database and perform any resources @@ -231,7 +231,7 @@ class DatabaseFileSource : public FileSource { * will be executed on the database thread; it is the responsibility of the * SDK bindings to re-execute a user-provided callback on the main thread. */ - virtual void deleteOfflineRegion(const OfflineRegion&, std::function); + virtual void deleteOfflineRegion(const OfflineRegion&, ExceptionCallback&&); /** * Invalidate all the tiles from an offline region forcing Mapbox GL to @@ -239,7 +239,7 @@ class DatabaseFileSource : public FileSource { * than deleting the offline region and downloading it again because if the * data on the cache matches the server, no new data gets transmitted. */ - virtual void invalidateOfflineRegion(const OfflineRegion&, std::function); + virtual void invalidateOfflineRegion(const OfflineRegion&, ExceptionCallback&&); /** * Changing or bypassing this limit without permission from Mapbox is diff --git a/include/mbgl/storage/file_source.hpp b/include/mbgl/storage/file_source.hpp index 4c896c4d6fc..1fcbb9cb142 100644 --- a/include/mbgl/storage/file_source.hpp +++ b/include/mbgl/storage/file_source.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include #include @@ -34,24 +35,28 @@ enum FileSourceType : uint8_t { // GeoJSONSource, RasterSource, VectorSource, CustomGeometrySource and other *Sources. class FileSource { public: + template + using Callback = std23::move_only_function; + template + using CopyableCallback = std::function; + using ExceptionCallback = Callback; + FileSource& operator=(const FileSource&) = delete; virtual ~FileSource() = default; - using Callback = std::function; - /// Request a resource. The callback will be called asynchronously, in the /// same thread as the request was made. This thread must have an active /// RunLoop. The request may be cancelled before completion by releasing the /// returned AsyncRequest. If the request is cancelled before the callback /// is executed, the callback will not be executed. - virtual std::unique_ptr request(const Resource&, Callback) = 0; + virtual std::unique_ptr request(const Resource&, CopyableCallback&&) = 0; /// Allows to forward response from one source to another. /// Optionally, callback can be provided to receive notification for forward /// operation. // // NOLINTNEXTLINE(performance-unnecessary-value-param) - virtual void forward(const Resource&, const Response&, std::function) {} + virtual void forward(const Resource&, const Response&, Scheduler::Task&&) {} /// When a file source supports consulting a local cache only, it must /// return true. Cache-only requests are requests that aren't as urgent, but diff --git a/include/mbgl/storage/online_file_source.hpp b/include/mbgl/storage/online_file_source.hpp index 797e4070047..9c9bf4abd90 100644 --- a/include/mbgl/storage/online_file_source.hpp +++ b/include/mbgl/storage/online_file_source.hpp @@ -19,7 +19,7 @@ class OnlineFileSource : public FileSource { private: // FileSource overrides - std::unique_ptr request(const Resource&, Callback) override; + std::unique_ptr request(const Resource&, CopyableCallback&&) override; bool canRequest(const Resource&) const override; void pause() override; void resume() override; diff --git a/include/mbgl/util/run_loop.hpp b/include/mbgl/util/run_loop.hpp index 755f4aeb7fd..1a91d5eba1c 100644 --- a/include/mbgl/util/run_loop.hpp +++ b/include/mbgl/util/run_loop.hpp @@ -54,10 +54,10 @@ class RunLoop : public Scheduler, private util::noncopyable { /// loop. It will be called from any thread and is up to the platform /// to, after receiving the callback, call RunLoop::runOnce() from the /// same thread as the Map object lives. - void setPlatformCallback(std::function callback) { platformCallback = std::move(callback); } + void setPlatformCallback(Scheduler::Task&& callback) { platformCallback = std::move(callback); } // So far only needed by the libcurl backend. - void addWatch(int fd, Event, std::function&& callback); + void addWatch(int fd, Event, std23::move_only_function&& callback); void removeWatch(int fd); // Invoke fn(args...) on this RunLoop. @@ -80,8 +80,8 @@ class RunLoop : public Scheduler, private util::noncopyable { return std::make_unique(task); } - void schedule(std::function&& fn) override { invoke(std::move(fn)); } - void schedule(const util::SimpleIdentity, std::function&& fn) override { schedule(std::move(fn)); } + void schedule(Scheduler::Task&& fn) override { invoke(std::move(fn)); } + void schedule(const util::SimpleIdentity, Scheduler::Task&& fn) override { schedule(std::move(fn)); } ::mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } void waitForEmpty(const util::SimpleIdentity = util::SimpleIdentity::Empty) override; @@ -131,7 +131,7 @@ class RunLoop : public Scheduler, private util::noncopyable { } } - std::function platformCallback; + Scheduler::Task platformCallback; Queue defaultQueue; Queue highPriorityQueue; diff --git a/include/mbgl/util/unique_function.hpp b/include/mbgl/util/unique_function.hpp new file mode 100644 index 00000000000..880748ad79e --- /dev/null +++ b/include/mbgl/util/unique_function.hpp @@ -0,0 +1,71 @@ +#pragma once + +#include + +namespace mbgl { +namespace util { + +// C++20 non-copying lambda capture, pending C++23 `move_only_function` +// Adapted from https://stackoverflow.com/a/52358928/135138 +template +class unique_function : public std::function { + using base = std::function; + +public: + unique_function() noexcept = default; + + unique_function(std::nullptr_t) noexcept + : base(nullptr) {} + + template + requires requires(Fn f) { + { f() } -> std::same_as>; + } + unique_function(Fn &&f) + : base(wrapper>{std::forward>(f)}) {} + + unique_function(unique_function &&) = default; + + unique_function &operator=(unique_function &&) = default; + + template + requires requires(Fn f) { + { f() } -> std::same_as>; + } + unique_function &operator=(Fn &&f) { + base::operator=(wrapper>{std::forward>(f)}); + return *this; + } + + using base::operator(); + +private: + template + struct wrapper; + + // specialization for MoveConstructible-only Fn + template + struct wrapper && std::is_move_constructible_v>> { + Fn fn; + + wrapper(Fn &&fn_) + : fn(std::forward(fn_)) {} + wrapper(wrapper &&) = default; + wrapper &operator=(wrapper &&) = default; + + // these two functions are instantiated by std::function and are never called + // We can't delete this or `fn` is uninitialized for non-DefaultContructible types + wrapper(const wrapper &rhs) + : fn(const_cast(rhs.fn)) { + throw std::runtime_error{{}}; + } + wrapper &operator=(const wrapper &) = delete; + + template + auto operator()(Args &&...args) { + return fn(std::forward(args)...); + } + }; +}; +} // namespace util +} // namespace mbgl diff --git a/platform/android/MapLibreAndroid/src/cpp/CMakeLists.txt b/platform/android/MapLibreAndroid/src/cpp/CMakeLists.txt index 05902547e94..80a4beb2060 100644 --- a/platform/android/MapLibreAndroid/src/cpp/CMakeLists.txt +++ b/platform/android/MapLibreAndroid/src/cpp/CMakeLists.txt @@ -241,6 +241,7 @@ target_link_libraries(maplibre $<$:-fuse-ld=gold> Mapbox::Base::jni.hpp mbgl-core + mbgl-vendor-nontype_functional mbgl-vendor-unique_resource) install(TARGETS maplibre LIBRARY DESTINATION lib) diff --git a/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.cpp b/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.cpp index 0f5ddb66235..0dba1aacb4f 100644 --- a/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/android_renderer_frontend.cpp @@ -1,17 +1,16 @@ #include "android_renderer_frontend.hpp" +#include "android_renderer_backend.hpp" -#include #include #include #include +#include #include #include #include #include #include -#include "android_renderer_backend.hpp" - namespace mbgl { namespace android { @@ -45,8 +44,8 @@ class ForwardingRendererObserver : public RendererObserver { void onDidFinishRenderingMap() override { delegate.invoke(&RendererObserver::onDidFinishRenderingMap); } - void onStyleImageMissing(const std::string& id, const StyleImageMissingCallback& done) override { - delegate.invoke(&RendererObserver::onStyleImageMissing, id, done); + void onStyleImageMissing(const std::string& id, Scheduler::Task&& done) override { + delegate.invoke(&RendererObserver::onStyleImageMissing, id, std::move(done)); } void onRemoveUnusedStyleImages(const std::vector& ids) override { diff --git a/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.cpp b/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.cpp index d7b5b6e9647..2a7a0933d70 100644 --- a/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.cpp @@ -70,7 +70,8 @@ AssetManagerFileSource::AssetManagerFileSource(jni::JNIEnv& env, AssetManagerFileSource::~AssetManagerFileSource() = default; -std::unique_ptr AssetManagerFileSource::request(const Resource& resource, Callback callback) { +std::unique_ptr AssetManagerFileSource::request(const Resource& resource, + CopyableCallback&& callback) { auto req = std::make_unique(std::move(callback)); impl->actor().invoke(&Impl::request, resource.url, req->actor()); @@ -79,7 +80,7 @@ std::unique_ptr AssetManagerFileSource::request(const Resource& re } bool AssetManagerFileSource::canRequest(const Resource& resource) const { - return 0 == resource.url.rfind(mbgl::util::ASSET_PROTOCOL, 0); + return resource.url.starts_with(mbgl::util::ASSET_PROTOCOL); } void AssetManagerFileSource::setResourceOptions(ResourceOptions options) { diff --git a/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.hpp b/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.hpp index 166fc03980f..0d5805e64b1 100644 --- a/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.hpp @@ -23,7 +23,7 @@ class AssetManagerFileSource : public FileSource { const ClientOptions); ~AssetManagerFileSource() override; - std::unique_ptr request(const Resource&, Callback) override; + std::unique_ptr request(const Resource&, CopyableCallback&&) override; bool canRequest(const Resource&) const override; void setResourceOptions(ResourceOptions options) override; diff --git a/platform/android/MapLibreAndroid/src/cpp/file_source.cpp b/platform/android/MapLibreAndroid/src/cpp/file_source.cpp index 6ad4ee95d6d..4ccdaf83940 100644 --- a/platform/android/MapLibreAndroid/src/cpp/file_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/file_source.cpp @@ -30,13 +30,13 @@ FileSource::FileSource(jni::JNIEnv& _env, mbgl::FileSourceManager::get()->registerFileSourceFactory( mbgl::FileSourceType::Asset, - [](const mbgl::ResourceOptions& resourceOptions, const mbgl::ClientOptions& clientOptions) { + [](const mbgl::ResourceOptions& resourceOptions_, const mbgl::ClientOptions& clientOptions_) { auto env{android::AttachEnv()}; std::unique_ptr assetFileSource; if (android::MapLibre::hasInstance(*env)) { auto assetManager = android::MapLibre::getAssetManager(*env); assetFileSource = std::make_unique( - *env, assetManager, resourceOptions.clone(), clientOptions.clone()); + *env, assetManager, resourceOptions_.clone(), clientOptions_.clone()); } return assetFileSource; }); @@ -161,7 +161,7 @@ void FileSource::setResourceCachePath(jni::JNIEnv& env, pathChangeCallback = {}; }); - databaseSource->setDatabasePath(newPath + DATABASE_FILE, pathChangeCallback); + databaseSource->setDatabasePath(newPath + DATABASE_FILE, std::move(pathChangeCallback)); } void FileSource::resume(jni::JNIEnv&) { diff --git a/platform/android/MapLibreAndroid/src/cpp/file_source.hpp b/platform/android/MapLibreAndroid/src/cpp/file_source.hpp index 421ef2e4602..136f9f69573 100644 --- a/platform/android/MapLibreAndroid/src/cpp/file_source.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/file_source.hpp @@ -24,6 +24,8 @@ namespace android { */ class FileSource { public: + using Callback = std23::move_only_function; + static constexpr auto Name() { return "org/maplibre/android/storage/FileSource"; }; struct ResourceTransformCallback { @@ -93,7 +95,7 @@ class FileSource { mbgl::ResourceOptions resourceOptions; mbgl::ClientOptions clientOptions; std::unique_ptr> resourceTransform; - std::function pathChangeCallback; + Callback pathChangeCallback; std::shared_ptr databaseSource; std::shared_ptr onlineSource; std::shared_ptr resourceLoader; diff --git a/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp b/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp index 92420ca7d8e..9aaa21a6a19 100644 --- a/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp @@ -39,8 +39,8 @@ class HTTPRequest : public AsyncRequest { public: static constexpr auto Name() { return "org/maplibre/android/http/NativeHttpRequest"; }; - HTTPRequest(jni::JNIEnv&, const Resource&, FileSource::Callback); - ~HTTPRequest(); + HTTPRequest(jni::JNIEnv&, const Resource&, FileSource::CopyableCallback&&); + ~HTTPRequest() override; void onFailure(jni::JNIEnv&, int type, const jni::String& message); void onResponse(jni::JNIEnv&, @@ -57,7 +57,7 @@ class HTTPRequest : public AsyncRequest { private: Resource resource; - FileSource::Callback callback; + FileSource::CopyableCallback callback; Response response; util::AsyncTask async{[this] { @@ -88,9 +88,11 @@ void RegisterNativeHTTPRequest(jni::JNIEnv& env) { } // namespace android -HTTPRequest::HTTPRequest(jni::JNIEnv& env, const Resource& resource_, FileSource::Callback callback_) +HTTPRequest::HTTPRequest(jni::JNIEnv& env, + const Resource& resource_, + FileSource::CopyableCallback&& callback_) : resource(resource_), - callback(callback_) { + callback(std::move(callback_)) { std::string etagStr; std::string modifiedStr; @@ -145,7 +147,7 @@ void HTTPRequest::onResponse(jni::JNIEnv& env, } if (cacheControl) { - const auto cc = http::CacheControl::parse(jni::Make(env, cacheControl).c_str()); + const auto cc = http::CacheControl::parse(jni::Make(env, cacheControl)); response.expires = cc.toTimePoint(); response.mustRevalidate = cc.mustRevalidate; } @@ -157,7 +159,7 @@ void HTTPRequest::onResponse(jni::JNIEnv& env, if (code == 200) { if (body) { auto data = std::make_shared(body.Length(env), char()); - jni::GetArrayRegion(env, *body, 0, data->size(), reinterpret_cast(&(*data)[0])); + jni::GetArrayRegion(env, *body, 0, data->size(), reinterpret_cast(data->data())); response.data = data; } else { response.data = std::make_shared(); @@ -214,8 +216,9 @@ HTTPFileSource::HTTPFileSource(const ResourceOptions& resourceOptions, const Cli HTTPFileSource::~HTTPFileSource() = default; -std::unique_ptr HTTPFileSource::request(const Resource& resource, Callback callback) { - return std::make_unique(*impl->env, resource, callback); +std::unique_ptr HTTPFileSource::request(const Resource& resource, + CopyableCallback&& callback) { + return std::make_unique(*impl->env, resource, std::move(callback)); } void HTTPFileSource::setResourceOptions(ResourceOptions options) { diff --git a/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp b/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp index fc235a67f4c..763a40c608a 100644 --- a/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/map_renderer.cpp @@ -63,7 +63,7 @@ ActorRef MapRenderer::actor() const { return *rendererRef; } -void MapRenderer::schedule(std::function&& scheduled) { +void MapRenderer::schedule(Task&& scheduled) { MLN_TRACE_FUNC(); try { // Create a runnable diff --git a/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp b/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp index 9fc962c0d35..1e6a1ec740b 100644 --- a/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/map_renderer.hpp @@ -72,8 +72,8 @@ class MapRenderer : public Scheduler { // From Scheduler. Schedules by using callbacks to the // JVM to process the mailbox on the right thread. - void schedule(std::function&& scheduled) override; - void schedule(const util::SimpleIdentity, std::function&& fn) override { schedule(std::move(fn)); }; + void schedule(Task&& scheduled) override; + void schedule(const util::SimpleIdentity, Task&& fn) override { schedule(std::move(fn)); }; mapbox::base::WeakPtr makeWeakPtr() override { return weakFactory.makeWeakPtr(); } diff --git a/platform/android/MapLibreAndroid/src/cpp/map_renderer_runnable.cpp b/platform/android/MapLibreAndroid/src/cpp/map_renderer_runnable.cpp index 805bcac8f9f..f7a2d24d92e 100644 --- a/platform/android/MapLibreAndroid/src/cpp/map_renderer_runnable.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/map_renderer_runnable.cpp @@ -5,7 +5,7 @@ namespace mbgl { namespace android { -MapRendererRunnable::MapRendererRunnable(jni::JNIEnv& env, std::function function_) +MapRendererRunnable::MapRendererRunnable(jni::JNIEnv& env, Scheduler::Task&& function_) : function(std::move(function_)) { // Create the Java peer and hold on to a global reference // Not using a weak reference here as this might oerflow diff --git a/platform/android/MapLibreAndroid/src/cpp/map_renderer_runnable.hpp b/platform/android/MapLibreAndroid/src/cpp/map_renderer_runnable.hpp index 392e8a002d0..d96c3c7f053 100644 --- a/platform/android/MapLibreAndroid/src/cpp/map_renderer_runnable.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/map_renderer_runnable.hpp @@ -23,7 +23,7 @@ class MapRendererRunnable { static void registerNative(jni::JNIEnv&); - MapRendererRunnable(jni::JNIEnv&, std::function); + MapRendererRunnable(jni::JNIEnv&, Scheduler::Task&&); // Only for jni registration, unused MapRendererRunnable(jni::JNIEnv&) { assert(false); } @@ -37,7 +37,7 @@ class MapRendererRunnable { private: jni::Global> javaPeer; - std::function function; + Scheduler::Task function; }; } // namespace android diff --git a/platform/android/android.cmake b/platform/android/android.cmake index 0f14d074f46..221c891e308 100644 --- a/platform/android/android.cmake +++ b/platform/android/android.cmake @@ -8,6 +8,7 @@ target_compile_definitions( ) include(${PROJECT_SOURCE_DIR}/vendor/icu.cmake) +include(${PROJECT_SOURCE_DIR}/vendor/nontype_functional.cmake) include(${PROJECT_SOURCE_DIR}/vendor/sqlite.cmake) # cmake-format: off @@ -106,6 +107,7 @@ target_link_libraries( jnigraphics log mbgl-vendor-icu + mbgl-vendor-nontype_functional mbgl-vendor-sqlite z ) diff --git a/platform/android/src/run_loop.cpp b/platform/android/src/run_loop.cpp index 71d5b759ee2..f3b80a4ee19 100644 --- a/platform/android/src/run_loop.cpp +++ b/platform/android/src/run_loop.cpp @@ -290,7 +290,7 @@ void RunLoop::stop() { }); } -void RunLoop::addWatch(int fd, Event event, std::function&& cb) { +void RunLoop::addWatch(int fd, Event event, std23::move_only_function&& cb) { MBGL_VERIFY_THREAD(tid); if (event == Event::Read) { diff --git a/platform/android/src/run_loop_impl.hpp b/platform/android/src/run_loop_impl.hpp index 4f0eff4103a..b9b381c0a6b 100644 --- a/platform/android/src/run_loop_impl.hpp +++ b/platform/android/src/run_loop_impl.hpp @@ -16,7 +16,7 @@ struct ALooper; namespace mbgl { namespace util { -using WatchCallback = std::function; +using WatchCallback = std23::move_only_function; template class Thread; diff --git a/platform/default/include/mbgl/storage/file_source_request.hpp b/platform/default/include/mbgl/storage/file_source_request.hpp index 79ad53ff289..6e508f6af61 100644 --- a/platform/default/include/mbgl/storage/file_source_request.hpp +++ b/platform/default/include/mbgl/storage/file_source_request.hpp @@ -13,7 +13,7 @@ class Mailbox; class FileSourceRequest final : public AsyncRequest { public: - FileSourceRequest(FileSource::Callback&& callback); + FileSourceRequest(FileSource::CopyableCallback&& callback); ~FileSourceRequest() final; void onCancel(std::function&& callback); @@ -22,8 +22,8 @@ class FileSourceRequest final : public AsyncRequest { ActorRef actor(); private: - FileSource::Callback responseCallback = nullptr; - std::function cancelCallback = nullptr; + FileSource::CopyableCallback responseCallback; + FileSource::Callback<> cancelCallback; std::shared_ptr mailbox; }; diff --git a/platform/default/src/mbgl/map/map_snapshotter.cpp b/platform/default/src/mbgl/map/map_snapshotter.cpp index e42fe0719c4..f1c3145efa2 100644 --- a/platform/default/src/mbgl/map/map_snapshotter.cpp +++ b/platform/default/src/mbgl/map/map_snapshotter.cpp @@ -46,8 +46,8 @@ class ForwardingRendererObserver final : public RendererObserver { delegate.invoke(f, mode, repaintNeeded, placementChanged, frameEncodingTime, frameRenderingTime); } - void onStyleImageMissing(const std::string& image, const StyleImageMissingCallback& cb) override { - delegate.invoke(&RendererObserver::onStyleImageMissing, image, cb); + void onStyleImageMissing(const std::string& image, Scheduler::Task&& cb) override { + delegate.invoke(&RendererObserver::onStyleImageMissing, image, std::move(cb)); } private: @@ -95,8 +95,8 @@ class SnapshotterRenderer final : public RendererObserver { mode, repaintNeeded, placementChanged, frameEncodingTime, frameRenderingTime); } - void onStyleImageMissing(const std::string& id, const StyleImageMissingCallback& done) override { - rendererObserver->onStyleImageMissing(id, done); + void onStyleImageMissing(const std::string& id, Scheduler::Task&& done) override { + rendererObserver->onStyleImageMissing(id, std::move(done)); } void setObserver(std::shared_ptr observer) { diff --git a/platform/default/src/mbgl/storage/asset_file_source.cpp b/platform/default/src/mbgl/storage/asset_file_source.cpp index a43c0be9679..c2d5a7649c8 100644 --- a/platform/default/src/mbgl/storage/asset_file_source.cpp +++ b/platform/default/src/mbgl/storage/asset_file_source.cpp @@ -77,7 +77,8 @@ AssetFileSource::AssetFileSource(const ResourceOptions& resourceOptions, const C AssetFileSource::~AssetFileSource() = default; -std::unique_ptr AssetFileSource::request(const Resource& resource, Callback callback) { +std::unique_ptr AssetFileSource::request(const Resource& resource, + CopyableCallback&& callback) { auto req = std::make_unique(std::move(callback)); impl->actor().invoke(&Impl::request, resource.url, req->actor()); diff --git a/platform/default/src/mbgl/storage/database_file_source.cpp b/platform/default/src/mbgl/storage/database_file_source.cpp index fa674c55512..11901b2ca8f 100644 --- a/platform/default/src/mbgl/storage/database_file_source.cpp +++ b/platform/default/src/mbgl/storage/database_file_source.cpp @@ -38,68 +38,64 @@ class DatabaseFileSourceThread { req.invoke(&FileSourceRequest::setResponse, *offlineResponse); } - void setDatabasePath(const std::string& path, const std::function& callback) { + void setDatabasePath(const std::string& path, FileSource::Callback<>&& callback) { db->changePath(path); if (callback) { callback(); } } - void forward(const Resource& resource, const Response& response, const std::function& callback) { + void forward(const Resource& resource, const Response& response, Scheduler::Task&& callback) { db->put(resource, response); if (callback) { callback(); } } - void resetDatabase(const std::function& callback) { callback(db->resetDatabase()); } + void resetDatabase(FileSource::ExceptionCallback&& callback) { callback(db->resetDatabase()); } - void packDatabase(const std::function& callback) { callback(db->pack()); } + void packDatabase(FileSource::ExceptionCallback&& callback) { callback(db->pack()); } void runPackDatabaseAutomatically(bool autopack) { db->runPackDatabaseAutomatically(autopack); } void put(const Resource& resource, const Response& response) { db->put(resource, response); } - void invalidateAmbientCache(const std::function& callback) { - callback(db->invalidateAmbientCache()); - } + void invalidateAmbientCache(FileSource::ExceptionCallback&& callback) { callback(db->invalidateAmbientCache()); } - void clearAmbientCache(const std::function& callback) { - callback(db->clearAmbientCache()); - } + void clearAmbientCache(FileSource::ExceptionCallback&& callback) { callback(db->clearAmbientCache()); } - void setMaximumAmbientCacheSize(uint64_t size, const std::function& callback) { + void setMaximumAmbientCacheSize(uint64_t size, FileSource::ExceptionCallback&& callback) { callback(db->setMaximumAmbientCacheSize(size)); } - void listRegions(const std::function)>& callback) { + void listRegions(FileSource::Callback)>&& callback) { callback(db->listRegions()); } void getRegion(const int64_t regionID, - const std::function, std::exception_ptr>)>& callback) { + FileSource::Callback, std::exception_ptr>)>&& callback) { callback(db->getRegion(regionID)); } void createRegion(const OfflineRegionDefinition& definition, const OfflineRegionMetadata& metadata, - const std::function)>& callback) { + FileSource::Callback)>&& callback) { callback(db->createRegion(definition, metadata)); } void mergeOfflineRegions(const std::string& sideDatabasePath, - const std::function)>& callback) { + FileSource::Callback)>&& callback) { callback(db->mergeDatabase(sideDatabasePath)); } void updateMetadata(const int64_t regionID, const OfflineRegionMetadata& metadata, - const std::function)>& callback) { + FileSource::Callback)>&& callback) { callback(db->updateMetadata(regionID, metadata)); } void getRegionStatus(int64_t regionID, - const std::function)>& callback) { + FileSource::Callback)>&& callback) { if (auto download = getDownload(regionID)) { callback(download.value()->getStatus()); } else { @@ -107,12 +103,12 @@ class DatabaseFileSourceThread { } } - void deleteRegion(OfflineRegion region, const std::function& callback) { + void deleteRegion(OfflineRegion region, FileSource::ExceptionCallback&& callback) { downloads.erase(region.getID()); callback(db->deleteRegion(std::move(region))); } - void invalidateRegion(int64_t regionID, const std::function& callback) { + void invalidateRegion(int64_t regionID, FileSource::ExceptionCallback&& callback) { callback(db->invalidateRegion(regionID)); } @@ -211,16 +207,17 @@ DatabaseFileSource::DatabaseFileSource(const ResourceOptions& resourceOptions, c DatabaseFileSource::~DatabaseFileSource() = default; -std::unique_ptr DatabaseFileSource::request(const Resource& resource, Callback callback) { +std::unique_ptr DatabaseFileSource::request(const Resource& resource, + CopyableCallback&& callback) { auto req = std::make_unique(std::move(callback)); impl->actor().invoke(&DatabaseFileSourceThread::request, resource, req->actor()); return req; } -void DatabaseFileSource::forward(const Resource& res, const Response& response, std::function callback) { +void DatabaseFileSource::forward(const Resource& res, const Response& response, Scheduler::Task&& callback) { if (res.storagePolicy == Resource::StoragePolicy::Volatile) return; - std::function wrapper; + Scheduler::Task wrapper; if (callback) { wrapper = Scheduler::GetCurrent()->bindOnce(std::move(callback)); } @@ -233,15 +230,15 @@ bool DatabaseFileSource::canRequest(const Resource& resource) const { resource.url.rfind(mbgl::util::FILE_PROTOCOL, 0) == std::string::npos; } -void DatabaseFileSource::setDatabasePath(const std::string& path, std::function callback) { +void DatabaseFileSource::setDatabasePath(const std::string& path, Callback<>&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::setDatabasePath, path, std::move(callback)); } -void DatabaseFileSource::resetDatabase(std::function callback) { +void DatabaseFileSource::resetDatabase(ExceptionCallback&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::resetDatabase, std::move(callback)); } -void DatabaseFileSource::packDatabase(std::function callback) { +void DatabaseFileSource::packDatabase(ExceptionCallback&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::packDatabase, std::move(callback)); } @@ -253,59 +250,55 @@ void DatabaseFileSource::put(const Resource& resource, const Response& response) impl->actor().invoke(&DatabaseFileSourceThread::put, resource, response); } -void DatabaseFileSource::invalidateAmbientCache(std::function callback) { +void DatabaseFileSource::invalidateAmbientCache(ExceptionCallback&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::invalidateAmbientCache, std::move(callback)); } -void DatabaseFileSource::clearAmbientCache(std::function callback) { +void DatabaseFileSource::clearAmbientCache(ExceptionCallback&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::clearAmbientCache, std::move(callback)); } -void DatabaseFileSource::setMaximumAmbientCacheSize(uint64_t size, std::function callback) { +void DatabaseFileSource::setMaximumAmbientCacheSize(uint64_t size, ExceptionCallback&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::setMaximumAmbientCacheSize, size, std::move(callback)); } -void DatabaseFileSource::listOfflineRegions( - std::function)> callback) { +void DatabaseFileSource::listOfflineRegions(Callback)>&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::listRegions, std::move(callback)); } void DatabaseFileSource::getOfflineRegion( - const int64_t regionID, std::function, std::exception_ptr>)> callback) { + const int64_t regionID, Callback, std::exception_ptr>)>&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::getRegion, regionID, std::move(callback)); } -void DatabaseFileSource::createOfflineRegion( - const OfflineRegionDefinition& definition, - const OfflineRegionMetadata& metadata, - std::function)> callback) { +void DatabaseFileSource::createOfflineRegion(const OfflineRegionDefinition& definition, + const OfflineRegionMetadata& metadata, + Callback)>&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::createRegion, definition, metadata, std::move(callback)); } -void DatabaseFileSource::mergeOfflineRegions( - const std::string& sideDatabasePath, std::function)> callback) { +void DatabaseFileSource::mergeOfflineRegions(const std::string& sideDatabasePath, + Callback)>&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::mergeOfflineRegions, sideDatabasePath, std::move(callback)); } void DatabaseFileSource::updateOfflineMetadata( const int64_t regionID, const OfflineRegionMetadata& metadata, - std::function)> callback) { + Callback)>&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::updateMetadata, regionID, metadata, std::move(callback)); } -void DatabaseFileSource::deleteOfflineRegion(const OfflineRegion& region, - std::function callback) { +void DatabaseFileSource::deleteOfflineRegion(const OfflineRegion& region, ExceptionCallback&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::deleteRegion, region, std::move(callback)); } -void DatabaseFileSource::invalidateOfflineRegion(const OfflineRegion& region, - std::function callback) { +void DatabaseFileSource::invalidateOfflineRegion(const OfflineRegion& region, ExceptionCallback&& callback) { impl->actor().invoke(&DatabaseFileSourceThread::invalidateRegion, region.getID(), std::move(callback)); } void DatabaseFileSource::setOfflineRegionObserver(const OfflineRegion& region, - std::unique_ptr observer) { + std::unique_ptr&& observer) { impl->actor().invoke(&DatabaseFileSourceThread::setRegionObserver, region.getID(), std::move(observer)); } @@ -314,8 +307,7 @@ void DatabaseFileSource::setOfflineRegionDownloadState(const OfflineRegion& regi } void DatabaseFileSource::getOfflineRegionStatus( - const OfflineRegion& region, - std::function)> callback) const { + const OfflineRegion& region, Callback)>&& callback) const { impl->actor().invoke(&DatabaseFileSourceThread::getRegionStatus, region.getID(), std::move(callback)); } diff --git a/platform/default/src/mbgl/storage/file_source_request.cpp b/platform/default/src/mbgl/storage/file_source_request.cpp index c8d5ae46a1f..279d9fb5448 100644 --- a/platform/default/src/mbgl/storage/file_source_request.cpp +++ b/platform/default/src/mbgl/storage/file_source_request.cpp @@ -5,8 +5,8 @@ namespace mbgl { -FileSourceRequest::FileSourceRequest(FileSource::Callback&& callback) - : responseCallback(callback), +FileSourceRequest::FileSourceRequest(FileSource::CopyableCallback&& callback) + : responseCallback(std::move(callback)), mailbox(std::make_shared(*Scheduler::GetCurrent())) {} FileSourceRequest::~FileSourceRequest() { diff --git a/platform/default/src/mbgl/storage/local_file_source.cpp b/platform/default/src/mbgl/storage/local_file_source.cpp index c42ad520c1a..61dd2b34558 100644 --- a/platform/default/src/mbgl/storage/local_file_source.cpp +++ b/platform/default/src/mbgl/storage/local_file_source.cpp @@ -13,7 +13,7 @@ namespace { bool acceptsURL(const std::string& url) { - return 0 == url.rfind(mbgl::util::FILE_PROTOCOL, 0); + return url.starts_with(mbgl::util::FILE_PROTOCOL); } } // namespace @@ -74,7 +74,8 @@ LocalFileSource::LocalFileSource(const ResourceOptions& resourceOptions, const C LocalFileSource::~LocalFileSource() = default; -std::unique_ptr LocalFileSource::request(const Resource& resource, Callback callback) { +std::unique_ptr LocalFileSource::request(const Resource& resource, + CopyableCallback&& callback) { auto req = std::make_unique(std::move(callback)); impl->actor().invoke(&Impl::request, resource.url, req->actor()); diff --git a/platform/default/src/mbgl/storage/main_resource_loader.cpp b/platform/default/src/mbgl/storage/main_resource_loader.cpp index 66699128fe9..5543744a060 100644 --- a/platform/default/src/mbgl/storage/main_resource_loader.cpp +++ b/platform/default/src/mbgl/storage/main_resource_loader.cpp @@ -53,7 +53,7 @@ class MainResourceLoaderThread { " Action: " << "Requesting," << " URL: " << res.url.c_str() << " Size: " << (response.data != nullptr ? response.data->size() : 0) << "B," - << " Time") + << " Time"); } callback(response); }); @@ -160,7 +160,7 @@ class MainResourceLoader::Impl { resourceOptions(resourceOptions_.clone()), clientOptions(clientOptions_.clone()) {} - std::unique_ptr request(const Resource& resource, Callback callback) { + std::unique_ptr request(const Resource& resource, CopyableCallback&& callback) { auto req = std::make_unique(std::move(callback)); req->onCancel([actorRef = thread->actor(), req = req.get()]() { @@ -244,7 +244,8 @@ bool MainResourceLoader::supportsCacheOnlyRequests() const { return impl->supportsCacheOnlyRequests(); } -std::unique_ptr MainResourceLoader::request(const Resource& resource, Callback callback) { +std::unique_ptr MainResourceLoader::request(const Resource& resource, + CopyableCallback&& callback) { return impl->request(resource, std::move(callback)); } diff --git a/platform/default/src/mbgl/storage/mbtiles_file_source.cpp b/platform/default/src/mbgl/storage/mbtiles_file_source.cpp index 29b9be2de1d..7e69d460d92 100644 --- a/platform/default/src/mbgl/storage/mbtiles_file_source.cpp +++ b/platform/default/src/mbgl/storage/mbtiles_file_source.cpp @@ -29,7 +29,7 @@ namespace { bool acceptsURL(const std::string &url) { - return 0 == url.rfind(mbgl::util::MBTILES_PROTOCOL, 0); + return url.starts_with(mbgl::util::MBTILES_PROTOCOL); } std::string url_to_path(const std::string &url) { @@ -107,7 +107,7 @@ class MBTilesFileSource::Impl { auto format_ptr = values.find("format"); std::string format = format_ptr == values.end() ? "png" : format_ptr->second; - if (format != "pbf" && values.count("scale") == 0) { + if (format != "pbf" && !values.contains("scale")) { values["scale"] = "1"; } @@ -184,19 +184,19 @@ class MBTilesFileSource::Impl { // Load data for specific tile void request_tile(const Resource &resource, ActorRef req) { - std::string base_path = url_to_path(resource.url); - std::string path = db_path(base_path); + const std::string base_path = url_to_path(resource.url); + const std::string path = db_path(base_path); auto &db = get_db(path); - int iy = resource.tileData->y; - int iz = resource.tileData->z; + const int iy = resource.tileData->y; + const int iz = static_cast(resource.tileData->z); - auto x = std::to_string(resource.tileData->x); - auto y = std::to_string((int)(pow(2, iz) - 1) - iy); - auto z = std::to_string(iz); + const auto x = std::to_string(resource.tileData->x); + const auto y = std::to_string((int)(pow(2, iz) - 1) - iy); + const auto z = std::to_string(iz); - std::string sql = "SELECT tile_data FROM tiles where zoom_level = " + z + " AND tile_column = " + x + - " AND tile_row = " + y; + const std::string sql = "SELECT tile_data FROM tiles where zoom_level = " + z + " AND tile_column = " + x + + " AND tile_row = " + y; mapbox::sqlite::Statement stmt(db, sql.c_str()); Response response; @@ -255,7 +255,7 @@ class MBTilesFileSource::Impl { auto ptr = db_cache.find(path); if (ptr != db_cache.end()) { return ptr->second; - }; + } auto ptr2 = db_cache.insert(std::pair( path, mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly))); @@ -275,7 +275,8 @@ MBTilesFileSource::MBTilesFileSource(const ResourceOptions &resourceOptions, con resourceOptions.clone(), clientOptions.clone())) {} -std::unique_ptr MBTilesFileSource::request(const Resource &resource, FileSource::Callback callback) { +std::unique_ptr MBTilesFileSource::request(const Resource &resource, + FileSource::CopyableCallback &&callback) { auto req = std::make_unique(std::move(callback)); // assume if there is a tile request, that the mbtiles file has been validated @@ -296,7 +297,7 @@ std::unique_ptr MBTilesFileSource::request(const Resource &resourc // file must exist auto path = url_to_path(resource.url); - struct stat buffer; + struct stat buffer{}; int result = stat(path.c_str(), &buffer); if (result == -1 && errno == ENOENT) { Response response; diff --git a/platform/default/src/mbgl/storage/online_file_source.cpp b/platform/default/src/mbgl/storage/online_file_source.cpp index 077fc915fc7..5b0604b43f7 100644 --- a/platform/default/src/mbgl/storage/online_file_source.cpp +++ b/platform/default/src/mbgl/storage/online_file_source.cpp @@ -303,7 +303,7 @@ class OnlineFileSourceThread { std::set activeRequests; bool online = true; - uint32_t maximumConcurrentRequests; + uint32_t maximumConcurrentRequests = util::DEFAULT_MAXIMUM_CONCURRENT_REQUESTS; HTTPFileSource httpFileSource; util::AsyncTask reachability{std::bind(&OnlineFileSourceThread::networkIsReachableAgain, this)}; std::map> tasks; @@ -320,7 +320,7 @@ class OnlineFileSource::Impl { resourceOptions.clone(), clientOptions.clone())) {} - std::unique_ptr request(Callback callback, Resource res) { + std::unique_ptr request(CopyableCallback&& callback, Resource res) { auto req = std::make_unique(std::move(callback)); req->onCancel( [actorRef = thread->actor(), req = req.get()]() { actorRef.invoke(&OnlineFileSourceThread::cancel, req); }); @@ -616,7 +616,8 @@ OnlineFileSource::OnlineFileSource(const ResourceOptions& resourceOptions, const OnlineFileSource::~OnlineFileSource() = default; -std::unique_ptr OnlineFileSource::request(const Resource& resource, Callback callback) { +std::unique_ptr OnlineFileSource::request(const Resource& resource, + CopyableCallback&& callback) { Resource res = resource; const TileServerOptions options = impl->getResourceOptions().tileServerOptions(); diff --git a/render-test/file_source.hpp b/render-test/file_source.hpp index c06d223079d..703711f2305 100644 --- a/render-test/file_source.hpp +++ b/render-test/file_source.hpp @@ -12,7 +12,7 @@ class ProxyFileSource : public FileSource { ProxyFileSource(std::shared_ptr, const ResourceOptions&, const ClientOptions&); ~ProxyFileSource(); - std::unique_ptr request(const Resource&, Callback) override; + std::unique_ptr request(const Resource&, CopyableCallback&&) override; bool canRequest(const Resource&) const override { return true; } /** diff --git a/src/mbgl/actor/scheduler.cpp b/src/mbgl/actor/scheduler.cpp index 707de734ddc..4e15a2b38ff 100644 --- a/src/mbgl/actor/scheduler.cpp +++ b/src/mbgl/actor/scheduler.cpp @@ -5,7 +5,7 @@ namespace mbgl { -std::function Scheduler::bindOnce(std::function fn) { +Scheduler::Task Scheduler::bindOnce(Scheduler::Task fn) { assert(fn); return [scheduler = makeWeakPtr(), scheduled = std::move(fn)]() mutable { if (!scheduled) return; // Repeated call. diff --git a/src/mbgl/map/map_impl.cpp b/src/mbgl/map/map_impl.cpp index 6b5a6454f18..3bf43a3ed2c 100644 --- a/src/mbgl/map/map_impl.cpp +++ b/src/mbgl/map/map_impl.cpp @@ -64,7 +64,7 @@ Map::Impl::~Impl() { // Explicitly reset the RendererFrontend first to ensure it releases // All shared resources (AnnotationManager) rendererFrontend.reset(); -}; +} // MARK: - Map::Impl StyleObserver @@ -212,12 +212,10 @@ void Map::Impl::onWillStartRenderingMap() { void Map::Impl::onDidFinishRenderingMap() { if (mode == MapMode::Continuous && loading) { observer.onDidFinishRenderingMap(MapObserver::RenderMode::Full); - if (loading) { - loading = false; - observer.onDidFinishLoadingMap(); - } + loading = false; + observer.onDidFinishLoadingMap(); } -}; +} void Map::Impl::jumpTo(const CameraOptions& camera) { cameraMutated = true; @@ -225,7 +223,7 @@ void Map::Impl::jumpTo(const CameraOptions& camera) { onUpdate(); } -void Map::Impl::onStyleImageMissing(const std::string& id, const std::function& done) { +void Map::Impl::onStyleImageMissing(const std::string& id, Scheduler::Task&& done) { if (!style->getImage(id)) observer.onStyleImageMissing(id); done(); diff --git a/src/mbgl/map/map_impl.hpp b/src/mbgl/map/map_impl.hpp index d093b30f227..c1cee52636f 100644 --- a/src/mbgl/map/map_impl.hpp +++ b/src/mbgl/map/map_impl.hpp @@ -52,7 +52,7 @@ class Map::Impl final : public style::Observer, public RendererObserver { void onDidFinishRenderingFrame(RenderMode, bool, bool, double, double) final; void onWillStartRenderingMap() final; void onDidFinishRenderingMap() final; - void onStyleImageMissing(const std::string&, const std::function&) final; + void onStyleImageMissing(const std::string&, Scheduler::Task&&) final; void onRemoveUnusedStyleImages(const std::vector&) final; void onRegisterShaders(gfx::ShaderRegistry&) final; @@ -88,7 +88,7 @@ class Map::Impl final : public style::Observer, public RendererObserver { uint8_t prefetchZoomDelta = util::DEFAULT_PREFETCH_ZOOM_DELTA; bool loading = false; - bool rendererFullyLoaded; + bool rendererFullyLoaded{false}; std::unique_ptr stillImageRequest; }; diff --git a/src/mbgl/renderer/image_manager.cpp b/src/mbgl/renderer/image_manager.cpp index c2b7bca9358..2ed998d8c27 100644 --- a/src/mbgl/renderer/image_manager.cpp +++ b/src/mbgl/renderer/image_manager.cpp @@ -231,7 +231,7 @@ void ImageManager::checkMissingAndNotify(ImageRequestor& requestor, const ImageR if (!missingDependencies.empty()) { ImageRequestor* requestorPtr = &requestor; - assert(!missingImageRequestors.count(requestorPtr)); + assert(!missingImageRequestors.contains(requestorPtr)); missingImageRequestors.emplace(requestorPtr, pair); for (const auto& dependency : missingDependencies) { @@ -260,7 +260,7 @@ void ImageManager::checkMissingAndNotify(ImageRequestor& requestor, const ImageR requestor.addPendingRequest(missingImage); } - auto removePendingRequests = [this, missingImage] { + Scheduler::Task removePendingRequests = [this, missingImage] { std::lock_guard readWriteLock(rwLock); auto existingRequest = requestedImages.find(missingImage); if (existingRequest == requestedImages.end()) { @@ -271,8 +271,8 @@ void ImageManager::checkMissingAndNotify(ImageRequestor& requestor, const ImageR req->removePendingRequest(missingImage); } }; - observer->onStyleImageMissing(missingImage, - Scheduler::GetCurrent()->bindOnce(std::move(removePendingRequests))); + Scheduler::Task bindRemove = Scheduler::GetCurrent()->bindOnce(std::move(removePendingRequests)); + observer->onStyleImageMissing(missingImage, std::move(bindRemove)); } } else { // Associate requestor with an image that was provided by the client. diff --git a/src/mbgl/renderer/image_manager_observer.hpp b/src/mbgl/renderer/image_manager_observer.hpp index a057c99f847..f5f7ba624e1 100644 --- a/src/mbgl/renderer/image_manager_observer.hpp +++ b/src/mbgl/renderer/image_manager_observer.hpp @@ -11,7 +11,7 @@ class ImageManagerObserver { public: virtual ~ImageManagerObserver() = default; - virtual void onStyleImageMissing(const std::string&, const std::function& done) { done(); } + virtual void onStyleImageMissing(const std::string&, Scheduler::Task&& done) { done(); } virtual void onRemoveUnusedStyleImages(const std::vector&) {} }; diff --git a/src/mbgl/renderer/render_orchestrator.cpp b/src/mbgl/renderer/render_orchestrator.cpp index c9762831a1b..8cd82d1b834 100644 --- a/src/mbgl/renderer/render_orchestrator.cpp +++ b/src/mbgl/renderer/render_orchestrator.cpp @@ -54,7 +54,7 @@ const std::string& LayerRenderItem::getName() const { } #if MLN_DRAWABLE_RENDERER -void LayerRenderItem::updateDebugDrawables(DebugLayerGroupMap&, PaintParameters&) const {}; +void LayerRenderItem::updateDebugDrawables(DebugLayerGroupMap&, PaintParameters&) const {} #endif namespace { @@ -1065,10 +1065,10 @@ void RenderOrchestrator::onTileAction(RenderSource&, observer->onTileAction(op, id, sourceID); } -void RenderOrchestrator::onStyleImageMissing(const std::string& id, const std::function& done) { +void RenderOrchestrator::onStyleImageMissing(const std::string& id, Scheduler::Task&& done) { MLN_TRACE_FUNC(); - observer->onStyleImageMissing(id, done); + observer->onStyleImageMissing(id, std::move(done)); } void RenderOrchestrator::onRemoveUnusedStyleImages(const std::vector& unusedImageIDs) { diff --git a/src/mbgl/renderer/render_orchestrator.hpp b/src/mbgl/renderer/render_orchestrator.hpp index 5065b189ae6..95d4179e0eb 100644 --- a/src/mbgl/renderer/render_orchestrator.hpp +++ b/src/mbgl/renderer/render_orchestrator.hpp @@ -186,7 +186,7 @@ class RenderOrchestrator final : public GlyphManagerObserver, public ImageManage void onTileAction(RenderSource&, TileOperation, const OverscaledTileID&, const std::string&) override; // ImageManagerObserver implementation - void onStyleImageMissing(const std::string&, const std::function&) override; + void onStyleImageMissing(const std::string&, Scheduler::Task&&) override; void onRemoveUnusedStyleImages(const std::vector&) override; #if MLN_DRAWABLE_RENDERER diff --git a/src/mbgl/storage/asset_file_source.hpp b/src/mbgl/storage/asset_file_source.hpp index 8db775592d1..e9d9495fe3b 100644 --- a/src/mbgl/storage/asset_file_source.hpp +++ b/src/mbgl/storage/asset_file_source.hpp @@ -16,7 +16,7 @@ class AssetFileSource : public FileSource { AssetFileSource(const ResourceOptions& resourceOptions, const ClientOptions& clientOptions); ~AssetFileSource() override; - std::unique_ptr request(const Resource&, Callback) override; + std::unique_ptr request(const Resource&, CopyableCallback&&) override; bool canRequest(const Resource&) const override; void pause() override; void resume() override; diff --git a/src/mbgl/storage/http_file_source.hpp b/src/mbgl/storage/http_file_source.hpp index 7a123395896..ec38b70f9ce 100644 --- a/src/mbgl/storage/http_file_source.hpp +++ b/src/mbgl/storage/http_file_source.hpp @@ -13,7 +13,7 @@ class HTTPFileSource : public FileSource { HTTPFileSource(const ResourceOptions& resourceOptions, const ClientOptions& clientOptions); ~HTTPFileSource() override; - std::unique_ptr request(const Resource&, Callback) override; + std::unique_ptr request(const Resource&, CopyableCallback&&) override; bool canRequest(const Resource& resource) const override { return resource.hasLoadingMethod(Resource::LoadingMethod::Network); } diff --git a/src/mbgl/storage/local_file_source.hpp b/src/mbgl/storage/local_file_source.hpp index 79a90de6720..2f46e9a42dd 100644 --- a/src/mbgl/storage/local_file_source.hpp +++ b/src/mbgl/storage/local_file_source.hpp @@ -16,7 +16,7 @@ class LocalFileSource : public FileSource { LocalFileSource(const ResourceOptions& resourceOptions, const ClientOptions& clientOptions); ~LocalFileSource() override; - std::unique_ptr request(const Resource&, Callback) override; + std::unique_ptr request(const Resource&, CopyableCallback&&) override; bool canRequest(const Resource&) const override; void pause() override; void resume() override; diff --git a/src/mbgl/storage/main_resource_loader.hpp b/src/mbgl/storage/main_resource_loader.hpp index 6b6ddc54266..b91d51a37c4 100644 --- a/src/mbgl/storage/main_resource_loader.hpp +++ b/src/mbgl/storage/main_resource_loader.hpp @@ -14,7 +14,7 @@ class MainResourceLoader final : public FileSource { ~MainResourceLoader() override; bool supportsCacheOnlyRequests() const override; - std::unique_ptr request(const Resource&, Callback) override; + std::unique_ptr request(const Resource&, CopyableCallback&&) override; bool canRequest(const Resource&) const override; void pause() override; void resume() override; diff --git a/src/mbgl/storage/mbtiles_file_source.hpp b/src/mbgl/storage/mbtiles_file_source.hpp index 876d348c653..788295e8e6b 100644 --- a/src/mbgl/storage/mbtiles_file_source.hpp +++ b/src/mbgl/storage/mbtiles_file_source.hpp @@ -13,7 +13,7 @@ class MBTilesFileSource : public FileSource { MBTilesFileSource(const ResourceOptions& resourceOptions, const ClientOptions& clientOptions); ~MBTilesFileSource() override; - std::unique_ptr request(const Resource&, Callback) override; + std::unique_ptr request(const Resource&, CopyableCallback&&) override; bool canRequest(const Resource&) const override; void setResourceOptions(ResourceOptions) override; diff --git a/src/mbgl/tile/geometry_tile_worker.cpp b/src/mbgl/tile/geometry_tile_worker.cpp index 24a81c4ca70..7e90e7c477c 100644 --- a/src/mbgl/tile/geometry_tile_worker.cpp +++ b/src/mbgl/tile/geometry_tile_worker.cpp @@ -14,12 +14,13 @@ #include #include #include -#include -#include #include -#include +#include #include +#include +#include #include +#include #include #include @@ -372,9 +373,9 @@ void GeometryTileWorker::parse() { return; } - MBGL_TIMING_START(watch) + MBGL_TIMING_START(watch); - std::unordered_map> symbolLayoutMap; + mbgl::unordered_map> symbolLayoutMap; renderData.clear(); layouts.clear(); @@ -495,7 +496,7 @@ void GeometryTileWorker::finalizeLayout() { return; } - MBGL_TIMING_START(watch) + MBGL_TIMING_START(watch); std::optional glyphAtlasImage; ImageAtlas iconAtlas = makeImageAtlas(imageMap, patternMap, versionMap); if (!layouts.empty()) { diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp index 9985352e022..5201e9b9dad 100644 --- a/src/mbgl/tile/tile_cache.cpp +++ b/src/mbgl/tile/tile_cache.cpp @@ -2,6 +2,7 @@ #include #include +#include #include @@ -17,6 +18,7 @@ TileCache::~TileCache() { deferredSignal.wait(counterLock, [&]() { return deferredDeletionsPending == 0; }); } + void TileCache::setSize(size_t size_) { MLN_TRACE_FUNC(); @@ -37,25 +39,6 @@ void TileCache::setSize(size_t size_) { assert(orderedKeys.size() <= size); } -namespace { -/// This exists solely to prevent a problem where temporary lambda captures -/// are retained for the duration of the scope instead of being destroyed immediately. -struct CaptureWrapper { - CaptureWrapper(std::vector>&& items_) - : items(items_.size()) { - std::ranges::move(items_, items.begin()); - } - CaptureWrapper(CaptureWrapper&&) = default; - - // coverage:ignore-start - /// This copy constructor is required to build, but doesn't seem to be called. - CaptureWrapper(const CaptureWrapper& other) - : items(other.items) {} - // coverage:ignore-end - - std::vector> items; -}; -} // namespace void TileCache::deferredRelease(std::unique_ptr&& tile) { MLN_TRACE_FUNC(); @@ -78,27 +61,22 @@ void TileCache::deferPendingReleases() { deferredDeletionsPending++; } - CaptureWrapper wrap{std::move(pendingReleases)}; + // Move elements to a disposable container to be captured by the lambda + decltype(pendingReleases) pending{pendingReleases.size()}; + std::ranges::move(pendingReleases, pending.begin()); pendingReleases.clear(); - - // The `std::function` must be created in a separate statement from the `schedule` call. - // Creating a `std::function` from a lambda involves a copy, which is why we must use - // `shared_ptr` rather than `unique_ptr` for the capture. As a result, a temporary holds - // a reference until the construction is complete and the lambda is destroyed. - // If this temporary outlives the `schedule` call, and the function is executed immediately - // by a waiting thread and is already complete, that temporary reference ends up being the - // last one and the destruction actually occurs here on this thread. - std::function func{[tile_{CaptureWrapper{std::move(wrap)}}, this]() mutable { + threadPool.schedule({[items{std::move(pending)}, this]() mutable { MLN_TRACE_ZONE(deferPendingReleases lambda); MLN_ZONE_VALUE(wrap_.releases.size()); - tile_.items.clear(); + // Run the deletions + items.clear(); + // Wake up a waiting destructor std::lock_guard counterLock(deferredSignalLock); deferredDeletionsPending--; deferredSignal.notify_all(); - }}; - - threadPool.schedule(std::move(func)); + }}); + pendingReleases.clear(); } void TileCache::add(const OverscaledTileID& key, std::unique_ptr&& tile) { diff --git a/src/mbgl/util/stopwatch.hpp b/src/mbgl/util/stopwatch.hpp index 3e29122c0ab..693c4266e44 100644 --- a/src/mbgl/util/stopwatch.hpp +++ b/src/mbgl/util/stopwatch.hpp @@ -21,10 +21,10 @@ namespace util { std::stringstream messageStream; \ messageStream << message; \ watch->report(messageStream.str()); \ - } while (0); + } while (0) #else -#define MBGL_TIMING_START(watch) -#define MBGL_TIMING_FINISH(watch, message) +#define MBGL_TIMING_START(watch) ((void)0) +#define MBGL_TIMING_FINISH(watch, message) ((void)0) #endif #ifndef DISABLE_STOPWATCH diff --git a/src/mbgl/util/thread_pool.cpp b/src/mbgl/util/thread_pool.cpp index bf2ad316df7..655e076eed8 100644 --- a/src/mbgl/util/thread_pool.cpp +++ b/src/mbgl/util/thread_pool.cpp @@ -59,7 +59,7 @@ std::thread ThreadedSchedulerBase::makeSchedulerThread(size_t index) { // 2. Visit a task from each for (auto& q : pending) { - std::function tasklet; + Task tasklet; { std::lock_guard lock(q->lock); if (q->queue.size()) { @@ -105,11 +105,11 @@ std::thread ThreadedSchedulerBase::makeSchedulerThread(size_t index) { }); } -void ThreadedSchedulerBase::schedule(std::function&& fn) { +void ThreadedSchedulerBase::schedule(Task&& fn) { schedule(uniqueID, std::move(fn)); } -void ThreadedSchedulerBase::schedule(const util::SimpleIdentity tag, std::function&& fn) { +void ThreadedSchedulerBase::schedule(const util::SimpleIdentity tag, Task&& fn) { MLN_TRACE_FUNC(); assert(fn); if (!fn) return; diff --git a/src/mbgl/util/thread_pool.hpp b/src/mbgl/util/thread_pool.hpp index b1614666179..afa30b09f5b 100644 --- a/src/mbgl/util/thread_pool.hpp +++ b/src/mbgl/util/thread_pool.hpp @@ -22,12 +22,12 @@ class ThreadedSchedulerBase : public Scheduler { /// @brief Schedule a generic task not assigned to any particular owner. /// The scheduler itself will own the task. /// @param fn Task to run - void schedule(std::function&& fn) override; + void schedule(Task&& fn) override; /// @brief Schedule a task assigned to the given owner `tag`. /// @param tag Identifier object to indicate ownership of `fn` /// @param fn Task to run - void schedule(const util::SimpleIdentity tag, std::function&& fn) override; + void schedule(const util::SimpleIdentity tag, Task&& fn) override; const util::SimpleIdentity uniqueID; protected: @@ -56,10 +56,10 @@ class ThreadedSchedulerBase : public Scheduler { // Task queues bucketed by tag address struct Queue { - std::atomic runningCount; /* running tasks */ - std::condition_variable cv; /* queue empty condition */ - std::mutex lock; /* lock */ - std::queue> queue; /* pending task queue */ + std::atomic runningCount; /* running tasks */ + std::condition_variable cv; /* queue empty condition */ + std::mutex lock; /* lock */ + std::queue queue; /* pending task queue */ }; mbgl::unordered_map> taggedQueue; }; @@ -90,7 +90,7 @@ class ThreadedScheduler : public ThreadedSchedulerBase { } } - void runOnRenderThread(const util::SimpleIdentity tag, std::function&& fn) override { + void runOnRenderThread(const util::SimpleIdentity tag, Task&& fn) override { std::shared_ptr queue; { std::lock_guard lock(taggedRenderQueueLock); @@ -149,7 +149,7 @@ class ThreadedScheduler : public ThreadedSchedulerBase { std::vector threads; struct RenderQueue { - std::queue> queue; + std::queue queue; std::mutex mutex; }; mbgl::unordered_map> taggedRenderQueue; diff --git a/test/src/mbgl/test/fake_file_source.hpp b/test/src/mbgl/test/fake_file_source.hpp index 95752b0037c..b68ba036f88 100644 --- a/test/src/mbgl/test/fake_file_source.hpp +++ b/test/src/mbgl/test/fake_file_source.hpp @@ -27,12 +27,14 @@ class FakeFileSource : public FileSource { class FakeFileRequest : public AsyncRequest { public: Resource resource; - Callback callback; + CopyableCallback callback; std::list& list; std::list::iterator link; - FakeFileRequest(Resource resource_, Callback callback_, std::list& list_) + FakeFileRequest(Resource resource_, + CopyableCallback&& callback_, + std::list& list_) : resource(std::move(resource_)), callback(std::move(callback_)), list(list_), @@ -47,8 +49,9 @@ class FakeFileSource : public FileSource { FakeFileSource() : FakeFileSource(ResourceOptions::Default(), ClientOptions()) {} - std::unique_ptr request(const Resource& resource, Callback callback) override { - return std::make_unique(resource, callback, requests); + std::unique_ptr request(const Resource& resource, + CopyableCallback&& callback) override { + return std::make_unique(resource, std::move(callback), requests); } bool canRequest(const Resource&) const override { return true; } @@ -62,7 +65,7 @@ class FakeFileSource : public FileSource { if (requestFound) { // Copy the callback, in case calling it deallocates the AsyncRequest. - Callback callback_ = (*it)->callback; + auto callback_ = (*it)->callback; callback_(response); } @@ -89,8 +92,9 @@ class FakeOnlineFileSource : public FakeFileSource { FakeOnlineFileSource(const ResourceOptions& resourceOptions_, const ClientOptions& clientOptions_) : FakeFileSource(resourceOptions_, clientOptions_) {} - std::unique_ptr request(const Resource& resource, Callback callback) override { - return FakeFileSource::request(resource, callback); + std::unique_ptr request(const Resource& resource, + CopyableCallback&& callback) override { + return FakeFileSource::request(resource, std::move(callback)); } bool respond(Resource::Kind kind, const Response& response) { return FakeFileSource::respond(kind, response); } diff --git a/test/src/mbgl/test/stub_file_source.hpp b/test/src/mbgl/test/stub_file_source.hpp index 1bad4a74d22..85e1189daf2 100644 --- a/test/src/mbgl/test/stub_file_source.hpp +++ b/test/src/mbgl/test/stub_file_source.hpp @@ -23,8 +23,8 @@ class StubFileSource : public FileSource { StubFileSource(ResponseType = ResponseType::Asynchronous); ~StubFileSource() override; - std::unique_ptr request(const Resource&, Callback) override; - bool canRequest(const Resource&) const override { return true; } + std::unique_ptr request(const Resource&, CopyableCallback&&) override; + bool canRequest(const Resource&) const noexcept override { return true; } void remove(AsyncRequest*); void setProperty(const std::string&, const mapbox::base::Value&) override; mapbox::base::Value getProperty(const std::string&) const override; diff --git a/test/storage/sync_file_source.test.cpp b/test/storage/sync_file_source.test.cpp index 8a87ba1412c..95ee2f1f050 100644 --- a/test/storage/sync_file_source.test.cpp +++ b/test/storage/sync_file_source.test.cpp @@ -18,7 +18,8 @@ using namespace mbgl; class SyncFileSource : public FileSource { public: - std::unique_ptr request(const Resource& resource, FileSource::Callback callback) override { + std::unique_ptr request(const Resource& resource, + CopyableCallback&& callback) override { Response response; auto it = assets.find(resource.url); if (it == assets.end()) { diff --git a/vendor/BUILD.bazel b/vendor/BUILD.bazel index 66815d83b44..46e4b178e24 100644 --- a/vendor/BUILD.bazel +++ b/vendor/BUILD.bazel @@ -226,3 +226,12 @@ cc_library( strip_include_prefix = "unordered_dense/include/ankerl", visibility = ["//visibility:public"], ) + +cc_library( + name = "nontype_functional", + srcs = [], + hdrs = ["nontype_functional/include"], + include_prefix = "ankerl", + strip_include_prefix = "nontype_functional/include", + visibility = ["//visibility:public"], +) diff --git a/vendor/nontype_functional b/vendor/nontype_functional new file mode 160000 index 00000000000..6fae3505138 --- /dev/null +++ b/vendor/nontype_functional @@ -0,0 +1 @@ +Subproject commit 6fae3505138a74c0bf2380ca769b4193b891d13e diff --git a/vendor/nontype_functional.cmake b/vendor/nontype_functional.cmake new file mode 100644 index 00000000000..0673e9c1ca3 --- /dev/null +++ b/vendor/nontype_functional.cmake @@ -0,0 +1,21 @@ +if(TARGET mbgl-vendor-nontype_functional) + return() +endif() + +add_library( + mbgl-vendor-nontype_functional INTERFACE +) + +target_include_directories( + mbgl-vendor-nontype_functional SYSTEM + INTERFACE ${CMAKE_CURRENT_LIST_DIR}/nontype_functional/include +) + +set_target_properties( + mbgl-vendor-nontype_functional + PROPERTIES + INTERFACE_MAPLIBRE_NAME "nontype_functional" + INTERFACE_MAPLIBRE_URL "https://github.com/zhihaoy/nontype_functional" + INTERFACE_MAPLIBRE_AUTHOR "zhihaoy" + INTERFACE_MAPLIBRE_LICENSE ${CMAKE_CURRENT_LIST_DIR}/nontype_functional/LICENSE +) From 90c6a0b7a5c92f3bb00036387d052490af167ddc Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Thu, 21 Nov 2024 07:38:38 -0800 Subject: [PATCH 08/14] xcode build --- BUILD.bazel | 1 + platform/darwin/src/http_file_source.mm | 10 +++++----- render-test/file_source.cpp | 3 ++- vendor/BUILD.bazel | 6 +++--- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 9e7d53e0373..8ca2cb9d62d 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -128,6 +128,7 @@ cc_library( "//vendor:earcut.hpp", "//vendor:eternal", "//vendor:mapbox-base", + "//vendor:nontype_functional", "//vendor:parsedate", "//vendor:polylabel", "//vendor:protozero", diff --git a/platform/darwin/src/http_file_source.mm b/platform/darwin/src/http_file_source.mm index 007a0b902d4..bb4571a8b42 100644 --- a/platform/darwin/src/http_file_source.mm +++ b/platform/darwin/src/http_file_source.mm @@ -55,9 +55,9 @@ void cancel() { class HTTPRequest : public AsyncRequest { public: - HTTPRequest(FileSource::Callback callback_) + HTTPRequest(FileSource::CopyableCallback&& callback_) : shared(std::make_shared(response, async)), - callback(callback_) { + callback(std::move(callback_)) { } ~HTTPRequest() override { @@ -71,7 +71,7 @@ void cancel() { NSURLSessionDataTask* task = nil; private: - FileSource::Callback callback; + FileSource::CopyableCallback callback; Response response; util::AsyncTask async { [this] { @@ -248,8 +248,8 @@ BOOL isValidMapboxEndpoint(NSURL *url) { return url; } -std::unique_ptr HTTPFileSource::request(const Resource& resource, Callback callback) { - auto request = std::make_unique(callback); +std::unique_ptr HTTPFileSource::request(const Resource& resource, CopyableCallback&& callback) { + auto request = std::make_unique(std::move(callback)); auto shared = request->shared; // Explicit copy so that it also gets copied into the completion handler block below. @autoreleasepool { diff --git a/render-test/file_source.cpp b/render-test/file_source.cpp index 30b1f679e1f..5b684560aec 100644 --- a/render-test/file_source.cpp +++ b/render-test/file_source.cpp @@ -32,7 +32,8 @@ ProxyFileSource::ProxyFileSource(std::shared_ptr defaultResourceLoad ProxyFileSource::~ProxyFileSource() = default; -std::unique_ptr ProxyFileSource::request(const Resource& resource, Callback callback) { +std::unique_ptr ProxyFileSource::request(const Resource& resource, + CopyableCallback&& callback) { auto transformed = resource; // If offline, force always loading the resource from the cache diff --git a/vendor/BUILD.bazel b/vendor/BUILD.bazel index 46e4b178e24..dbe6104d16a 100644 --- a/vendor/BUILD.bazel +++ b/vendor/BUILD.bazel @@ -230,8 +230,8 @@ cc_library( cc_library( name = "nontype_functional", srcs = [], - hdrs = ["nontype_functional/include"], - include_prefix = "ankerl", - strip_include_prefix = "nontype_functional/include", + hdrs = glob(["nontype_functional/include/std23/*.h"]), + include_prefix = "std23", + strip_include_prefix = "nontype_functional/include/std23", visibility = ["//visibility:public"], ) From c7f4deadbb619e5dc87de9b110b5955ed5de8c1e Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Thu, 21 Nov 2024 08:02:33 -0800 Subject: [PATCH 09/14] improve type reference --- platform/default/src/mbgl/storage/online_file_source.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/platform/default/src/mbgl/storage/online_file_source.cpp b/platform/default/src/mbgl/storage/online_file_source.cpp index 5b0604b43f7..5849e443d08 100644 --- a/platform/default/src/mbgl/storage/online_file_source.cpp +++ b/platform/default/src/mbgl/storage/online_file_source.cpp @@ -37,9 +37,9 @@ constexpr const char* ONLINE_STATUS_KEY = "online-status"; class OnlineFileSourceThread; struct OnlineFileRequest { - using Callback = std::function; + using Callback = FileSource::CopyableCallback; - OnlineFileRequest(Resource resource_, Callback callback_, OnlineFileSourceThread& impl_); + OnlineFileRequest(Resource resource_, Callback&& callback_, OnlineFileSourceThread& impl_); ~OnlineFileRequest(); void networkIsReachableAgain(); @@ -429,7 +429,7 @@ class OnlineFileSource::Impl { const std::unique_ptr> thread; }; -OnlineFileRequest::OnlineFileRequest(Resource resource_, Callback callback_, OnlineFileSourceThread& impl_) +OnlineFileRequest::OnlineFileRequest(Resource resource_, Callback&& callback_, OnlineFileSourceThread& impl_) : impl(impl_), resource(std::move(resource_)), callback(std::move(callback_)) { From 5b8a962b908af260d9e677a036874139f983b344 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Thu, 21 Nov 2024 08:02:58 -0800 Subject: [PATCH 10/14] Disable static downcast warnings --- .clang-tidy | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.clang-tidy b/.clang-tidy index e50cf88144e..5394510eeda 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -113,7 +113,8 @@ Checks: [ -performance-enum-size, -misc-include-cleaner, -readability-redundant-inline-specifier, - -readability-avoid-nested-conditional-operator + -readability-avoid-nested-conditional-operator, + -cppcoreguidelines-pro-type-static-cast-downcast # no RTTI ] WarningsAsErrors: '*' HeaderFilterRegex: '.*' From 0511a86c4b36a25485a84c7d03e4e66814f17863 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Wed, 4 Dec 2024 11:32:43 -0800 Subject: [PATCH 11/14] cherry-pick 366ebcc7d0 --- .github/workflows/android-ci.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/android-ci.yml b/.github/workflows/android-ci.yml index a26ad33a352..c121ca7bc01 100644 --- a/.github/workflows/android-ci.yml +++ b/.github/workflows/android-ci.yml @@ -212,6 +212,18 @@ jobs: working-directory: test/android steps: + - name: Free Disk Space (Ubuntu) + if: startsWith(runner.name, 'GitHub Actions') + uses: jlumbroso/free-disk-space@main + with: + tool-cache: false + android: false + dotnet: true + haskell: true + large-packages: true + docker-images: true + swap-storage: false + - uses: actions/checkout@v4 with: submodules: recursive From 8ce96e454f114a9ea52c2f8874c2b976aa1b9859 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Wed, 4 Dec 2024 12:10:01 -0800 Subject: [PATCH 12/14] fix glfw/xcode build --- platform/glfw/CMakeLists.txt | 1 + src/mbgl/tile/tile_cache.cpp | 8 +++----- src/mbgl/vulkan/context.cpp | 2 +- src/mbgl/vulkan/descriptor_set.cpp | 12 +++++++----- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/platform/glfw/CMakeLists.txt b/platform/glfw/CMakeLists.txt index e8db2bc77c4..784ef8d107f 100644 --- a/platform/glfw/CMakeLists.txt +++ b/platform/glfw/CMakeLists.txt @@ -124,6 +124,7 @@ target_link_libraries( Mapbox::Map mbgl-compiler-options Mapbox::Base::cheap-ruler-cpp + mbgl-vendor-nontype_functional ) if(NOT WIN32 AND MLN_WITH_OPENGL) diff --git a/src/mbgl/tile/tile_cache.cpp b/src/mbgl/tile/tile_cache.cpp index 5201e9b9dad..270f3d7ec50 100644 --- a/src/mbgl/tile/tile_cache.cpp +++ b/src/mbgl/tile/tile_cache.cpp @@ -18,7 +18,6 @@ TileCache::~TileCache() { deferredSignal.wait(counterLock, [&]() { return deferredDeletionsPending == 0; }); } - void TileCache::setSize(size_t size_) { MLN_TRACE_FUNC(); @@ -39,7 +38,6 @@ void TileCache::setSize(size_t size_) { assert(orderedKeys.size() <= size); } - void TileCache::deferredRelease(std::unique_ptr&& tile) { MLN_TRACE_FUNC(); @@ -67,11 +65,11 @@ void TileCache::deferPendingReleases() { pendingReleases.clear(); threadPool.schedule({[items{std::move(pending)}, this]() mutable { MLN_TRACE_ZONE(deferPendingReleases lambda); - MLN_ZONE_VALUE(wrap_.releases.size()); - // Run the deletions + MLN_ZONE_VALUE(items.size()); + // Run the deletions items.clear(); - // Wake up a waiting destructor + // Wake up a waiting destructor std::lock_guard counterLock(deferredSignalLock); deferredDeletionsPending--; deferredSignal.notify_all(); diff --git a/src/mbgl/vulkan/context.cpp b/src/mbgl/vulkan/context.cpp index 1acb4d691d4..bfa821aa6ef 100644 --- a/src/mbgl/vulkan/context.cpp +++ b/src/mbgl/vulkan/context.cpp @@ -584,7 +584,7 @@ void Context::buildUniformDescriptorSetLayout(vk::UniqueDescriptorSetLayout& lay for (size_t i = 0; i < uniformCount; ++i) { bindings.push_back(vk::DescriptorSetLayoutBinding() - .setBinding(i) + .setBinding(static_cast(i)) .setStageFlags(stageFlags) .setDescriptorType(vk::DescriptorType::eUniformBuffer) .setDescriptorCount(1)); diff --git a/src/mbgl/vulkan/descriptor_set.cpp b/src/mbgl/vulkan/descriptor_set.cpp index d8d6866e5e7..f27f0a23c3e 100644 --- a/src/mbgl/vulkan/descriptor_set.cpp +++ b/src/mbgl/vulkan/descriptor_set.cpp @@ -50,7 +50,7 @@ void DescriptorSet::createDescriptorPool(DescriptorPoolGrowable& growablePool) { const auto descriptorPoolInfo = vk::DescriptorPoolCreateInfo(poolFlags).setPoolSizes(size).setMaxSets(maxSets); growablePool.pools.emplace_back(device->createDescriptorPoolUnique(descriptorPoolInfo), maxSets); - growablePool.currentPoolIndex = growablePool.pools.size() - 1; + growablePool.currentPoolIndex = static_cast(growablePool.pools.size() - 1); }; void DescriptorSet::allocate() { @@ -73,7 +73,8 @@ void DescriptorSet::allocate() { growablePool.pools.begin(), growablePool.pools.end(), [&](const auto& p) { return !p.unusedSets.empty(); }); if (unusedPoolIt != growablePool.pools.end()) { - growablePool.currentPoolIndex = std::distance(growablePool.pools.begin(), unusedPoolIt); + growablePool.currentPoolIndex = static_cast( + std::distance(growablePool.pools.begin(), unusedPoolIt)); } else #endif { @@ -83,7 +84,8 @@ void DescriptorSet::allocate() { [&](const auto& p) { return p.remainingSets >= layouts.size(); }); if (freePoolIt != growablePool.pools.end()) { - growablePool.currentPoolIndex = std::distance(growablePool.pools.begin(), freePoolIt); + growablePool.currentPoolIndex = static_cast( + std::distance(growablePool.pools.begin(), freePoolIt)); } else { createDescriptorPool(growablePool); } @@ -161,7 +163,7 @@ void UniformDescriptorSet::update(const gfx::UniformBufferArray& uniforms, .setBufferInfo(descriptorBufferInfo) .setDescriptorCount(1) .setDescriptorType(vk::DescriptorType::eUniformBuffer) - .setDstBinding(index) + .setDstBinding(static_cast(index)) .setDstSet(descriptorSets[frameIndex]); device->updateDescriptorSets(writeDescriptorSet, nullptr); @@ -197,7 +199,7 @@ void ImageDescriptorSet::update(const std::array(id)) .setDstSet(descriptorSets[frameIndex]); device->updateDescriptorSets(writeDescriptorSet, nullptr); From 18341ea6efdfe041e2c0ec2bb8b0d4a30c9565da Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Wed, 4 Dec 2024 14:01:51 -0800 Subject: [PATCH 13/14] use public dep --- CMakeLists.txt | 2 +- platform/glfw/CMakeLists.txt | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e736f861f0c..e6b25e2f944 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1474,7 +1474,6 @@ target_link_libraries( mbgl-vendor-earcut.hpp mbgl-vendor-eternal $<$:mbgl-vendor-metal-cpp> - mbgl-vendor-nontype_functional mbgl-vendor-parsedate mbgl-vendor-polylabel mbgl-vendor-protozero @@ -1488,6 +1487,7 @@ target_link_libraries( Mapbox::Base::geojson.hpp Mapbox::Base::geometry.hpp Mapbox::Base::variant + mbgl-vendor-nontype_functional $<$:TracyClient> unordered_dense ) diff --git a/platform/glfw/CMakeLists.txt b/platform/glfw/CMakeLists.txt index 784ef8d107f..e8db2bc77c4 100644 --- a/platform/glfw/CMakeLists.txt +++ b/platform/glfw/CMakeLists.txt @@ -124,7 +124,6 @@ target_link_libraries( Mapbox::Map mbgl-compiler-options Mapbox::Base::cheap-ruler-cpp - mbgl-vendor-nontype_functional ) if(NOT WIN32 AND MLN_WITH_OPENGL) From 3154c34582326251a1bb86650cdd3ef859c56cc9 Mon Sep 17 00:00:00 2001 From: Tim Sylvester Date: Wed, 4 Dec 2024 23:15:06 +0000 Subject: [PATCH 14/14] linux build --- include/mbgl/renderer/renderer_observer.hpp | 10 +++++----- include/mbgl/storage/database_file_source.hpp | 2 +- include/mbgl/storage/file_source.hpp | 2 +- include/mbgl/storage/online_file_source.hpp | 2 +- .../src/cpp/asset_manager_file_source.cpp | 2 +- .../src/cpp/asset_manager_file_source.hpp | 2 +- .../MapLibreAndroid/src/cpp/http_file_source.cpp | 6 +++--- platform/darwin/src/http_file_source.mm | 4 ++-- .../include/mbgl/storage/file_source_request.hpp | 2 +- .../default/src/mbgl/storage/asset_file_source.cpp | 2 +- .../default/src/mbgl/storage/database_file_source.cpp | 2 +- .../default/src/mbgl/storage/file_source_request.cpp | 2 +- platform/default/src/mbgl/storage/http_file_source.cpp | 10 +++++----- .../default/src/mbgl/storage/local_file_source.cpp | 2 +- .../default/src/mbgl/storage/main_resource_loader.cpp | 4 ++-- .../default/src/mbgl/storage/mbtiles_file_source.cpp | 2 +- .../default/src/mbgl/storage/online_file_source.cpp | 4 ++-- platform/default/src/mbgl/util/run_loop.cpp | 6 +++--- render-test/file_source.cpp | 2 +- render-test/file_source.hpp | 2 +- src/mbgl/storage/asset_file_source.hpp | 2 +- src/mbgl/storage/http_file_source.hpp | 2 +- src/mbgl/storage/local_file_source.hpp | 2 +- src/mbgl/storage/main_resource_loader.hpp | 2 +- src/mbgl/storage/mbtiles_file_source.hpp | 2 +- test/actor/actor.test.cpp | 4 ++-- test/renderer/image_manager.test.cpp | 2 +- test/src/mbgl/test/fake_file_source.hpp | 6 +++--- test/src/mbgl/test/stub_file_source.cpp | 4 ++-- test/src/mbgl/test/stub_file_source.hpp | 4 ++-- test/storage/asset_file_source.test.cpp | 2 +- test/storage/sync_file_source.test.cpp | 2 +- 32 files changed, 52 insertions(+), 52 deletions(-) diff --git a/include/mbgl/renderer/renderer_observer.hpp b/include/mbgl/renderer/renderer_observer.hpp index d1d1594c356..693451d9a38 100644 --- a/include/mbgl/renderer/renderer_observer.hpp +++ b/include/mbgl/renderer/renderer_observer.hpp @@ -1,15 +1,15 @@ #pragma once -#include -#include -#include -#include +#include #include #include +#include +#include +#include +#include #include #include -#include #include namespace mbgl { diff --git a/include/mbgl/storage/database_file_source.hpp b/include/mbgl/storage/database_file_source.hpp index 1b781776ae8..f83abb7f561 100644 --- a/include/mbgl/storage/database_file_source.hpp +++ b/include/mbgl/storage/database_file_source.hpp @@ -16,7 +16,7 @@ class DatabaseFileSource : public FileSource { ~DatabaseFileSource() override; /// FileSource overrides - std::unique_ptr request(const Resource&, CopyableCallback&&) override; + std::unique_ptr request(const Resource&, CopyableCallback) override; void forward(const Resource&, const Response&, Scheduler::Task&& callback) override; bool canRequest(const Resource&) const override; void setProperty(const std::string&, const mapbox::base::Value&) override; diff --git a/include/mbgl/storage/file_source.hpp b/include/mbgl/storage/file_source.hpp index 1fcbb9cb142..1cc150311aa 100644 --- a/include/mbgl/storage/file_source.hpp +++ b/include/mbgl/storage/file_source.hpp @@ -49,7 +49,7 @@ class FileSource { /// RunLoop. The request may be cancelled before completion by releasing the /// returned AsyncRequest. If the request is cancelled before the callback /// is executed, the callback will not be executed. - virtual std::unique_ptr request(const Resource&, CopyableCallback&&) = 0; + virtual std::unique_ptr request(const Resource&, CopyableCallback) = 0; /// Allows to forward response from one source to another. /// Optionally, callback can be provided to receive notification for forward diff --git a/include/mbgl/storage/online_file_source.hpp b/include/mbgl/storage/online_file_source.hpp index 9c9bf4abd90..b4ae4380923 100644 --- a/include/mbgl/storage/online_file_source.hpp +++ b/include/mbgl/storage/online_file_source.hpp @@ -19,7 +19,7 @@ class OnlineFileSource : public FileSource { private: // FileSource overrides - std::unique_ptr request(const Resource&, CopyableCallback&&) override; + std::unique_ptr request(const Resource&, CopyableCallback) override; bool canRequest(const Resource&) const override; void pause() override; void resume() override; diff --git a/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.cpp b/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.cpp index 2a7a0933d70..073099b855f 100644 --- a/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.cpp @@ -71,7 +71,7 @@ AssetManagerFileSource::AssetManagerFileSource(jni::JNIEnv& env, AssetManagerFileSource::~AssetManagerFileSource() = default; std::unique_ptr AssetManagerFileSource::request(const Resource& resource, - CopyableCallback&& callback) { + CopyableCallback callback) { auto req = std::make_unique(std::move(callback)); impl->actor().invoke(&Impl::request, resource.url, req->actor()); diff --git a/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.hpp b/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.hpp index 0d5805e64b1..3a67e9be04c 100644 --- a/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.hpp +++ b/platform/android/MapLibreAndroid/src/cpp/asset_manager_file_source.hpp @@ -23,7 +23,7 @@ class AssetManagerFileSource : public FileSource { const ClientOptions); ~AssetManagerFileSource() override; - std::unique_ptr request(const Resource&, CopyableCallback&&) override; + std::unique_ptr request(const Resource&, CopyableCallback) override; bool canRequest(const Resource&) const override; void setResourceOptions(ResourceOptions options) override; diff --git a/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp b/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp index 9aaa21a6a19..3ec1361bd92 100644 --- a/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp +++ b/platform/android/MapLibreAndroid/src/cpp/http_file_source.cpp @@ -39,7 +39,7 @@ class HTTPRequest : public AsyncRequest { public: static constexpr auto Name() { return "org/maplibre/android/http/NativeHttpRequest"; }; - HTTPRequest(jni::JNIEnv&, const Resource&, FileSource::CopyableCallback&&); + HTTPRequest(jni::JNIEnv&, const Resource&, FileSource::CopyableCallback); ~HTTPRequest() override; void onFailure(jni::JNIEnv&, int type, const jni::String& message); @@ -90,7 +90,7 @@ void RegisterNativeHTTPRequest(jni::JNIEnv& env) { HTTPRequest::HTTPRequest(jni::JNIEnv& env, const Resource& resource_, - FileSource::CopyableCallback&& callback_) + FileSource::CopyableCallback callback_) : resource(resource_), callback(std::move(callback_)) { std::string etagStr; @@ -217,7 +217,7 @@ HTTPFileSource::HTTPFileSource(const ResourceOptions& resourceOptions, const Cli HTTPFileSource::~HTTPFileSource() = default; std::unique_ptr HTTPFileSource::request(const Resource& resource, - CopyableCallback&& callback) { + CopyableCallback callback) { return std::make_unique(*impl->env, resource, std::move(callback)); } diff --git a/platform/darwin/src/http_file_source.mm b/platform/darwin/src/http_file_source.mm index bb4571a8b42..e247adcbe74 100644 --- a/platform/darwin/src/http_file_source.mm +++ b/platform/darwin/src/http_file_source.mm @@ -55,7 +55,7 @@ void cancel() { class HTTPRequest : public AsyncRequest { public: - HTTPRequest(FileSource::CopyableCallback&& callback_) + HTTPRequest(FileSource::CopyableCallback callback_) : shared(std::make_shared(response, async)), callback(std::move(callback_)) { } @@ -248,7 +248,7 @@ BOOL isValidMapboxEndpoint(NSURL *url) { return url; } -std::unique_ptr HTTPFileSource::request(const Resource& resource, CopyableCallback&& callback) { +std::unique_ptr HTTPFileSource::request(const Resource& resource, CopyableCallback callback) { auto request = std::make_unique(std::move(callback)); auto shared = request->shared; // Explicit copy so that it also gets copied into the completion handler block below. diff --git a/platform/default/include/mbgl/storage/file_source_request.hpp b/platform/default/include/mbgl/storage/file_source_request.hpp index 6e508f6af61..97f373cbda5 100644 --- a/platform/default/include/mbgl/storage/file_source_request.hpp +++ b/platform/default/include/mbgl/storage/file_source_request.hpp @@ -13,7 +13,7 @@ class Mailbox; class FileSourceRequest final : public AsyncRequest { public: - FileSourceRequest(FileSource::CopyableCallback&& callback); + FileSourceRequest(FileSource::CopyableCallback callback); ~FileSourceRequest() final; void onCancel(std::function&& callback); diff --git a/platform/default/src/mbgl/storage/asset_file_source.cpp b/platform/default/src/mbgl/storage/asset_file_source.cpp index c2d5a7649c8..e427bf2d025 100644 --- a/platform/default/src/mbgl/storage/asset_file_source.cpp +++ b/platform/default/src/mbgl/storage/asset_file_source.cpp @@ -78,7 +78,7 @@ AssetFileSource::AssetFileSource(const ResourceOptions& resourceOptions, const C AssetFileSource::~AssetFileSource() = default; std::unique_ptr AssetFileSource::request(const Resource& resource, - CopyableCallback&& callback) { + CopyableCallback callback) { auto req = std::make_unique(std::move(callback)); impl->actor().invoke(&Impl::request, resource.url, req->actor()); diff --git a/platform/default/src/mbgl/storage/database_file_source.cpp b/platform/default/src/mbgl/storage/database_file_source.cpp index 11901b2ca8f..e78c0918720 100644 --- a/platform/default/src/mbgl/storage/database_file_source.cpp +++ b/platform/default/src/mbgl/storage/database_file_source.cpp @@ -208,7 +208,7 @@ DatabaseFileSource::DatabaseFileSource(const ResourceOptions& resourceOptions, c DatabaseFileSource::~DatabaseFileSource() = default; std::unique_ptr DatabaseFileSource::request(const Resource& resource, - CopyableCallback&& callback) { + CopyableCallback callback) { auto req = std::make_unique(std::move(callback)); impl->actor().invoke(&DatabaseFileSourceThread::request, resource, req->actor()); return req; diff --git a/platform/default/src/mbgl/storage/file_source_request.cpp b/platform/default/src/mbgl/storage/file_source_request.cpp index 279d9fb5448..e0a740bb6ee 100644 --- a/platform/default/src/mbgl/storage/file_source_request.cpp +++ b/platform/default/src/mbgl/storage/file_source_request.cpp @@ -5,7 +5,7 @@ namespace mbgl { -FileSourceRequest::FileSourceRequest(FileSource::CopyableCallback&& callback) +FileSourceRequest::FileSourceRequest(FileSource::CopyableCallback callback) : responseCallback(std::move(callback)), mailbox(std::make_shared(*Scheduler::GetCurrent())) {} diff --git a/platform/default/src/mbgl/storage/http_file_source.cpp b/platform/default/src/mbgl/storage/http_file_source.cpp index 0b0d191c2d5..b65a2b3e6aa 100644 --- a/platform/default/src/mbgl/storage/http_file_source.cpp +++ b/platform/default/src/mbgl/storage/http_file_source.cpp @@ -79,7 +79,7 @@ class HTTPFileSource::Impl { class HTTPRequest : public AsyncRequest { public: - HTTPRequest(HTTPFileSource::Impl *, Resource, FileSource::Callback); + HTTPRequest(HTTPFileSource::Impl *, Resource, FileSource::CopyableCallback); ~HTTPRequest() override; void handleResult(CURLcode code); @@ -90,7 +90,7 @@ class HTTPRequest : public AsyncRequest { HTTPFileSource::Impl *context = nullptr; Resource resource; - FileSource::Callback callback; + FileSource::CopyableCallback callback; // Will store the current response. std::shared_ptr data; @@ -264,7 +264,7 @@ ClientOptions HTTPFileSource::Impl::getClientOptions() { return clientOptions.clone(); } -HTTPRequest::HTTPRequest(HTTPFileSource::Impl *context_, Resource resource_, FileSource::Callback callback_) +HTTPRequest::HTTPRequest(HTTPFileSource::Impl *context_, Resource resource_, FileSource::CopyableCallback callback_) : context(context_), resource(std::move(resource_)), callback(std::move(callback_)), @@ -452,8 +452,8 @@ HTTPFileSource::HTTPFileSource(const ResourceOptions &resourceOptions, const Cli HTTPFileSource::~HTTPFileSource() = default; -std::unique_ptr HTTPFileSource::request(const Resource &resource, Callback callback) { - return std::make_unique(impl.get(), resource, callback); +std::unique_ptr HTTPFileSource::request(const Resource &resource, CopyableCallback callback) { + return std::make_unique(impl.get(), resource, std::move(callback)); } void HTTPFileSource::setResourceOptions(ResourceOptions options) { diff --git a/platform/default/src/mbgl/storage/local_file_source.cpp b/platform/default/src/mbgl/storage/local_file_source.cpp index 61dd2b34558..74a5540623c 100644 --- a/platform/default/src/mbgl/storage/local_file_source.cpp +++ b/platform/default/src/mbgl/storage/local_file_source.cpp @@ -75,7 +75,7 @@ LocalFileSource::LocalFileSource(const ResourceOptions& resourceOptions, const C LocalFileSource::~LocalFileSource() = default; std::unique_ptr LocalFileSource::request(const Resource& resource, - CopyableCallback&& callback) { + CopyableCallback callback) { auto req = std::make_unique(std::move(callback)); impl->actor().invoke(&Impl::request, resource.url, req->actor()); diff --git a/platform/default/src/mbgl/storage/main_resource_loader.cpp b/platform/default/src/mbgl/storage/main_resource_loader.cpp index 5543744a060..42a745bdf61 100644 --- a/platform/default/src/mbgl/storage/main_resource_loader.cpp +++ b/platform/default/src/mbgl/storage/main_resource_loader.cpp @@ -160,7 +160,7 @@ class MainResourceLoader::Impl { resourceOptions(resourceOptions_.clone()), clientOptions(clientOptions_.clone()) {} - std::unique_ptr request(const Resource& resource, CopyableCallback&& callback) { + std::unique_ptr request(const Resource& resource, CopyableCallback callback) { auto req = std::make_unique(std::move(callback)); req->onCancel([actorRef = thread->actor(), req = req.get()]() { @@ -245,7 +245,7 @@ bool MainResourceLoader::supportsCacheOnlyRequests() const { } std::unique_ptr MainResourceLoader::request(const Resource& resource, - CopyableCallback&& callback) { + CopyableCallback callback) { return impl->request(resource, std::move(callback)); } diff --git a/platform/default/src/mbgl/storage/mbtiles_file_source.cpp b/platform/default/src/mbgl/storage/mbtiles_file_source.cpp index 7e69d460d92..89bce0eca69 100644 --- a/platform/default/src/mbgl/storage/mbtiles_file_source.cpp +++ b/platform/default/src/mbgl/storage/mbtiles_file_source.cpp @@ -276,7 +276,7 @@ MBTilesFileSource::MBTilesFileSource(const ResourceOptions &resourceOptions, con clientOptions.clone())) {} std::unique_ptr MBTilesFileSource::request(const Resource &resource, - FileSource::CopyableCallback &&callback) { + FileSource::CopyableCallback callback) { auto req = std::make_unique(std::move(callback)); // assume if there is a tile request, that the mbtiles file has been validated diff --git a/platform/default/src/mbgl/storage/online_file_source.cpp b/platform/default/src/mbgl/storage/online_file_source.cpp index 5849e443d08..5b258f1ed6b 100644 --- a/platform/default/src/mbgl/storage/online_file_source.cpp +++ b/platform/default/src/mbgl/storage/online_file_source.cpp @@ -320,7 +320,7 @@ class OnlineFileSource::Impl { resourceOptions.clone(), clientOptions.clone())) {} - std::unique_ptr request(CopyableCallback&& callback, Resource res) { + std::unique_ptr request(CopyableCallback callback, Resource res) { auto req = std::make_unique(std::move(callback)); req->onCancel( [actorRef = thread->actor(), req = req.get()]() { actorRef.invoke(&OnlineFileSourceThread::cancel, req); }); @@ -617,7 +617,7 @@ OnlineFileSource::OnlineFileSource(const ResourceOptions& resourceOptions, const OnlineFileSource::~OnlineFileSource() = default; std::unique_ptr OnlineFileSource::request(const Resource& resource, - CopyableCallback&& callback) { + CopyableCallback callback) { Resource res = resource; const TileServerOptions options = impl->getResourceOptions().tileServerOptions(); diff --git a/platform/default/src/mbgl/util/run_loop.cpp b/platform/default/src/mbgl/util/run_loop.cpp index 0076299c655..c444737260c 100644 --- a/platform/default/src/mbgl/util/run_loop.cpp +++ b/platform/default/src/mbgl/util/run_loop.cpp @@ -48,8 +48,8 @@ struct Watch { uv_poll_t poll; int fd; - std::function eventCallback; - std::function closeCallback; + std23::move_only_function eventCallback; + std23::move_only_function closeCallback; }; RunLoop* RunLoop::Get() { @@ -171,7 +171,7 @@ void RunLoop::waitForEmpty([[maybe_unused]] const mbgl::util::SimpleIdentity tag } } -void RunLoop::addWatch(int fd, Event event, std::function&& callback) { +void RunLoop::addWatch(int fd, Event event, std23::move_only_function&& callback) { MBGL_VERIFY_THREAD(tid); Watch* watch = nullptr; diff --git a/render-test/file_source.cpp b/render-test/file_source.cpp index 5b684560aec..71e0324a633 100644 --- a/render-test/file_source.cpp +++ b/render-test/file_source.cpp @@ -33,7 +33,7 @@ ProxyFileSource::ProxyFileSource(std::shared_ptr defaultResourceLoad ProxyFileSource::~ProxyFileSource() = default; std::unique_ptr ProxyFileSource::request(const Resource& resource, - CopyableCallback&& callback) { + CopyableCallback callback) { auto transformed = resource; // If offline, force always loading the resource from the cache diff --git a/render-test/file_source.hpp b/render-test/file_source.hpp index 703711f2305..5bb670e36f1 100644 --- a/render-test/file_source.hpp +++ b/render-test/file_source.hpp @@ -12,7 +12,7 @@ class ProxyFileSource : public FileSource { ProxyFileSource(std::shared_ptr, const ResourceOptions&, const ClientOptions&); ~ProxyFileSource(); - std::unique_ptr request(const Resource&, CopyableCallback&&) override; + std::unique_ptr request(const Resource&, CopyableCallback) override; bool canRequest(const Resource&) const override { return true; } /** diff --git a/src/mbgl/storage/asset_file_source.hpp b/src/mbgl/storage/asset_file_source.hpp index e9d9495fe3b..4fb1ef9e64b 100644 --- a/src/mbgl/storage/asset_file_source.hpp +++ b/src/mbgl/storage/asset_file_source.hpp @@ -16,7 +16,7 @@ class AssetFileSource : public FileSource { AssetFileSource(const ResourceOptions& resourceOptions, const ClientOptions& clientOptions); ~AssetFileSource() override; - std::unique_ptr request(const Resource&, CopyableCallback&&) override; + std::unique_ptr request(const Resource&, CopyableCallback) override; bool canRequest(const Resource&) const override; void pause() override; void resume() override; diff --git a/src/mbgl/storage/http_file_source.hpp b/src/mbgl/storage/http_file_source.hpp index ec38b70f9ce..6e7d601305e 100644 --- a/src/mbgl/storage/http_file_source.hpp +++ b/src/mbgl/storage/http_file_source.hpp @@ -13,7 +13,7 @@ class HTTPFileSource : public FileSource { HTTPFileSource(const ResourceOptions& resourceOptions, const ClientOptions& clientOptions); ~HTTPFileSource() override; - std::unique_ptr request(const Resource&, CopyableCallback&&) override; + std::unique_ptr request(const Resource&, CopyableCallback) override; bool canRequest(const Resource& resource) const override { return resource.hasLoadingMethod(Resource::LoadingMethod::Network); } diff --git a/src/mbgl/storage/local_file_source.hpp b/src/mbgl/storage/local_file_source.hpp index 2f46e9a42dd..03f379aa1e2 100644 --- a/src/mbgl/storage/local_file_source.hpp +++ b/src/mbgl/storage/local_file_source.hpp @@ -16,7 +16,7 @@ class LocalFileSource : public FileSource { LocalFileSource(const ResourceOptions& resourceOptions, const ClientOptions& clientOptions); ~LocalFileSource() override; - std::unique_ptr request(const Resource&, CopyableCallback&&) override; + std::unique_ptr request(const Resource&, CopyableCallback) override; bool canRequest(const Resource&) const override; void pause() override; void resume() override; diff --git a/src/mbgl/storage/main_resource_loader.hpp b/src/mbgl/storage/main_resource_loader.hpp index b91d51a37c4..22beef1d0ea 100644 --- a/src/mbgl/storage/main_resource_loader.hpp +++ b/src/mbgl/storage/main_resource_loader.hpp @@ -14,7 +14,7 @@ class MainResourceLoader final : public FileSource { ~MainResourceLoader() override; bool supportsCacheOnlyRequests() const override; - std::unique_ptr request(const Resource&, CopyableCallback&&) override; + std::unique_ptr request(const Resource&, CopyableCallback) override; bool canRequest(const Resource&) const override; void pause() override; void resume() override; diff --git a/src/mbgl/storage/mbtiles_file_source.hpp b/src/mbgl/storage/mbtiles_file_source.hpp index 788295e8e6b..792b8474f01 100644 --- a/src/mbgl/storage/mbtiles_file_source.hpp +++ b/src/mbgl/storage/mbtiles_file_source.hpp @@ -13,7 +13,7 @@ class MBTilesFileSource : public FileSource { MBTilesFileSource(const ResourceOptions& resourceOptions, const ClientOptions& clientOptions); ~MBTilesFileSource() override; - std::unique_ptr request(const Resource&, CopyableCallback&&) override; + std::unique_ptr request(const Resource&, CopyableCallback) override; bool canRequest(const Resource&) const override; void setResourceOptions(ResourceOptions) override; diff --git a/test/actor/actor.test.cpp b/test/actor/actor.test.cpp index 406f788d70a..e94f23ab9e8 100644 --- a/test/actor/actor.test.cpp +++ b/test/actor/actor.test.cpp @@ -94,14 +94,14 @@ TEST(Actor, DestructionBlocksOnSend) { void waitForEmpty(const util::SimpleIdentity) override { assert(false); } - void schedule(std::function&&) final { + void schedule(Task&&) final { promise.set_value(); future.wait(); std::this_thread::sleep_for(1ms); waited = true; } - void schedule(const util::SimpleIdentity, std::function&& fn) override final { + void schedule(const util::SimpleIdentity, Task&& fn) override final { schedule(std::move(fn)); } diff --git a/test/renderer/image_manager.test.cpp b/test/renderer/image_manager.test.cpp index 282b4982259..8f92f472087 100644 --- a/test/renderer/image_manager.test.cpp +++ b/test/renderer/image_manager.test.cpp @@ -149,7 +149,7 @@ class StubImageManagerObserver : public ImageManagerObserver { int count = 0; std::function imageMissing = [](const std::string&) { }; - void onStyleImageMissing(const std::string& id, const std::function& done) override { + void onStyleImageMissing(const std::string& id, Scheduler::Task&& done) override { count++; imageMissing(id); done(); diff --git a/test/src/mbgl/test/fake_file_source.hpp b/test/src/mbgl/test/fake_file_source.hpp index b68ba036f88..dc802419ac9 100644 --- a/test/src/mbgl/test/fake_file_source.hpp +++ b/test/src/mbgl/test/fake_file_source.hpp @@ -33,7 +33,7 @@ class FakeFileSource : public FileSource { std::list::iterator link; FakeFileRequest(Resource resource_, - CopyableCallback&& callback_, + CopyableCallback callback_, std::list& list_) : resource(std::move(resource_)), callback(std::move(callback_)), @@ -50,7 +50,7 @@ class FakeFileSource : public FileSource { : FakeFileSource(ResourceOptions::Default(), ClientOptions()) {} std::unique_ptr request(const Resource& resource, - CopyableCallback&& callback) override { + CopyableCallback callback) override { return std::make_unique(resource, std::move(callback), requests); } @@ -93,7 +93,7 @@ class FakeOnlineFileSource : public FakeFileSource { : FakeFileSource(resourceOptions_, clientOptions_) {} std::unique_ptr request(const Resource& resource, - CopyableCallback&& callback) override { + CopyableCallback callback) override { return FakeFileSource::request(resource, std::move(callback)); } diff --git a/test/src/mbgl/test/stub_file_source.cpp b/test/src/mbgl/test/stub_file_source.cpp index 598b20b666a..89b7f7de357 100644 --- a/test/src/mbgl/test/stub_file_source.cpp +++ b/test/src/mbgl/test/stub_file_source.cpp @@ -60,7 +60,7 @@ StubFileSource::StubFileSource(const ResourceOptions& resourceOptions_, StubFileSource::~StubFileSource() = default; -std::unique_ptr StubFileSource::request(const Resource& resource, Callback callback) { +std::unique_ptr StubFileSource::request(const Resource& resource, CopyableCallback callback) { auto req = std::make_unique(*this); if (type == ResponseType::Synchronous) { std::optional res = response(resource); @@ -68,7 +68,7 @@ std::unique_ptr StubFileSource::request(const Resource& resource, callback(*res); } } else { - pending.emplace(req.get(), std::make_tuple(resource, response, callback)); + pending.emplace(req.get(), std::make_tuple(resource, response, std::move(callback))); } return req; } diff --git a/test/src/mbgl/test/stub_file_source.hpp b/test/src/mbgl/test/stub_file_source.hpp index 85e1189daf2..8690861b3cc 100644 --- a/test/src/mbgl/test/stub_file_source.hpp +++ b/test/src/mbgl/test/stub_file_source.hpp @@ -23,7 +23,7 @@ class StubFileSource : public FileSource { StubFileSource(ResponseType = ResponseType::Asynchronous); ~StubFileSource() override; - std::unique_ptr request(const Resource&, CopyableCallback&&) override; + std::unique_ptr request(const Resource&, CopyableCallback) override; bool canRequest(const Resource&) const noexcept override { return true; } void remove(AsyncRequest*); void setProperty(const std::string&, const mapbox::base::Value&) override; @@ -57,7 +57,7 @@ class StubFileSource : public FileSource { // The default behavior is to throw if no per-kind callback has been set. std::optional defaultResponse(const Resource&); - std::unordered_map> pending; + std::unordered_map>> pending; ResponseType type; util::Timer timer; std::map properties; diff --git a/test/storage/asset_file_source.test.cpp b/test/storage/asset_file_source.test.cpp index efe9d36facd..cbb9c864279 100644 --- a/test/storage/asset_file_source.test.cpp +++ b/test/storage/asset_file_source.test.cpp @@ -57,7 +57,7 @@ TEST(AssetFileSource, Load) { mbgl::AssetFileSource* fs; std::unique_ptr request; - std::function requestCallback; + mbgl::FileSource::CopyableCallback requestCallback; }; std::vector>> threads; diff --git a/test/storage/sync_file_source.test.cpp b/test/storage/sync_file_source.test.cpp index 95ee2f1f050..ba409213866 100644 --- a/test/storage/sync_file_source.test.cpp +++ b/test/storage/sync_file_source.test.cpp @@ -19,7 +19,7 @@ using namespace mbgl; class SyncFileSource : public FileSource { public: std::unique_ptr request(const Resource& resource, - CopyableCallback&& callback) override { + CopyableCallback callback) override { Response response; auto it = assets.find(resource.url); if (it == assets.end()) {