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

Fix: Response::hover_pos returns incorrect positions with layer transforms #4679

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

Creative0708
Copy link
Contributor

When layer transforms are present, the Response::hover_pos function in egui returns transform * pos instead of transform.inverse() * pos. This is incorrect and isn't consistent with how egui calculates other interaction positions.

See: https://github.com/emilk/egui/blob/master/crates/egui/src/response.rs#L471

This PR fixes this bug, changing transform to transform.inverse(). Nothing fancy here, just a one-line change. scripts/check.sh runs successfully.

Below are videos of before and after with a modified version of the web demo. I added another entry after https://github.com/emilk/egui/blob/master/crates/egui_demo_lib/src/demo/pan_zoom.rs#L108 with a debug rectangle drawn at the return value of hover_pos:

(
    egui::Pos2::new(120.0, 120.0),
    Box::new(|ui, _state| {
        let response =
            ui.allocate_response(egui::Vec2::splat(128.0), egui::Sense::hover());
        ui.painter().rect_filled(
            egui::Rect::from_center_size(
                response.hover_pos().unwrap_or_default(),
                egui::Vec2::splat(8.0),
            ),
            egui::Rounding::ZERO,
            egui::Color32::DEBUG_COLOR,
        );
        response
    }),
),

Without the fix:
https://github.com/emilk/egui/assets/104604363/241cfcab-88ab-459b-8f4d-3367da3aa392

With the fix:
https://github.com/emilk/egui/assets/104604363/e52a7263-44c7-42c1-be46-1ecadc025625

Copy link
Owner

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Nice!

@emilk emilk changed the title Fix: Response::hover_pos returns incorrect positions with layer transforms Fix: Response::hover_pos returns incorrect positions with layer transforms Jun 20, 2024
@emilk emilk added bug Something is broken egui labels Jun 20, 2024
@emilk emilk merged commit cd3e9ea into emilk:master Jun 20, 2024
20 of 21 checks passed
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
…nsforms (emilk#4679)

<!--
Please read the "Making a PR" section of
[`CONTRIBUTING.md`](https://github.com/emilk/egui/blob/master/CONTRIBUTING.md)
before opening a Pull Request!

* Keep your PR:s small and focused.
* The PR title is what ends up in the changelog, so make it descriptive!
* If applicable, add a screenshot or gif.
* If it is a non-trivial addition, consider adding a demo for it to
`egui_demo_lib`, or a new example.
* Do NOT open PR:s from your `master` branch, as that makes it hard for
maintainers to add commits to your PR.
* Remember to run `cargo fmt` and `cargo clippy`.
* Open the PR as a draft until you have self-reviewed it and run
`./scripts/check.sh`.
* When you have addressed a PR comment, mark it as resolved.

Please be patient! I will review your PR, but my time is limited!
-->

When layer transforms are present, the `Response::hover_pos` function in
egui returns `transform * pos` instead of `transform.inverse() * pos`.
This is incorrect and isn't consistent with [how egui calculates other
interaction
positions](https://github.com/emilk/egui/blob/master/crates/egui/src/context.rs#L1193).

See:
https://github.com/emilk/egui/blob/master/crates/egui/src/response.rs#L471

This PR fixes this bug, changing `transform` to `transform.inverse()`.
Nothing fancy here, just a one-line change. `scripts/check.sh` runs
successfully.


Below are videos of before and after with a modified version of the web
demo. I added another entry after
https://github.com/emilk/egui/blob/master/crates/egui_demo_lib/src/demo/pan_zoom.rs#L108
with a debug rectangle drawn at the return value of `hover_pos`:
```rust
(
    egui::Pos2::new(120.0, 120.0),
    Box::new(|ui, _state| {
        let response =
            ui.allocate_response(egui::Vec2::splat(128.0), egui::Sense::hover());
        ui.painter().rect_filled(
            egui::Rect::from_center_size(
                response.hover_pos().unwrap_or_default(),
                egui::Vec2::splat(8.0),
            ),
            egui::Rounding::ZERO,
            egui::Color32::DEBUG_COLOR,
        );
        response
    }),
),
```

Without the fix:

https://github.com/emilk/egui/assets/104604363/241cfcab-88ab-459b-8f4d-3367da3aa392

With the fix:

https://github.com/emilk/egui/assets/104604363/e52a7263-44c7-42c1-be46-1ecadc025625
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken egui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants