Drag and zoom support for plots #317

EmbersArc · 2021-04-18T15:51:18Z

Closes #316.

egui/src/widgets/plot.rs

Co-authored-by: ilya sheprut <optitel223@gmail.com>

emilk

Thank you for this PR; it works really well, and nice use of the new memory feature!

emilk · 2021-04-19T20:16:19Z

egui/src/widgets/plot.rs

+impl Default for PlotMemory {
+    fn default() -> Self {
+        Self {
+            bounds: Bounds::new_symmetrical(1.),


This is quite arbitrary. Would it make more sense to use Bounds::EMPTY here, and have that to mean "automatic"? Then at the first frame we would get the automatic bounds

emilk · 2021-04-19T20:21:00Z

egui/src/widgets/plot.rs

-impl Default for Plot {
-    fn default() -> Self {
+impl Plot {
+    pub fn new(name: impl Into<String>) -> Self {


I think this should be either changed to id_source: impl Hash, or we should actually use it as a String - i.e. show it on hover. Though then I wonder what the difference would be between a plot name and on_hover_ui-tooltip. Probably better to go with id_source: impl Hash and change the examples to use something that looks less like a user-visible name and more like an identifier ("my_plot")

How about we use the same system that Window uses with a constructor that takes the shown name and
an id method to set a custom id when required? The title will be visible to the user at some point,
I just haven't thought about how to show it yet.

emilk · 2021-04-19T20:22:39Z

egui/src/widgets/plot.rs

+    /// If true, the bounds will be set based on the data.
+    pub fn automatic_bounds(mut self, enabled: bool) -> Self {


This is a bit under-documented (what is the default? what does it mean for it to be false?).

I wonder if we really need this at all? I like the behavior of automatically calculating bounds at the start and on double-click, but otherwise allow users to scroll and zoom.

egui/src/widgets/plot.rs

s-nie

Nevermind, wrong account

EmbersArc · 2021-04-19T22:04:04Z

Thanks for the review! I share your concerns about how to handle the plot bounds.
I got a bit carried away and started refactoring this whole thing quite a bit and added new features. If it's alright with you then I'll address the bounds issues there so that those changes don't have to be made twice. The bounds need a bit more thought anyway since there is no way for the user to define them manually or to make them fixed yet. I'll open a PR for these changes as soon as possible.

Is it alright with you when the plotting functionality keeps growing? In #155 you mentioned that
you'd prefer if it didn't get too extensive. But I really like the foundation that you've built here
and see potential for this to grow into something that can be used as much more than just a demo.

emilk · 2021-04-20T16:11:35Z

If it's alright with you then I'll address the bounds issues there so that those changes don't have to be made twice

I'm a bit hesitant to merge a PR that removes a desired feature (include_x/include_y).
Maybe you can open the other PR as a draft so I can take a look?

Is it alright with you when the plotting functionality keeps growing?

I'm alright with that, but it would be good to sync exactly in what direction it is growing before you do too much work! Perhaps you can open a tracking issue with your plans?

EmbersArc · 2021-04-20T16:51:14Z

I'm a bit hesitant to merge a PR that removes a desired feature

Understandable, I added back those methods, and made automatic bounds the default.

I'll open a tracking issue and a PR soon for the coming changes.

Question: Is there a way to tell if a window has been drawn for the first time?

emilk

I just realized there is an important use-case that this breaks: plotting dynamic data. For instance, plotting the last few seconds of a recorded time series. In this case we want the bounds to automatically follow the data each frame.

I suggest a (hopefully) simple change: add a auto_bounds: bool to PlotMemory and initialize it to true. This is set to false on zoom/pan, and reset to true on double-tap. Then we will maintain previous behavior (tracking dynamic plots) while still allowing the new awesome zoom/pan behavior.

A second use case that need to work is the include_x/include_y (e.g. .include_y(0.0) to include the X-axis in a plot). This should be used when computing automatic bounds, but ignored when users pan and zoom. I suggest changing Plot::bounds to be called Plot::min_auto_bounds and use it as the starting point when computing automatic bounds.

PS: thanks for your patience and work :)

egui/src/widgets/plot.rs

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

EmbersArc · 2021-04-20T22:37:38Z

Thanks for the great suggestions! I tried to apply all of them. Might not work in all cases thought, but I don't have time to test it more thoroughly right now.

EmbersArc · 2021-04-21T18:16:34Z

I've opened up a PR based on this one: #331

emilk

Awesome work!

blagovestb2c2 · 2021-06-15T12:19:38Z

Hey guys, awesome library first of all. I found this PR after having problems with zoom. I am not sure how i'm supposed to achieve that on a Plot (mac with magic mouse if it matters). I tried reading the code and zoom seems to require hovering but i only see labels if i hover? And if I hover + track (or slide my finger on my mouse rather), the plot is just dragged. Am I doing something dumb? :D

egui = "0.12.0"
epi = "0.12.0"
eframe = "0.12.0"

EmbersArc · 2021-06-15T17:30:46Z

@blagovestb2c2 You need to hold control while scrolling to zoom

EmbersArc changed the title ~~drag and zoom support for plots~~ Drag and zoom support for plots Apr 18, 2021

drag and zoom support for plots

ec3bda0

EmbersArc force-pushed the plot-drag-zoom branch from bff4e3d to ec3bda0 Compare April 18, 2021 15:54

EmbersArc added 3 commits April 18, 2021 18:05

update doctest

a398183

use impl ToString

5022279

revert back to Into<String> until emilk#302 is solved

a245b9f

EmbersArc marked this pull request as ready for review April 18, 2021 18:11

optozorax reviewed Apr 19, 2021

View reviewed changes

egui/src/widgets/plot.rs Outdated Show resolved Hide resolved

egui/src/widgets/plot.rs Outdated Show resolved Hide resolved

EmbersArc and others added 2 commits April 19, 2021 07:39

Apply suggestions from code review

bbd6d28

Co-authored-by: ilya sheprut <optitel223@gmail.com>

use persistence feature for PlotMemory

1def4ca

emilk requested changes Apr 19, 2021

View reviewed changes

rename shift -> translate

b3002ff

s-nie reviewed Apr 19, 2021

View reviewed changes

EmbersArc added 2 commits April 20, 2021 18:47

remove automatic bounds

c1dd793

removed unused methods

5ab7f6b

Into<String> -> ToString

80599ab

emilk requested changes Apr 20, 2021

View reviewed changes

egui/src/widgets/plot.rs Show resolved Hide resolved

egui/src/widgets/plot.rs Show resolved Hide resolved

egui/src/widgets/plot.rs Outdated Show resolved Hide resolved

egui/src/widgets/plot.rs Outdated Show resolved Hide resolved

EmbersArc and others added 5 commits April 21, 2021 00:05

Apply suggestions from code review

80ed5e1

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

avoid potential invalid bounds bug

1b00e7f

use new is_valid method

35e11f3

improve auto bounds behavior as suggested

2ce3242

use NOTHING to initialize min_auto_bounds

7ba683a

emilk approved these changes Apr 21, 2021

View reviewed changes

emilk merged commit 012542d into emilk:master Apr 21, 2021

EmbersArc deleted the plot-drag-zoom branch April 21, 2021 21:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drag and zoom support for plots #317

Drag and zoom support for plots #317

EmbersArc commented Apr 18, 2021

emilk left a comment

emilk Apr 19, 2021

emilk Apr 19, 2021 •

edited

Loading

EmbersArc Apr 19, 2021

emilk Apr 19, 2021

s-nie left a comment •

edited

Loading

EmbersArc commented Apr 19, 2021 •

edited

Loading

emilk commented Apr 20, 2021 •

edited

Loading

EmbersArc commented Apr 20, 2021

emilk left a comment •

edited

Loading

EmbersArc commented Apr 20, 2021

EmbersArc commented Apr 21, 2021

emilk left a comment

blagovestb2c2 commented Jun 15, 2021

EmbersArc commented Jun 15, 2021

		/// If true, the bounds will be set based on the data.
		pub fn automatic_bounds(mut self, enabled: bool) -> Self {

Drag and zoom support for plots #317

Drag and zoom support for plots #317

Conversation

EmbersArc commented Apr 18, 2021

emilk left a comment

Choose a reason for hiding this comment

emilk Apr 19, 2021

Choose a reason for hiding this comment

emilk Apr 19, 2021 • edited Loading

Choose a reason for hiding this comment

EmbersArc Apr 19, 2021

Choose a reason for hiding this comment

emilk Apr 19, 2021

Choose a reason for hiding this comment

s-nie left a comment • edited Loading

Choose a reason for hiding this comment

EmbersArc commented Apr 19, 2021 • edited Loading

emilk commented Apr 20, 2021 • edited Loading

EmbersArc commented Apr 20, 2021

emilk left a comment • edited Loading

Choose a reason for hiding this comment

EmbersArc commented Apr 20, 2021

EmbersArc commented Apr 21, 2021

emilk left a comment

Choose a reason for hiding this comment

blagovestb2c2 commented Jun 15, 2021

EmbersArc commented Jun 15, 2021

emilk Apr 19, 2021 •

edited

Loading

s-nie left a comment •

edited

Loading

EmbersArc commented Apr 19, 2021 •

edited

Loading

emilk commented Apr 20, 2021 •

edited

Loading

emilk left a comment •

edited

Loading