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

Multi-Monitor DPI scaling breaks dialog sizes #15304

Merged
merged 3 commits into from
Apr 20, 2024

Conversation

ltetak
Copy link
Contributor

@ltetak ltetak commented Apr 10, 2024

What does the pull request do?

There is a bug that the new window is the wrong size
It may be platform-specific since some platforms may have the possibility to set the window position + size in one call. Windows can't do that.

What is the current behavior?

  1. have two screens with different DPI settings
  2. move the main window to the secondary screen
  3. open a dialog with the "center owner" setting and predefined size (e.g. 200x200)
    The dialog should be 200x200 but it is not because a lot of things happen
  4. the window is created in the default location (somewhere on the main screen) and with the default size
  5. the window is then moved to the proper position and size but this happens in two steps (position and then size)
    2a. position - the window is moved to the center of the window on the secondary screen - but this immediately changes its size since these screens have different DPI settings
    2b. the window had of course wrong size at this point but the resize callback breaks the window.Width and Height settings
    2c. window sets the size - but the Width and Height is already broken

What is the updated/expected behavior with this PR?

Ignore Resize callbacks until the initial size is set

How was the solution implemented (if it's not obvious)?

ugly flag - maybe somebody has a better idea
no unit tests - feel free to add some

@ltetak
Copy link
Contributor Author

ltetak commented Apr 10, 2024

I moved the flag up immediately after setting the initial position.
This was broken here #12833

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0047140-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6
Copy link
Member

We can technically revert #12833, it's not too late yet.
Linux scaling might need to be fixed on X11 backend side.

On the other hand, this PR doesn't look like a big hack either.

cc @kekekeks @emmauss

@maxkatz6 maxkatz6 merged commit 2ae1628 into AvaloniaUI:master Apr 20, 2024
10 checks passed
@maxkatz6 maxkatz6 added the backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch label Apr 20, 2024
maxkatz6 pushed a commit that referenced this pull request Apr 20, 2024
* Multi-Monitor DPI scaling breaks dialog sizes

* move the flag up

* second location
@maxkatz6 maxkatz6 added backported-11.1.x and removed backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Apr 20, 2024
@ltetak
Copy link
Contributor Author

ltetak commented Apr 24, 2024

@maxkatz6 sorry, but I found an issue with this fix - in some cases, the size is correct but the position is wrong. Reverting #12833 may be a better fix after all.

@maxkatz6
Copy link
Member

@ltetak honestly, I feel like we should stop trying to fix the unfixable, until we got linux automation and linux e2e tests, like we have on Windows/macOS. It might take a while to get there, as we don't have priority to do it soon enough.

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

Successfully merging this pull request may close these issues.

3 participants