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

Win32 - set internal _shown flag if ShowWindow will make window visible #16029

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

emmauss
Copy link
Contributor

@emmauss emmauss commented Jun 14, 2024

What does the pull request do?

Fixes an issue where, if a window starts out with window state Maximized, the _shown flag isn't set because Windows doesn't send the WM_SHOWWINDOW message for Maximize state. This flag is used internal for resize operations, and should be properly synced.

What is the current behavior?

What is the updated/expected behavior with this PR?

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

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

@TomEdwardsEnscape
Copy link
Contributor

This solution leaves gaps. A window can be shown without hitting the method containing the new code, either because the window was directly manipulated by non-Avalonia code, or because another Avalonia type is showing it (e.g. popups).

There must be a message which is sent to the window when this situation occurs, otherwise it wouldn't know to start repainting. We should identify it and set the bool when that message is received.

@emmauss emmauss force-pushed the fixes/win32_ensure_shown_set branch from ee724e5 to c4ae3b7 Compare June 17, 2024 07:51
@emmauss
Copy link
Contributor Author

emmauss commented Jun 17, 2024

This solution leaves gaps. A window can be shown without hitting the method containing the new code, either because the window was directly manipulated by non-Avalonia code, or because another Avalonia type is showing it (e.g. popups).

There must be a message which is sent to the window when this situation occurs, otherwise it wouldn't know to start repainting. We should identify it and set the bool when that message is received.

We cannot account for changes users make to the window using native apis. As said in the pr description, WM_SHOWWINDOW message isn't sent when the show command is Maximize or Minimize. As for popups, they always shown with ShowNoActivate command, which will send the message.

@avaloniaui-bot
Copy link

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

@MrJul
Copy link
Member

MrJul commented Jun 17, 2024

The proper message to use here is probably WM_WINDOWPOSCHANGED.

The lParam will point to a WINDOWPOS which contains SWP_SHOWWINDOW on show, SWP_HIDEWINDOW on hide, and neither for other position changes.

@emmauss emmauss force-pushed the fixes/win32_ensure_shown_set branch from c4ae3b7 to 5261367 Compare June 17, 2024 20:10
@emmauss
Copy link
Contributor Author

emmauss commented Jun 17, 2024

The proper message to use here is probably WM_WINDOWPOSCHANGED.

The lParam will point to a WINDOWPOS which contains SWP_SHOWWINDOW on show, SWP_HIDEWINDOW on hide, and neither for other position changes.

This works quite well. Updated to use it.

@avaloniaui-bot
Copy link

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

@maxkatz6 maxkatz6 requested a review from MrJul June 18, 2024 03:12
@MrJul MrJul added this pull request to the merge queue Jun 18, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 18, 2024
@Gillibald Gillibald added this pull request to the merge queue Jun 18, 2024
Merged via the queue into master with commit 87ea0d2 Jun 18, 2024
11 checks passed
@Gillibald Gillibald deleted the fixes/win32_ensure_shown_set branch June 18, 2024 10:00
@maxkatz6 maxkatz6 added backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch bug os-windows labels Jul 4, 2024
grokys pushed a commit that referenced this pull request Jul 22, 2024
…le (#16029)

* win32- set internal _shown flag if ShowWindow will make window visible

* check window visibility state from WM_WINDOWPOSCHANGED message
grokys pushed a commit that referenced this pull request Jul 22, 2024
…le (#16029)

* win32- set internal _shown flag if ShowWindow will make window visible

* check window visibility state from WM_WINDOWPOSCHANGED message
@grokys grokys added backported-11.1.x and removed backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Jul 22, 2024
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.

7 participants