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.

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

Introduce a persistent zoom_factor #3602

Closed
emilk opened this issue Nov 21, 2023 · 0 comments · Fixed by #3608
Closed

Introduce a persistent zoom_factor #3602

emilk opened this issue Nov 21, 2023 · 0 comments · Fixed by #3608
Assignees
Labels
egui rerun Desired for Rerun.io
Milestone

Comments

@emilk
Copy link
Owner

emilk commented Nov 21, 2023

Currently we have pixels_per_point which the user can change with ctx.set_pixels_per_point. However, it is confusing how that works when e.g. dragging an eframe window from one monitor to another with a different pixel density.

I suggest we introducer a zoom_factor: f32 (default 1.0) which would be a multiplier on top of the native pixels_per_point. This means dragging a window from one monitor to another would keep the same zoom_factor (e.g. 120%), even though the monitors might have different pixel densities.

I'd also like to build-in the cmd+/- gui scaling to egui, which would then operator on zoom_factor.

zoom_factor would thus replace a settable pixels_per_point. Context would instead computer pixels_per_point as zoom_factor * native_pixels_per_point, where zoom_factor is read/write, and pixels_per_point is read-only.

We would then persist zoom_factor in egui::Memory.

This is essentially what we do manually at Rerun: https://github.com/rerun-io/rerun/pull/1448/files


One wrinkle is: what unit to use for input/output? Currently we use UI "points" everywhere, e.g. when describing mouse coordinates and output triangle vertices.

This means integrations would need to ask for Context::pixels_per_point() to calculate it.
We could also change the input/output interface to use native pixels for everything, but that's a pretty big change, and can lead to many subtle bugs.

@emilk emilk added rerun Desired for Rerun.io egui labels Nov 21, 2023
@emilk emilk added this to the 0.25.0 milestone Nov 21, 2023
@emilk emilk changed the title Introduce gui_scale Introduce a persistent gui_scale Nov 21, 2023
@emilk emilk changed the title Introduce a persistent gui_scale Introduce a persistent zoom_factor Nov 21, 2023
@emilk emilk self-assigned this Nov 22, 2023
@emilk emilk modified the milestones: 0.25.0, 0.24.0 Nov 22, 2023
emilk added a commit that referenced this issue Nov 22, 2023
* Closes #3602

You can now zoom any egui app by pressing Cmd+Plus, Cmd+Minus or Cmd+0,
just like in a browser. This will change the current `zoom_factor`
(default 1.0) which is persisted in the egui memory, and is the same for
all viewports.
You can turn off the keyboard shortcuts with `ctx.options_mut(|o|
o.zoom_with_keyboard = false);`

`zoom_factor` can also be explicitly read/written with
`ctx.zoom_factor()` and `ctx.set_zoom_factor()`.

This redefines `pixels_per_point` as `zoom_factor *
native_pixels_per_point`, where `native_pixels_per_point` is whatever is
the native scale factor for the monitor that the current viewport is in.

This adds some complexity to the interaction with winit, since we need
to know the current `zoom_factor` in a lot of places, because all egui
IO is done in ui points. I'm pretty sure this PR fixes a bunch of subtle
bugs though that used to be in this code.

`egui::gui_zoom::zoom_with_keyboard_shortcuts` is now gone, and is no
longer needed, as this is now the default behavior.

`Context::set_pixels_per_point` is still there, but it is recommended
you use `Context::set_zoom_factor` instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
egui rerun Desired for Rerun.io
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant