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 all 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
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ NOTE: [`epaint`](crates/epaint/CHANGELOG.md), [`eframe`](crates/eframe/CHANGELOG
## Unreleased
* Add `char_limit` to `TextEdit` singleline mode to limit the amount of characters
* ⚠️ BREAKING: `Plot::link_axis` and `Plot::link_cursor` now take the name of the group ([#2410](https://github.com/emilk/egui/pull/2410)).

* Update `Plot::allow_zoom` and `Plot::allow_drag` to allow setting those values for X and Y axes independently ([#2901](https://github.com/emilk/egui/pull/2901)).

## 0.21.0 - 2023-02-08 - Deadlock fix and style customizability
* ⚠️ BREAKING: `egui::Context` now use closures for locking ([#2625](https://github.com/emilk/egui/pull/2625)):
Expand Down
59 changes: 42 additions & 17 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 @@ -72,15 +72,19 @@ impl Default for CoordinatesFormatter {
const MIN_LINE_SPACING_IN_POINTS: f64 = 6.0; // TODO(emilk): large enough for a wide label

#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
#[derive(Copy, Clone)]
struct AxisBools {
x: bool,
y: bool,
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub struct AxisBools {
pub x: bool,
pub y: bool,
}

impl AxisBools {
pub fn new(x: bool, y: bool) -> Self {
Self { x, y }
}

#[inline]
fn any(&self) -> bool {
pub fn any(&self) -> bool {
self.x || self.y
}
}
Expand Down Expand Up @@ -165,8 +169,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 @@ -207,8 +211,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 @@ -305,8 +309,13 @@ impl Plot {
}

/// 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;
///
/// Note: Allowing zoom in one axis but not the other may lead to unexpected results if used in combination with `data_aspect`.
pub fn allow_zoom<T>(mut self, on: T) -> Self
where
T: Into<AxisBools>,
{
self.allow_zoom = on.into();
self
}

Expand Down Expand Up @@ -346,8 +355,11 @@ impl Plot {
}

/// 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;
pub fn allow_drag<T>(mut self, on: T) -> Self
where
T: Into<AxisBools>,
{
self.allow_drag = on.into();
self
}

Expand Down Expand Up @@ -829,9 +841,16 @@ impl Plot {
}

// Dragging
if allow_drag && response.dragged_by(PointerButton::Primary) {
if allow_drag.any() && 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 @@ -889,12 +908,18 @@ impl Plot {

let hover_pos = response.hover_pos();
if let Some(hover_pos) = hover_pos {
if allow_zoom {
let zoom_factor = if data_aspect.is_some() {
if allow_zoom.any() {
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
49 changes: 39 additions & 10 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
@@ -1,7 +1,7 @@
use std::f64::consts::TAU;
use std::ops::RangeInclusive;

use egui::plot::{GridInput, GridMark};
use egui::plot::{AxisBools, GridInput, GridMark};
use egui::*;
use plot::{
Arrows, Bar, BarChart, BoxElem, BoxPlot, BoxSpread, CoordinatesFormatter, Corner, HLine,
Expand Down Expand Up @@ -812,29 +812,53 @@ impl Default for Chart {
struct ChartsDemo {
chart: Chart,
vertical: bool,
allow_zoom: AxisBools,
allow_drag: AxisBools,
}

impl Default for ChartsDemo {
fn default() -> Self {
Self {
vertical: true,
chart: Chart::default(),
allow_zoom: true.into(),
allow_drag: true.into(),
}
}
}

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 @@ -867,6 +891,8 @@ impl ChartsDemo {
Plot::new("Normal Distribution Demo")
.legend(Legend::default())
.clamp_grid(true)
.allow_zoom(self.allow_zoom)
.allow_drag(self.allow_drag)
.show(ui, |plot_ui| plot_ui.bar_chart(chart))
.response
}
Expand Down Expand Up @@ -925,6 +951,7 @@ impl ChartsDemo {
Plot::new("Stacked Bar Chart Demo")
.legend(Legend::default())
.data_aspect(1.0)
.allow_drag(self.allow_drag)
.show(ui, |plot_ui| {
plot_ui.bar_chart(chart1);
plot_ui.bar_chart(chart2);
Expand Down Expand Up @@ -968,6 +995,8 @@ impl ChartsDemo {

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