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): Use SDL2's centering strategy for create window #1059

Merged
merged 4 commits into from
Apr 22, 2021

Conversation

bryphe
Copy link
Member

@bryphe bryphe commented Apr 21, 2021

Issue: Came from onivim/oni2#3349 - the centering strategy implemented in 08c2131 does not handle multi-monitor setups well (it will place the application window in between multiple monitors, if there are 2)

Fix: Just use SDL2's centering strategy, which will center on the relevant monitor based on where the application was started

@bryphe bryphe merged commit 5a82f13 into master Apr 22, 2021
@bryphe bryphe deleted the fix/window/correct-centering branch April 22, 2021 00:39
bryphe added a commit to onivim/oni2 that referenced this pull request Apr 22, 2021
…3443)

__Issue:__ As described in #3349 , when there are two monitors, Onivim will by default start in the middle of both of them.

__Defect:__ The root cause is this commit to Revery - revery-ui/revery@08c2131 - the centering logic causes the calculated position to be in the center of the two monitors.

In addition, in the presence of this bug, there weren't good tools in place to workaround it - ideally, there should be a way to manually specify the window position in the absolute worst case.

__Fix:__
The fix for the bug is in Revery, here: revery-ui/revery#1059

In addition, this PR adds a couple of command-line options:
__`--list-displays`__ - shows a list of available displays and positions, like: 

```sh
oni2 --list-displays
Displays:
0: LG Ultra HD - x: 0 y: 0 width: 1920 height: 1080
1: Color LCD - x: 305 y: 1080 width: 1440 height: 900
```

__Todo:__
- [x] Need to account for the 'upgrade' case - there may be values persisted for `windowX` and `windowY` in the store, so that users hitting #3349 would continue to hit it, even with this fix.

__`--window-position` - set the window position manually, in the display space. For example, if I wanted to start the Onivim on the second display above, I could use `oni2 --window-position=305,1080` to position it on the second display.

- [x] Depends on revery-ui/revery#1059

Fixes #3349
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant