linebender / kurbo

A Rust library for manipulating curves
Apache License 2.0
701 stars 67 forks source link

Imprecise BezPath::bounding_box on some paths? #223

Closed RazrFalcon closed 2 years ago

RazrFalcon commented 2 years ago

Hi! A user reported a bug in resvg that relates to Path bbox calculation. While resvg doesn't use BezPath::bounding_box directly, it relies on kurbo::CubicBez::bounding_box (like this). And it seems like BezPath::bounding_box is affected in a similar way.

Here is a sample SVG:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 210 297">
    <path d="M 13.68752 15.483618 C 6.1165633 12.08989 4.412796 12.770705 3.231074 15.322257 C 2.0493531 17.873812 -2.4950337 7.8268027 2.2409132 4.4050307 C 6.97686 0.98325753 12.049765 -0.34644848 14.740673 0.3714725 C 17.431581 1.0893925 19.921005 4.5336003 20.673872 6.8366413 C 21.42674 9.139683 21.344765 14.534013 20.593191 17.385418 C 19.84162 20.236822 16.858624 16.604939 13.68752 15.483618 Z"/>
</svg>

I've tried calculating a bbox of this path using various libraries and it seems like tiny-skia and CoreGraphics return the same results, while kurbo is different.

Results:

Kurbo (master):

let mut path = BezPath::new();
path.move_to(Point::new(13.68752, 15.483618));
path.curve_to(Point::new(6.1165632, 12.08989), Point::new(4.4127962, 12.770705), Point::new(3.2310742, 15.322257));
path.curve_to(Point::new(2.0493532, 17.873811), Point::new(-2.4950338, 7.8268025), Point::new(2.2409132, 4.4050305));
path.curve_to(Point::new(6.9768602, 0.98325751), Point::new(12.049765, -0.34644849), Point::new(14.740673, 0.37147251));
path.curve_to(Point::new(17.431580999999998, 1.0893925), Point::new(19.921006, 4.5336005), Point::new(20.673872, 6.8366415));
path.curve_to(Point::new(21.426741, 9.1396825), Point::new(21.344765, 14.534013), Point::new(20.593192, 17.385417));
path.curve_to(Point::new(19.841618999999998, 20.236823), Point::new(16.858624, 16.604938), Point::new(13.68752, 15.483618));
path.close_path();
let bbox = path.bounding_box();
println!("{} {} {} {}", bbox.x0, bbox.y0, bbox.width(), bbox.height());
// 0.12983728305837466 0.1829278818272776 21.06950589682364 18.271265147354878

tiny-skia (master) (aka Skia):

let mut pb = PathBuilder::new();
pb.move_to(13.68752, 15.483618);
pb.cubic_to(6.1165632, 12.08989, 4.4127962, 12.770705, 3.2310742, 15.322257);
pb.cubic_to(2.0493532, 17.873811, -2.4950338, 7.8268025, 2.2409132, 4.4050305);
pb.cubic_to(6.9768602, 0.98325751, 12.049765, -0.34644849, 14.740673, 0.37147251);
pb.cubic_to(17.431580999999998, 1.0893925, 19.921006, 4.5336005, 20.673872, 6.8366415);
pb.cubic_to(21.426741, 9.1396825, 21.344765, 14.534013, 20.593192, 17.385417);
pb.cubic_to(19.841618999999998, 20.236823, 16.858624, 16.604938, 13.68752, 15.483618);
pb.close();
let path = pb.finish().unwrap();
let bbox = path.bounds();
println!("{} {} {} {}", bbox.x(), bbox.y(), bbox.width(), bbox.height());
// -2.4950337 -0.34644848 23.921774 20.583271

CoreGraphics on mac 12.4

let path = CGMutablePath()
path.move(to: CGPoint(x: 13.68752, y: 15.483618))
path.addCurve(to: CGPoint(x: 6.1165632, y: 12.08989), control1: CGPoint(x: 4.4127962, y: 12.770705), control2: CGPoint(x: 3.2310742, y: 15.322257))
path.addCurve(to: CGPoint(x: 2.0493532, y: 17.873811), control1: CGPoint(x: -2.4950338, y: 7.8268025), control2: CGPoint(x: 2.2409132, y: 4.4050305))

path.addCurve(to: CGPoint(x: 6.9768602, y: 0.98325751), control1: CGPoint(x: 12.049765, y: -0.34644849), control2: CGPoint(x: 14.740673, y: 0.37147251))
path.addCurve(to: CGPoint(x: 17.431580999999998, y: 1.0893925), control1: CGPoint(x: 19.921006, y: 4.5336005), control2: CGPoint(x: 20.673872, y: 6.8366415))
path.addCurve(to: CGPoint(x: 21.426741, y: 9.1396825), control1: CGPoint(x: 21.344765, y: 14.534013), control2: CGPoint(x: 20.593192, y: 17.385417))
path.addCurve(to: CGPoint(x: 19.841618999999998, y: 20.236823), control1: CGPoint(x: 16.858624, y: 16.604938), control2: CGPoint(x: 13.68752, y: 15.483618))
path.closeSubpath()
var bbox = path.boundingBox
print(bbox.minX, bbox.minY, bbox.width, bbox.height)
// -2.4950338 -0.34644849 23.9217748 20.58327149
bbox = path.boundingBoxOfPath
print(bbox.minX, bbox.minY, bbox.width, bbox.height)
// 0.920599560877915 0.7534006725439468 20.506141439122086 19.483422327456054
cmyr commented 2 years ago

Thanks, I don't have any intuition here so I defer to @raphlinus.

raphlinus commented 2 years ago

I will take a look, hopefully soon.

raphlinus commented 2 years ago

This looks like a correct bounding box to me. Is the problem that you're looking for the coarse bounding box, ie enclosing box including Bézier control points?

Screen Shot 2022-08-10 at 1 09 28 PM
RazrFalcon commented 2 years ago

Ok, nevermind, I was really dumb. I misunderstood the issue and messed up points order in CGPath (the first point should be an end point and not p1...). The bug was in my code and also very dumb.

Sorry for bothering you.