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

Add Plot::allow_auto_bounds #2569

Closed
wants to merge 12 commits into from
Closed

Add Plot::allow_auto_bounds #2569

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
599e919
add Plot::allow_auto_bounds
haricot Jan 10, 2023
6b057c4
reformulate docstring: allow_auto_bounds
haricot Aug 14, 2023
53ed8ec
Update CHANGELOG.md
haricot Aug 14, 2023
57d2642
Update formating
haricot Aug 15, 2023
8c80132
Merge remote-tracking branch 'origin/master' into allow_auto_bounds
haricot Aug 16, 2023
bd77c27
Update logic allow_auto_bounds
haricot Aug 16, 2023
3da8c63
Update logic allow_auto_bounds
haricot Aug 16, 2023
ad80536
Update doc allow_auto_bounds
haricot Aug 16, 2023
2617738
Update doc allow_auto_bounds if set_plot_bounds
haricot Aug 16, 2023
1bd2e72
Merge remote-tracking branch 'origin/master' into allow_auto_bounds
haricot Aug 16, 2023
d8a80d2
Update formating
haricot Aug 17, 2023
f7b5fd6
update doc allow_auto_bounds
haricot Aug 17, 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
47 changes: 36 additions & 11 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 @@ -200,6 +200,7 @@ pub struct Plot {
allow_scroll: bool,
allow_double_click_reset: bool,
allow_boxed_zoom: bool,
allow_auto_bounds: bool,
auto_bounds: AxisBools,
min_auto_bounds: PlotBounds,
margin_fraction: Vec2,
Expand Down Expand Up @@ -242,6 +243,7 @@ impl Plot {
allow_scroll: true,
allow_double_click_reset: true,
allow_boxed_zoom: true,
allow_auto_bounds: true,
auto_bounds: false.into(),
min_auto_bounds: PlotBounds::NOTHING,
margin_fraction: Vec2::splat(0.05),
Expand Down Expand Up @@ -375,6 +377,22 @@ impl Plot {
self
}

/// Whether to allow auto bounds. Default: `true`.
Copy link
Owner

Choose a reason for hiding this comment

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

I think you should try to explain what it means for the user to set this to true or false.

Does "bounds" here mean the view of the plot (zoom/pan)?
How does this relate to PlotUi::set_plot_bounds?
What bounds will be used when this is set to false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suggest this:

If false, it set bounds_modified to true to prevent auto adjusting bounds,
also it check if the user double clicked to reset bounds. If so, it set bounds_modified to false.

It's needed when a new plot is started, such as when resetting egui memory and pan/zoom that
have not been used before, as the limits can be saved as persistent data.

Currently, if allow_auto_bounds(false) and plot_ui.set_plot_bounds(PlotBounds) are used in
same time set_plot_bounds will be ignored.

TODO: Refactor Plot::allow_auto_bounds and Plot::set_plot_bounds feature to work together.

Possible solution for allow_auto_bounds and set_plot_bounds by passing something like this when initializing plot:
Plot::new("myplot").allow_auto_bounds(false).preset_bounds(plotbounds) or something a enum.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or something like:

enum LogicBound{
  SkipRecalculated,
  Bounds(PlotBounds)
}

Plot::set_plot_bounds( LogicBounds::SkipRecalculated )

I will try to test this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

or rather just this:

widgets/plot/mod.rs

        .unwrap_or_else(|| 
            PlotMemory {
            bounds_modified: if let Some(custom_bounds) = custom_bounds {
                true.into()
                } else { 
                false.into()
                }  ,
            hovered_entry: None,
            hidden_items: Default::default(),
            last_plot_transform: PlotTransform::new(
                rect,
                if let Some(custom_bounds) = custom_bounds {
                    custom_bounds
                    } else { 
                        min_auto_bounds
                    },
                center_axis.x,
                center_axis.y,
            ),
            last_click_pos_for_zoom: None,
        });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opened issue #3371 related this pull request, how i think it resolve this indirect approach.

Also i realize i think we can add set_bounds and dynamic zoom and translate under process default bounds (currently enforce aspect ratio) like vec ![bounds_modifications] but in setting by using factor_delta_changed slider.

let data_zoom_aspect = scalar_aspect_zoom.current ;
let last_data_aspect_zoom  = scalar_aspect_zoom.last_changed;

let data_zoom_aspect  = if last_data_aspect_zoom > data_zoom_aspect {
    1.0 * (data_zoom_aspect / last_data_aspect_zoom)
} else {
    (data_zoom_aspect / last_data_aspect_zoom) / 1.0
};

scalar_aspect_zoom.last_changed  =  scalar_aspect_zoom.current ;

scalar_aspect_zoom.factor_delta_changed  =  data_zoom_aspect;

/// If `false`, it set `bounds_modified` to true to prevent auto adjusting bounds,
/// also it check if the user double clicked to reset bounds. If so, it set `bounds_modified` to false.
///
/// It's needed when a new plot is started, such as when resetting egui memory and pan/zoom that
/// have not been used before, as the limits can be saved as persistent data.
///
/// Currently, if `allow_auto_bounds(false)` and `plot_ui.set_plot_bounds(PlotBounds)` are used in
/// same time `set_plot_bounds` will be ignored.
///
/// TODO: Refactor `Plot::allow_auto_bounds` and `Plot::set_plot_bounds` feature to work together.
pub fn allow_auto_bounds(mut self, on: bool) -> Self {
self.allow_auto_bounds = on;
self
}

/// Config the button pointer to use for boxed zooming. Default: [`Secondary`](PointerButton::Secondary)
pub fn boxed_zoom_pointer_button(mut self, boxed_zoom_pointer_button: PointerButton) -> Self {
self.boxed_zoom_pointer_button = boxed_zoom_pointer_button;
Expand Down Expand Up @@ -711,6 +729,7 @@ impl Plot {
show_grid,
linked_axes,
linked_cursors,
allow_auto_bounds,

clamp_grid,
grid_spacers,
Expand Down Expand Up @@ -951,23 +970,29 @@ impl Plot {
});
};

let reset_bounds = allow_double_click_reset && response.double_clicked();

// Allow double clicking to reset to the initial bounds.
if allow_double_click_reset && response.double_clicked() {
if reset_bounds {
bounds_modified = false.into();
}

// Apply bounds modifications.
for modification in bounds_modifications {
match modification {
BoundsModification::Set(new_bounds) => {
bounds = new_bounds;
bounds_modified = true.into();
}
BoundsModification::Translate(delta) => {
bounds.translate(delta);
bounds_modified = true.into();
if allow_auto_bounds {
// Apply bounds modifications.
for modification in bounds_modifications {
match modification {
BoundsModification::Set(new_bounds) => {
bounds = new_bounds;
bounds_modified = true.into();
}
BoundsModification::Translate(delta) => {
bounds.translate(delta);
bounds_modified = true.into();
}
}
}
} else if !reset_bounds {
bounds_modified = true.into();
}

// Reset bounds to initial bounds if they haven't been modified.
Expand Down