-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix HDPI: Position of a Modal-Form (and Modeless) is not centered to parent #14664
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4290,7 +4290,7 @@ protected virtual void OnLoad(EventArgs e) | |
|
|
||
| // Also, at this time we can now locate the form on the correct area of the screen. | ||
| // We must do this after applying any autoscaling. | ||
| if (GetState(States.Modal)) | ||
| if (GetState(States.Modal) || (FormStartPosition)_formState[s_formStateStartPos] == FormStartPosition.CenterParent) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously only modal forms ( childForm.StartPosition = FormStartPosition.CenterParent;
childForm.Show(); // no ownerDoes |
||
| { | ||
| AdjustFormPosition(); | ||
| } | ||
|
|
@@ -4564,6 +4564,10 @@ private void WmDpiChanged(ref Message m) | |
| DeviceDpiInternal = e.DeviceDpiNew; | ||
|
|
||
| OnDpiChanged(e); | ||
| if (!Visible && (FormStartPosition)_formState[s_formStateStartPos] == FormStartPosition.CenterParent) | ||
| { | ||
| AdjustFormPosition(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The // Only re-center before the form is shown to the user. Once visible,
// the user may have repositioned it and we must not override that. |
||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2763,6 +2763,77 @@ public void Form_DoesNot_DisposeUserIcon() | |
| smallIcon.Handle.Should().NotBe(0); | ||
| } | ||
|
|
||
| [WinFormsFact] | ||
| public void Form_ShowDialog_WithCenterParent_PositionsCenteredOnParent() | ||
| { | ||
| using Form parentForm = new() | ||
| { | ||
| Size = new Size(400, 400), | ||
| StartPosition = FormStartPosition.Manual, | ||
| Location = new Point(200, 200) | ||
| }; | ||
|
|
||
| parentForm.Show(); | ||
| using Form childForm = new() | ||
| { | ||
| Size = new Size(200, 200), | ||
| StartPosition = FormStartPosition.CenterParent | ||
| }; | ||
|
|
||
| Point capturedLocation = Point.Empty; | ||
| childForm.Load += (sender, e) => | ||
| { | ||
| capturedLocation = childForm.Location; | ||
| childForm.Close(); | ||
| }; | ||
|
|
||
| childForm.ShowDialog(parentForm); | ||
| // Expected center: parentForm center minus half of childForm size. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Consider capturing the size inside the Size capturedSize = Size.Empty;
childForm.Load += (sender, e) =>
{
capturedLocation = childForm.Location;
capturedSize = childForm.Size;
childForm.Close();
};Then use |
||
| int expectedX = parentForm.Left + (parentForm.Width - childForm.Width) / 2; | ||
| int expectedY = parentForm.Top + (parentForm.Height - childForm.Height) / 2; | ||
| int deltaX = SystemInformation.Border3DSize.Width + 1; | ||
| int deltaY = SystemInformation.Border3DSize.Height + 1; | ||
|
|
||
| capturedLocation.X.Should().BeInRange(expectedX - deltaX, expectedX + deltaX); | ||
| capturedLocation.Y.Should().BeInRange(expectedY - deltaY, expectedY + deltaY); | ||
| } | ||
|
|
||
| [WinFormsFact] | ||
| public void Form_Show_WithCenterParent_PositionsCenteredOnParent() | ||
| { | ||
| using Form parentForm = new() | ||
| { | ||
| Size = new Size(400, 400), | ||
| StartPosition = FormStartPosition.Manual, | ||
| Location = new Point(200, 200) | ||
| }; | ||
|
|
||
| parentForm.Show(); | ||
| using Form childForm = new() | ||
| { | ||
| Size = new Size(200, 200), | ||
| StartPosition = FormStartPosition.CenterParent | ||
| }; | ||
|
|
||
| Point capturedLocation = Point.Empty; | ||
| childForm.Load += (sender, e) => | ||
| { | ||
| capturedLocation = childForm.Location; | ||
| }; | ||
|
|
||
| childForm.Show(parentForm); | ||
| childForm.Close(); | ||
|
|
||
| // Expected center: parentForm center minus half of childForm size. | ||
| int expectedX = parentForm.Left + (parentForm.Width - childForm.Width) / 2; | ||
| int expectedY = parentForm.Top + (parentForm.Height - childForm.Height) / 2; | ||
| int deltaX = SystemInformation.Border3DSize.Width + 1; | ||
| int deltaY = SystemInformation.Border3DSize.Height + 1; | ||
|
|
||
| capturedLocation.X.Should().BeInRange(expectedX - deltaX, expectedX + deltaX); | ||
| capturedLocation.Y.Should().BeInRange(expectedY - deltaY, expectedY + deltaY); | ||
| } | ||
|
|
||
| public partial class ParentedForm : Form | ||
| { | ||
| private ParentingForm _parentForm; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
In a cross-DPI scenario (e.g., parent is on a hi-DPI monitor), the sequence could be:
CreateHandle()— window created on primary monitorWM_DPICHANGEDfires (!Visible && CenterParent) →AdjustFormPosition()OnLoadfires (CenterParent) →AdjustFormPosition()againThis results in
AdjustFormPosition()being called twice. It’s harmless (idempotent), but worth noting. Consider adding a comment here explaining why the double call is acceptable, or adding a guard to skip if already adjusted.