-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fixed Incorrect Window.Y and Window.Height values when closing a maximized window #29253
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
base: main
Are you sure you want to change the base?
Conversation
Hey there @@Dhivya-SF4094! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
int cbAttribute); | ||
|
||
internal static Rect GetExtendedFrameBounds(this IntPtr hwnd) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can check hwnd
to avoid unnecessary calls with invalid handles.
if (hwnd == IntPtr.Zero)
{
return Rect.Zero;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsuarezruiz Added a check to avoid unnecessary calls.
|
||
internal static Rect GetExtendedFrameBounds(this IntPtr hwnd) | ||
{ | ||
if (DwmGetWindowAttribute(hwnd, DWMWA_EXTENDED_FRAME_BOUNDS, out PlatformMethods.RECT rect, Marshal.SizeOf<PlatformMethods.RECT>()) == 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we wrapped in a try-catch and return an empty Rect if there are any exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsuarezruiz Wrapped the logic in a try-catch block to return an empty Rect in case of exceptions.
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test SizeChangedOnlyFiresWhenSizeChanges is failing on Windows:
Assert.That(initialValue, Is.EqualTo(finalValue))
String lengths are both 15. Strings differ at index 8.
Expected: "resized 3 times"
But was: "resized 1 times"
-------------------^
Is depending on Window size changes, could you verify if is related with the changes?
{ | ||
if (hwnd == IntPtr.Zero) | ||
{ | ||
return new Rect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these allocations necessary? Would having private static readonly Rect EmptyRect = new()
prevent them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MartyIX Initialized emptyRect within the method scope to avoid unnecessary memory allocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The end effect is the same, I believe. I just wanted to point out that perhaps there can be just one "empty" rect instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MartyIX Updated the code to use a static EmptyRect instance to avoid repeated allocations
@jsuarezruiz Currently looking into the SizeChangedOnlyFiresWhenSizeChanges test failure on Windows. |
@jsuarezruiz Fixed CI failure. Could you please review it once. |
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
void UpdateVirtualViewFrame(AppWindow appWindow) | ||
{ | ||
var hwnd = PlatformView.GetWindowHandle(); | ||
var bounds = hwnd.GetExtendedFrameBounds(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before calling GetExtendedFrameBounds(), confirm hwnd is not null or invalid to prevent runtime errors.
if (hwnd == IntPtr.Zero)
return;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsuarezruiz, Added condition to check if hwnd is not null before calling GetExtendedFrameBounds() to ensure safe execution and prevent potential runtime errors.
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
Issue Detail:
On the Windows platform, if the application window is maximized, the main window X,Y values are in negative (-7, -7). For a non-maximized window, values are correct.
Root Cause
On Windows, when the application window is maximized, the values of Window.Y and Window.Height reported during Window_Destroying are offset by 8 pixels. This occurs because the default window bounds include the non-client area (e.g., title bar and borders), which is excluded by the OS when maximizing the window. As a result, the bounds do not accurately reflect the actual client area used by the application, leading to incorrect frame values being reported to the virtual view.
Description of Change
To resolve this, the implementation now uses the Windows Desktop Window Manager (DWM) API to retrieve the extended frame bounds via DwmGetWindowAttribute with the DWMWA_EXTENDED_FRAME_BOUNDS attribute. These bounds reflect the full screen-space dimensions of the window, even when maximized.
In the UpdateVirtualViewFrame method, when the window is in a normal state, the position and size reported by AppWindow are used directly. When the window is maximized, the logic switches to use the bounds provided by the DWM API. This ensures that the virtual view receives accurate and consistent frame data (X, Y, Width, and Height) regardless of the window state.
Validated the behaviour in the following platforms
Issues Fixed:
Fixes #29066
Screenshots