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

Support for optional properties (content, stretchX & stretchY) #53

Closed
DerZade opened this issue May 10, 2023 · 6 comments
Closed

Support for optional properties (content, stretchX & stretchY) #53

DerZade opened this issue May 10, 2023 · 6 comments

Comments

@DerZade
Copy link
Contributor

DerZade commented May 10, 2023

I don't think there is currently a way to set the content, stretchX and stretchY optional properties in the inext file, correct? (take a look at the mapbox docs)

Maybe these values could be read from data attributes of the SVG root element or an optional json file with the same name as the svg or another JSON as another input, which includes optional properties for all sprites.

Thoughts?

@flother
Copy link
Owner

flother commented Jun 18, 2023

You're right to say Spreet doesn't provide a way to add the optional properties to the index file directly, but the good news is that it is possible. You can combine Spreet with jq to output the data you need.

You need to create an index file that contains only the optional properties, and then merge that with Spreet's output. Say you have three sprite SVGs in an images directory, bicycle.svg, tram.svg, and train.svg, that need the extra properties. Create an optional.json that includes only the optional props:

{
  "bicycle": {
    "stretchX": [16, 32],
    "stretchY": [10, 24]
  },
  "tram": {
    "content": [0, 0, 32, 32],
  },
  "train": {
    "stretchY": [0, 32]
  }
}

And then run Spreet in the usual manner:

$ spreet images output

Now you can merge Spreet's output.json index file with your optional.json index file using jq:

$ jq --slurp '.[0] * .[1]' output.json base.json

That will output a merged JSON file that you can use as your map's index file.

It's mildly annoying that it's two steps rather than one, but Spreet can't output to stdout because it creates two files. I suppose we could build the JSON merging into Spreet, but jq is so much more flexible than Spreet would ever be, and it's nice to follow the Unix philosophy of doing one thing and doing it well.

How does that work for you?

@DerZade
Copy link
Contributor Author

DerZade commented Jun 26, 2023

That would work fairly well if I only have ratio 1. The values for stretchX, stretchY and content will differ for different ratios tho. This will get complicated and a lot of things to do manually quite fast 🙈

@flother
Copy link
Owner

flother commented Oct 2, 2023

Yeah, you're right. My suggestion does feel a little bit hacky, and since Spritezero supports this then Spreet really should too.

The original spec for stretchable icons is in mapbox/mapbox-gl-js#8917 and was implemented in Spritezero in mapbox/spritezero#68. From an icon author's perspective it seems the idea is that you create transparent shapes (i.e. no fill, no stroke) in Adobe Illustrator (or equivalent), and they represent the icon's stretch and content areas. In an exported SVG version of the icon the invisible shapes would be <path> or <rect> elements with specific id attributes:

  • mapbox-icon-stretch
  • mapbox-icon-stretch-x
  • mapbox-icon-stretch-y
  • mapbox-icon-content

mapbox-icon-stretch is a shorthand for mapbox-icon-stretch-x and mapbox-icon-stretch-y, and the bounding boxes of the <path>/<rect> elements are used as the values for the stretchX and stretchY properties in the sprite index file. There can be multiple "stretch" ids to define multiple areas — mapbox-icon-stretch-x-1, mapbox-icon-stretch-x-2, and so on. The mapbox-icon-content element maps to the index file's content property; an icon can only have one of those.

An example from the RFC:

<svg viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg">
    <path d="M..."/>
    <path d="M..."/>
    <rect id="mapbox-icon-stretch-x" x="3" y="0" width="14" height="1" style="fill:none;"/>
</svg>

As someone who loves the command-line and abhors point-and-click interfaces, I dislike the idea of setting metadata using a GUI image-editing tool. But in this case I think it makes sense for Spreet to follow prior art rather than inventing its own way of doing things. It should support maplibre-icon-* ids too — or perhaps just unprefixed ids like icon-content.

I've tinkered with usvg (the SVG parser Spreet uses) and it seems like it should be possible to support stretchable icons in the same way Spritezero does. I'd really like to try and add support for this just as soon as I have more time to spend on Spreet.

@nvkelso
Copy link

nvkelso commented Oct 2, 2023

+1 as this can reduce the size of highway shield spritesheet significantly (eg having 1 icon per shield plus this metadata, instead of 5 or 6 per shield pattern and shield text character width.

flother added a commit that referenced this issue Oct 4, 2023
Adds support for the sprite index file's content, stretchX, and stretchY
properties, which are required to support stretchable icons. Currently
has support with a few caveats:

- SVG elements containing content/stretchX/stretchY metadata that are
  hidden (`style="display: none"`) cannot be used. This is a due to how
  SVGs are parsed by the underlying usvg library. This may have an
  impact on icons exported from Inkscape
- Parallel loading of SVG files is currently disabled because the parsed
  SVG trees can't be passed between threads
- When included in an index file, the content/stretchX/stretchY
  properties are always floats (spritezero, because it's JavaScript,
  will happily mix and match integers and floats)

See #53.
@flother
Copy link
Owner

flother commented Oct 5, 2023

I have an implementation ready in #64. It passes the units tests in Spritezero and I've checked it works on a quick-and-dirty demo map, so I'm fairly confident it does the right thing. But I don't have access to any "real" stretchable icons, used in a production-level map style. Does anyone here have anything we can use to try it out?

If you have access to the Rust toolchain you can try the PR yourself:

$ git clone https://github.com/flother/spreet.git
$ cd spreet/
$ git switch stretchable-icons
$ cargo run -- <INPUT> <OUTPUT>

flother added a commit that referenced this issue Oct 8, 2023
Add support for outputting the optional icon properties in the sprite
index file (`content`, `stretchX`, and `stretchY`) when the input SVGs
have the required metadata. See discussion in #53 for details. Some
differences from Spritezero's original implementation:

- If an SVG metadata element is hidden (e.g.
  `<path id="mapbox-content" d="..." style="display: none"/>`) then its
  metadata isn't included in the index file. This is due to how SVGs are
  parsed by the underlying [usvg](https://docs.rs/usvg/) library
- Storing metadata in a `<text>` element isn't supported
- While there are some guarantees on metadata values (no negative
  heights or widths, left <= right edge, top <= bottom), metadata isn't
  otherwise validated
@flother
Copy link
Owner

flother commented Oct 8, 2023

I've tidied up the code and merged it to master. I'll cut a new release (0.9.0) so pre-compiled binaries will be available for anyone to try.

One last thing to mention: I didn't bother adding support for maplibre-* prefixes, it's just mapbox-* for now. The documentation on stretchable icons is minimal enough as it is, so I didn't feel like it was worth adding code to support what would essentially be a hidden feature.

@flother flother closed this as completed Oct 8, 2023
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

No branches or pull requests

3 participants