Skip to content

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
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