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

fixes #892: support mapbox:// style URLs #1161

Merged
merged 1 commit into from
Mar 30, 2015
Merged

fixes #892: support mapbox:// style URLs #1161

merged 1 commit into from
Mar 30, 2015

Conversation

incanus
Copy link
Contributor

@incanus incanus commented Mar 30, 2015

Addresses #892 and ports over mapbox/mapbox-gl-js@699826a.

This lets the user continue to pass a bundled style like styles/emerald-v7.json from Cocoa, which will get converted into an asset:// URL as before.

But if a style URL like mapbox://justin.80b0e36d is passed instead, it gets converted to the style API format and loaded via the regular remote style URL facility.

One thing I'm not clear on:

  • I'm not quite sure what base should be in StyleInfo. Does this matter for URL-based styles @kkaefer @jfirebaugh? Going to keep digging into this but can't quite figure it out yet.

@incanus incanus added this to the iOS Beta 1 milestone Mar 30, 2015
@incanus incanus added the GL JS parity For feature parity with Mapbox GL JS label Mar 30, 2015
@incanus
Copy link
Contributor Author

incanus commented Mar 30, 2015

As far as I can gather, we don't need to set a base for online hosted styles, as their sprite/glyph/etc. URLs would be specified as absolute and not relative. /cc @samanpwbb

mbglMap->setStyleURL(std::string("asset://") + [filePathURL UTF8String]);
std::string styleURL([styleURLString UTF8String]);

if ( ! [[NSURL URLWithString:styleURLString] scheme])
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we drop this case and require the app developer to use "asset://..." if that's what they want?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

setStyleURL: is private right now. At such time that we support in-framework setting of a style URL (versus download of JSON externally and passing that) I think we could, but also, it's weird to use asset:// — this has no meaning outside of GL and typically a Cocoa dev would use +[NSURL fileURLWithPath].

Copy link
Member

Choose a reason for hiding this comment

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

It may have no meaning outside of our codebase, but we use to to make our styles compatible across renderers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, no qualms with it, I'd just rather not expose the scheme to end users if we can avoid it.

@jfirebaugh
Copy link
Contributor

As far as I can gather, we don't need to set a base for online hosted styles, as their sprite/glyph/etc. URLs would be specified as absolute and not relative.

Yes, they should be using mapbox:// URLs in turn. Though for sprites that's pending a sprites API.

@incanus
Copy link
Contributor Author

incanus commented Mar 30, 2015

Ok, I'm going to merge this then once CI passes.

@jfirebaugh
Copy link
Contributor

👍

incanus added a commit that referenced this pull request Mar 30, 2015
fixes #892: support mapbox:// style URLs
@incanus incanus merged commit 7e878d0 into master Mar 30, 2015
@incanus incanus deleted the mapbox-style-urls branch March 30, 2015 23:44
@kkaefer
Copy link
Member

kkaefer commented Mar 31, 2015

Can you please add tests for this?

@incanus
Copy link
Contributor Author

incanus commented Mar 31, 2015

Will do.

@incanus
Copy link
Contributor Author

incanus commented Mar 31, 2015

Tests done in #1167.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
GL JS parity For feature parity with Mapbox GL JS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants