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

Treat "images" as nested collection of a style, like sources or layers #4085

Closed
lucaswoj opened this issue Feb 1, 2017 · 23 comments
Closed
Labels
breaking change ⚠️ Requires a backwards-incompatible change to the API

Comments

@lucaswoj
Copy link
Contributor

lucaswoj commented Feb 1, 2017

From @jfirebaugh on November 20, 2014 2:17

We want something more convenient than the current automagical practice of appending permutations of .json, .png, and @2x to the sprite property and making requests to two separate resources which need to be in sync.

Options that have been proposed:

  • External protobuf container A format inspired by glyph pbfs. The protobuf message would be a series of [metadata, image data] tuples. Metadata would be id, image format, pixel ratio, and sdf boolean. Open question: would we still have separate @1x and @2x files, or just use @2x images everywhere, or permit including multiple images per id (with different pixel ratios) in the same file?
  • Inline JSON + base64 image data Include sprite data directly in the style. Metadata in JSON format, image data base64 encoded (can't really do better than that). Same question regarding @2x images as above.

@mapbox/gl

Copied from original issue: mapbox/mapbox-gl-style-spec#220

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @incanus on November 20, 2014 2:41

Is this speaking strictly to the storage format, or mechanism for use in GL, or both? Keep in mind that per mapbox/mapbox-gl-native#507 we will need to support arbitrary imagery for points and other annotations on the map in a rapidly-changing sprite. This could be (and probably should be) separate from the style but a similar approach could be used.

@lucaswoj lucaswoj added the breaking change ⚠️ Requires a backwards-incompatible change to the API label Feb 1, 2017
@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on November 20, 2014 2:44

Is this speaking strictly to the storage format, or mechanism for use in GL, or both?

Both, I think -- not sure what you mean by "mechanism for use in GL".

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @ljbade on November 20, 2014 8:37

On Android we can have more than just 1x and 2x, Android supports 0.75, 1, 1.5, 2, 3 and 4x screen densities.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @mikemorris on November 20, 2014 16:44

I'd definitely be in favor of keeping the actual image assets out of the stylesheet, simply for ease of editing the style and updating the images separately. Let's figure out some sort of extensible spec for sprite protobuf containers, perhaps with a [ratio, imagedata] repeated field to allow for any number of pixel densities.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @incanus on November 20, 2014 19:15

not sure what you mean by "mechanism for use in GL"

I mean both the preparation & storage in a sprite as well as the use of a single sprite bound to a texture unit. Mostly that we can think about multiple sprite sheets, since we'll likely need a rapidly changing one for more open/dynamic imagery.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @ljbade on November 20, 2014 20:8

@incanus, oh so you mean have the ability encode sprites in a texture atlas, e.g. contiguous pixel data, but then have a seperate 'structure' index that says sprite i is found at pixels x, y + size of sprite?

@mikemorris, yeah make ratio a float and just choose the sprite closest to what the system says is needed

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on November 20, 2014 22:19

This ticket is about storage format. In memory representation is a separable concern. In js we'll use a sprite atlas like native already uses and copy image data from its storage representation.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @incanus on November 21, 2014 0:12

This ticket is about storage format.

👍

FYI what I'm talking about is this, but agree that an in-memory representation will probably be what we use directly in these cases:

https://gist.github.com/incanus/e9cba3b5f11d97654cc2

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @kkaefer on November 24, 2014 12:25

@jfirebaugh I agree with your proposal. The two ways you suggest are almost equal; both require special parsing and generation routines. However, I'd opt for the first (separate protobuf container) instead of encoding the information into the PNGs (e.g. as separate data sections). While it would have the advantage of being able to view the sprite, the disadvantage is that it may tempt users into quickly "updating" the image with a graphics program, without realizing that they also have to update the included metadata. Instead, we should follow a Maki-style approach of having a folder full of icons/SVG images, and write a small tool that produces sprite PBFs.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @kkaefer on December 9, 2014 10:43

Implemented a first draft in https://github.com/mapbox/spritenik

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @kkaefer on December 9, 2014 10:49

Issue with embedding data into PNG images is also that there are proxies out there which process/shrink images (e.g. my mobile phone provider does that), which would potentially remove embedded information. Using JSON to store Base64 images would add double compression (compressed data inside base64 images, and another layer of compression on the actual JSON file in transfer) and parsing in the browser will become trickier.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on February 4, 2016 20:1

I think we should:

  • Eliminate the "sprite" concept and terminology. Instead a style has a collection of icons.
  • Icons are embedded directly as an "icons" property of the style.
  • An icon has a style spec JSON representation consisting of dimensions, pixel ratio, content type, and base64 encoded data. Content type must be PNG for now.
  • The style API in gl-js (Add map.setSprite(spriteUrl)method #2058 / Add Map#{add,remove}Image #2059) and gl-native will be a standard CRUD API for the "icon" subresource of a style.
  • api.mapbox.com/styles/v2 renames the "sprite" subresource to "icons", but the interface is basically the same.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @ansis on February 4, 2016 20:17

Icons are embedded directly as an "icons" property of the style.

Would there be different styles for different device pixel ratios?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on February 4, 2016 20:26

Multi-resolution support is TBD. But for starters, I think we could have Mapbox APIs always serve styles containing @2x icons, and for homesteaders, leave it up to them what resolutions they include with a style.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @scothis on February 4, 2016 20:30

Icons are embedded directly as an "icons" property of the style.

Would there be different styles for different device pixel ratios?

If we make inlining optional, we can preserve a link to the icons resource that can be loaded resolution independent, or with a new images format. There are advantages to inlining and externalizing depending on the circumstance.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @ansis on February 4, 2016 21:11

I think we could have Mapbox APIs always serve styles containing @2x icons

This would kill pixel snapping for icons for 1x devices, which could be ok, but could also make icons too blurry at that size and resolution.

2x icons on 1x device:
screen shot 2016-02-04 at 1 06 27 pm screen shot 2016-02-04 at 1 05 53 pm screen shot 2016-02-04 at 1 05 24 pm

1x icons on 1x device:
screen shot 2016-02-04 at 1 06 48 pm screen shot 2016-02-04 at 1 07 03 pm screen shot 2016-02-04 at 1 07 51 pm

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @mourner on February 4, 2016 21:17

Would images for patterns (fill textures etc.) also be called icons?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on February 4, 2016 21:29

@mourner Good point -- the terminology should be consistent within the rendering property names as well. How about: images, icon-image, and {background,fill,line}-pattern-image?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @jfirebaugh on February 4, 2016 21:53

Thoughts on pixel ratios:

  • I'm warming to the idea of having the API be able to generate styles with images of a particular pixel ratio. In general I think Style API v2 should start to make a distinction between "raw"/"uncompiled" and "for-renderer"/"compiled" forms of a style and the desired pixel ratio could be part of the latter.
  • However, I think we should preserve the ability to (hand-)author a single style document containing images at multiple resolutions and have the optimal resolution selected at render time. This has implications for how we structure the "images" style property and its sub-objects in the style. (Array or object? If object, what form are the keys in?)
  • I'm not sold on the advantage of optional inlining. I think many of the problems we have encountered with sprites are due to the fact that a style isn't as self-contained as possible. Self-contained-ness just makes things way simpler. I'd like to push for inlining sources as well.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

I worry about the naming collision between "images" as proposed in this ticket and the image source type. Users may become confused that the same word can refer to either thing.

cc @1ec5 @tmcw

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @1ec5 on November 17, 2016 3:18

…should image sources be called something else? 🤔

We have raster layers and raster sources; seems like style images and image sources could coexist at least as well.

How would you describe the difference between the two? Reusable images versus georeferenced images?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

Raster layers consume raster sources. That makes sense 👍.

Image sources would not relate to "images" (by some definitions of the thing). That makes a less sense to me.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

We could continue to call these new things "sprites" though we change how sprites are specified.

@lucaswoj lucaswoj closed this as completed Feb 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ⚠️ Requires a backwards-incompatible change to the API
Projects
None yet
Development

No branches or pull requests

1 participant