Plot interaction methods #766

EmbersArc · 2021-10-03T22:08:30Z

Closes #734.

I went with option 2 described in #734. This is a small breaking change and updating shouldn't be difficult. In the long run we can use this to add even more methods for interacting with the plot.

Might be a bit rough around the edges so feel free to suggest improvements.

emilk · 2021-10-22T08:43:53Z

Sorry for the slow review - I'll get to it after the next egui release! 😓

emilk · 2021-10-25T20:54:21Z

I'm a bit worried about Plot being split in two (Plot and PlotUi), and becoming so different from how other widgets work. But then again, Plot is very different from other widgets, so perhaps this is motivated.

I wrote yet another alternative in #734 (comment)

This just seems like a very big change for something as simple as "read point position so I can show it the next frame" (what this PR demos). Is there actually something more complex you want to accomplish?

emilk · 2021-10-25T21:00:24Z

Btw, I've just created a egui discord server where I was hoping we could more easily have design discussions like these - perhaps you care to join it? https://discord.gg/qzvmcrUe

coderedart · 2021-10-25T21:41:18Z

Btw, I've just created a egui discord server where I was hoping we could more easily have design discussions like these - perhaps you care to join it? https://discord.gg/qzvmcrUe

Finally! I was secretly hoping for a egui discord for a long time. I would recommend that you put this as a badge in readme where it shows how many people are online in discord

emilk

Overall this looks great!

What do you think about using Value instead of Pos2 for the values in plot-coordinates?

We could add helpers to Value (to_pos2 and to_vec2) instead.

Value could also do with a better name, but that's another problem :)

emilk · 2021-11-01T18:48:19Z

CHANGELOG.md

