emilk / emilk/egui_plot

`Plot` wrong bounds data aspect ratio corresponding directive and allow auto_bounds if [false,false]

Open
#12 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
Rust
Stars
468
Forks
108
Avg merge
15m
Merged PRs (30d)
3

Description

About process default bounds (enforce aspect ratio) and process allow auto_bounds.

****
**Wrong bounds aspect ratio corresponding directive:**
- auto_bounds.x && auto_bounds.y
- auto_bounds.x && !auto_bounds.y
- !auto_bounds.x && auto_bounds

https://github.com/emilk/egui/blob/d77c44657206d6a5a9eda0d3b10fac6e189acc27/crates/egui_plot/src/lib.rs#L1024-L1033

_My Actual comprehension of the process default bounds:
The Plot fields `min_auto_bounds: PlotBounds` and `auto_bounds : AxisBool` are used for determined the enforce aspect ratio. `min_auto_bounds` is determined once is initialized and on reset to initial bound else it is last_plot_transform.bounds if any axis `bounds_modified.any()` or if any Nan axis `!bounds_modified.any()` is preserving the current bounds._

The issue is to pass good axis in `PlotTransform::set_aspect_by_changing_axis`

****
**Wrong allow auto_bounds when it used with !auto_bounds.x && !auto_bounds.**

Only allow auto_bounds need to be true if it used like plot.auto_bounds_x() or plot.auto_bounds_y() or plot.auto_bounds_x() and plot.auto_bounds_y(), but it less live auto_bounds if is !auto_bounds.x && !auto_bounds.

https://github.com/emilk/egui/blob/d77c44657206d6a5a9eda0d3b10fac6e189acc27/crates/egui_plot/src/lib.rs#L997-L999

Proposal for resolving this two issues:
```

if let Some(data_aspect) = data_aspect {
if let Some((_, linked_axes)) = &linked_axes {
let change_x = linked_axes.y && !linked_axes.x;
transform.set_aspect_by_changing_axis(data_aspect as f64, change_x);
// bounds_modified = false.into()
} else if auto_bounds.x && auto_bounds.y {
transform.set_aspect_by_expanding(data_aspect as f64);
} else if auto_bounds.any() {
let change_x = auto_bounds.y && !auto_bounds.x;
transform.set_aspect_by_changing_axis(data_aspect as f64, change_x);
} else {
transform.set_aspect_by_changing_axis(data_aspect as f64, false);
bounds_modified.allow_auto_bounds = false;
}
}
[...]

let auto_x = (!bounds_modified.x && bounds_modified.allow_auto_bounds)
&& (!min_auto_bounds.is_valid_x()
|| (auto_bounds.x && bounds_modified.allow_auto_bounds));
let auto_y = (!bounds_modified.x && bounds_modified.allow_auto_bounds)
&& (!min_auto_bounds.is_valid_y()
|| (auto_bounds.y && bounds_modified.allow_auto_bounds));

bounds_modified.allow_auto_bounds = true;
```

or something like that :

```
bounds_modified = AxisChoice::reset(bounds_modified.choice);
if let Some(data_aspect) = data_aspect {
if let Some((_, linked_axes)) = &linked_axes {
let change_x = linked_axes.y && !linked_axes.x;
transform.set_aspect_by_changing_axis(data_aspect as f64, change_x);
} else if auto_bounds.x && auto_bounds.y {
transform.set_aspect_by_expanding(data_aspect as f64);
} else if auto_bounds.any() {
let change_x = auto_bounds.y && !auto_bounds.x;
transform.set_aspect_by_changing_axis(data_aspect as f64, change_x);
} else {
transform.set_aspect_by_changing_axis(data_aspect as f64, false);
bounds_modified = AxisBools::from(([true, true], AxisChoice::None));

}
}
[...]

not needed change like previous proposal.
```

**unfixed:**
auto_bounds : None

[wrong_none.webm](https://github.com/emilk/egui/assets/344493/bd8ab1de-b603-4e06-b0ce-de36a57e561e)

**fixed:**
auto_bounds : None

[fixed_autobounds_none.webm](https://github.com/emilk/egui/assets/344493/4eadc100-8747-430b-b05a-6a6717ebff43)

auto_bounds : X

[auto_bounds_X.webm](https://github.com/emilk/egui/assets/344493/9bbbc241-b1ea-48a3-bfcc-291dd6ac22db)

auto_bounds : Y

[auto_bounds_Y.webm](https://github.com/emilk/egui/assets/344493/9610be0a-a359-4c1b-b492-14dbc7877655)

auto_bounds : X_or_Y

[auto_bounds_X_or_Y.webm](https://github.com/emilk/egui/assets/344493/7d024050-5d45-4f27-a1a2-cdd2be814955)

Which approach is best, is there a more suitable resolution?

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.