plotters-rs / plotters

A rust drawing library for high quality data plotting for both WASM and native, statically and realtimely 🦀 📈🚀
https://plotters-rs.github.io/home/
MIT License
3.85k stars 278 forks source link

[BUG] `draw_series` with `BitMapBackend` and `LineSeries` hangs in very specific scenario #475

Open matteopolak opened 1 year ago

matteopolak commented 1 year ago

Describe the bug Under a very specific set of parameters, the call to draw_series hangs while still consuming a decent chunk of CPU (14% with a Ryzen 9 6900HS). The scenario is extremely specific, changing the X axis range (out of a certain range), removing any of the three points, changing the stroke width to 1, and changing the dimensions of the plot all fix the issue.

To Reproduce

use std::error::Error;

use plotters::{
    prelude::{BitMapBackend, ChartBuilder, IntoDrawingArea},
    series::LineSeries,
    style::{RGBColor, ShapeStyle},
};

fn main() -> Result<(), Box<dyn Error>> {
    // Try using 750x500 and it will work
    const PIXELS: usize = 750 * 389;
    const BUF_LEN_RGB: usize = 3 * PIXELS;

    let mut buffer = vec![0; BUF_LEN_RGB];
    // Try using 750x500 and it will work *change PIXELS as well*
    let backend = BitMapBackend::with_buffer(&mut buffer, (750, 389)).into_drawing_area();

    // Try removing any of these three points and it will work
    let points = [
        (1_683_150_612_458, 1_339_071_534),
        (1_683_150_669_670, 1_039_071_534),
        (1_683_150_842_108, 1_041_566_132),
    ];

    // Try changing the upper limit of the y_spec to 1_000_000_000 and it will work
    let mut chart =
        ChartBuilder::on(&backend).build_cartesian_2d(0u64..2_000_000_000_000, 0u64..2_000_000_000)?;

    let colour: ShapeStyle = RGBColor(0, 0, 0).into();

    println!("pre-hang");

    // Change the `stroke_width` to 1 and it will work
    chart.draw_series(LineSeries::new(points, colour.stroke_width(2)))?;

    panic!("did not hang");
}

Version Information Tested on nightly and stable

rustc 1.71.0-nightly (8b4b20836 2023-05-22)
rustc 1.69.0 (84c898d65 2023-04-16)

I'm using the next-release-devel branch at commit 3fe48e3debf13cf053b04a9315e2749b428f4220 (latest at the time of writing), since the release branch crashes with an alignment issue.


Edit: the (I presume) overflow is coming somewhere from here: https://github.com/plotters-rs/plotters/blob/next-release-devel/plotters-backend/src/rasterizer/path.rs#L147

Edit: The culprit is somewhere in these few lines: https://github.com/plotters-rs/plotters/blob/next-release-devel/plotters-backend/src/rasterizer/path.rs#L64-L65 . x and y and set to f64::INFINITY and all of the if statements are skipped, resulting in f64::INFINITY.round() as i32, which is 2147483647.

The following values are computed in compute_polygon_vertex:

let a0 = 0.0000000000000000;
let b0 = -0.0000000000000000;
let c0 = 2.0000000000000000;
let a1 = 1.0000000000000000;
let b1 = -1.0000000000000000;
let c1 = 0.0000000000000000;

Which results in (a0 * b1 - a1 * b0).abs() being 0 (which is less than f64::EPSILON) and a_t.0 * b_t.1 - a_t.1 * b_t.0 being 0 (which is both not less than 0.0 and not greater than 0.0), skipping everything.

I am not entirely sure what the purpose of this exactly is, but I would be more than happy to submit a PR for it once I figure it out.

matteopolak commented 1 year ago

I forked the branch and added a temporary fix that seems to display fine, I'm sure there's a more permanent fix to the issue though.

Replaced https://github.com/plotters-rs/plotters/blob/next-release-devel/plotters-backend/src/rasterizer/path.rs#L87 with:

if x != f64::INFINITY && y != f64::INFINITY {
    buf.push((x.round() as i32, y.round() as i32));
}
juntyr commented 8 months ago

I just came across this bug as well and can confirm that @matteopolak's fix works - thank you so much for finding it and saving me lots of debugging time! A slightly nicer version of the fix is:

if x.is_finite() && y.is_finite() {
    buf.push((x.round() as i32, y.round() as i32));
}