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

[WIP] [ios, macos] Calculate number of tiles for an offline region #11965

Closed
wants to merge 5 commits into from

Conversation

captainbarbosa
Copy link
Contributor

Draft implementation for #11504

Adds a new method which calculates the number of tiles within an MGLTilePyramidOfflineRegion.

This is implemented within the MGLOfflineRegion protocol in hopes of this one day applying to offline regions with arbitrary shapes, as mentioned in #11447. This is very much a work in progress.

@captainbarbosa captainbarbosa added iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS offline labels May 21, 2018
@captainbarbosa captainbarbosa self-assigned this May 21, 2018
@captainbarbosa captainbarbosa requested a review from 1ec5 May 21, 2018 22:40
@@ -53,6 +53,11 @@ - (instancetype)initWithStyleURL:(NSURL *)styleURL bounds:(MGLCoordinateBounds)b
return self;
}

-(uint64_t)tileCount {
auto tilePyramidOfflineRegion = [self offlineRegionDefinition];
return tilePyramidOfflineRegion.tileCount(mbgl::style::SourceType::Vector, 512, {static_cast<unsigned char>(tilePyramidOfflineRegion.minZoom), static_cast<unsigned char>(tilePyramidOfflineRegion.maxZoom)});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For testing purposes, I'm assuming the source type and tile size. We will need to figure out a way for to pass these in dynamically, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

@asheemmamoowala, it would be great if this method could somehow call tileCount() automatically for each source in the style, so that the developer doesn’t have to enumerate the sources themselves. But I’m unsure how we could accomplish that for arbitrary style URLs, especially if the URL hasn’t been loaded yet. #6386 would make it possible to pass in a full style in some cases, but there’s still the possibility of an unloaded style.

In the meantime, perhaps we could make it a method of MGLTileSource that takes an offline region as a parameter?

Copy link
Contributor

Choose a reason for hiding this comment

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

In the meantime, perhaps we could make it a method of MGLTileSource that takes an offline region as a parameter?

I like this approach. Another option is to extend MGLOfflineRegion... to use the currently loaded MGLStyle, and the MGLStlye.sources property.

Copy link
Contributor

@1ec5 1ec5 May 22, 2018

Choose a reason for hiding this comment

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

The offline map API and runtime styling APIs currently have little to do with each other. I think it would be problematic if we hooked up MGLOfflinePack or MGLOfflineRegion with MGLStyle in any way, because that would imply you could add a source to the style and have it reflected in the tile count, or change a layer in the style and persist that change when loading the offline pack in the future.

For now, let’s turn the MGLOfflineRegion.tileCount property into a method that takes an MGLTileSource as a parameter. We can frame it as a way to get the tile count for a single source within the offline pack. Then the developer can create an MGLTileSource to pass into this method. The MGLTileSource knows all the details we need for that tileCount() call even without loading the style.

It’ll be a bit weird that the developer will be able to pass an unrelated source into this method, but that weirdness comes from how the style URL is part of the offline region, something we can’t change just yet.

@@ -10,6 +10,12 @@ NS_ASSUME_NONNULL_BEGIN
*/
@protocol MGLOfflineRegion <NSObject>

/**
The number of tiles that the object conforming to the `MGLOfflineRegion`
protocol contains.
Copy link
Contributor

Choose a reason for hiding this comment

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

More precisely, it’s the number of tiles needed to load one of the style’s sources within the region.

@stale
Copy link

stale bot commented Oct 24, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the archived Archived because of inactivity label Oct 24, 2018
@julianrex
Copy link
Contributor

This is still in flight.

@stale stale bot removed the archived Archived because of inactivity label Oct 24, 2018
@stale stale bot added the archived Archived because of inactivity label Dec 23, 2018
@stale
Copy link

stale bot commented Dec 24, 2018

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this Dec 24, 2018
@captainbarbosa
Copy link
Contributor Author

This is quite outdated, and I'm worried we might need to scrap this and start over with a fresh implementation if things have changed. @1ec5 what do you think?

@stale stale bot removed the archived Archived because of inactivity label Dec 24, 2018
@stale stale bot added the archived Archived because of inactivity label Feb 22, 2019
@stale
Copy link

stale bot commented Feb 22, 2019

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

@stale stale bot closed this Feb 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived Archived because of inactivity iOS Mapbox Maps SDK for iOS macOS Mapbox Maps SDK for macOS offline
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants