Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor issues in plotters output #179

Closed
bluenote10 opened this issue Oct 15, 2024 · 4 comments
Closed

Minor issues in plotters output #179

bluenote10 opened this issue Oct 15, 2024 · 4 comments

Comments

@bluenote10
Copy link
Contributor

I noticed that the plotters integration has some minor rendering issues when compared to the "reference" plotters backend. Given the two minimal reproduction examples:

Example using cushy/kludgine backend
// Note that this requires to enable the corresponding plotters feature,
// i.e., default-feature = false will not work.
use cushy::widget::MakeWidget;
use cushy::widgets::Canvas;
use cushy::Run;
use plotters::prelude::*;

pub fn basic_plot<A>(
    root: &DrawingArea<A, plotters::coord::Shift>,
) -> Result<(), Box<dyn std::error::Error>>
where
    A: DrawingBackend,
    A::ErrorType: 'static,
{
    root.fill(&WHITE)?;
    let mut chart = ChartBuilder::on(&root)
        .caption("y=x^2", ("sans-serif", 50).into_font())
        .margin(5)
        .x_label_area_size(30)
        .y_label_area_size(30)
        .build_cartesian_2d(-2f32..2f32, -0.1f32..1f32)?;

    chart.configure_mesh().draw()?;

    chart
        .draw_series(LineSeries::new(
            (-100..=100).map(|x| x as f32 / 50.0).map(|x| (x, x * x)),
            &RED,
        ))?
        .label("y = x^2")
        .legend(|(x, y)| PathElement::new(vec![(x, y), (x + 20, y)], &RED));

    chart
        .configure_series_labels()
        .background_style(&WHITE.mix(0.8))
        .border_style(&BLACK)
        .draw()?;

    Ok(())
}

fn plotters() -> impl MakeWidget {
    Canvas::new({
        move |context| {
            basic_plot(&context.gfx.as_plot_area()).unwrap();
        }
    })
}

fn main() -> cushy::Result<()> {
    plotters().run()
}
Example using plain plotters backend
use plotters::prelude::*;

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let root = BitMapBackend::new("basic_plot.png", (640, 480)).into_drawing_area();

    root.fill(&WHITE)?;
    let mut chart = ChartBuilder::on(&root)
        .caption("y=x^2", ("sans-serif", 50).into_font())
        .margin(5)
        .x_label_area_size(30)
        .y_label_area_size(30)
        .build_cartesian_2d(-2f32..2f32, -0.1f32..1f32)?;

    chart.configure_mesh().draw()?;

    chart
        .draw_series(LineSeries::new(
            (-100..=100).map(|x| x as f32 / 50.0).map(|x| (x, x * x)),
            &RED,
        ))?
        .label("y = x^2")
        .legend(|(x, y)| PathElement::new(vec![(x, y), (x + 20, y)], &RED));

    chart
        .configure_series_labels()
        .background_style(&WHITE.mix(0.8))
        .border_style(&BLACK)
        .draw()?;

    root.present()?;

    Ok(())
}

The cushy/kludgine based output is:

image

The plotters based output is:

basic_plot

The main issue is that text labels are wrongly aligned. Minor issues are that the line anti-aliasing doesn't look as good, and that the grid get aliased instead of being pixel perfect (but likely that is expected if the size of the drawing area doesn't allow for integer multiples of the grid size).

Since I'm not entirely sure if this a cushy or kludgine issue (or even further upstream?), I decided to report it on cushy. Do you generally prefer such issues being raised further upstream if possible?

Note that the lack of clipping is an upsteam issue in plotters plotters-rs/plotters#622

@ecton
Copy link
Member

ecton commented Oct 16, 2024

Thank you for reporting this! I knew I had under-tested some of the plotters code, but some of that label drawing is pretty bad too.

The fuzziness is almost certainly due to me trying to automatically scale the plotters output based on DPI scaling. It seems like the more correct approach is going to be to let it be pixel-perfect rendering, and require that the plotters-integrator worry about dpi scaling themselves.

As for where to report the issue, I'm happy with most issues being reported on this repository -- it's where most users might actually encounter a bug. I can always create a separate issue and link them if I think someone else might tackle it, but right now I'm usually the person to fix something in Kludgine!

@ecton
Copy link
Member

ecton commented Oct 18, 2024

image

Things are looking better! I can't seem to figure out how the labels are sized -- I put a lot of logging code in, and I notice plotters measures the chart caption, but not the labels. My picture shows two series because I was trying to see if that would cause it to measure the text... but it didn't.

The issues were not what I originally suspected. It was that to draw a pixel-perfect 1px wide line, the coordinates need to be offset because lyon generates a rectangle centered on the path coordinates. To ensure the 1px wide line isn't split across two lines with Kludgine's subpixel rendering support, the coordinates need to be offset by 1/2 a pixel, or more generalized, half of the stroke width.

Beyond that, the text drawing code didn't support anchoring or alignment or rotation. This plot doesn't rotate text, but in theory everything is handled correctly now -- don't hesitate to let me know if you run into other issues! If you test the main branch, be sure to use cargo update to get Kludgine's changes.

@ecton
Copy link
Member

ecton commented Oct 18, 2024

Actually I'm reopening this because I'd like to have you weigh in on what's happening with the labels, if you know more about how plotters works.

@ecton ecton reopened this Oct 18, 2024
@bluenote10
Copy link
Contributor Author

Thanks for the fix!

Actually I'm reopening this because I'd like to have you weigh in on what's happening with the labels, if you know more about how plotters works.

In my opinion, this looks pretty much what I would expect now, so closing the issue should be fine. In case anything else comes up I'll let you know. I also don't have much expertise with plotters, mainly just trying things out so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants