RFC: checked_trunc(f64) -> Result<i32>
When checking the tarball of crashes in #588 (closed), I noticed that rsvg-convert fails on this:
<?xml version="1.0" encoding="UTF-8"?>
<svg ion="1.1" baseProfile="b" id="svg-root"
width="100%" height="100%" viewBox="0 0 4822222222222222222220 360"
xmlns="http://www.w3.org/2000/svg" xmlns:xlink="httk">
<g>
<g opacity="0.5">
<rect x="60" y="230" width="80" height="40" fill="+0000ff" opacity=".5"/>
<rect x="71" y="240" width="80" height="40" fill="#00ff00" opacity=".5"/>
</g>
</g>
</svg>
It produces:
(rsvg-convert:30213): librsvg-WARNING **: 12:24:29.842: cannot render on a cairo_t with a failure status (status="invalid value (typically too big) for the size of the input (surface, pattern, etc.)")
This is because rsvg_handle_get_dimensions_sub()
returns a negative number, due to the huge viewBox
, and rsvg-convert creates an image surface with negative dimensions.
The culprit is this; note that RsvgDimensionData
has i32
values (strictly c_int
), while RsvgRectangle
is f64
:
get_dimensions_sub -> RsvgDimensionData
fishy conversion in the call to size_callback
get_geometry_sub -> RsvgRectangle
gets the Svg::get_size() (bug is here)
or calls get_node_geometry_with_viewport() -> cairo::Rectangle
Then, Svg::get_size()
does this:
impl Svg {
pub fn get_size(&self, values: &ComputedValues, dpi: Dpi) -> Option<(i32, i32)> {
let (w, h) = self.get_unnormalized_size();
match (w, h, self.vbox) {
(w, h, Some(vbox)) => {
let params = ViewParams::new(dpi.x(), dpi.y(), vbox.0.width(), vbox.0.height());
Some((
w.normalize(values, ¶ms).round() as i32,
h.normalize(values, ¶ms).round() as i32,
))
}
The w.normalize(...).round as i32
is the problem. It has a big positive number, which doesn't fit in i32, and gets cast as a negative integer. This is incorrect. In this case we don't want a saturating cast; we actually want to return "this is too big to fit in the supported range".
I'll start refactoring this to push f64 values as far up in the code as possible, and then have the legacy C API decide what to do if it gets values that are too large to fit.
CC @pborelli for the following reason. While Rust decides what to do with respect to checked numeric casts, what do you think of adding a CheckedTrunc
trait, something like this:
trait CheckedTrunc<T> {
fn checked_trunc(self) -> Result<T, ()>;
}
impl CheckedTrunc<i32> for f64 { ... }
And then having the equivalent to convert from Rect
to IRect
?
(Simon Sapin just told me that something like this would be contemplated in this pre-RFC, in the "Future possibilities" section ("fallible approximate").