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 icon-text-fit-stretch to alter existing sprites #9169

Closed
yapalexei opened this issue Jan 10, 2020 · 3 comments
Closed

Add icon-text-fit-stretch to alter existing sprites #9169

yapalexei opened this issue Jan 10, 2020 · 3 comments
Labels
cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) feature 🍏

Comments

@yapalexei
Copy link

yapalexei commented Jan 10, 2020

Motivation

I would like to be able to use the sprite icons provided with the loaded style as stretchable assets; to be able to define stretch points as documented under the sprites section. I can certainly add an image and define all the relevant properties but since our designers are adding all their icons to the Studio it make better sense to also add the stretchable-label-icons to the same sprite sheet.

At this time the sprite icons can be used for the icon-text-fit but because the sprite.json file, containing position/dimension props, does not contain stretch props they stretch beyond their intended dimensions when rendered to cover the text. I would like to alter these props if it is not already possible.

Design Alternatives

There are several design alternatives that can be done. All have their costs and benefits, it just depends on which one can be done sooner and provide the most long term value.

  1. The first possibility is that all sprite icons are defined, by default, to have stretch points at the center of the vertical and horizontal pixels. This is b/c no one want's all their pixels stretched equally when using icon-text-fit.
  2. One could also provide a way to add properties to the sprite.json file when importing your own sprites from Mapbox Studio. At this time the Studio only allows you to add icons but not alter any properties related to the icon such as the stretch point arrays. This one might be more relevant on the Studio project but I don't know where that is to open the issue.
  3. A final alternative could be, when defining a layer, is to add a layout property (something like icon-text-fit-stretch) to alter/add icon stretch points.

Design

The first possibility is probably the least effort that brings quite a lot of relative value. If one uses the icon-text-fit prop on an icon that has no stretch points defined but then gets assigned default center horizontal/vertical pixels stretch values, something like a circle will do perfectly for rounded corner labels. I don't really know enough about the inner workings of the mapbox-gl to have any idea of any drawbacks but I'm only guessing that this could be as simple as assigning defaults to the sprite-icon object.

The second option I'm less interested in but is also a valid direction to allow the icon being imported to have it's properties editable so that in the receiving end the client can stretch according to the designer set values. This obviously helps empower our designers to define such things with in Mapbox Studio. Us developers usually work closely with designers and if there's anything we can help free them up to make decisions on this would help us a lot.

The direction that will provide the most benefit though is Number 3. This is because it empowers the developer to choose an icon from the sprite sheet sourced from the style and define which pixels to stretch on. Given that the sprite sheet is alterable from Mapbox Studio (by our designers) one could conceivably add a stretchable-label-icon and in the code define which pixels to stretch on.

Mock-Up

The property could look like this:

layout: {
    ...
    // Doing this would automatically assign default stretch points if not present.
    'icon-text-fit': 'rectangle-white-2'
    ...
}
// or
layout: {
    ...
    'icon-text-fit': 'rectangle-white-2',
    'icon-text-fit-stretch': [[[10,10]],[[10,10]]], // this would override existing values if present
    ...
}

The multi-dimensional array is simple a combination of the stretchX and stretchY values

[
  [[xStart: Number, xEnd: Number], ...etc],
  [[yStart: Number, yEnd: Number], ...etc],
]

Concepts

This should be pretty much the same language defined by the stretchX/Y topic and perhaps a new example can be made to illustrate the proposed property.

Implementation

For the first point above, the part of mapbox-gl that would have to change is (possibly) src/style/load_sprite.js:maybeComplete. As the code traverses through the images it could assign defaults where there are none. This would then provide a way to stretch an image, I assume. I have yet to try this myself b/c I'm encountering some blockers on running the repo code in my tests. I think I just need more time to poke around and get acquainted.

@mourner mourner added cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) feature 🍏 labels Jan 13, 2020
@ansis
Copy link
Contributor

ansis commented Jan 13, 2020

Thanks for putting this together! We're currently working on adding support for adding stretchable icons through Studio. The plan is to allow icon creators to add some special shapes to their svgs that show which parts can be stretched. Uploading the icons to Studio would add them to the sprite with all the correct metadata.

If this were implemented would you still need additional apis in mapbox-gl-js?

@yapalexei
Copy link
Author

Oh that's great! Thank you for the response!

The above improvement would significantly reduce the need for additional apis but, long term, I still see value in enabling the user to stretch sprite/icons on stock maps/styles.

@kkaefer
Copy link
Contributor

kkaefer commented Jan 14, 2020

@yapalexei thanks for your thoughtful writeup! When designing stretchable icons, we've considered 1) but ultimately decided against it. Arguably, the default behavior of stretching isn't desirable in most cases, but changing it now breaks existing styles that rely on this behavior.

As @ansis mentioned, we will add the ability for designers to add stretch zones in their icons, and upload those icons to Studio. https://github.com/mapbox/spritezero already implements parsing those specially designed SVG images.

While I can see how adding icon-text-fit-stretch could solve a particular design problem, I don't think it'd be good to add it: stretch zones are fundamentally associated with an icon, and not with a layer or feature. Therefore, I think that the metadata should be with the icon/in the sprite JSON where it is now. We already have a similar property, icon-text-fit-padding, and it's not very useful because it's hard/impossible to use with most POI layers because they use data-driven styling and require a distinct padding for every feature/icon. The same applies to stretches as well.

I'll therefore close this issue and ask you to wait for Studio support for stretchable icons.

@kkaefer kkaefer closed this as completed Jan 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-platform 📺 Requires coordination with Mapbox GL Native (style specification, rendering tests, etc.) feature 🍏
Projects
None yet
Development

No branches or pull requests

4 participants