Skip to content

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.

Sign up for GitHub

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

Open
Sathish-087 wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix HDPI: Position of a Modal-Form (and Modeless) is not centered to parent #14664

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb3b91e
Added fix for issue 10422
Sathish-087 Jun 11, 2026
26fc742
Merge branch 'dotnet:main' into Fix_Issue_10422
Sathish-087 Jun 22, 2026
236d1a1
Address review comments and Added unit test cases
Sathish-087 Jun 22, 2026
dee1450
Remove unwanted comments
Sathish-087 Jun 22, 2026
ef2285d
Address review correction
Sathish-087 Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/System.Windows.Forms/System/Windows/Forms/Form.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

@LeafShi1 LeafShi1 Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

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:

  1. CreateHandle() — window created on primary monitor
  2. WM_DPICHANGED fires (!Visible && CenterParent) → AdjustFormPosition()
  3. OnLoad fires (CenterParent) → AdjustFormPosition() again

This 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.

@Sathish-087 Sathish-087 Jun 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a guard in CenterToParent to skip the Location update if the position is already at the calculated value. This avoids any unnecessary layout processing when AdjustFormPosition is called twice (e.g., from both WmDpiChanged and OnLoad in a cross-DPI scenario).

@LeafShi1 LeafShi1 Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously only modal forms (ShowDialog) reached this path, and ShowDialog always has an owner. Now non-modal Show() can also reach here. What happens if the user calls:

childForm.StartPosition = FormStartPosition.CenterParent;
childForm.Show();  // no owner

Does AdjustFormPosition() handle Owner == null gracefully? Could you verify this edge case doesn’t throw or produce unexpected positioning?

@Sathish-087 Sathish-087 Jun 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added an Owner is not null guard in OnLoad so AdjustFormPosition is only called for non-modal forms when there's an actual owner. Without this, Previously CenterToParent falls through to CenterToScreen behavior now its works as previous behaviour.

{
AdjustFormPosition();
}
Expand Down Expand Up @@ -4564,6 +4564,10 @@ private void WmDpiChanged(ref Message m)
DeviceDpiInternal = e.DeviceDpiNew;

OnDpiChanged(e);
if (!Visible && (FormStartPosition)_formState[s_formStateStartPos] == FormStartPosition.CenterParent)
{
AdjustFormPosition();

@LeafShi1 LeafShi1 Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The !Visible guard is correct — it prevents overriding the user’s position after the form is shown. However, the intent isn’t immediately obvious to future readers. Consider adding a comment, e.g.:

// 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.

@Sathish-087 Sathish-087 Jun 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment in WmDpiChanged to explain why the !Visible guard is there and what this block does.

}
}

/// <summary>
Expand Down
71 changes: 71 additions & 0 deletions src/test/unit/System.Windows.Forms/System/Windows/Forms/FormTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

@LeafShi1 LeafShi1 Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

childForm.Width and childForm.Height are read after ShowDialog returns (i.e., after Close() was called inside the Load handler). If Close() triggers a WM_DPICHANGED (e.g., the form moves to a different monitor during teardown), the size could change.

Consider capturing the size inside the Load handler alongside the location:

Size capturedSize = Size.Empty;
childForm.Load += (sender, e) =>
{
    capturedLocation = childForm.Location;
    capturedSize = childForm.Size;
    childForm.Close();
};

Then use capturedSize for the expected position calculation.

@Sathish-087 Sathish-087 Jun 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated unit tests to capture childForm.Size inside the Load handler along with Location. The expected center position is now calculated using capturedSize instead of reading childForm.Width/Height after the form is closed.

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;
Expand Down
Loading