plotters icon indicating copy to clipboard operation
plotters copied to clipboard

Advanced Flexbox Layout

Open siefkenj opened this issue 3 years ago • 13 comments

Here's some initial work towards ideas pointed out in #258

Most of the work so far is in nodes.rs, which builds the layout and interfaces with the stretch library. I am now working on ChartLayout which should be similar to ChartBuilder but with additional flexibility. However, I could use some pointers...

Something with how I am doing error handling in ChartLayout isn't right. In examples/layout.rs, if I use ? to unwrap the errors, I get a compile error:

returns a value referencing data owned by the current function

which I assume is because the error is somehow related to the DrawingArea to which I hold a mutable reference. However, I tried to mirror your code closely, so I'm not sure what's wrong. (If I use .unwrap() instead of ? it works.) Any ideas?

siefkenj avatar May 26 '21 03:05 siefkenj

Codecov Report

Merging #259 (8cc45ea) into master (b054a80) will increase coverage by 2.39%. The diff coverage is 82.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #259      +/-   ##
==========================================
+ Coverage   68.53%   70.92%   +2.39%     
==========================================
  Files          61       64       +3     
  Lines        4322     5204     +882     
==========================================
+ Hits         2962     3691     +729     
- Misses       1360     1513     +153     
Impacted Files Coverage Δ
src/chart/builder.rs 75.78% <ø> (ø)
src/coord/mod.rs 50.00% <ø> (ø)
src/coord/ranged1d/combinators/logarithmic.rs 58.33% <ø> (ø)
src/coord/ranged1d/mod.rs 92.59% <ø> (ø)
src/coord/ranged2d/cartesian.rs 78.26% <ø> (ø)
src/drawing/area.rs 91.58% <0.00%> (-1.43%) :arrow_down:
src/style/palette.rs 0.00% <ø> (ø)
src/style/text.rs 30.66% <ø> (ø)
src/chart/layout/mod.rs 75.34% <75.34%> (ø)
src/element/basic_shapes.rs 94.44% <88.88%> (-0.43%) :arrow_down:
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b054a80...8cc45ea. Read the comment docs.

codecov[bot] avatar May 26 '21 03:05 codecov[bot]

I think I figured it out...I switch to concrete error types instead of dyn std::error::Error and now it works. I don't know why the dyn Error was causing issues, though...

siefkenj avatar May 26 '21 16:05 siefkenj

@38 The bulk of the API should be implemented now. Please let me know what you think.

One question I had is about the LayoutError enum. I modified it so that it could encapsulate errors for ChartLayout. It doesn't look like LayoutError is used anywhere in plotters or any of the plotters backends. Does anyone use this? Is it a breaking change?

So far I have implemented labels and extents, but not yet the passdown to the sub-region (chart_area) where plotting actually takes place. I plan to do something similar to ChartBuilder and just pass back a drawing area restricted to that location. The only thing I really need to work out is how to figure out if a label will spill into the margin so that I can automatically adjust the margin size.

Here's an example of some ouput of the current code. Sorry for the huge codedump!

layout2

use plotters::prelude::*;

const OUT_FILE_NAME: &'static str = "plotters-doc-data/layout2.png";

fn main() -> Result<(), Box<dyn std::error::Error>> {
    const W: u32 = 600;
    const H: u32 = 400;
    let root = BitMapBackend::new(OUT_FILE_NAME, (W, H)).into_drawing_area();
    root.fill(&full_palette::WHITE)?;

    let mut chart = ChartLayout::new(&root);
    chart
        .set_chart_title_text("Chart Title")?
        .set_top_label_text("A label at the top")?
        .set_chart_title_style(("serif", 60.).into_font().with_color(&RED))?
        .set_left_label_text("Left label")?
        .set_right_label_text("Right label")?
        .set_bottom_label_text("Bottom label")?
        .set_bottom_label_margin(10.)?
        .set_top_label_margin(10.)?
        .draw()?;

    let extent = chart.get_chart_area_extent()?;
    root.draw(&Rectangle::new(extent.into_array_of_tuples(), &BLUE))?;
    let extent = chart.get_chart_title_extent()?;
    root.draw(&Rectangle::new(extent.into_array_of_tuples(), &BLUE))?;

    Ok(())
}

siefkenj avatar May 27 '21 18:05 siefkenj

This should be ready now. I currently I just pass a sub-drawing area where the chart area is to allow for graphing. I tried to figure out how to get tick locations so I could compute their size and automatically resize the chart, but it was just too hard and I couldn't find out the types.

For now we get an output that looks like

layout2

as a result of examples/layout.rs. I think maybe the automatic sizing of the tick labels should come in a separate PR, since this one s already getting pretty long.

Please let me know what you think!

siefkenj avatar May 28 '21 20:05 siefkenj

Hi Jason,

Thanks for the PR, I am so excited to try this out. The general idea looks really interesting and I feel this actually makes plotters easier to use.

