`Rect::round_out` is either incorrect or documented wrong

[danieldg]

The documentation claims that this function:

Converts into an IntRect rounding outwards.

However, the following code prints Some(IntRect { x: 1, y: 1, width: 2, height: 2 }) which is rounding the right and bottom edges inwards:

let r = Rect::from_ltrb(1.8, 1.8, 3.1, 3.1).unwrap();
println!("{:?}", r.round_out());

src/scan/path_aa.rs uses the "correct" implementation and calls out that Rect::round_out returns the wrong result.

I would suggest that the current implementation is useless, and replace it with the one from path_aa.rs that actually rounds out.

[RazrFalcon]

Hm... I'm not sure what is the correct result here should be. We either have Rect::from_xywh(1.8, 1.8, 1.3, 1.3) rounded out to IntRect::from_xywh(1, 1, 2, 2) or IntRect::from_xywh(1, 1, 3, 3). The jump to 3 is kinda questionable to me. But this is what Skia does as well, so I guess we have to follow.

[RazrFalcon]

And of course it affects rendering, which makes it even more complicated.

[RazrFalcon]

Even worse, it somehow causes a different output each time... Will take a closer look later.

[danieldg]

I believe the goal of round_out (at least what I was using it for) is to turn bounding boxes into "all the pixels that could be touched by this shape". When looked at in terms of xywh, it seems unintuitive, but when looked at in terms of ltrb it's pretty clear.

Also, I looked at path_aa again, and the implementation there is overly complex. You can get away with a much simpler:

IntRect::from_ltrb(
    orig.left().floor() as i32,
    orig.top().floor() as i32,
    orig.right().ceil() as i32,
    orig.bottom().ceil() as i32,
)

Checks for overflowing i32::MAX on the original rectangle would be needed if you want to return None in that case, otherwise thanks to https://doc.rust-lang.org/stable/reference/expressions/operator-expr.html#numeric-cast it will just use that value as the edge, which seems reasonable to me.

[RazrFalcon]

Yes, we should switch the current implementation to from_ltrb internally.

Also, I looked at path_aa again, and the implementation there is overly complex.

Patches are welcome. tiny-skia is 16 KLOC. There would always be ways to improve it.