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

Add optional line to Points PlotItem #4143

Closed
wants to merge 3 commits into from
Closed

Conversation

jsalort
Copy link

@jsalort jsalort commented Mar 7, 2024

This is a proposal to add the option to connect the points in the Points PlotItem, similar to "o-" in Matlab or Python Matplotlib.
I don't know if such addition would be welcome in egui_plot, but I needed this in a project, and initially thought about defining a custom PlotItem. But it is not straightforward to implement custom PlotItem, as some methods are private, so it was simpler to just add the options I needed in the existing Points struct. Besides, other people may want to do this.

It is not the same thing as adding two PlotItem: a Line and a Points, because then clicking on the name in the legend would mask only the Points. There are surely workarounds, but I found simpler to just have the option the connect the points in the Points PlotItem.

@emilk emilk added the egui_plot Related to egui_plot label Mar 8, 2024
.for_each(|center| {
let tf = |dx: f32, dy: f32| -> Pos2 { center + radius * vec2(dx, dy) };
.map(|v| transform.position_from_point(v))
.collect();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is paying the price of the collect wether or not we have a line

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds a "line" feature to egui_plot::Points.
We could instead add a marker feature to egui_plot::Line

But better yet would probably be to combine Points and Line into one thing, instead of replicating features between them. What do you think?

@jsalort
Copy link
Author

jsalort commented Mar 8, 2024

This adds a "line" feature to egui_plot::Points. We could instead add a marker feature to egui_plot::Line

But better yet would probably be to combine Points and Line into one thing, instead of replicating features between them. What do you think?

Well yes, I agree completely. In the analogy with other plotting tools like Matplotlib, both kinds of plots are just obtained with plt.plot, with option for markers and/or line. Providing both features within the same PlotItem makes sense to me, but deprecating one of the existing things would be a backward-incompatible change (for people using the one that would be removed).
Or maybe it is possible to have just one thing, perhaps something like egui_plot::Chart, but keep both previous names, egui_plot::Points and egui_plot::Line which would be aliases for Chart only with different defaults, e.g. Points with only markers by default, and Line with only a line by default (if such a thing is possible with the Rust type system, I am too new to Rust to know).

@emilk
Copy link
Owner

emilk commented Jul 15, 2024

egui_plot has recently been moved to its own repository, at https://github.com/emilk/egui_plot

This will hopefully speed up its development by having more reviewers and maintainers.

Please re-open this PR at https://github.com/emilk/egui_plot/pulls

See also:

@emilk emilk closed this Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
egui_plot Related to egui_plot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants