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: Window position creeps between executions on scaled monitors #4443

Merged

Conversation

avery-radmacher
Copy link
Contributor

Compare this gif to the one on the issue report.
egui_window_position_no_creep

Investigation notes

  • WindowSettings.inner_position_pixels and WindowSettings.outer_position_pixels are stored in physical/pixel coordinates.
  • ViewportBuilder::with_position expects to be passed a position in logical coordinates.
  • Prior to this PR, the position was being passed from WindowSettings to with_position without any scaling. This was the root cause of the issue.
  • The fix is to first convert the position to logical coordinates, respecting the scaling factor of the active monitor. This requires us to first determine the active monitor, so I factored out some of the logic in clamp_pos_to_monitor to find the active monitor.

`ViewportBuilder` expects position to be in logical coordinates, but it was being passed in pixels. To fix, scale by the active monitor size and `egui_zoom_factor`.
@avery-radmacher avery-radmacher marked this pull request as ready for review May 2, 2024 14:49
@avery-radmacher avery-radmacher changed the title Fix persistent window position creep Fix: Window position creeps between executions on scaled monitors May 2, 2024
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.

Thanks for fixing this! Have you tested this with differing egui_zoom_factors and monitor dpi scales too?

@emilk emilk added bug Something is broken eframe Relates to epi and eframe egui-winit porblems related to winit and removed egui-winit porblems related to winit labels May 10, 2024
@avery-radmacher
Copy link
Contributor Author

I only tested on 100% and 150% monitors, and an egui_zoom_factor of 1.0 I will do some more testing this weekend!

@avery-radmacher
Copy link
Contributor Author

More testing is complete. This works on other zoom factors and monitor scales too!

@emilk emilk merged commit e06b225 into emilk:master May 11, 2024
23 of 24 checks passed
@emilk
Copy link
Owner

emilk commented May 11, 2024

Thanks for the additional testing!

@avery-radmacher avery-radmacher deleted the fix-persistent-window-position-creep branch May 12, 2024 18:35
@emilk emilk mentioned this pull request May 29, 2024
7 tasks
emilk added a commit to rerun-io/rerun that referenced this pull request May 29, 2024
* Uses: rerun-io/egui_tiles#67
* Uses: lampsitter/egui_commonmark#51
* Split off from #6171
* Closes #5280

### Test
* [x] image loading
* [x] gltf
 
### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6448?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/6448?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!

