search

mapbox / spritezero

small opinionated sprites
Other
173 stars 62 forks source link

Don't use Mapnik in generateLayout? #51

Open mcwhittemore opened 7 years ago

mcwhittemore commented 7 years ago

Currently, spritezero uses Mapnik in the generateLayout step. While this is helpful when the layout is passed to generateImage, it is a lot of unneeded work when the layout json is all an application needs.

Can we move this logic to the generateImage step? It would mean needing to calculate an icons width with something other than Mapnik Image.

springmeyer commented 7 years ago

It would mean needing to calculate an icons width with something other than Mapnik Image.

Good idea. However, interpreting the width/height is non-trivial due to what SVG supports: variable units and viewBox layout. (refs https://github.com/mapnik/mapnik/commit/fb77657dfd7807508e4d370197e6da0b1ae70f12 to see @artemp latest fixes to this code). So, my preference would be to use Mapnik rather than duplicate parsing logic.

This could be done by exposing an api in node-mapnik like:

mapnik.parseSVGMeta(buffer, options, function(err,meta) {
 ....
});

The meta object could contain properties like width, height, and an array of warnings/errors encountered during parsing. It would use just this code in node-mapnik: https://github.com/mapnik/node-mapnik/blob/fe80ce5d79c0e90cfbb5a2b992bf0ae2b8f88198/src/mapnik_image.cpp#L2625-L2629

mcwhittemore commented 7 years ago

This sounds like a good option. I'll take a stab at a PR to do this.

I'm assuming the api should be mapnik.Image.parseSVGMeta rather than mapnik.parseSVGMeta.