From d4435d9fef0087a6b250e66646f6639929ee6f96 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Fri, 17 Apr 2020 14:27:46 +0300 Subject: [PATCH 1/9] [core] Introduce `Resource::minimumUpdateInterval` Introduce `Resource::minimumUpdateInterval` and consider it in the online file source. The `minimumUpdateInterval` is used to throttle the requests, which were initiated due to resource expiration. --- include/mbgl/storage/resource.hpp | 1 + .../src/mbgl/storage/online_file_source.cpp | 55 ++++++++++--------- src/mbgl/util/http_timeout.cpp | 6 +- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/include/mbgl/storage/resource.hpp b/include/mbgl/storage/resource.hpp index b21265ef54c..b7b394efc27 100644 --- a/include/mbgl/storage/resource.hpp +++ b/include/mbgl/storage/resource.hpp @@ -95,6 +95,7 @@ class Resource { optional priorExpires = {}; optional priorEtag = {}; std::shared_ptr priorData; + Duration minimumUpdateInterval{Duration::zero()}; }; inline bool Resource::hasLoadingMethod(Resource::LoadingMethod method) const { diff --git a/platform/default/src/mbgl/storage/online_file_source.cpp b/platform/default/src/mbgl/storage/online_file_source.cpp index a5274c6b38b..ca51234b56c 100644 --- a/platform/default/src/mbgl/storage/online_file_source.cpp +++ b/platform/default/src/mbgl/storage/online_file_source.cpp @@ -40,14 +40,15 @@ struct OnlineFileRequest { ~OnlineFileRequest(); void networkIsReachableAgain(); - void schedule(); - void schedule(optional expires); + void activate(); + void schedule(Duration timeout); void completed(Response); void setTransformedURL(const std::string& url); ActorRef actor(); void onCancel(std::function); + Duration getUpdateInterval(optional expires) const; OnlineFileSourceThread& impl; Resource resource; std::unique_ptr request; @@ -100,7 +101,7 @@ class OnlineFileSourceThread { ref.invoke(&OnlineFileRequest::setTransformedURL, url); }); } else { - req->schedule(); + req->activate(); } } @@ -176,11 +177,11 @@ class OnlineFileSourceThread { maximumConcurrentRequests = maximumConcurrentRequests_; } - void setAPIBaseURL(const std::string& t) { apiBaseURL = t; } - std::string getAPIBaseURL() const { return apiBaseURL; } + void setAPIBaseURL(std::string t) { apiBaseURL = std::move(t); } + const std::string& getAPIBaseURL() const { return apiBaseURL; } - void setAccessToken(const std::string& t) { accessToken = t; } - std::string getAccessToken() const { return accessToken; } + void setAccessToken(std::string t) { accessToken = std::move(t); } + const std::string& getAccessToken() const { return accessToken; } private: friend struct OnlineFileRequest; @@ -383,15 +384,6 @@ OnlineFileRequest::OnlineFileRequest(Resource resource_, Callback callback_, Onl impl.add(this); } -void OnlineFileRequest::schedule() { - // Force an immediate first request if we don't have an expiration time. - if (resource.priorExpires) { - schedule(resource.priorExpires); - } else { - schedule(util::now()); - } -} - OnlineFileRequest::~OnlineFileRequest() { if (mailbox) { mailbox->close(); @@ -431,17 +423,21 @@ Timestamp interpolateExpiration(const Timestamp& current, optional pr return now + std::max(delta, util::CLOCK_SKEW_RETRY_TIMEOUT); } -void OnlineFileRequest::schedule(optional expires) { +void OnlineFileRequest::activate() { + // Force an immediate first request if we don't have an expiration time. + Duration timeout = Duration::zero(); + if (resource.priorExpires) { + timeout = getUpdateInterval(resource.priorExpires); + } + schedule(timeout); +} + +void OnlineFileRequest::schedule(Duration timeout) { if (impl.isPending(this) || impl.isActive(this)) { // There's already a request in progress; don't start another one. return; } - // If we're not being asked for a forced refresh, calculate a timeout that depends on how many - // consecutive errors we've encountered, and on the expiration time, if present. - Duration timeout = std::min(http::errorRetryTimeout(failedRequestReason, failedRequests, retryAfter), - http::expirationTimeout(std::move(expires), expiredRequests)); - if (timeout == Duration::max()) { return; } @@ -460,6 +456,15 @@ void OnlineFileRequest::schedule(optional expires) { }); } +Duration OnlineFileRequest::getUpdateInterval(optional expires) const { + // Calculate a timeout that depends on how many + // consecutive errors we've encountered, and on the expiration time, if present. + Duration errorRetryTimeout = http::errorRetryTimeout(failedRequestReason, failedRequests, retryAfter); + Duration expirationTimeout = + std::max(http::expirationTimeout(std::move(expires), expiredRequests), resource.minimumUpdateInterval); + return std::min(errorRetryTimeout, expirationTimeout); +} + namespace { inline std::string sanitizeURL(std::string& url) { @@ -519,7 +524,7 @@ void OnlineFileRequest::completed(Response response) { failedRequestReason = Response::Error::Reason::Success; } - schedule(response.expires); + schedule(getUpdateInterval(response.expires)); // Calling the callback may result in `this` being deleted. It needs to be done last, // and needs to make a local copy of the callback to ensure that it remains valid for @@ -532,13 +537,13 @@ void OnlineFileRequest::networkIsReachableAgain() { // We need all requests to fail at least once before we are going to start retrying // them, and we only immediately restart request that failed due to connection issues. if (failedRequestReason == Response::Error::Reason::Connection) { - schedule(util::now()); + schedule(Duration::zero()); } } void OnlineFileRequest::setTransformedURL(const std::string& url) { resource.url = url; - schedule(); + activate(); } ActorRef OnlineFileRequest::actor() { diff --git a/src/mbgl/util/http_timeout.cpp b/src/mbgl/util/http_timeout.cpp index 04842e48bee..cf4085d5dc8 100644 --- a/src/mbgl/util/http_timeout.cpp +++ b/src/mbgl/util/http_timeout.cpp @@ -32,11 +32,11 @@ Duration errorRetryTimeout(Response::Error::Reason failedRequestReason, uint32_t Duration expirationTimeout(optional expires, uint32_t expiredRequests) { if (expiredRequests) { return Seconds(1u << std::min(expiredRequests - 1, 31u)); - } else if (expires) { + } + if (expires) { return std::max(Seconds::zero(), *expires - util::now()); - } else { - return Duration::max(); } + return Duration::max(); } } // namespace http From 4b4f3a4bfd9ad819124311b6e89b4e7d467940d6 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Fri, 17 Apr 2020 17:44:45 +0300 Subject: [PATCH 2/9] Add OnlineFileSource.RespectMinimumUpdateInterval unit test --- test/storage/online_file_source.test.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/storage/online_file_source.test.cpp b/test/storage/online_file_source.test.cpp index 1bed5f9618d..050b9d0f3f0 100644 --- a/test/storage/online_file_source.test.cpp +++ b/test/storage/online_file_source.test.cpp @@ -217,6 +217,25 @@ TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RespectPriorExpires)) { loop.run(); } +TEST(OnlineFileSource, TEST_REQUIRES_SERVER(RespectMinimumUpdateInterval)) { + util::RunLoop loop; + std::unique_ptr fs = std::make_unique(); + + auto start = util::now(); + Resource resource{Resource::Unknown, "http://127.0.0.1:3000/test"}; + resource.priorExpires = start + std::chrono::duration_cast(Milliseconds(100)); + resource.minimumUpdateInterval = Seconds(1); + + std::unique_ptr req = fs->request(resource, [&](Response) { + auto wait = util::now() - start; + EXPECT_GE(wait, resource.minimumUpdateInterval); + EXPECT_LT(wait, resource.minimumUpdateInterval + Milliseconds(10)); + loop.stop(); + }); + + loop.run(); +} + TEST(OnlineFileSource, TEST_REQUIRES_SERVER(Load)) { util::RunLoop loop; std::unique_ptr fs = std::make_unique(); From 3134f953ba10cf574cc2827cdf7c2b8649b4d3a0 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Fri, 17 Apr 2020 17:47:28 +0300 Subject: [PATCH 3/9] [core] Introduce Tile::setMinimumUpdateInterval() --- src/mbgl/tile/raster_dem_tile.cpp | 4 ++++ src/mbgl/tile/raster_dem_tile.hpp | 3 ++- src/mbgl/tile/raster_tile.cpp | 4 ++++ src/mbgl/tile/raster_tile.hpp | 3 ++- src/mbgl/tile/tile.hpp | 1 + src/mbgl/tile/tile_loader.hpp | 17 +++++++---------- src/mbgl/tile/tile_loader_impl.hpp | 27 ++++++++++++++++++++++++++- src/mbgl/tile/vector_tile.cpp | 4 ++++ src/mbgl/tile/vector_tile.hpp | 1 + 9 files changed, 51 insertions(+), 13 deletions(-) diff --git a/src/mbgl/tile/raster_dem_tile.cpp b/src/mbgl/tile/raster_dem_tile.cpp index b70e581ea19..83b41ddd835 100644 --- a/src/mbgl/tile/raster_dem_tile.cpp +++ b/src/mbgl/tile/raster_dem_tile.cpp @@ -122,4 +122,8 @@ void RasterDEMTile::setNecessity(TileNecessity necessity) { loader.setNecessity(necessity); } +void RasterDEMTile::setMinimumUpdateInterval(Duration interval) { + loader.setMinimumUpdateInterval(interval); +} + } // namespace mbgl diff --git a/src/mbgl/tile/raster_dem_tile.hpp b/src/mbgl/tile/raster_dem_tile.hpp index 68526fdb34a..d29c5337bef 100644 --- a/src/mbgl/tile/raster_dem_tile.hpp +++ b/src/mbgl/tile/raster_dem_tile.hpp @@ -67,7 +67,8 @@ class RasterDEMTile final : public Tile { ~RasterDEMTile() override; std::unique_ptr createRenderData() override; - void setNecessity(TileNecessity) final; + void setNecessity(TileNecessity) override; + void setMinimumUpdateInterval(Duration) override; void setError(std::exception_ptr); void setMetadata(optional modified, optional expires); diff --git a/src/mbgl/tile/raster_tile.cpp b/src/mbgl/tile/raster_tile.cpp index 66e32b7ca09..1045097a050 100644 --- a/src/mbgl/tile/raster_tile.cpp +++ b/src/mbgl/tile/raster_tile.cpp @@ -78,4 +78,8 @@ void RasterTile::setNecessity(TileNecessity necessity) { loader.setNecessity(necessity); } +void RasterTile::setMinimumUpdateInterval(Duration interval) { + loader.setMinimumUpdateInterval(interval); +} + } // namespace mbgl diff --git a/src/mbgl/tile/raster_tile.hpp b/src/mbgl/tile/raster_tile.hpp index 3ff05d2f9d4..78dbd95c0ac 100644 --- a/src/mbgl/tile/raster_tile.hpp +++ b/src/mbgl/tile/raster_tile.hpp @@ -23,7 +23,8 @@ class RasterTile final : public Tile { ~RasterTile() override; std::unique_ptr createRenderData() override; - void setNecessity(TileNecessity) final; + void setNecessity(TileNecessity) override; + void setMinimumUpdateInterval(Duration) override; void setError(std::exception_ptr); void setMetadata(optional modified, optional expires); diff --git a/src/mbgl/tile/tile.hpp b/src/mbgl/tile/tile.hpp index b73383f989f..bf84f8bd98c 100644 --- a/src/mbgl/tile/tile.hpp +++ b/src/mbgl/tile/tile.hpp @@ -52,6 +52,7 @@ class Tile { void setObserver(TileObserver* observer); virtual void setNecessity(TileNecessity) {} + virtual void setMinimumUpdateInterval(Duration) {} // Mark this tile as no longer needed and cancel any pending work. virtual void cancel(); diff --git a/src/mbgl/tile/tile_loader.hpp b/src/mbgl/tile/tile_loader.hpp index 65f4ceb118c..c6d0d61fef3 100644 --- a/src/mbgl/tile/tile_loader.hpp +++ b/src/mbgl/tile/tile_loader.hpp @@ -22,16 +22,8 @@ class TileLoader { const Tileset&); ~TileLoader(); - void setNecessity(TileNecessity newNecessity) { - if (newNecessity != necessity) { - necessity = newNecessity; - if (necessity == TileNecessity::Required) { - makeRequired(); - } else { - makeOptional(); - } - } - } + void setNecessity(TileNecessity newNecessity); + void setMinimumUpdateInterval(Duration); private: // called when the tile is one of the ideal tiles that we want to show definitely. the tile source @@ -48,11 +40,16 @@ class TileLoader { void loadedData(const Response&); void loadFromNetwork(); + bool hasPendingNetworkRequest() const { + return resource.loadingMethod == Resource::LoadingMethod::NetworkOnly && request; + } + T& tile; TileNecessity necessity; Resource resource; std::shared_ptr fileSource; std::unique_ptr request; + Duration minimumUpdateInterval{Duration::zero()}; }; } // namespace mbgl diff --git a/src/mbgl/tile/tile_loader_impl.hpp b/src/mbgl/tile/tile_loader_impl.hpp index b518ac6782d..78b9fe6df33 100644 --- a/src/mbgl/tile/tile_loader_impl.hpp +++ b/src/mbgl/tile/tile_loader_impl.hpp @@ -56,6 +56,30 @@ TileLoader::TileLoader(T& tile_, template TileLoader::~TileLoader() = default; +template +void TileLoader::setNecessity(TileNecessity newNecessity) { + if (newNecessity != necessity) { + necessity = newNecessity; + if (necessity == TileNecessity::Required) { + makeRequired(); + } else { + makeOptional(); + } + } +} + +template +void TileLoader::setMinimumUpdateInterval(Duration interval) { + if (minimumUpdateInterval != interval) { + minimumUpdateInterval = interval; + if (hasPendingNetworkRequest()) { + // Update the pending request. + request.reset(); + loadFromNetwork(); + } + } +} + template void TileLoader::loadFromCache() { assert(!request); @@ -99,7 +123,7 @@ void TileLoader::makeRequired() { template void TileLoader::makeOptional() { - if (resource.loadingMethod == Resource::LoadingMethod::NetworkOnly && request) { + if (hasPendingNetworkRequest()) { // Abort the current request, but only when we know that we're specifically querying for a // network resource only. request.reset(); @@ -135,6 +159,7 @@ void TileLoader::loadFromNetwork() { // Instead of using Resource::LoadingMethod::All, we're first doing a CacheOnly, and then a // NetworkOnly request. resource.loadingMethod = Resource::LoadingMethod::NetworkOnly; + resource.minimumUpdateInterval = minimumUpdateInterval; request = fileSource->request(resource, [this](const Response& res) { loadedData(res); }); } diff --git a/src/mbgl/tile/vector_tile.cpp b/src/mbgl/tile/vector_tile.cpp index e3e7a2dff82..2e2dabc66e7 100644 --- a/src/mbgl/tile/vector_tile.cpp +++ b/src/mbgl/tile/vector_tile.cpp @@ -16,6 +16,10 @@ void VectorTile::setNecessity(TileNecessity necessity) { loader.setNecessity(necessity); } +void VectorTile::setMinimumUpdateInterval(Duration interval) { + loader.setMinimumUpdateInterval(interval); +} + void VectorTile::setMetadata(optional modified_, optional expires_) { modified = std::move(modified_); expires = std::move(expires_); diff --git a/src/mbgl/tile/vector_tile.hpp b/src/mbgl/tile/vector_tile.hpp index 205fd642f0f..fdefb14e20c 100644 --- a/src/mbgl/tile/vector_tile.hpp +++ b/src/mbgl/tile/vector_tile.hpp @@ -16,6 +16,7 @@ class VectorTile : public GeometryTile { const Tileset&); void setNecessity(TileNecessity) final; + void setMinimumUpdateInterval(Duration interval) final; void setMetadata(optional modified, optional expires); void setData(const std::shared_ptr& data); From 802f1cd27a8474138bb4594f29ac3dabc4642229 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Mon, 20 Apr 2020 12:19:35 +0300 Subject: [PATCH 4/9] [core] Introduce Source::setMinimumTileUpdateInterval API The `Source::setMinimumTileUpdateInterval()` method sets the minimum tile update interval, which is used to throttle the tile update network requests. Default value is `Duration::zero()`. --- include/mbgl/style/source.hpp | 19 +++++++++++++++---- src/mbgl/style/custom_tile_loader.hpp | 6 ++++-- src/mbgl/style/source.cpp | 12 ++++++++++++ src/mbgl/style/source_impl.hpp | 6 +++++- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/include/mbgl/style/source.hpp b/include/mbgl/style/source.hpp index 32b15bdf47f..a1650ad06a0 100644 --- a/include/mbgl/style/source.hpp +++ b/include/mbgl/style/source.hpp @@ -1,9 +1,9 @@ #pragma once -#include -#include -#include #include +#include +#include +#include #include #include @@ -40,8 +40,11 @@ struct LayerTypeInfo; * * auto vectorSource = std::make_unique("my-vector-source"); */ -class Source : public mbgl::util::noncopyable { +class Source { public: + Source(const Source&) = delete; + Source& operator=(const Source&) = delete; + virtual ~Source(); // Check whether this source is of the given subtype. @@ -76,6 +79,14 @@ class Source : public mbgl::util::noncopyable { void setPrefetchZoomDelta(optional delta) noexcept; optional getPrefetchZoomDelta() const noexcept; + // If the given source supports loading tiles from a server, + // sets the minimum tile update interval, which is used to + // throttle the tile update network requests. + // + // Default value is `Duration::zero()`. + void setMinimumTileUpdateInterval(Duration) noexcept; + Duration getMinimumTileUpdateInterval() const noexcept; + // Sets a limit for how much a parent tile can be overscaled. // // When a set of tiles for a current zoom level is being rendered and some of the diff --git a/src/mbgl/style/custom_tile_loader.hpp b/src/mbgl/style/custom_tile_loader.hpp index b27a8a15217..78317fdcb1f 100644 --- a/src/mbgl/style/custom_tile_loader.hpp +++ b/src/mbgl/style/custom_tile_loader.hpp @@ -1,9 +1,9 @@ #pragma once +#include #include #include #include -#include #include #include @@ -14,8 +14,10 @@ class CustomGeometryTile; namespace style { -class CustomTileLoader : private util::noncopyable { +class CustomTileLoader { public: + CustomTileLoader(const CustomTileLoader&) = delete; + CustomTileLoader& operator=(const CustomTileLoader&) = delete; using OverscaledIDFunctionTuple = std::tuple>; diff --git a/src/mbgl/style/source.cpp b/src/mbgl/style/source.cpp index 51a3c800894..42ad367d66d 100644 --- a/src/mbgl/style/source.cpp +++ b/src/mbgl/style/source.cpp @@ -43,6 +43,18 @@ optional Source::getPrefetchZoomDelta() const noexcept { return baseImpl->getPrefetchZoomDelta(); } +void Source::setMinimumTileUpdateInterval(Duration interval) noexcept { + if (getMinimumTileUpdateInterval() == interval) return; + auto newImpl = createMutable(); + newImpl->setMinimumTileUpdateInterval(interval); + baseImpl = std::move(newImpl); + observer->onSourceChanged(*this); +} + +Duration Source::getMinimumTileUpdateInterval() const noexcept { + return baseImpl->getMinimumTileUpdateInterval(); +} + void Source::setMaxOverscaleFactorForParentTiles(optional overscaleFactor) noexcept { if (getMaxOverscaleFactorForParentTiles() == overscaleFactor) return; auto newImpl = createMutable(); diff --git a/src/mbgl/style/source_impl.hpp b/src/mbgl/style/source_impl.hpp index af8435537ae..b16b763766f 100644 --- a/src/mbgl/style/source_impl.hpp +++ b/src/mbgl/style/source_impl.hpp @@ -22,15 +22,19 @@ class Source::Impl { virtual optional getAttribution() const = 0; void setPrefetchZoomDelta(optional delta) noexcept; optional getPrefetchZoomDelta() const noexcept; + void setMinimumTileUpdateInterval(Duration interval) { minimumTileUpdateInterval = interval; } + Duration getMinimumTileUpdateInterval() const { return minimumTileUpdateInterval; } void setMaxOverscaleFactorForParentTiles(optional overscaleFactor) noexcept; optional getMaxOverscaleFactorForParentTiles() const noexcept; const SourceType type; const std::string id; + +protected: optional prefetchZoomDelta; optional maxOverscaleFactor; + Duration minimumTileUpdateInterval{Duration::zero()}; -protected: Impl(SourceType, std::string); Impl(const Impl&) = default; }; From c9339751c79d4ff79d38fdb28b64df95877d9f5f Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Mon, 20 Apr 2020 13:15:12 +0300 Subject: [PATCH 5/9] [core] TilePyramid::update accepts source impl --- .../annotation/render_annotation_source.cpp | 6 ++--- .../sources/render_custom_geometry_source.cpp | 27 +++++++++---------- .../sources/render_geojson_source.cpp | 25 ++++++++--------- .../sources/render_raster_dem_source.cpp | 6 ++--- .../renderer/sources/render_raster_source.cpp | 6 ++--- .../renderer/sources/render_vector_source.cpp | 25 ++++++++--------- src/mbgl/renderer/tile_pyramid.cpp | 10 ++++--- src/mbgl/renderer/tile_pyramid.hpp | 15 +++++------ test/style/source.test.cpp | 21 +++++++-------- 9 files changed, 62 insertions(+), 79 deletions(-) diff --git a/src/mbgl/annotation/render_annotation_source.cpp b/src/mbgl/annotation/render_annotation_source.cpp index b03b7662d8a..efa1d1987c8 100644 --- a/src/mbgl/annotation/render_annotation_source.cpp +++ b/src/mbgl/annotation/render_annotation_source.cpp @@ -33,15 +33,13 @@ void RenderAnnotationSource::update(Immutable baseImpl_, needsRendering, needsRelayout, parameters, - SourceType::Annotations, + *baseImpl, util::tileSize, // Zoom level 16 is typically sufficient for annotations. // See https://github.com/mapbox/mapbox-gl-native/issues/10197 {0, 16}, optional{}, - [&](const OverscaledTileID& tileID) { return std::make_unique(tileID, parameters); }, - baseImpl->getPrefetchZoomDelta(), - baseImpl->getMaxOverscaleFactorForParentTiles()); + [&](const OverscaledTileID& tileID) { return std::make_unique(tileID, parameters); }); } std::unordered_map> diff --git a/src/mbgl/renderer/sources/render_custom_geometry_source.cpp b/src/mbgl/renderer/sources/render_custom_geometry_source.cpp index ecb74b65034..fafa81f27ae 100644 --- a/src/mbgl/renderer/sources/render_custom_geometry_source.cpp +++ b/src/mbgl/renderer/sources/render_custom_geometry_source.cpp @@ -40,21 +40,18 @@ void RenderCustomGeometrySource::update(Immutable baseImpl_ return; } - tilePyramid.update( - layers, - needsRendering, - needsRelayout, - parameters, - SourceType::CustomVector, - util::tileSize, - impl().getZoomRange(), - {}, - [&](const OverscaledTileID& tileID) { - return std::make_unique( - tileID, impl().id, parameters, impl().getTileOptions(), *tileLoader); - }, - baseImpl->getPrefetchZoomDelta(), - baseImpl->getMaxOverscaleFactorForParentTiles()); + tilePyramid.update(layers, + needsRendering, + needsRelayout, + parameters, + *baseImpl, + util::tileSize, + impl().getZoomRange(), + {}, + [&](const OverscaledTileID& tileID) { + return std::make_unique( + tileID, impl().id, parameters, impl().getTileOptions(), *tileLoader); + }); } } // namespace mbgl diff --git a/src/mbgl/renderer/sources/render_geojson_source.cpp b/src/mbgl/renderer/sources/render_geojson_source.cpp index 2cf5c9f3b9b..7f81ff903a6 100644 --- a/src/mbgl/renderer/sources/render_geojson_source.cpp +++ b/src/mbgl/renderer/sources/render_geojson_source.cpp @@ -106,20 +106,17 @@ void RenderGeoJSONSource::update(Immutable baseImpl_, if (!data_) return; - tilePyramid.update( - layers, - needsRendering, - needsRelayout, - parameters, - SourceType::GeoJSON, - util::tileSize, - impl().getZoomRange(), - optional{}, - [&, data_](const OverscaledTileID& tileID) { - return std::make_unique(tileID, impl().id, parameters, data_); - }, - baseImpl->getPrefetchZoomDelta(), - baseImpl->getMaxOverscaleFactorForParentTiles()); + tilePyramid.update(layers, + needsRendering, + needsRelayout, + parameters, + *baseImpl, + util::tileSize, + impl().getZoomRange(), + optional{}, + [&, data_](const OverscaledTileID& tileID) { + return std::make_unique(tileID, impl().id, parameters, data_); + }); } mapbox::util::variant diff --git a/src/mbgl/renderer/sources/render_raster_dem_source.cpp b/src/mbgl/renderer/sources/render_raster_dem_source.cpp index 9b2345f8e06..1ad6a6c2575 100644 --- a/src/mbgl/renderer/sources/render_raster_dem_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_dem_source.cpp @@ -32,13 +32,11 @@ void RenderRasterDEMSource::updateInternal(const Tileset& tileset, needsRendering, needsRelayout, parameters, - SourceType::RasterDEM, + *baseImpl, impl().getTileSize(), tileset.zoomRange, tileset.bounds, - [&](const OverscaledTileID& tileID) { return std::make_unique(tileID, parameters, tileset); }, - baseImpl->getPrefetchZoomDelta(), - baseImpl->getMaxOverscaleFactorForParentTiles()); + [&](const OverscaledTileID& tileID) { return std::make_unique(tileID, parameters, tileset); }); algorithm::updateTileMasks(tilePyramid.getRenderedTiles()); } diff --git a/src/mbgl/renderer/sources/render_raster_source.cpp b/src/mbgl/renderer/sources/render_raster_source.cpp index f759623cdd3..f1a4a612817 100644 --- a/src/mbgl/renderer/sources/render_raster_source.cpp +++ b/src/mbgl/renderer/sources/render_raster_source.cpp @@ -30,13 +30,11 @@ void RenderRasterSource::updateInternal(const Tileset& tileset, needsRendering, needsRelayout, parameters, - SourceType::Raster, + *baseImpl, impl().getTileSize(), tileset.zoomRange, tileset.bounds, - [&](const OverscaledTileID& tileID) { return std::make_unique(tileID, parameters, tileset); }, - baseImpl->getPrefetchZoomDelta(), - baseImpl->getMaxOverscaleFactorForParentTiles()); + [&](const OverscaledTileID& tileID) { return std::make_unique(tileID, parameters, tileset); }); algorithm::updateTileMasks(tilePyramid.getRenderedTiles()); } diff --git a/src/mbgl/renderer/sources/render_vector_source.cpp b/src/mbgl/renderer/sources/render_vector_source.cpp index 3ac16a136f8..b581d244d4c 100644 --- a/src/mbgl/renderer/sources/render_vector_source.cpp +++ b/src/mbgl/renderer/sources/render_vector_source.cpp @@ -21,20 +21,17 @@ void RenderVectorSource::updateInternal(const Tileset& tileset, const bool needsRendering, const bool needsRelayout, const TileParameters& parameters) { - tilePyramid.update( - layers, - needsRendering, - needsRelayout, - parameters, - SourceType::Vector, - util::tileSize, - tileset.zoomRange, - tileset.bounds, - [&](const OverscaledTileID& tileID) { - return std::make_unique(tileID, baseImpl->id, parameters, tileset); - }, - baseImpl->getPrefetchZoomDelta(), - baseImpl->getMaxOverscaleFactorForParentTiles()); + tilePyramid.update(layers, + needsRendering, + needsRelayout, + parameters, + *baseImpl, + util::tileSize, + tileset.zoomRange, + tileset.bounds, + [&](const OverscaledTileID& tileID) { + return std::make_unique(tileID, baseImpl->id, parameters, tileset); + }); } } // namespace mbgl diff --git a/src/mbgl/renderer/tile_pyramid.cpp b/src/mbgl/renderer/tile_pyramid.cpp index 57086cb9159..013e6a7f37b 100644 --- a/src/mbgl/renderer/tile_pyramid.cpp +++ b/src/mbgl/renderer/tile_pyramid.cpp @@ -53,13 +53,11 @@ void TilePyramid::update(const std::vector>& l const bool needsRendering, const bool needsRelayout, const TileParameters& parameters, - const SourceType type, + const style::Source::Impl& sourceImpl, const uint16_t tileSize, const Range zoomRange, optional bounds, - std::function(const OverscaledTileID&)> createTile, - const optional& sourcePrefetchZoomDelta, - const optional& maxParentTileOverscaleFactor) { + std::function(const OverscaledTileID&)> createTile) { // If we need a relayout, abandon any cached tiles; they're now stale. if (needsRelayout) { cache.clear(); @@ -86,11 +84,15 @@ void TilePyramid::update(const std::vector>& l handleWrapJump(parameters.transformState.getLatLng().longitude()); + const auto type = sourceImpl.type; // Determine the overzooming/underzooming amounts and required tiles. int32_t overscaledZoom = util::coveringZoomLevel(parameters.transformState.getZoom(), type, tileSize); int32_t tileZoom = overscaledZoom; int32_t panZoom = zoomRange.max; + const optional& sourcePrefetchZoomDelta = sourceImpl.getPrefetchZoomDelta(); + const optional& maxParentTileOverscaleFactor = sourceImpl.getMaxOverscaleFactorForParentTiles(); + std::vector idealTiles; std::vector panTiles; diff --git a/src/mbgl/renderer/tile_pyramid.hpp b/src/mbgl/renderer/tile_pyramid.hpp index 7dd2ab5341c..f1c310849a2 100644 --- a/src/mbgl/renderer/tile_pyramid.hpp +++ b/src/mbgl/renderer/tile_pyramid.hpp @@ -1,11 +1,12 @@ #pragma once -#include -#include +#include +#include +#include #include #include -#include -#include +#include +#include #include #include @@ -37,13 +38,11 @@ class TilePyramid { bool needsRendering, bool needsRelayout, const TileParameters&, - style::SourceType type, + const style::Source::Impl&, uint16_t tileSize, Range zoomRange, optional bounds, - std::function(const OverscaledTileID&)> createTile, - const optional& sourcePrefetchZoomDelta, - const optional& maxParentTileOverscaleFactor); + std::function(const OverscaledTileID&)> createTile); const std::map>& getRenderedTiles() const { return renderedTiles; } Tile* getTile(const OverscaledTileID&); diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp index dae2aef9caa..06afd6427fe 100644 --- a/test/style/source.test.cpp +++ b/test/style/source.test.cpp @@ -767,18 +767,15 @@ class FakeTileSource : public RenderTileSetSource { const bool needsRendering, const bool needsRelayout, const TileParameters& parameters) override { - tilePyramid.update( - layers, - needsRendering, - needsRelayout, - parameters, - SourceType::Vector, - util::tileSize, - tileset.zoomRange, - tileset.bounds, - [&](const OverscaledTileID& tileID) { return std::make_unique(*this, tileID); }, - baseImpl->getPrefetchZoomDelta(), - baseImpl->getMaxOverscaleFactorForParentTiles()); + tilePyramid.update(layers, + needsRendering, + needsRelayout, + parameters, + *baseImpl, + util::tileSize, + tileset.zoomRange, + tileset.bounds, + [&](const OverscaledTileID& tileID) { return std::make_unique(*this, tileID); }); } const optional& getTileset() const override { From fa547fc6c390528e5ca69737184c79f6363344be Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Mon, 20 Apr 2020 14:29:58 +0300 Subject: [PATCH 6/9] [core] Tile pyramid passes minimum update interval to tiles --- src/mbgl/renderer/tile_pyramid.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/mbgl/renderer/tile_pyramid.cpp b/src/mbgl/renderer/tile_pyramid.cpp index 013e6a7f37b..0e4986c7e9c 100644 --- a/src/mbgl/renderer/tile_pyramid.cpp +++ b/src/mbgl/renderer/tile_pyramid.cpp @@ -92,6 +92,7 @@ void TilePyramid::update(const std::vector>& l const optional& sourcePrefetchZoomDelta = sourceImpl.getPrefetchZoomDelta(); const optional& maxParentTileOverscaleFactor = sourceImpl.getMaxOverscaleFactorForParentTiles(); + const Duration minimumUpdateInterval = sourceImpl.getMinimumTileUpdateInterval(); std::vector idealTiles; std::vector panTiles; @@ -131,6 +132,7 @@ void TilePyramid::update(const std::vector>& l auto retainTileFn = [&](Tile& tile, TileNecessity necessity) -> void { if (retain.emplace(tile.id).second) { + tile.setMinimumUpdateInterval(minimumUpdateInterval); tile.setNecessity(necessity); } @@ -158,14 +160,11 @@ void TilePyramid::update(const std::vector>& l std::unique_ptr tile = cache.pop(tileID); if (!tile) { tile = createTile(tileID); - if (tile) { - tile->setObserver(observer); - tile->setLayers(layers); - } - } - if (!tile) { - return nullptr; + if (!tile) return nullptr; } + + tile->setObserver(observer); + tile->setLayers(layers); return tiles.emplace(tileID, std::move(tile)).first->second.get(); }; From 80b8c8f427753fc72dc7ab355722c058a6b6d87c Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Mon, 20 Apr 2020 15:55:13 +0300 Subject: [PATCH 7/9] Add Source.SourceMinimumUpdateInterval unit test --- test/style/source.test.cpp | 49 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/test/style/source.test.cpp b/test/style/source.test.cpp index 06afd6427fe..a77be5523c0 100644 --- a/test/style/source.test.cpp +++ b/test/style/source.test.cpp @@ -749,6 +749,7 @@ class FakeTile : public Tile { renderable = true; } void setNecessity(TileNecessity necessity) override; + void setMinimumUpdateInterval(Duration) override; bool layerPropertiesUpdated(const Immutable&) override { return true; } std::unique_ptr createRenderData() override { return nullptr; } @@ -760,6 +761,7 @@ class FakeTile : public Tile { class FakeTileSource : public RenderTileSetSource { public: MOCK_METHOD1(tileSetNecessity, void(TileNecessity)); + MOCK_METHOD1(tileSetMinimumUpdateInterval, void(Duration)); explicit FakeTileSource(Immutable impl_) : RenderTileSetSource(std::move(impl_)) {} void updateInternal(const Tileset& tileset, @@ -787,6 +789,10 @@ void FakeTile::setNecessity(TileNecessity necessity) { source.tileSetNecessity(necessity); } +void FakeTile::setMinimumUpdateInterval(Duration interval) { + source.tileSetMinimumUpdateInterval(interval); +} + } // namespace TEST(Source, InvisibleSourcesTileNecessity) { @@ -812,6 +818,49 @@ TEST(Source, InvisibleSourcesTileNecessity) { renderSource->update(initialized.baseImpl, layers, true, false, test.tileParameters()); } +TEST(Source, SourceMinimumUpdateInterval) { + SourceTest test; + VectorSource initialized("source", Tileset{{"tiles"}}); + initialized.loadDescription(*test.fileSource); + + FakeTileSource renderTilesetSource{initialized.baseImpl}; + RenderSource* renderSource = &renderTilesetSource; + LineLayer layer("id", "source"); + Immutable layerProperties = + makeMutable(staticImmutableCast(layer.baseImpl)); + std::vector> layers{layerProperties}; + + Duration minimumTileUpdateInterval = initialized.getMinimumTileUpdateInterval(); + auto baseImpl = initialized.baseImpl; + EXPECT_EQ(Duration::zero(), minimumTileUpdateInterval); + EXPECT_CALL(renderTilesetSource, tileSetMinimumUpdateInterval(minimumTileUpdateInterval)).Times(1); + renderSource->update(baseImpl, layers, true, false, test.tileParameters()); + + initialized.setMinimumTileUpdateInterval(Seconds(1)); + EXPECT_NE(baseImpl, initialized.baseImpl) << "Source impl was updated"; + baseImpl = initialized.baseImpl; + + initialized.setMinimumTileUpdateInterval(Seconds(1)); // Set the same interval again. + EXPECT_EQ(baseImpl, initialized.baseImpl) << "Source impl was not updated"; + + minimumTileUpdateInterval = initialized.getMinimumTileUpdateInterval(); + EXPECT_EQ(Seconds(1), minimumTileUpdateInterval); + EXPECT_CALL(renderTilesetSource, tileSetMinimumUpdateInterval(minimumTileUpdateInterval)).Times(1); + renderSource->update(baseImpl, layers, true, false, test.tileParameters()); + + initialized.setMinimumTileUpdateInterval(Seconds(2)); + minimumTileUpdateInterval = initialized.getMinimumTileUpdateInterval(); + EXPECT_EQ(Seconds(2), minimumTileUpdateInterval); + + // No network activity for invisible tiles, and no reason to set the update interval. + EXPECT_CALL(renderTilesetSource, tileSetMinimumUpdateInterval(minimumTileUpdateInterval)).Times(0); + renderSource->update(initialized.baseImpl, layers, false, false, test.tileParameters()); + + // Tiles got visible, set the update interval now. + EXPECT_CALL(renderTilesetSource, tileSetMinimumUpdateInterval(minimumTileUpdateInterval)).Times(1); + renderSource->update(initialized.baseImpl, layers, true, false, test.tileParameters()); +} + TEST(Source, RenderTileSetSourceUpdate) { SourceTest test; From 8e6c01aef3b0df40ede503869a6f12f6601eefa5 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Mon, 20 Apr 2020 19:02:01 +0300 Subject: [PATCH 8/9] Add Map.SourceMinimumUpdateIntervalOverride unit test --- test/map/map.test.cpp | 57 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/test/map/map.test.cpp b/test/map/map.test.cpp index 7a8078df27b..10cd3080c62 100644 --- a/test/map/map.test.cpp +++ b/test/map/map.test.cpp @@ -1349,6 +1349,63 @@ TEST(Map, TEST_REQUIRES_SERVER(ExpiredSpriteSheet)) { test.runLoop.run(); } +TEST(Map, SourceMinimumUpdateIntervalOverride) { + MapTest<> test{1, MapMode::Continuous}; + + test.map.getStyle().loadJSON( + R"STYLE({ + "layers": [{ + "id": "a", + "type": "fill", + "source": "source-a", + "minzoom": 0, + "maxzoom": 24 + }, + { + "id": "b", + "type": "fill", + "source": "source-b", + "minzoom": 0, + "maxzoom": 24 + }] + })STYLE"); + + // Vector source + auto vectorSourceA = std::make_unique("source-a", Tileset{{"a/{z}/{x}/{y}"}}); + vectorSourceA->setMinimumTileUpdateInterval(Seconds(1)); + test.map.getStyle().addSource(std::move(vectorSourceA)); + + auto vectorSourceB = std::make_unique("source-b", Tileset{{"b/{z}/{x}/{y}"}}); + test.map.getStyle().addSource(std::move(vectorSourceB)); + + std::atomic_int requestedTilesA(0); + std::atomic_int requestedTilesB(0); + test.fileSource->tileResponse = [&](const Resource& resource) -> Response { + assert(!resource.url.empty()); + char firstSymbol = resource.url[0]; + if (firstSymbol == 'a') { + EXPECT_EQ(Seconds(1), resource.minimumUpdateInterval); + ++requestedTilesA; + } else if (firstSymbol == 'b') { + EXPECT_EQ(Duration::zero(), resource.minimumUpdateInterval); + ++requestedTilesB; + } else { + EXPECT_FALSE(true) << "Never reached"; + } + + Response res; + res.noContent = true; + return res; + }; + + test.map.jumpTo(CameraOptions().withZoom(double(16))); + test.observer.didFinishLoadingMapCallback = [&] { test.runLoop.stop(); }; + test.runLoop.run(); + + EXPECT_EQ(8, requestedTilesA); + EXPECT_EQ(8, requestedTilesB); +} + namespace { int requestsCount = 0; From 0c4cbaa361f8bcd2a7a425a5c937dc863a8d0a12 Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Mon, 20 Apr 2020 19:10:46 +0300 Subject: [PATCH 9/9] Add change log entry --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf6107519c2..8f81d852bc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ ## master +### ✨ New features + +- [core] Introduce Source::setMinimumTileUpdateInterval API ([#16416](https://github.com/mapbox/mapbox-gl-native/pull/16416)) + + The `Source::setMinimumTileUpdateInterval(Duration)` method sets the minimum tile update interval, which is used to throttle the tile update network requests. + + The corresponding `Source::getMinimumTileUpdateInterval()` getter is added too. + + Default minimum tile update interval value is `Duration::zero()`. + ## maps-v1.6.0-rc.1 ### ✨ New features