-
Notifications
You must be signed in to change notification settings - Fork 276
Support floating point viewbox in resvg binary #875
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
base: main
Are you sure you want to change the base?
Conversation
Avoid rounding sizes for images until creating the pixmap, and always use the ceil for that to avoid truncating images. This requires a corresponding change to tiny-skia to add scale_by/scale_to_width/scale_to_height functions to tiny_skia_path::Size to match the implementations in IntSize.
@@ -526,13 +525,13 @@ fn parse_args() -> Result<Args, String> { | |||
let mut default_size = usvg::Size::from_wh(100.0, 100.0).unwrap(); | |||
if let (Some(w), Some(h)) = (args.width, args.height) { | |||
default_size = usvg::Size::from_wh(w as f32, h as f32).unwrap(); | |||
fit_to = FitTo::Size(w, h); | |||
fit_to = FitTo::Size(w as f32, h as f32); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense for these arguments to themselves be floats? I think probably not, because they need to be positive and finite. But creating a massive image can still cause issues.
don't want to be annoying, but - ping! I'm still waiting on this so I can stop having inkscape as a dependency in my software =w= |
a-another ping 😓 sorry… impatient |
What’s missing for this? Just a bump in tiny skia? |
That's my understanding, yes. Once that's available then we can bump dependencies here (and consider a resvg release?). The only question is if it's useful to have width and height directly parsed from the command line as f32 as @DJMcNab has pondered. If you used |
Thanks, will do my best to take a closer soon. Sorry for the delay. |
# Conflicts: # crates/resvg/Cargo.toml # crates/usvg/Cargo.toml
Had to update two test cases, but only with very minimal pixel differences. I added a git dependency now because I think it'll still be a while until the next release. I think we can merge this as is, @DJMcNab wdyt? |
I'd be hesistant to land a git dependency here - I'm not sure when we would otherwise made a Tiny Skia release, so using this to force that is probably worthwhile. @jermy, would you be willing to help here? The way to unblock a Tiny Skia release is to go through the changes since the last release, and check if any of them are breaking. At the same time, it would be great to add them to the Unreleased section of the changelog. If they aren't, we should be able to make a bump fairly easily. Unfortunately, if there are breaking changes, the path forward is less clear. |
It is a breaking change I think, because new methods were added in the PR to |
Avoid rounding sizes for images until creating the pixmap, and always use the ceil for that to avoid truncating images. This fixes #810.
This requires a corresponding change to tiny-skia to add scale_by/scale_to_width/scale_to_height functions to tiny_skia_path::Size to match the implementations in IntSize.
This won't compile as-is, since it depends on linebender/tiny-skia#146 or a similar change.