Plot borrowed data

[GunnarMorrigan]

• Keep your PR:s small and focused
  Sorry this is a big change for through egui_plot. I can not make it smaller.
• cargo fmt
• cargo cranky
• Open the PR as a draft until you have self-reviewed it and run ./scripts/check.sh.
  The script does not work on 1.72.0

Partially closes emilk/egui_plot#18.

After much trial and error this PR enables the user to plot borrowed data.

It allows the user to plot Iterators, slices, PlotPoints. While there is still optimization possible per my message in emilk/egui_plot#18. This is a good first step and allows for easy performance improvement on the users side.
E.g. the user can now easily create a slice &[PlotPoint] that represents their point data or go a step further and represent their data using an iter().filter(...) approach.

Will this break current builds: Yes

• PlotPoint has been renamed to Coordinate
• Points has been made generic
• build_fn in plot.show() has gotten a different signature
• other changes

[emilk]

Did you benchmark to see what kind of performance gains this can give?

[GunnarMorrigan]

Did you benchmark to see what kind of performance gains this can give?

I have done profiling to see what was hurting performance. The recreation of all Points was the first obvious problem.

In my case I store roughly 350k PlotPoints to draw, cloning or recalculating them every repaint gets in the way.
If I only need to map my data to PlotPoints once it is much more efficient.

In the case of a dynamic element (e.g. a filter) data needs to be recomputed every time. There is still some improvement here as now you can use a lazy evaluated filter over your static PlotPoints. Additionally, there is added ergonomics in using an Iterator as you can easily apply filter / filter_map / skip etc

[emilk]

Nice, this is looking much better!

It would be great if one of the examples in crates/egui_demo_lib/src/demo/plot_demo.rs used the new borrowing mechanism

[GunnarMorrigan]

Nice, this is looking much better!

It would be great if one of the examples in crates/egui_demo_lib/src/demo/plot_demo.rs used the new borrowing mechanism

Added Scatter plot example with decently cool step_by to show off the iterator part. It plots 50k dots, not sure how much web could handle

[emilk]

First of all, I appreciate all the effort you've put into this!

However, this is a lot of new complexity, so I want to make sure this is justified. I'm likely the one who is going to be maintaining it for the foreseeable future.

Whenever I've ran into plot performance issues, it's been on the tessellator side, not here. And the solution is usually to aggregate the data, i.e. downsample it before handing it to egui_plot (see for instance rerun-io/rerun#4865).

So I'd be very interested in seeing some before/after flamegraph profiler screenshots, or an added benchmark to justify all this.

[emilk]

This is a slightly annoying name change, but maybe fine

[GunnarMorrigan]

I can replace it with a single function but would need to expose PlotItem as it would go into a public interface.

pub fn points<T: 'a>(&mut self, mut points: Points<T>) -> &mut Self
    where
        Points<T>: PlotItem,
    {
        // Give the points an automatic color if no color has been assigned.
        if points.color == Color32::TRANSPARENT {
            points.color = self.auto_color();
        }
        self.items.push(Box::new(points));

        self
    }

This is something I am not sure about. If it is exposed maybe it should be sealed?

[emilk]

This should be called something like from_iter, and have a docstring

[GunnarMorrigan]

This might not be the best name:

method from_iter can be confused for the standard trait method std::iter::FromIterator::from_iter
consider implementing the trait std::iter::FromIterator or choosing a less ambiguous method name
for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait
-W clippy::should-implement-trait implied by -W clippy::all

Also implementing FromIterator trait is not an option.

[emilk]

It would be nice if we could combine owned_points and borrowed_points.

[GunnarMorrigan]

I agree that it would be nicer.

We need to require that everything impls PlotItem and this is either done through a generic type constraint or well defined types that impl PlotItem.

[GunnarMorrigan]

First of all, I appreciate all the effort you've put into this!

However, this is a lot of new complexity, so I want to make sure this is justified. I'm likely the one who is going to be maintaining it for the foreseeable future.

Whenever I've ran into plot performance issues, it's been on the tessellator side, not here. And the solution is usually to aggregate the data, i.e. downsample it before handing it to egui_plot (see for instance rerun-io/rerun#4865).

So I'd be very interested in seeing some before/after flamegraph profiler screenshots, or an added benchmark to justify all this.

In my profiling there were some three things in performance

• Tessellation
  Vec the growth is amortized but if you know it will grow very large the moving takes a lot of time.
• Shape Vec
  Idem
• PlotPoint construction

There is as I see it no easy way to benchmark anything in egui as egui::__run_test_ui does not actually do any work. This makes it extremely hard to perform any reliable performance profiling as you can't see when something slows down or speeds up if the benchmark is not the same every time.

I have a flamegraph where egui spends significant time in the shape vec reallocating which should be preventable:

In my optimizeShapeVec branch I made a simple function to count the shapes. Which should already help a bit.
This optimization is also more focused on egui_plot as the shape vec only grows so large in egui_plot due to the number of points that are drawn.

Tesselation vec (the reserves are nice but it would be nicer if it could be done at once)

These are two examples of likely relatively straightforward performance improvements.

I will say I think there is indeed more 'real' gain to be had with shapes and tessellation then plot points but it is not insignificant. Additionally, iterators makes down sampling simpler to reconstructing the data every frame.

This PR is a start to performance improvements. However, right now I feel like there is a requirement to invest a significant amount of additional time to make this or other PRs to increase performance land.
I really like egui and what you have created, but I also have limited free time.