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

[WIP] Working with svgs + new geometry options #41

Closed
wants to merge 4 commits into from

Conversation

jamesdaniels
Copy link
Contributor

Hey Daniel, I wanted to open a PR for this work in progress change I have that manipulates SVGs in badge. This leads to better quality badges when resized and makes an especially big difference on Android.

Just wanted to start a conversation around this change.

Curb was added as a dependency due to a problem in my local environment, that keeps net/http from requesting from shield.io. I haven't figured out what this is yet. I'll see about at least separating this from the PR.

rsvg-convert is now a CLI dependency, as even when compiled with SVG support magick still has compatibility issues (inexplicable black badges on Travis for example). I need to add some error handling here.

I added new options, shield_scale and shield_geometry which allow you to resize the shield and move it around respectively.

Example usage: bundle exec badge --glob "/../app/src/main/res/**/ic_launcher.png" --shield $SHIELD --no_badge --shield_geometry "+0+25%" --shield_scale 0.75

Let me know what you think!

Now directly working with the SVGs, rather than downloading them as
PNGs. This allows resizing to be lossless.

I've also added new options --shield_scale and --shield_geometry.

TODOs:
* Figure out why 90.5 is the density factor rather than 72 or 96
* Figure out why the composite is losing the alpha from the shield
* Figure out why Net:HTTP is choking on the SVG (why I used curb)
@HazAT
Copy link
Owner

HazAT commented Feb 16, 2017

Hey,
Thanks for the PR ...
That's pretty awesome that will improve the overall quality of the shields by a lot 🎉
I left a few review comments.


if icon.width > current_shield.width && !shield_no_resize
current_shield.resize "#{icon.width}x#{icon.height}<"
svg_shield = MiniMagick::Image.open(shield.path)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This var is unused

file.write(open(url).read)
file.close
end
Curl::Easy.download(url, file_name)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be ok ... but I am not sure if the download fails the above rescue OpenURI::HTTPError => error will catch the error then ... please check that


png_shield = MiniMagick::Image.open(new_path)

result = composite(result, png_shield, alpha_channel, shield_gravity || "north", shield_geometry)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could remove the png_shield and directly pass MiniMagick::Image.open(new_path)

@HazAT
Copy link
Owner

HazAT commented Mar 13, 2017

@jamesdaniels I had some time and merged it locally, thx again for your PR 🥇
Update to 0.8.0

@HazAT HazAT closed this Mar 13, 2017
@jamesdaniels
Copy link
Contributor Author

Thanks and glad it made it in! Sorry didn't have time to address your feedback myself.

@jamesdaniels jamesdaniels deleted the working_with_svgs branch March 13, 2017 19:27
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

Successfully merging this pull request may close these issues.

None yet

2 participants