Repository navigation
Support floating point viewbox in resvg binary - #875
Conversation
| 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.
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.
There was a problem hiding this comment.
Let's ignore this for now, if you want to render such big images you are going to have other problems. 😄
|
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. |
|
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 |
|
hey, hate to bump, but what is the course of action from here? is there any way I can help get this through faster? I would really like to have this so I can replace inkscape with resvg as my primary SVG rasterizer… |
|
Sorry about that. :( The reason nothing is happening is basically 1) No one's really been actively developing resvg in recent months (I guess mostly because of being busy with other stuff) and 2) tiny-skia is basically unmaintained at this point (we have our own new CPU renderer that we are developing), and therefore there's little motivation to add new features and updates to the crate... So things are basically not moving at all at the moment. 😅 Hopefully this can change soon, but hard to say. :/ |
|
I'm wondering though, is that really such a critical thing to have? To me it seems like a pretty small edge case, no? |
|
in my use-case, a lot of the images I use a rasterizer on are small enough (in SVG units) to notice discrepancies at larger scales (in pixels) the only reason I even noticed is because it really fucked with a bunch of my images. for example:
↑ one of the less bad examples. I don't have time to look for a worse one rn, but notice how the bottom and right sides are cut off a bit in the resvg render |
|
This is unblocked now. The reason it stalled (needing I confirmed the bug is still live on main. Red extends to pixel 60, i.e. 1.2 ÷ 2.0, so the 1.5 viewBox is being rounded up. I rebased locally to check it works: dropped the git pins, resolved the conflicts, builds clean and all 1,793 tests pass. Afterwards red extends to pixel 80 (1.2 ÷ 1.5), the corner is blue, and there's no green anywhere. Two things worth noting from the rebase:
@jermy are you still up for finishing this? Happy to push the rebase if it's easier. @RiedleroD , sorry it took so long. |
|
all that matters is that it's getting done eventually ^^ I'm just happy it's not being dropped |
I'm fairly busy at the moment - I don't have any issue with you taking this and doing that rebase and rework as you see fit. Thank you. |
Hey @luisbg are you a new maintainer to resvg? |
|
I am just helping, not a maintainer. Let me ask when a new version will be released. |
|
No release scheduled, unless there is something important. |
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.
5f52038 to
f9bc393
Compare
|
@jermy Thank you again, and very sorry that this took so long. :( |
|
thanks everyone for making this happen ^^ once this lands in a release I can finally go back to using resvg for my image converter script |
6 participants
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.