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.

Sign up for GitHub

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

Plot widget - allow disabling zoom and drag for x and y separately #2901

Merged
merged 9 commits into from
Apr 18, 2023
Merged

Plot widget - allow disabling zoom and drag for x and y separately #2901

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bec7012
Set whether zooming allowed for x and y separately
OmegaJak Mar 28, 2023
02e7fa1
Set whether dragging allowed for x and y separately
OmegaJak Mar 29, 2023
40601a5
Add disclaimers about interaction with data_aspect
OmegaJak Apr 14, 2023
6159620
Show zoom & drag behavior in plot demo/charts instead of context menu…
OmegaJak Apr 14, 2023
a516aa2
Code review suggestions - use AxisBools::any()
OmegaJak Apr 18, 2023
0603570
Simplify allow_drag and allow_zoom APIs to take in Into<AxisBools>
OmegaJak Apr 18, 2023
188efe9
Remove unnecessary using...
OmegaJak Apr 18, 2023
dcf506e
Remove unrelated change
OmegaJak Apr 18, 2023
0bffe9e
Merge branch 'master' into separate-x-y-zoom-drag
OmegaJak Apr 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 43 additions & 14 deletions crates/egui/src/widgets/plot/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,8 @@ pub struct Plot {

center_x_axis: bool,
center_y_axis: bool,
allow_zoom: bool,
allow_drag: bool,
allow_zoom: AxisBools,
allow_drag: AxisBools,
allow_scroll: bool,
allow_double_click_reset: bool,
allow_boxed_zoom: bool,
Expand Down Expand Up @@ -305,8 +305,8 @@ impl Plot {

center_x_axis: false,
center_y_axis: false,
allow_zoom: true,
allow_drag: true,
allow_zoom: true.into(),
allow_drag: true.into(),
allow_scroll: true,
allow_double_click_reset: true,
allow_boxed_zoom: true,
Expand Down Expand Up @@ -402,9 +402,19 @@ impl Plot {
self
}

/// Whether to allow zooming in the plot. Default: `true`.
pub fn allow_zoom(mut self, on: bool) -> Self {
Copy link
Contributor Author

@OmegaJak OmegaJak Apr 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It would be very easy to maintain backwards compatibility here by keeping these methods and having them set both x and y at the same time.

Question to the reviewers: what's the preference - keep backwards compatibility here or go with a more minimal API?

I'll update the changelog once this is decided.

Copy link
Owner

Choose a reason for hiding this comment

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

I vote keeping the old one. Not just for backwards compatibility, but also ergonomics: in most cases, you want to control both the same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, good point. It occurs to me now that having it be generic in T where T : Into<AxisBools> would produce the best of both worlds. That way there's only the one method needed, and it maintains the current ergonomics for setting both at once, while allowing the new flexibility.

The main cost I see there is that it requires exposing AxisBools, which has been private until now. I guess it will also be a little more verbose to disable just one of the axes, as you'll have to do something like .allow_zoom(AxisBools::new(false, true)) (unless we keep the allow_zoom_x/y methods, which doesn't seem worth it). I'll go ahead and update with the new signature, and probably drop the x/y-specific methods.

self.allow_zoom = on;
/// Whether to allow zooming in the plot's x-axis. Default: `true`.
///
/// Note: Allowing zoom in x but not y (or vice versa) may lead to unexpected results if used in combination with `data_aspect`.
pub fn allow_zoom_x(mut self, on: bool) -> Self {
self.allow_zoom.x = on;
self
}

/// Whether to allow zooming in the plot's y-axis. Default: `true`.
///
/// Note: Allowing zoom in x but not y (or vice versa) may lead to unexpected results if used in combination with `data_aspect`.
pub fn allow_zoom_y(mut self, on: bool) -> Self {
self.allow_zoom.y = on;
self
}

Expand Down Expand Up @@ -443,9 +453,15 @@ impl Plot {
self
}

/// Whether to allow dragging in the plot to move the bounds. Default: `true`.
pub fn allow_drag(mut self, on: bool) -> Self {
self.allow_drag = on;
/// Whether to allow dragging in the plot to move the bounds in the x direction. Default: `true`.
pub fn allow_drag_x(mut self, on: bool) -> Self {
self.allow_drag.x = on;
self
}

/// Whether to allow dragging in the plot to move the bounds in the y direction. Default: `true`.
pub fn allow_drag_y(mut self, on: bool) -> Self {
self.allow_drag.y = on;
self
}

Expand Down Expand Up @@ -894,9 +910,16 @@ impl Plot {
}

// Dragging
if allow_drag && response.dragged_by(PointerButton::Primary) {
if (allow_drag.x || allow_drag.y) && response.dragged_by(PointerButton::Primary) {
response = response.on_hover_cursor(CursorIcon::Grabbing);
transform.translate_bounds(-response.drag_delta());
let mut delta = -response.drag_delta();
if !allow_drag.x {
delta.x = 0.0;
}
if !allow_drag.y {
delta.y = 0.0;
}
transform.translate_bounds(delta);
bounds_modified = true.into();
}

Expand Down Expand Up @@ -953,12 +976,18 @@ impl Plot {
}

if let Some(hover_pos) = response.hover_pos() {
if allow_zoom {
let zoom_factor = if data_aspect.is_some() {
if allow_zoom.x || allow_zoom.y {
let mut zoom_factor = if data_aspect.is_some() {
Vec2::splat(ui.input(|i| i.zoom_delta()))
} else {
ui.input(|i| i.zoom_delta_2d())
};
if !allow_zoom.x {
zoom_factor.x = 1.0;
}
if !allow_zoom.y {
zoom_factor.y = 1.0;
}
if zoom_factor != Vec2::splat(1.0) {
transform.zoom(zoom_factor, hover_pos);
bounds_modified = true.into();
Expand Down
7 changes: 5 additions & 2 deletions crates/egui_demo_lib/src/demo/context_menu.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,14 @@ impl ContextMenus {
);
egui::plot::Plot::new("example_plot")
.show_axes(self.show_axes)
.allow_drag(self.allow_drag)
.allow_zoom(self.allow_zoom)
.allow_drag_x(self.allow_drag)
.allow_drag_y(self.allow_drag)
.allow_zoom_x(self.allow_zoom)
.allow_zoom_y(self.allow_zoom)
.allow_scroll(self.allow_scroll)
.center_x_axis(self.center_x_axis)
.center_x_axis(self.center_y_axis)
.allow_boxed_zoom(false)
.width(self.width)
.height(self.height)
.data_aspect(1.0)
Expand Down
56 changes: 47 additions & 9 deletions crates/egui_demo_lib/src/demo/plot_demo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -818,29 +818,57 @@ impl Default for Chart {
struct ChartsDemo {
chart: Chart,
vertical: bool,
allow_zoom_x: bool,
allow_zoom_y: bool,
allow_drag_x: bool,
allow_drag_y: bool,
}

impl Default for ChartsDemo {
fn default() -> Self {
Self {
vertical: true,
chart: Chart::default(),
allow_zoom_x: true,
allow_zoom_y: true,
allow_drag_x: true,
allow_drag_y: true,
}
}
}

impl ChartsDemo {
fn ui(&mut self, ui: &mut Ui) -> Response {
ui.label("Type:");
ui.horizontal(|ui| {
ui.selectable_value(&mut self.chart, Chart::GaussBars, "Histogram");
ui.selectable_value(&mut self.chart, Chart::StackedBars, "Stacked Bar Chart");
ui.selectable_value(&mut self.chart, Chart::BoxPlot, "Box Plot");
});
ui.label("Orientation:");
ui.horizontal(|ui| {
ui.selectable_value(&mut self.vertical, true, "Vertical");
ui.selectable_value(&mut self.vertical, false, "Horizontal");
ui.vertical(|ui| {
ui.label("Type:");
ui.horizontal(|ui| {
ui.selectable_value(&mut self.chart, Chart::GaussBars, "Histogram");
ui.selectable_value(&mut self.chart, Chart::StackedBars, "Stacked Bar Chart");
ui.selectable_value(&mut self.chart, Chart::BoxPlot, "Box Plot");
});
ui.label("Orientation:");
ui.horizontal(|ui| {
ui.selectable_value(&mut self.vertical, true, "Vertical");
ui.selectable_value(&mut self.vertical, false, "Horizontal");
});
});
ui.vertical(|ui| {
ui.group(|ui| {
ui.add_enabled_ui(self.chart != Chart::StackedBars, |ui| {
ui.horizontal(|ui| {
ui.label("Allow zoom:");
ui.checkbox(&mut self.allow_zoom_x, "X");
ui.checkbox(&mut self.allow_zoom_y, "Y");
});
});
ui.horizontal(|ui| {
ui.label("Allow drag:");
ui.checkbox(&mut self.allow_drag_x, "X");
ui.checkbox(&mut self.allow_drag_y, "Y");
});
});
});
});
match self.chart {
Chart::GaussBars => self.bar_gauss(ui),
Expand Down Expand Up @@ -873,6 +901,10 @@ impl ChartsDemo {
Plot::new("Normal Distribution Demo")
.legend(Legend::default())
.clamp_grid(true)
.allow_zoom_x(self.allow_zoom_x)
.allow_zoom_y(self.allow_zoom_y)
.allow_drag_x(self.allow_drag_x)
.allow_drag_y(self.allow_drag_y)
.show(ui, |plot_ui| plot_ui.bar_chart(chart))
.response
}
Expand Down Expand Up @@ -931,6 +963,8 @@ impl ChartsDemo {
Plot::new("Stacked Bar Chart Demo")
.legend(Legend::default())
.data_aspect(1.0)
.allow_drag_x(self.allow_drag_x)
.allow_drag_y(self.allow_drag_y)
.show(ui, |plot_ui| {
plot_ui.bar_chart(chart1);
plot_ui.bar_chart(chart2);
Expand Down Expand Up @@ -974,6 +1008,10 @@ impl ChartsDemo {

Plot::new("Box Plot Demo")
.legend(Legend::default())
.allow_zoom_x(self.allow_zoom_x)
.allow_zoom_y(self.allow_zoom_y)
.allow_drag_x(self.allow_drag_x)
.allow_drag_y(self.allow_drag_y)
.show(ui, |plot_ui| {
plot_ui.box_plot(box1);
plot_ui.box_plot(box2);
Expand Down