@@ -43,6 +44,7 @@ NOTE: [`epaint`](epaint/CHANGELOG.md), [`eframe`](eframe/CHANGELOG.md), [`egui_w
 * MSRV (Minimum Supported Rust Version) is now `1.54.0`.
 * By default, `DragValue`:s no longer show a tooltip when hovered. Change with `Style::explanation_tooltips`.
 * Smaller and nicer color picker.
+* Plots now provide a `build` method that has to be used to add items to and show the plot.


Thanks to my slow review, these lines now need to be moved to new ## Unreleased header! Please also add a PR link (it's something new I'm trying to do).

emilk · 2021-11-01T18:49:47Z

CHANGELOG.md

@@ -30,6 +30,7 @@ NOTE: [`epaint`](epaint/CHANGELOG.md), [`eframe`](eframe/CHANGELOG.md), [`egui_w
 * Add `ui.add_enabled_ui(bool, |ui| …)` to create a possibly disabled UI section.
 * Add feature `"serialize"` separatedly from `"persistence"`.
 * Add `egui::widgets::global_dark_light_mode_buttons` to easily add buttons for switching the egui theme.
+* Add interaction methods to the plot.


Can we improve this wording a bit? Maybe "You can now read the Plot coordinates of the mouse when building a Plot." ? 🤷

emilk · 2021-11-01T18:52:43Z

egui/src/widgets/plot/mod.rs

-impl Widget for Plot {
-    fn ui(self, ui: &mut Ui) -> Response {
+    /// Interact with and add items to the plot and finally draw it.
+    pub fn build(self, ui: &mut Ui, build_fn: impl FnOnce(&mut PlotUi)) -> Response {


For better or for worse, the name show is used for the similar thing in CollapsingHeader, Window etc. I think we should be consistent with that.

egui/src/widgets/plot/mod.rs

emilk · 2021-11-01T19:02:41Z

egui/src/widgets/plot/mod.rs

+    }
+
+    /// Transform the screen coordinates to plot coordinates.
+    pub fn screen_to_plot_coordinates(&self, position: Pos2) -> Pos2 {


I prefer x_from_y to y_to_x for a few reasons:

For one, it is more consistent with stlib (let string = String::from_utf8(utf8); etc)

But more importantly, it puts the spaces in sequence:

V-------------V let pos_in_plot = plot_ui.plot_from_screen(pos_in_screen); ^--------------^

vs

V-----------------------V let pos_in_plot = plot_ui.screen_to_plot(pos_in_screen); ^---------------------^

emilk · 2021-11-01T19:05:46Z

egui/src/widgets/plot/mod.rs

+    }
+
+    /// The pointer position in plot coordinates, if the pointer is inside the plot area.
+    pub fn pointer_coordinate(&self) -> Option<Pos2> {


I wonder if we should not use Value here instead of Pos2. f32 is not enough accuracy in many situations. For instance, if I am plotting things as a function of time I may be using seconds since unix epoch as the X coordinate, and that won't round very well to f32.

The added type-safety may also be nice (we can see from the type in what space it is in).

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>

…into plot-get-mouse-pos

EmbersArc · 2021-11-11T22:10:08Z

I had completely missed your review and only just saw it. Sorry about the delay. I agree with all of your suggestions and have made the changes.

EmbersArc · 2021-11-12T23:48:48Z

I tested this today and am pretty happy with how it works. I'd like to add the following features as well though:

Access to Ui inside the closure as well. Either through the plot_ui struct or as a second argument.
A plot_to_screen conversion, to enable custom drawing in screen coordinates based on items in the plot.
A way to get the current plot bounds.

emilk

Excellent!

EmbersArc added 5 commits October 3, 2021 17:08

move to a basic plot builder with callback

203cbb0

add some interaction methods

8cd2de3

move interaction demo to its own panel

9cada8b

add another comment

9974524

update changelog

fb8b6d9

EmbersArc mentioned this pull request Oct 9, 2021

Detect mouse events in a Plot #759

Closed

EmbersArc added 3 commits October 10, 2021 22:03

avoid a clone

10ff7bc

Merge remote-tracking branch 'upstream/master' into plot-get-mouse-pos

cdb6f18

add a semicolon

1eba777

emilk mentioned this pull request Oct 31, 2021

Plot: Get mouse position #734

Closed

EmbersArc added 2 commits October 31, 2021 22:46

Merge remote-tracking branch 'upstream/master' into plot-get-mouse-pos

3ac148c

fix context menu

c89b11f

emilk requested changes Nov 1, 2021

View reviewed changes

EmbersArc and others added 5 commits November 11, 2021 22:42

Merge remote-tracking branch 'upstream/master' into plot-get-mouse-pos

8b97d8e

update changelog

2020fab

Update egui/src/widgets/plot/mod.rs

ed034b8

Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>

Merge branch 'plot-get-mouse-pos' of https://github.com/EmbersArc/egui …

98c2b26

…into plot-get-mouse-pos

apply suggestions

89b131a

EmbersArc added 2 commits November 11, 2021 23:32

fix test

4b6be4d

fix doctest

7ac223e

emilk approved these changes Nov 13, 2021

View reviewed changes

emilk merged commit 0bad1d0 into emilk:master Nov 13, 2021

EmbersArc mentioned this pull request Nov 13, 2021

Improved plot interaction methods #891

Closed

Bromeon mentioned this pull request Nov 14, 2021

Bar charts + box plots continued #863

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plot interaction methods #766

Plot interaction methods #766

EmbersArc commented Oct 3, 2021

emilk commented Oct 22, 2021

emilk commented Oct 25, 2021

emilk commented Oct 25, 2021

coderedart commented Oct 25, 2021

emilk left a comment

emilk Nov 1, 2021

emilk Nov 1, 2021

emilk Nov 1, 2021

emilk Nov 1, 2021

emilk Nov 1, 2021

EmbersArc commented Nov 11, 2021

EmbersArc commented Nov 12, 2021

emilk left a comment

Plot interaction methods #766

Plot interaction methods #766

Conversation

EmbersArc commented Oct 3, 2021

emilk commented Oct 22, 2021

emilk commented Oct 25, 2021

emilk commented Oct 25, 2021

coderedart commented Oct 25, 2021

emilk left a comment

Choose a reason for hiding this comment

emilk Nov 1, 2021

Choose a reason for hiding this comment

emilk Nov 1, 2021

Choose a reason for hiding this comment

emilk Nov 1, 2021

Choose a reason for hiding this comment

emilk Nov 1, 2021

Choose a reason for hiding this comment

emilk Nov 1, 2021

Choose a reason for hiding this comment

EmbersArc commented Nov 11, 2021

EmbersArc commented Nov 12, 2021

emilk left a comment

Choose a reason for hiding this comment