Michael Buschbeck commented on merge request !2273 at Inkscape / inkscape

Michael Buschbeck
2020-09-13T13:53:20Z

I've dug a bit deeper and found that redraw_area(Geom::Coord x0, y0, x1, y1) sometimes (actually, quite often) got nan in x1 and y1 – that my updated code just luckily happened to map to std::numeric_limits<int>::max().

This line is where the nans come from:

void
Canvas::redraw_area(Geom::Rect& area)
{
// Might need to round outward.
redraw_area(area.min().x(), area.min().y(), area.min().x()+area.width(), area.min().y()+area.height());
}

...because even though area.width() is inf as expected, -inf + inf is mathematically undefined, hence nan. Oops!

That method's circuitous way of calculating x1 and y1 from min plus width/height (to "round outward") doesn't make sense with my update's new Geom::Coord overload anymore, though. This is much simpler and works:

void
Canvas::redraw_area(Geom::Rect& area)
{
redraw_area(area.left(), area.top(), area.right(), area.bottom());
}

...and I've moved the rounding-outward to the Geom::Coord overload:

void
Canvas::redraw_area(Geom::Coord x0, Geom::Coord y0, Geom::Coord x1, Geom::Coord y1)
{
// Handle overflow during conversion gracefully.
// Round outward to make sure integral coordinates cover the entire area.

constexpr Geom::Coord min_int = static_cast<Geom::Coord>(std::numeric_limits<int>::min());
constexpr Geom::Coord max_int = static_cast<Geom::Coord>(std::numeric_limits<int>::max());

redraw_area(
static_cast<int>(std::floor(std::min(std::max(x0, min_int), max_int))),
static_cast<int>(std::floor(std::min(std::max(y0, min_int), max_int))),
static_cast<int>(std::ceil(std::min(std::max(x1, min_int), max_int))),
static_cast<int>(std::ceil(std::min(std::max(y1, min_int), max_int))));
}

I've also added some integer clamping to the int overload that makes sure that the Cairo::RectangleInt doesn't wrap around to negative width.