In general, I think the API workflow is a little bit odd. Currently, if my understand is correct, we need to build a ChartLayout and then pull out the chart_area from the layout then configure the chart context. But could we just build a chart context on top of ChartLayout?

I am pretty excited about what the current PR have, but I probably have some idea to tweak the workflow later. It looks good to me with the API if we can put it behind a feature switch.

Also I realized there are few warning introduced by the PR, we need to fix them.

Also, please don't mind I was not able to see this during the last weekend.

Thanks for this awesome PR! Cheers!

38 avatar Jun 01 '21 18:06 38

Thanks for the review! I have a few additional changes related to automatic axis labeling. Do you want those changes pushed to this branch, or to a separate one?

In general, I think the API workflow is a little bit odd. Currently, if my understand is correct, we need to build a ChartLayout and then pull out the chart_area from the layout then configure the chart context. But could we just build a chart context on top of ChartLayout?

I tried to do this, but I got into a type/lifetime soup and couldn't figure out how to make it work, so I did what was easiest first. Maybe you can help me figure out how to get it working.

In my new work, I have automatic axis labeling work, with space being allocated for the axis labels. I couldn't figure out how to do this with the existing axis format, so I'll need some help figuring that out.

I am pretty excited about what the current PR have, but I probably have some idea to tweak the workflow later. It looks good to me with the API if we can put it behind a feature switch.

Of course! I fully expect you to tweak things :-)

siefkenj avatar Jun 02 '21 02:06 siefkenj

I have addressed most of your comments and included the additional code for auto-sizing the label areas (so that tick marks don't fall off the edge of the canvas). Unfortunately, to do that I had to create a new Axis type, because I could notfigure out how to extract the necessary information from Ranged.

Here's an animation of what it can do:

layout2

Notice how room is automatically allocated for the .0 on the 3.0. When the graph starts scrolling, the .0 doesn't fall off the edge and so the axis runs right up to the edge of the chart instead (it looks like a flaw in the animation, but it actually isn't!)

In order to achieve this, I needed to compute the extents of all the tick marks and tick labels. I then union them together and resize the tick_label_areas accordingly. If an extent spills to the right/left/top/bottom, I check that area to make sure there is already enough space allocated. If there isn't, I allocate more space.

This process is iterative because, for example, if a very wide left tick label is allocated, then this will affect the width of the x-axis and will potentially affect how many tick marks there are.

Right now, I only implemented everything in f32. You may want to replace the axis stuff with your Ranged objects, so I will refrain from making it general, for now.

siefkenj avatar Jun 02 '21 18:06 siefkenj

For reference, here is what it looks like if a right label also exists. Since adequate space is always allocated, there's no bouncing!

layout2

siefkenj avatar Jun 02 '21 18:06 siefkenj

because I could notfigure out how to extract the necessary information from Ranged.

Just don't get the point. Could you please what is bouncing in the first animation ? Sorry for the ignorance

38 avatar Jun 02 '21 22:06 38

because I could notfigure out how to extract the necessary information from Ranged.

Just don't get the point. Could you please what is bouncing in the first animation ? Sorry for the ignorance

On the first GIF, watch the "Sine" label carefully as the number 4.0 scrolls by on the x axis. The Sine label is always aligned to the right of the chart. You can see it jump as the 4.0 passes by. That is the layout algorithm making room for the .0 on the right side.

siefkenj avatar Jun 02 '21 23:06 siefkenj

Ah, I see. But I'm not sure what information you need to avoid this. Probably I need to dig into your code a little bit.

38 avatar Jun 03 '21 01:06 38

Ah, I see. But I'm not sure what information you need to avoid this. Probably I need to dig into your code a little bit.

It's not a bug...it's how it is supposed to work.

The information I need relates to the separate issue of computing the extents of the axis labels. I couldn't figure out how to get the list of labels a Ranged was going to print, so I had to make a new struct, Axis, where I could figure out which tick labels were going to be printed so I could compute their sizes.

siefkenj avatar Jun 03 '21 01:06 siefkenj

Ah, I see. But I'm not sure what information you need to avoid this. Probably I need to dig into your code a little bit.

It's not a bug...it's how it is supposed to work.

The information I need relates to the separate issue of computing the extents of the axis labels. I couldn't figure out how to get the list of labels a Ranged was going to print, so I had to make a new struct, Axis, where I could figure out which tick labels were going to be printed so I could compute their sizes.

See example https://github.com/38/plotters/blob/b82174aa1ac758ca6afb22a555d8732209fb2bb8/src/chart/builder.rs#L179

Use X:AsRangedCoord instead of Range<f32> for coord spec will allowing you get tick labels.

    x_spec.into().key_points(BoldPoints(n))

where n is the maximum number of tick marks.

I believe if you make ChartLayout::build_cartesian_2d generic, you are able to retrieve tick marks information.

38 avatar Jun 03 '21 17:06 38