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

Incorrect pointer position during changing zoom factor #5379

Open
timstr opened this issue Nov 17, 2024 · 1 comment · May be fixed by #5380
Open

Incorrect pointer position during changing zoom factor #5379

timstr opened this issue Nov 17, 2024 · 1 comment · May be fixed by #5380
Labels
bug Something is broken

Comments

@timstr
Copy link
Contributor

timstr commented Nov 17, 2024

Thank you again for this wonderful library!

Describe the bug
Using the Winit backend, when modifying egui's zoom factor while querying the pointer position, egui simply uses the computed position from the last time the cursor was moved is used. However, this computed position bakes in the past zoom factor, meaning that the current position is scaled incorrectly relative to the top left corner of the window, until the cursor is moved again. Given that the screen rect is always up-to-date with the current zoom factor, this can lead to surprises when doing math that combines both the cursor position and the screen rect.

To Reproduce
Steps to reproduce the behavior:
Here's a quick demo that allows the egui zoom factor to be changed by scrolling and which displays the pointer interact position using a red circle. This is adapted from the eframe native demo at https://docs.rs/eframe/0.29.1/eframe/index.html#usage-native

impl eframe::App for MyEguiApp {
    fn update(&mut self, ctx: &egui::Context, _: &mut eframe::Frame) {
        egui::CentralPanel::default().show(ctx, |ui| {
            ui.heading("Hello World!");

            // START OF DEMO ====================================
            let (scroll, pointer_pos) =
                ui.input(|i| (i.smooth_scroll_delta, i.pointer.interact_pos()));

            ui.ctx().set_zoom_factor(
                (ui.ctx().zoom_factor() * (scroll.y * 0.01).exp()).clamp(0.25, 4.0),
            );

            if let Some(pos) = pointer_pos {
                ui.painter()
                    .circle_stroke(pos, 10.0, egui::Stroke::new(4.0, egui::Color32::RED));
            }
            // END OF DEMO ====================================
        });
    }
}

Here is a screen recording showing how egui's stored pointer position appears to move with the scale and differs from the actual pointer position:

Screencast.from.2024-11-16.04.09.08.PM.webm

Expected behavior

I would expect the pointer position given by egui to correspond to the visible location of the OS cursor at all times. Here I've written a quick patch that achieves this by deferring the zoom factor calculations to be per-frame, which I'll put up for review shortly.

Screencast.from.2024-11-16.04.13.54.PM.webm

Why this matters to me at all

I'm developing a sort of node graph editor with egui and was delighted recently to learn that using egui's zoom factor works shockingly well for global pan and zoom when combined with a little bit of positional translation logic. My goal is to be able to point and zoom in and out and pan around an arbitrarily large 2D canvas while keeping the point under the cursor fixed as I scroll. With egui 0.29.1, this suffers from weird sliding behaviour when I go to compute the normalized screenspace cursor position, since the screen rect is using the latest zoom factor but the cursor position is not.

With the same fix to make the cursor position up-to-date with respect to the latest zoom factor, we can have amazing point-and-zoom effects like this demo from my in-progress node editor. This effect is achieved using nothing but egui's zoom factor and a global Vec2 view offset used to translate all the widgets, plus a few lines of math.

Screencast.from.2024-11-16.04.20.10.PM.webm

Platform:

  • OS: Ubuntu 22.04
@timstr timstr added the bug Something is broken label Nov 17, 2024
@timstr timstr linked a pull request Nov 17, 2024 that will close this issue
1 task
@timstr
Copy link
Contributor Author

timstr commented Nov 21, 2024

Apropos my magical pan & zoom hack: I see now that there's actual support for this already and that there's an interactive demo devoted to it 🤦 I will use that instead and that should also let me have zoom-invariant things like HUDs which would always get scaled with my current approach.

This issue, while it might be surprising, is no longer a concern to me. Feel free to close this and the associated PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant