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 #4086

Open
lucaswoj opened this issue Feb 1, 2017 · 31 comments
Open
Labels
breaking change ⚠️ Requires a backwards-incompatible change to the API cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.)

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 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 @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
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
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @anandthakker on November 17, 2016 3:30

I may be missing some context or reading too fast here, but: if this is a replacement for sprite, could it still just be called sprite?

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

From @1ec5 on November 17, 2016 17:56

Regardless of the outcome of this discussion, it’s highly unlikely that we’ll ever introduce a “sprite” API to the iOS SDK or macOS SDK due to likely developer confusion. By far the most natural option on the platform would be “style image” (or more generally, “style asset”).

Sprite” is the most fundamental concept in Apple’s SpriteKit framework, akin to a view in UIKit and AppKit. (The closest analogue to GL sprites in SpriteKit is a “texture”. The closest analogue to SpriteKit sprites in GL is a symbol layer.) Meanwhile, a typical iOS SDK developer will never know about sprite sheets or even style JSON. The style specification’s current use of the word “sprite” makes a lot less sense in the absence of a sprite sheet.

iOS doesn’t wield veto power over style specification naming decisions, but it would be unfortunate if we call these images “sprites” in the name of disambiguation, only to face ambiguity in practice. We expect -[MGLStyle setImage:forName:] to be used at least as much as Studio’s sprite editor among iOS developers – and far more than manual authoring of style JSON.

@lucaswoj
Copy link
Contributor Author

lucaswoj commented Feb 1, 2017

[MGLStyle setImage:forName:] in iOS is a less ambiguous name than the proposed Map#addImage in Javascript. I would expect Map#addImage to add an image to the map. Associating this terminology with the style helps, for example Map#addStyleImage 👍

@1ec5 1ec5 added the cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) label Feb 3, 2017
@chriswhong
Copy link
Contributor

Adding my use case: We are trying to make all of our mapbox GL layers modular, so that they are more or less self contained and can be added and removed at-will from an existing mapboxGL map. We may end up having dozens of what I will call metalayers, which may contain one or more mapboxGL layers. To date all of these layers could be rendered without using sprites. We simply load a default mapboxGL style, and then add the additional layers after the map loads.

Now we need one of these add-on layers to include sprites, which breaks our modular design... the original style is no longer sufficient, we need to pre-load sprites for all layers that could ever possibly be added by the user in any part of our app.

Having images/sprites be something that can be added and removed at will just like sources and layers will help us achieve a more modular design for our add-on metalayers.

@lucaswoj lucaswoj self-assigned this Feb 14, 2017
@lucaswoj
Copy link
Contributor Author

@chriswhong Sounds like you'd be interested in #4000

@jaapster
Copy link

jaapster commented Sep 8, 2017

@chriswhong We are doing the same thing: creating "meta-layers" that contain multiple mapbox layers and sources (we are using multiple tile servers). For us it is crucial that all information needed for rendering a "meta-layer" is modular and has no dependencies.

@jacknkandy
Copy link

I also support this request due to difficulty rebuilding layer styles after changing the basemap.

As Mapbox has no concept of "basemaps" changing the basemap means that all custom layers are removed and need to be re-added after the new style has been loaded. I have been able to solve this particular issue - there is already talk about potentially addressing this issue.

The problem I am now having is that after this process, the images are no longer available and need to be re-added. We can easily access the source and layer information to rebuild the layer style, but the images used by particular layers cannot be accessed/obtained. This makes things quite difficult, especially considering the asynchronous nature of having to load images before adding them to the map. I will need to resort to maintaining a list of images and re-loading and re-adding them when the layer style is modified.

@pnorman
Copy link

pnorman commented May 13, 2020

#358 directs here via mapbox/mapbox-gl-style-spec#220. Is allowing styles to use multiple sprites and individual images within the scope of this issue?

I have three use cases

  • I want the ability to take a basemap and modify it by adding new features with new sprites without needing to rebuild the existing sprites, or, in general, combine a base spritesheet with additional style-specific spritesheets
  • I want to organize my 989 and growing sprites rather than one giant spritesheet
  • I want to be able to develop without needing to check generated files into git or require the sprite build infrastructure for people not making sprite changes.

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 cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.)
Projects
None yet
Development

No branches or pull requests

6 participants