Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] Add runtime API for setting tile prefetch delta for a Source #16179

Merged
merged 7 commits into from
Feb 11, 2020
Merged

[core] Add runtime API for setting tile prefetch delta for a Source #16179

merged 7 commits into from
Feb 11, 2020

Conversation

alexshalamov
Copy link
Contributor

The new Source::setPrefetchZoomDelta(optional<uint8_t> delta) method allows overriding default tile prefetch setting that is defined by the Map instance. The method can be moved to a generic style specification if found to be useful for gl-js engine.

Launch Checklist

  • briefly describe the changes in this PR
  • write tests for all new functionality
  • document any changes to public APIs
  • tagged @mapbox/maps-android @mapbox/maps-ios @mapbox/core-sdk if this PR adds or updates a public API
  • tagged @mapbox/gl-js if this PR includes shader changes or needs a js port
  • apply needs changelog label if a changelog is needed (remove label when added)

/cc @mapbox/gl-js @mapbox/core-sdk

@alexshalamov alexshalamov added the needs changelog Indicates PR needs a changelog entry prior to merging. label Feb 6, 2020
@alexshalamov alexshalamov force-pushed the alexshalamov_source_prefetch_delta branch 2 times, most recently from f0ebf79 to f2dc599 Compare February 7, 2020 16:12
@alexshalamov alexshalamov removed the needs changelog Indicates PR needs a changelog entry prior to merging. label Feb 7, 2020
@alexshalamov alexshalamov force-pushed the alexshalamov_source_prefetch_delta branch 2 times, most recently from 775234a to ec46f46 Compare February 7, 2020 17:43
@alexshalamov alexshalamov marked this pull request as ready for review February 7, 2020 18:09
@alexshalamov alexshalamov force-pushed the alexshalamov_source_prefetch_delta branch from ec46f46 to 8073c0e Compare February 10, 2020 10:22
@alexshalamov
[core] Add runtime API for setting tile prefetch delta for Source
04292d0 
New setPrefetchZoomDelta(optional<uint8_t> delta) method allow overriding
default tile prefetch setting that is defined by the Map instance. The
method can be moved to generic style specification if found to be useful
for gl-js engine.
pozdnyakov
pozdnyakov approved these changes Feb 10, 2020
View reviewed changes
Copy link
Contributor

@pozdnyakov pozdnyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm % questions

@alexshalamov
[core] Override default prefetch delta if source has it's own setting
c7ab18a
@alexshalamov
[core] Make CustomGeometrySource::TileOptions immutable
c9bbb86
@alexshalamov
[core] Clear tile pyramid for custom source only if there is a signif…
bc87272 
…icant change

Clear tile pyramid only if updated source has different tile options,
zoom range or initialization state for a custom tile loader.
@alexshalamov
[core] Add unit test for Source::setPrefetchZoomDelta
4aaea74 
Test verifies that tile pyramid will request only 4 tiles for
current zoom level, if Source's setPrefetchZoomDelta is 0.
@alexshalamov
[core] Update baselines
ae8c819
@alexshalamov
[core] Update changelog
a537489
@alexshalamov alexshalamov force-pushed the alexshalamov_source_prefetch_delta branch from 8073c0e to a537489 Compare February 10, 2020 20:35
@alexshalamov alexshalamov merged commit e0b1642 into master Feb 11, 2020
@alexshalamov alexshalamov deleted the alexshalamov_source_prefetch_delta branch February 11, 2020 08:40
@tobrun
Copy link
Member

tobrun commented Feb 14, 2020 •
edited
Loading

@alexshalamov downstream we are implementing code generation for Sources similar to what we already have for layers/properties/light etc from the style spec today. Should this configuration be exposed through the style spec? https://docs.mapbox.com/mapbox-gl-js/style-spec/sources/

update: just seeing the method can be moved to a generic style specification if found to be useful for gl-js engine.

@alexshalamov
Copy link
Contributor Author

@asheemmamoowala Do you think adding prefetchDelta to sources specification would be useful to gl-js? If so, I can start working on it next sprint.

@tobrun meanwhile, would it be possible to add an exception and generate setter / getter for all sources? I think generator already has exceptions, since some of the features are not implemented in native (e.g., promoteId).

@Chaoba Chaoba mentioned this pull request Feb 18, 2020
Closed
@chloekraw chloekraw mentioned this pull request Feb 24, 2020
Closed
@asheemmamoowala
Copy link
Contributor

asheemmamoowala commented Mar 18, 2020 •
edited
Loading

@asheemmamoowala Do you think adding prefetchDelta to sources specification would be useful to gl-js? If so, I can start working on it next sprint.

@alexshalamov I'm going to have to defer to @kkaefer @mourner on this. Cancelling and managing tasks in WebWorkers is different from native threads, so this may not work as well in gl-js as it does in native.

mapbox/mapbox-gl-js#2826

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@pozdnyakov pozdnyakov pozdnyakov approved these changes

@tmpsantos tmpsantos Awaiting requested review from tmpsantos

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@alexshalamov @tobrun @asheemmamoowala @pozdnyakov