search

neocotic / convert-svg

Node.js packages for converting SVG into other formats using headless Chromium
MIT License
198 stars 45 forks source link

Add scale option #3

Closed domoritz closed 7 years ago

domoritz commented 7 years ago

I'd love to scale all generated images to be 2x the resolution in the SVG.

neocotic commented 7 years ago

I like this idea!

What about cases where the size cannot be derived from the image and the width/height is required? Would you expect the scale option to be applied to the width/height options or completely ignored and the width/height options always taken as explicit?

I'm undecided on this. If the width/height is specified explicitly, the developer could have simply multiplied them already to their own scale, however, the default scale could simply be 1 and used to be applied to whatever the width/height is, whether specified explicitly or derived from the SVG source.

Thoughts?

domoritz commented 7 years ago

I think both make sense. Intuitively, I'd have used the first option because then all pixel units are in the same scale. Also, the 2x for hidpi screens requires devs to write in terms of pixels before scaling.

I'd check what other libraries that support scaling are doing.

On Fri, Oct 27, 2017, 02:43 Alasdair Mercer notifications@github.com wrote:

I like this idea!

What about cases where the size cannot be derived from the image and the width/height is required? Would you expect the scale option to be applied to the width/height options or completely ignored and the width/height options always taken as explicit? I'm for the latter as, if the width/height is specified explicitly, the developer could have simply multiplied them already to their own scale.

Thoughts?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/NotNinja/convert-svg-to-png/issues/3#issuecomment-339925349, or mute the thread https://github.com/notifications/unsubscribe-auth/AAj86vSpP8kLVwheFBgqK0YtyF5amUThks5swaW2gaJpZM4QIh73 .

neocotic commented 7 years ago

I'll implement this initially based on the scale option being applied to any specified or derived dimensions with a default of 1 and we can change in the future based on feedback. This is a very immature package, so I'm happy to make changes like this now and improve in the future, if needed.

domoritz commented 7 years ago

Fantastic!

neocotic commented 7 years ago

This work has been delivered by PR #11. It will be included in the upcoming 0.2.0 release.

neocotic commented 7 years ago

@domoritz I've just release v0.2.0. Check it out and let me know how you get on. There's some minor breaking changes to the API, but nothing I expect to affect many users considering how new this package is.