Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom @2x handling for mapbox v4 tile API when tileSize: 512 #1964

Closed
wants to merge 1 commit into from

Conversation

yhahn
Copy link
Member

@yhahn yhahn commented Jan 21, 2016

The v4 mapbox tile API supports 512x512 image tiles only when @2x is appended to the tile URL. If tileSize: 512 is specified for a Mapbox raster source force the @2x suffix even if a non hidpi device.

We can revisit this with the next version of the tile API which may look to provide 512x512 tiles to non-hidpi devices.

cc @kkaefer

@jfirebaugh
Copy link
Contributor

@yhahn Can you elaborate on the motivation here?

If tileSize: 512 is specified for a Mapbox raster source

My understand is that this shouldn't happen / you shouldn't do this. You should always specify tileSize: 256 for a Mapbox raster source.

yhahn added a commit to mapbox/mapbox-gl-native that referenced this pull request Jan 21, 2016
@yhahn
Copy link
Member Author

yhahn commented Jan 21, 2016

You should always specify tileSize: 256 for a Mapbox raster source.

Yeah, this is not necessarily revising this assumption but at least allowing us and others to start testing more realistically what using tileSize: 512 would be like.

All newer raster imagery on the mapbox API (default and custom uploaded imagery via upload API) now has 512x512 hires versions. At the moment there's no way to make use of this in conjunction with the 512x512 tile rendering size in either GL js or native -- if you want to try this out, you're stuck stretching 256x256 tiles out.

In the future we'll need to support 512x512@1x and 1024x1024@2x tiles from the Mapbox API to make the tileSize: 512 switch fully.

@yhahn
Copy link
Member Author

yhahn commented Jan 22, 2016

Adjusted approach after discussion: #1971

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants