4bb4 / implot-rs

Rust bindings to https://github.com/epezent/implot
Apache License 2.0
54 stars 22 forks source link

Shorter way to set ranges #15

Open dbr opened 3 years ago

dbr commented 3 years ago

I may be missing something in the API, but currently to set the ranges of the X and Y axes requires quite a long chunk of code

Plot::new()
    .x_limits(
        &ImPlotRange { Min: 0.0, Max: 2.0 },
        implot::Condition::Always,
    )
    .y_limits(
        &ImPlotRange {Min: 0.0, Max: 20.0},
        implot::YAxisChoice::First,
        implot::Condition::Always,
    )

..which is quite verbose

One simple change which would make things more consistent with imgui-rs would to take the ranges at [f32; 2] (and convert them to ImPlotRange internally if required), this would shorten the example to:

Plot::new("Example")
    .x_limits([0.0, 2.0], implot::Condition::Always)
    .y_limits(
        [0.0, 2.0],
        implot::YAxisChoice::First,
        implot::Condition::Always,
    )

I always wonder if the y_limits could be by for the first Y limit, and a second method for the secondary Y axis if present,

Plot::new("Example")
    .y_limits([0.0, 2.0], implot::Condition::Always)
    .y2_limits([0.0, 2.0], implot::Condition::Always)
    .y3_limits([0.0, 2.0], implot::Condition::Always)

..or something along those lines (not too sure about the specific naming, would probably be easier to find if y_limits_2 etc)

Only semi-related, but Plot::new().size(100.0, 200.0) could also use this same convention for passing sizes as an array, which would fit more neatly with imgui-rs:

let full_area = ui.content_region_avail();
Plot::new("Example")
    .size(full_area[0], full_area[1])

could become just:


Plot::new("Example")
    .size(ui.content_region_avail())
`
benmkw commented 3 years ago

I did not look into the specific code but I think this could be used instead https://doc.rust-lang.org/std/ops/struct.Range.html

or a From/ Into impl could be added.

4bb4 commented 3 years ago

As of https://github.com/4bb4/implot-rs/commit/f2a4c6a3d8919ec3438631873ce6a9f94135089c, one can do this almost as conveniently as listed in the issue, as demo-ed in https://github.com/4bb4/implot-rs/blob/f2a4c6a3d8919ec3438631873ce6a9f94135089c/implot-examples/examples-shared/src/line_plots.rs#L342.

I'll work on it a bit more to make it even more convenient and then create a release. The next few weeks are a bit busy though, so if you're in a rush, maybe fork the repo and point to that for the time being.

4bb4 commented 3 years ago

@dbr I finally got some time again to work on this - I added convenience functions y1_limits through y3_limits for direct limit setting, and improved the API so that you can directly pass in anything that is convertible to ImPlotRange, which means your example code from above should also work now. The code is currently on master. Let me know if that suits your needs, then I'll create a release (I'd also like to use that code in projects and not point to git revisions :sweat_smile:). I can't upgrade to a newer imgui-rs yet before I fix that WGPU issue, but at least we get a better API in the meantime.

dbr commented 3 years ago

The new API looks great! Can't properly test yet as I'm using it in project with imgui-rs 0.7 (via the 13-upgrade-to-imgui-rs-0.7 branch), but roughing in the new API lets rustfmt condense my 8 lines of "set Y axis range" into 1 which is exactly what I was hoping for :partying_face:

4bb4 commented 3 years ago

I've now also merged those API changes over into the branch you mention. I've done more debugging to try and fix the issue I'm referring to in https://github.com/4bb4/implot-rs/issues/13#issuecomment-798972470, but have not yet been able to resolve the problem.