search

aeirola / react-native-svg-asset-plugin

Asset plugin for importing SVG images in React Native
https://www.npmjs.com/package/react-native-svg-asset-plugin
MIT License
68 stars 9 forks source link

Added scaleMultiplicator that allows to control output image size #44

Closed devproivansurzhenko closed 4 years ago

devproivansurzhenko commented 4 years ago

It allows to manually scale images to the desired size (for example, if svg originally designed for another usage)

aeirola commented 4 years ago

Thanks for the proposed feature!

Before going into the actual code change, I'd be interested in hearing more about the use-case for the feature. You mention a case where an SVG would be originally designed for other usage, but I didn't really understand why not just change the SVG image width and height attributes to the desired size.

I see some potential usability issues in having a single global scaling factor for all SVG images. For example if some SVG images were originally designed for other use, while others are designed specifically for react native app usage. This makes me a bit hesitant towards directly merging this PR without more information about the use-cases and potential alternative solutions.

devproivansurzhenko commented 4 years ago

@aeirola Sorry, i forgot to mention that this PR is requiring some more testing from my side. So, please keep on hold it for a while.

Answering to your question: Company has a js lib that includes a set of icons and images that are making UI consistent over all components (i mean architectural components instead of React-components). Historically, company did the product for the web world only. So, everything in the lib were optimized for that usage.

Nowdays, we are developing a mobile client for that product which replicates some % of web functionality. For sure, UX of mobile app is slightly different... icon size is also different. Web one is using 16px while mobile one is using 32px for @1 (we just played with different sizes and found it most useful).

If we use react-native-svg-asset-plugin with our original SVGs, it makes our icons blurred because SVG files refer to 16px.

So, this big lib with the graphic content (more than 100 icons and pictures) requires to be scaled to a new size. It makes everyone worry because:

  1. Someone should go over all images and change it's sizes (btw, it is not enough to change just width/height in some cases) - one-time job
  2. Someone should sync changes from parent's big lib into a set of svg's inside of the mobile app - continuous infinite job with a big chance to lose something

With this PR, we can just scale PNG result without quality lose and without touching (copy-modify-pasting) sources of the original library.

aeirola commented 4 years ago

Thanks for the thorough explanation of the use case. Sounds like it is oddly specific for your company's situation where you are reusing existing icons from other projects.

I'm not sure this plugin is the proper place to handle that kind of image size conversion. I do understand the appeal to make a small additional feature to this plugin for your specific use case, but I would like to keep the plugin as simple and straightforward as possible. Specifically, just render the images as they are, without any extra processing. Similarly to how react native handles png and jpeg images without any build time manipulation.

In your situation I would probably push for the common asset library (or a separate mobile-specific repacakging) to include both web and mobile sized images. Either by manually maintaining the separate images, or with some automated conversation during packaging. I guess the latter would be quite straightforward using the SVG.js library.

aeirola commented 4 years ago

One hack you could try out is to only generate images at a higher scales, e.g. only 4x using the scales configuration option. This would fix blurry images, but produce a bit unnecessary large images for low dpi devices.

devproivansurzhenko commented 4 years ago

In case of functionality under this PR, it allows to generate right sizes for each scale without any side effects like memory overusage.

aeirola commented 4 years ago

Yeah, that is true. Configuring to use only large scales would be a hack and not an optimal solution.

In case it doesn't work for you, then I'd suggest that you focus on resizing the images before passing them to this plugin. I don't really see this PR being merged with this feature as it is. The use case seems way too specific, and the plugin configuration easily becomes confusing with too many narrow options.

In case there are other popular use cases for a feature which solves the issue you are having, then I'd be more than happy to consider incorporating it into this plugin. But for now, I'll close this PR as too specific use case.

I know this isn't what you hoped for, and I'm sorry about that.

devproivansurzhenko commented 4 years ago

jfyi, large scales will not work, because

https://github.com/aeirola/react-native-svg-asset-plugin/blob/8118734477ea51c092888ecb9b841add53652463/src/index.js#L120

it will store image@LARGESCALEx.png with 1x multiplication

devproivansurzhenko commented 4 years ago

ah.. no.. it will store it as image.png.. I missed the switch-case at getScaleSuffix

Anyway, it will not touch the real scale of image

aeirola commented 4 years ago

Ah, yeah. I was thinking of configuring something like scales: [4] which should produce only an @4x.png file, which should then be used by react native for all screen DPIs. Haven't tested it though, so can't confirm that it would work as expected.