- [PR Build Summary](https://build.rerun.io/pr/6448)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.

# egui changelog so far

Full diff at https://github.com/emilk/egui/compare/0.27.0..HEAD

#### ecolor
* Fix `hex_color!` macro by re-exporting `color_hex` crate from `ecolor`
[#4372](emilk/egui#4372) (thanks
[@dataphract](https://github.com/dataphract)!)
* Remove `extra_asserts` and `extra_debug_asserts` feature flags
[#4478](emilk/egui#4478)

#### eframe
* Add `register_native_texture` in `eframe::Frame`
[#4246](emilk/egui#4246) (thanks
[@Chaojimengnan](https://github.com/Chaojimengnan)!)
* Early-out from context switching the `glow` backend
[#4284](emilk/egui#4284)
* Fix `ViewportCommand::InnerSize` not resizing viewport on Wayland
(#4211) [#4211](emilk/egui#4211) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Only avoid glow context switching on Windows
[#4296](emilk/egui#4296)
* Improve IME support with new `Event::Ime`
[#4358](emilk/egui#4358) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Allow users to create viewports larger than monitor on Windows & macOS
[#4337](emilk/egui#4337) (thanks
[@lopo12123](https://github.com/lopo12123)!)
* Use `objc2` and its framework crates
[#4395](emilk/egui#4395) (thanks
[@madsmtm](https://github.com/madsmtm)!)
* Update to Rust 1.76 [#4411](emilk/egui#4411)
* Egui-winit: emit physical key presses when a non-Latin layout is
active [#4461](emilk/egui#4461) (thanks
[@TicClick](https://github.com/TicClick)!)
* IME for chinese [#4436](emilk/egui#4436)
(thanks [@rustbasic](https://github.com/rustbasic)!)
* Fix : In Windows, the 'egui_demo_app' screen does not appear.
[#4410](emilk/egui#4410) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Fix: Window position creeps between executions on scaled monitors
[#4443](emilk/egui#4443) (thanks
[@avery-radmacher](https://github.com/avery-radmacher)!)
* Update `image` crate to 0.25
[#4160](emilk/egui#4160)
* Ignore synthetic key presses
[#4514](emilk/egui#4514) (thanks
[@hut](https://github.com/hut)!)
* Fix: still track mouse when dragging outside web canvas
[#4522](emilk/egui#4522)
* Add `NativeOptions::persistence_path`
[#4423](emilk/egui#4423) (thanks
[@lucasmerlin](https://github.com/lucasmerlin)!)
* Use ResizeObserver instead of `resize` event
[#4536](emilk/egui#4536) (thanks
[@jprochazk](https://github.com/jprochazk)!)
* Fix: Don't `.forget()` RAF closure
[#4551](emilk/egui#4551) (thanks
[@jprochazk](https://github.com/jprochazk)!)

#### egui_extras
* Update `image` crate to 0.25
[#4160](emilk/egui#4160)

#### egui_plot
* `Plot::Items:allow_hover` give possibility to masked the interaction
on hovered item [#2558](emilk/egui#2558) (thanks
[@haricot](https://github.com/haricot)!)
* Expose `ClosestElem` and `PlotConfig`
[#4380](emilk/egui#4380) (thanks
[@Narcha](https://github.com/Narcha)!)
* Disable interaction for `ScrollArea` and `Plot` when UI is disabled
[#4457](emilk/egui#4457) (thanks
[@varphone](https://github.com/varphone)!)
* Make sure plot size is positive
[#4429](emilk/egui#4429) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Introduce lifetime to `egui_plot::Plot` to replace `'static` fields
[#4435](emilk/egui#4435) (thanks
[@Fabus1184](https://github.com/Fabus1184)!)
* Hide all other series when alt-clicking in the legend
[#4549](emilk/egui#4549) (thanks
[@abey79](https://github.com/abey79)!)
* Plot now respects the `interact_radius` set in the UI's style
[#4520](emilk/egui#4520) (thanks
[@YgorSouza](https://github.com/YgorSouza)!)

#### egui_glow
* Enable egui_glow's winit feature on wasm (#4420)
[#4421](emilk/egui#4421) (thanks
[@simon-frankau](https://github.com/simon-frankau)!)

#### egui-wgpu
* Update to wgpu 0.20 [#4433](emilk/egui#4433)
(thanks [@KeKsBoTer](https://github.com/KeKsBoTer)!)
* Revert update to wgpu 0.20 => downgrade to wgpu 0.19.1
[#4559](emilk/egui#4559)

#### egui-winit
* Update `webbrowser` to `v1.0.0`
[#4394](emilk/egui#4394) (thanks
[@torokati44](https://github.com/torokati44)!)
* Emit physical key presses when a non-Latin layout is active
[#4461](emilk/egui#4461) (thanks
[@TicClick](https://github.com/TicClick)!)
* IME for chinese [#4436](emilk/egui#4436)
(thanks [@rustbasic](https://github.com/rustbasic)!)
* Fix: Window position creeps between executions on scaled monitors
[#4443](emilk/egui#4443) (thanks
[@avery-radmacher](https://github.com/avery-radmacher)!)
* Ignore synthetic key presses
[#4514](emilk/egui#4514) (thanks
[@hut](https://github.com/hut)!)

#### egui
* Improve the UI for changing the egui theme
[#4257](emilk/egui#4257)
* Change the resize cursor when you reach the resize limit
[#4275](emilk/egui#4275)
* Make `TextEdit` an atomic widget
[#4276](emilk/egui#4276)
* Overload operators for `Rect + Margin`, `Rect - Margin` etc
[#4277](emilk/egui#4277)
* Implement blinking text cursor in `TextEdit`
[#4279](emilk/egui#4279)
* Rename `fn scroll2` to `fn scroll`
[#4282](emilk/egui#4282)
* Change `Frame::multiply_with_opacity` to multiply in gamma space
[#4283](emilk/egui#4283)
* Support order on windows
[#4301](emilk/egui#4301) (thanks
[@alexparlett](https://github.com/alexparlett)!)
* Fix wrong replacement function in deprecation notice of
`drag_released*` [#4314](emilk/egui#4314)
(thanks [@sornas](https://github.com/sornas)!)
* Consider layer transform when positioning text agent
[#4319](emilk/egui#4319) (thanks
[@juancampa](https://github.com/juancampa)!)
* Fix incorrect line breaks
[#4377](emilk/egui#4377) (thanks
[@juancampa](https://github.com/juancampa)!)
* Fix `hex_color!` macro by re-exporting `color_hex` crate from `ecolor`
[#4372](emilk/egui#4372) (thanks
[@dataphract](https://github.com/dataphract)!)
* Change `Ui::allocate_painter` to inherit properties from `Ui`
[#4343](emilk/egui#4343) (thanks
[@varphone](https://github.com/varphone)!)
* Use parent `Ui`s style for popups
[#4325](emilk/egui#4325) (thanks
[@alexparlett](https://github.com/alexparlett)!)
* Fix : take `rounding` into account when using `Slider::trailing_fill`
[#4308](emilk/egui#4308) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Add a way to specify Undoer settings and construct Undoers more easily
[#4357](emilk/egui#4357) (thanks
[@valadaptive](https://github.com/valadaptive)!)
* Add xtask crate [#4293](emilk/egui#4293)
(thanks [@YgorSouza](https://github.com/YgorSouza)!)
* Add `ViewportCommand::RequestCut`, `RequestCopy` and `RequestPaste` to
trigger Clipboard actions
[#4035](emilk/egui#4035) (thanks
[@bu5hm4nn](https://github.com/bu5hm4nn)!)
* Fix `Panel` incorrect size
[#4351](emilk/egui#4351) (thanks
[@zhatuokun](https://github.com/zhatuokun)!)
* Improve IME support with new `Event::Ime`
[#4358](emilk/egui#4358) (thanks
[@rustbasic](https://github.com/rustbasic)!)
* Allow users to create viewports larger than monitor on Windows & macOS
[#4337](emilk/egui#4337) (thanks
[@lopo12123](https://github.com/lopo12123)!)
* Added ability to define colors at UV coordinates along a path
[#4353](emilk/egui#4353) (thanks
[@murl-digital](https://github.com/murl-digital)!)
* Eframe: update ViewportBuilder.with_icon() documentation
[#4408](emilk/egui#4408) (thanks
[@roccoblues](https://github.com/roccoblues)!)
* Update to Rust 1.76 [#4411](emilk/egui#4411)
* Add a `Display` impl for `Vec2`, `Pos2`, and `Rect`
[#4428](emilk/egui#4428) (thanks
[@tgross35](https://github.com/tgross35)!)
* Remove `extra_asserts` and `extra_debug_asserts` feature flags
[#4478](emilk/egui#4478)
* Egui-winit: emit physical key presses when a non-Latin layout is
active [#4461](emilk/egui#4461) (thanks
[@TicClick](https://github.com/TicClick)!)
* Disable interaction for `ScrollArea` and `Plot` when UI is disabled
[#4457](emilk/egui#4457) (thanks
[@varphone](https://github.com/varphone)!)
* Update ahash 0.8.6 -> 0.8.11
[#4507](emilk/egui#4507) (thanks
[@hellodword](https://github.com/hellodword)!)
* `include_image!` now accepts expressions
[#4521](emilk/egui#4521) (thanks
[@YgorSouza](https://github.com/YgorSouza)!)
* Remove `Event::Scroll` and handle it in egui
[#4524](emilk/egui#4524)
* Remove scroll latency for smooth trackpads
[#4526](emilk/egui#4526)
* Smooth out zooming with discreet scroll wheel
[#4530](emilk/egui#4530)
* Add `Options::line_scroll_speed` and `scroll_zoom_speed`
[#4532](emilk/egui#4532)
* Don't panic when replacement glyph is not found
[#4542](emilk/egui#4542) (thanks
[@RyanBluth](https://github.com/RyanBluth)!)
* Make `TextEdit::return_key` optional
[#4543](emilk/egui#4543) (thanks
[@doonv](https://github.com/doonv)!)
* Add `TextEdit::hint_text_font`
[#4517](emilk/egui#4517) (thanks
[@zaaarf](https://github.com/zaaarf)!)
* Add `Options::reduce_texture_memory` to free up RAM
[#4431](emilk/egui#4431) (thanks
[@varphone](https://github.com/varphone)!)
* Fix `Ui::scroll_with_delta` only scrolling if the `ScrollArea` is
focused [#4303](emilk/egui#4303) (thanks
[@lucasmerlin](https://github.com/lucasmerlin)!)
* Add support for text truncation to `egui::Style`
[#4556](emilk/egui#4556) (thanks
[@abey79](https://github.com/abey79)!)
* Hide toolip when opening `ComboBox` drop-down
[#4546](emilk/egui#4546) (thanks
[@abey79](https://github.com/abey79)!)
* Better spacing and sizes for (menu) buttons
[#4558](emilk/egui#4558)

#### epaint
* Add `RectShape::blur_width` to implement shadows
[#4267](emilk/egui#4267)
* Overload operators for `Rect + Margin`, `Rect - Margin` etc
[#4277](emilk/egui#4277)
* Fix incorrect line breaks
[#4377](emilk/egui#4377) (thanks
[@juancampa](https://github.com/juancampa)!)
* Fix `hex_color!` macro by re-exporting `color_hex` crate from `ecolor`
[#4372](emilk/egui#4372) (thanks
[@dataphract](https://github.com/dataphract)!)
* Add `emath::OrderedFloat` (moved from `epaint::util::OrderedFloat`)
[#4389](emilk/egui#4389)
* Added ability to define colors at UV coordinates along a path
[#4353](emilk/egui#4353) (thanks
[@murl-digital](https://github.com/murl-digital)!)
* Add a `Display` impl for `Vec2`, `Pos2`, and `Rect`
[#4428](emilk/egui#4428) (thanks
[@tgross35](https://github.com/tgross35)!)
* Remove `extra_asserts` and `extra_debug_asserts` feature flags
[#4478](emilk/egui#4478)
* Make `epaint::mutex::RwLock` allow `?Sized` types
[#4485](emilk/egui#4485) (thanks
[@crumblingstatue](https://github.com/crumblingstatue)!)
* Update ahash 0.8.6 -> 0.8.11
[#4507](emilk/egui#4507) (thanks
[@hellodword](https://github.com/hellodword)!)
* Don't panic when replacement glyph is not found
[#4542](emilk/egui#4542) (thanks
[@RyanBluth](https://github.com/RyanBluth)!)

---------

Co-authored-by: Antoine Beyeler <[email protected]>
Co-authored-by: Andreas Reich <[email protected]>
hacknus pushed a commit to hacknus/egui that referenced this pull request Oct 30, 2024
…ilk#4443)

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

* Closes <emilk#4442>
* Refactors active monitor detection so it can be called from multiple
locations.

Compare this gif to the one on the issue report.

![egui_window_position_no_creep](https://github.com/emilk/egui/assets/45777186/8e05d4fb-266e-48b9-9223-b65f16500a99)

### Investigation notes

- [`WindowSettings.inner_position_pixels` and
`WindowSettings.outer_position_pixels`](https://github.com/emilk/egui/blob/master/crates/egui-winit/src/window_settings.rs#L8-L12)
are stored in physical/pixel coordinates.
- `ViewportBuilder::with_position` expects to be passed a position in
_logical_ coordinates.
- Prior to this PR, the position was being passed from `WindowSettings`
to `with_position` [without any
scaling](https://github.com/emilk/egui/blob/master/crates/egui-winit/src/window_settings.rs#L61-L68).
This was the root cause of the issue.
- The fix is to first convert the position to logical coordinates,
respecting the scaling factor of the active monitor. This requires us to
first determine the active monitor, so I factored out some of the logic
in
[`clamp_pos_to_monitor`](https://github.com/emilk/egui/blob/master/crates/egui-winit/src/window_settings.rs#L130)
to find the active monitor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken eframe Relates to epi and eframe egui-winit porblems related to winit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Window position creeps between executions on scaled monitors
2 participants