From 190a7afe372fa14934d8e6b20515bee82370c28b Mon Sep 17 00:00:00 2001 From: Zehuan Date: Mon, 4 Aug 2025 17:33:24 +1000 Subject: [PATCH 1/3] WI00938771 Temporary fix for WebBrowser leak (#29) Change WebBrowser references in WebBrowserSiteBase and WebBrowserEvent to a WeakReference. Temporary fix while waiting for https://github.com/dotnet/winforms/issues/13769 (cherry picked from commit 1ca4f6dcabb1944b4e2bd7fe44be769f9b371078) --- .../System.Windows.Forms.Legacy.Tests.csproj | 2 +- .../WebBrowser/WebBrowserTests.cs | 39 +++++++++++++ .../WebBrowser/WebBrowser.WebBrowserEvent.cs | 58 +++++++++++-------- .../Controls/WebBrowser/WebBrowserSiteBase.cs | 20 ++++++- 4 files changed, 92 insertions(+), 27 deletions(-) create mode 100644 src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/WebBrowser/WebBrowserTests.cs diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/System.Windows.Forms.Legacy.Tests.csproj b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/System.Windows.Forms.Legacy.Tests.csproj index ea08983bc41..19eae7d3917 100644 --- a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/System.Windows.Forms.Legacy.Tests.csproj +++ b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/System.Windows.Forms.Legacy.Tests.csproj @@ -6,7 +6,7 @@ true enable enable - $(NoWarn);SA1005;SA1028;SA1129;SA1513;SA1518;WFDEV006;CA1311;CA1507;CA1805;CA1852;CA2201;IDE0002;IDE0005;IDE0059;IDE0063;IDE0073;IDE0300;nullable + $(NoWarn);SA1005;SA1027;SA1028;SA1129;SA1507;SA1513;SA1518;WFDEV006;CA1311;CA1507;CA1805;CA1852;CA2201;IDE0002;IDE0005;IDE0017;IDE0059;IDE0063;IDE0073;IDE0300;nullable System.Windows.Forms.Tests false diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/WebBrowser/WebBrowserTests.cs b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/WebBrowser/WebBrowserTests.cs new file mode 100644 index 00000000000..680ec3f0b9f --- /dev/null +++ b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/WebBrowser/WebBrowserTests.cs @@ -0,0 +1,39 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using System.Text; + +namespace System.Windows.Forms.Legacy.Tests; + +public class WebBrowserTests +{ + [StaFact] + public void TestWebBrowserNoMemoryLeak() + { + var browserWeakRef = CreateWebBrowser(); + + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + var isBrowserGC = !browserWeakRef.IsAlive; + Assert.True(isBrowserGC, "WebBrowser should be collected after GC.Collect and GC.WaitForPendingFinalizers."); + + // NET CORE 8.0+: isBrowserGC is false; + // NET CORE 7.0: isBrowserGC is true; + // NET Framework 4.8: isBrowserGC is true; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static WeakReference CreateWebBrowser() + { + using (var browser = new WebBrowser()) + { + browser.Navigate("about:blank"); + return new WeakReference(browser); + } + } +} diff --git a/src/System.Windows.Forms/System/Windows/Forms/Controls/WebBrowser/WebBrowser.WebBrowserEvent.cs b/src/System.Windows.Forms/System/Windows/Forms/Controls/WebBrowser/WebBrowser.WebBrowserEvent.cs index 72a11305823..c2d5ca09667 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Controls/WebBrowser/WebBrowser.WebBrowserEvent.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Controls/WebBrowser/WebBrowser.WebBrowserEvent.cs @@ -13,12 +13,25 @@ public partial class WebBrowser [ClassInterface(ClassInterfaceType.None)] private class WebBrowserEvent : StandardOleMarshalObject, SHDocVw.DWebBrowserEvents2 { - private readonly WebBrowser _parent; + private readonly WeakReference _parent; + private WebBrowser Parent + { + get + { + if (_parent.TryGetTarget(out var target)) + { + return target; + } + + return null!; + } + } + private bool _haveNavigated; public WebBrowserEvent(WebBrowser parent) { - _parent = parent; + _parent = new(parent); } public bool AllowNavigation { get; set; } @@ -27,11 +40,11 @@ public void CommandStateChange(CommandStateChangeConstants command, bool enable) { if (command == CommandStateChangeConstants.CSC_NAVIGATEBACK) { - _parent.CanGoBackInternal = enable; + Parent.CanGoBackInternal = enable; } else if (command == CommandStateChangeConstants.CSC_NAVIGATEFORWARD) { - _parent.CanGoForwardInternal = enable; + Parent.CanGoForwardInternal = enable; } } @@ -44,8 +57,7 @@ public void BeforeNavigate2( ref object? headers, ref bool cancel) { - Debug.Assert(_parent is not null, "Parent should have been set"); - // Note: we want to allow navigation if we haven't already navigated. + Debug.Assert(Parent is not null, "Parent should have been set"); // Note: we want to allow navigation if we haven't already navigated. if (AllowNavigation || !_haveNavigated) { Debug.Assert(urlObject is null or string, "invalid url type"); @@ -62,7 +74,7 @@ public void BeforeNavigate2( string urlString = urlObject is null ? string.Empty : (string)urlObject; WebBrowserNavigatingEventArgs e = new( new Uri(urlString), targetFrameName is null ? string.Empty : (string)targetFrameName); - _parent.OnNavigating(e); + Parent.OnNavigating(e); cancel = e.Cancel; } else @@ -75,38 +87,38 @@ public unsafe void DocumentComplete(object pDisp, ref object? urlObject) { Debug.Assert(urlObject is null or string, "invalid url"); _haveNavigated = true; - if (_parent._documentStreamToSetOnLoad is not null && (string?)urlObject == "about:blank") + if (Parent._documentStreamToSetOnLoad is not null && (string?)urlObject == "about:blank") { - HtmlDocument? htmlDocument = _parent.Document; + HtmlDocument? htmlDocument = Parent.Document; if (htmlDocument is not null) { IPersistStreamInit.Interface? psi = htmlDocument.DomDocument as IPersistStreamInit.Interface; Debug.Assert(psi is not null, "The Document does not implement IPersistStreamInit"); - using var pStream = _parent._documentStreamToSetOnLoad.ToIStream(); + using var pStream = Parent._documentStreamToSetOnLoad.ToIStream(); psi.Load(pStream); htmlDocument.Encoding = "unicode"; } - _parent._documentStreamToSetOnLoad = null; + Parent._documentStreamToSetOnLoad = null; } else { string urlString = urlObject is null ? string.Empty : urlObject.ToString()!; WebBrowserDocumentCompletedEventArgs e = new( new Uri(urlString)); - _parent.OnDocumentCompleted(e); + Parent.OnDocumentCompleted(e); } } public void TitleChange(string text) { - _parent.OnDocumentTitleChanged(EventArgs.Empty); + Parent.OnDocumentTitleChanged(EventArgs.Empty); } public void SetSecureLockIcon(int secureLockIcon) { - _parent._encryptionLevel = (WebBrowserEncryptionLevel)secureLockIcon; - _parent.OnEncryptionLevelChanged(EventArgs.Empty); + Parent._encryptionLevel = (WebBrowserEncryptionLevel)secureLockIcon; + Parent.OnEncryptionLevelChanged(EventArgs.Empty); } public void NavigateComplete2(object pDisp, ref object? urlObject) @@ -115,29 +127,29 @@ public void NavigateComplete2(object pDisp, ref object? urlObject) string urlString = urlObject is null ? string.Empty : (string)urlObject; WebBrowserNavigatedEventArgs e = new( new Uri(urlString)); - _parent.OnNavigated(e); + Parent.OnNavigated(e); } public void NewWindow2(ref object ppDisp, ref bool cancel) { - CancelEventArgs e = new(); - _parent.OnNewWindow(e); + CancelEventArgs e = new CancelEventArgs(); + Parent.OnNewWindow(e); cancel = e.Cancel; } public void ProgressChange(int progress, int progressMax) { - WebBrowserProgressChangedEventArgs e = new(progress, progressMax); - _parent.OnProgressChanged(e); + WebBrowserProgressChangedEventArgs e = new WebBrowserProgressChangedEventArgs(progress, progressMax); + Parent.OnProgressChanged(e); } public void StatusTextChange(string text) { - _parent._statusText = text ?? string.Empty; - _parent.OnStatusTextChanged(EventArgs.Empty); + Parent._statusText = text ?? string.Empty; + Parent.OnStatusTextChanged(EventArgs.Empty); } - public void DownloadBegin() => _parent.OnFileDownload(EventArgs.Empty); + public void DownloadBegin() => Parent.OnFileDownload(EventArgs.Empty); public void FileDownload(ref bool cancel) { diff --git a/src/System.Windows.Forms/System/Windows/Forms/Controls/WebBrowser/WebBrowserSiteBase.cs b/src/System.Windows.Forms/System/Windows/Forms/Controls/WebBrowser/WebBrowserSiteBase.cs index 1beb1436970..f2aea680654 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Controls/WebBrowser/WebBrowserSiteBase.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Controls/WebBrowser/WebBrowserSiteBase.cs @@ -29,7 +29,7 @@ public unsafe class WebBrowserSiteBase : IPropertyNotifySink.Interface, IDisposable { - private readonly WebBrowserBase _host; + private readonly WeakReference _host; private AxHost.ConnectionPointCookie? _connectionPoint; // @@ -37,7 +37,10 @@ public unsafe class WebBrowserSiteBase : // this cannot be used as a standalone site. It has to be used in conjunction // with WebBrowserBase. Perhaps we can change it in future. // - internal WebBrowserSiteBase(WebBrowserBase h) => _host = h.OrThrowIfNull(); + internal WebBrowserSiteBase(WebBrowserBase h) + { + _host = new(h.OrThrowIfNull()); + } /// /// Dispose(release the cookie) @@ -61,7 +64,18 @@ protected virtual void Dispose(bool disposing) /// /// Retrieves the WebBrowserBase object set in the constructor. /// - internal WebBrowserBase Host => _host; + internal WebBrowserBase Host + { + get + { + if (_host.TryGetTarget(out var target)) + { + return target; + } + + return null!; + } + } // IOleControlSite methods: HRESULT IOleControlSite.Interface.OnControlInfoChanged() => HRESULT.S_OK; From 3b87cfaaa2e156a5a2d12b33782a458744f64e97 Mon Sep 17 00:00:00 2001 From: "sam.peng" Date: Thu, 26 Mar 2026 12:52:58 +0800 Subject: [PATCH 2/3] Add regression tests for PaintEventArgs.ResetGraphics FailFast crash and fix graphics state restoration in PaintEventArgs (WI00857973) --- .../PaintEventArgsResetGraphicsTests.cs | 161 ++++++++++++++++++ .../painteventargs-resetgraphics-failfast.md | 132 ++++++++++++++ .../Windows/Forms/Rendering/PaintEventArgs.cs | 22 ++- 3 files changed, 314 insertions(+), 1 deletion(-) create mode 100644 src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Rendering/PaintEventArgsResetGraphicsTests.cs create mode 100644 src/System.Windows.Forms.Legacy/painteventargs-resetgraphics-failfast.md diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Rendering/PaintEventArgsResetGraphicsTests.cs b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Rendering/PaintEventArgsResetGraphicsTests.cs new file mode 100644 index 00000000000..7d7cbc5de70 --- /dev/null +++ b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Rendering/PaintEventArgsResetGraphicsTests.cs @@ -0,0 +1,161 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Drawing; +using System.Drawing.Drawing2D; + +namespace System.Windows.Forms.Legacy.Tests; + +/// +/// Regression tests for the PaintEventArgs.ResetGraphics FailFast crash. +/// +/// +/// +/// When a is created from an HDC +/// (non-double-buffered path with DrawingEventFlags.SaveState), the graphics state must be +/// saved no matter which property — (public) or the internal +/// GraphicsInternal — first causes the underlying object +/// to be created. If state is not saved, ResetGraphics() silently skips the +/// graphics.Restore() call, causing any clip or transform applied in OnPaintBackground +/// to bleed into OnPaint. In DEBUG builds this also fires Debug.Fail, which in +/// .NET 10 escalates to Environment.FailFast and terminates the process. +/// +/// +/// IMPORTANT: Debug.Fail is decorated with [Conditional("DEBUG")] and is therefore +/// a compile-time no-op in release/test builds. Tests based purely on "no exception was thrown" +/// would pass even without the fix. Instead, these tests assert the observable behavioural +/// invariant: a clip applied during OnPaintBackground must not be visible during +/// OnPaint, which is only true when ResetGraphics() has correctly restored state. +/// +/// +/// See painteventargs-resetgraphics-failfast.md for the full analysis. +/// +/// +public class PaintEventArgsResetGraphicsTests +{ + /// + /// A tiny clip rectangle applied during OnPaintBackground to a region that does not + /// include the centre of the control. If ResetGraphics() fails to restore state, this + /// clip will still be active in OnPaint, and the centre point will appear clipped. + /// + private static readonly Rectangle s_backgroundClip = new(0, 0, 10, 10); + + /// + /// A control that: + /// + /// In : accesses the public Graphics + /// property first (triggering the bug path), then applies a small clip in the top-left corner. + /// In : records whether the centre of the control is + /// inside the current clip region. After a correct ResetGraphics() it must be visible; + /// if state was not restored, the narrow clip from background painting bleeds through and + /// the centre point appears clipped. + /// + /// + private sealed class ClipIsolationControl : Control + { + internal ClipIsolationControl(bool optimizedDoubleBuffer) + { + SetStyle(ControlStyles.AllPaintingInWmPaint, true); + SetStyle(ControlStyles.OptimizedDoubleBuffer, optimizedDoubleBuffer); + SetStyle(ControlStyles.UserPaint, true); + } + + /// + /// Set to when confirms the centre of the + /// control is visible (i.e. not still clipped by the background-paint clip region). + /// + internal bool CentrePointVisibleInOnPaint { get; private set; } + + protected override void OnPaintBackground(PaintEventArgs e) + { + // Access the public Graphics property FIRST so the Graphics object is created + // without the SaveStateIfNeeded callback — this is the exact trigger for the bug. + Graphics g = e.Graphics; + + // Narrow the clip to a small corner region that excludes the centre of the control. + // Without the fix, ResetGraphics() skips Restore(), so this clip persists into OnPaint. + g.SetClip(s_backgroundClip); + + base.OnPaintBackground(e); + } + + protected override void OnPaint(PaintEventArgs e) + { + // If ResetGraphics() correctly restored state, the clip here should be the full + // client rectangle, so the centre point is visible. + // If state was NOT restored, the narrow background clip is still active and the + // centre is clipped out. + Point centre = new(Width / 2, Height / 2); + CentrePointVisibleInOnPaint = e.Graphics.IsVisible(centre); + } + } + + /// + /// Verifies that a clip applied via the public property + /// in is properly isolated from + /// on the non-double-buffered (AllPaintingInWmPaint) code path. + /// + /// + /// + /// This test catches the bug in all build configurations. Without the fix, + /// ResetGraphics() silently skips graphics.Restore() (because + /// _savedGraphicsState is null), and the background clip bleeds into + /// OnPaint, causing CentrePointVisibleInOnPaint to be . + /// + /// + [StaFact] + public void ClipAppliedInOnPaintBackground_IsNotVisible_InOnPaint_NonDoubleBuffered() + { + // AllPaintingInWmPaint without OptimizedDoubleBuffer is the non-double-buffered HDC path + // that creates PaintEventArgs with DrawingEventFlags.SaveState — where the bug lives. + using ClipIsolationControl control = new(optimizedDoubleBuffer: false) + { + Size = new Size(100, 100) + }; + + using Form form = new() + { + Size = new Size(200, 200) + }; + + form.Controls.Add(control); + + // Show the form so WmPaint is dispatched and the full paint sequence runs: + // WmPaint → OnPaintBackground (applies clip via public Graphics) + // → ResetGraphics() → OnPaint (checks whether clip was restored) + form.Show(); + control.Refresh(); + + // The centre of the control must be visible in OnPaint — proving that + // ResetGraphics() correctly restored the graphics state after OnPaintBackground. + Assert.True(control.CentrePointVisibleInOnPaint, + "The clip set in OnPaintBackground bled into OnPaint. " + + "ResetGraphics() did not restore the graphics state, which means " + + "_savedGraphicsState was null when it was called (the bug)."); + } + + /// + /// Verifies the same clip-isolation invariant with double-buffering enabled, ensuring + /// the change to the Graphics getter does not break the double-buffered path. + /// + [StaFact] + public void ClipAppliedInOnPaintBackground_IsNotVisible_InOnPaint_DoubleBuffered() + { + using ClipIsolationControl control = new(optimizedDoubleBuffer: true) + { + Size = new Size(100, 100) + }; + + using Form form = new() + { + Size = new Size(200, 200) + }; + + form.Controls.Add(control); + form.Show(); + control.Refresh(); + + Assert.True(control.CentrePointVisibleInOnPaint, + "The clip set in OnPaintBackground bled into OnPaint on the double-buffered path."); + } +} diff --git a/src/System.Windows.Forms.Legacy/painteventargs-resetgraphics-failfast.md b/src/System.Windows.Forms.Legacy/painteventargs-resetgraphics-failfast.md new file mode 100644 index 00000000000..78d9c821c74 --- /dev/null +++ b/src/System.Windows.Forms.Legacy/painteventargs-resetgraphics-failfast.md @@ -0,0 +1,132 @@ +# PaintEventArgs.ResetGraphics – FailFast Crash + +## Problem + +Applications running on .NET 10 can terminate with a `FailFast` crash during painting: + +``` +Message: Called ResetGraphics more than once? + at System.Windows.Forms.PaintEventArgs.ResetGraphics() + at System.Windows.Forms.Control.WmPaint(Message& m) +``` + +The crash occurs when a `Form` (or any `Control`) is shown via `ShowDialog()` and its background is painted through the non-double-buffered code path. The process is terminated because `Debug.Fail` escalates to `Environment.FailFast` in .NET 10. + +## Root Cause + +`PaintEventArgs` has two internal ways to create the underlying `Graphics` object: + +| Property | Method called on `DrawingEventArgs` | Saves graphics state? | +|---|---|---| +| `Graphics` (public) | `GetOrCreateGraphicsInternal()` — no callback | **No** | +| `GraphicsInternal` (internal) | `GetOrCreateGraphicsInternal(SaveStateIfNeeded)` — passes callback | **Yes** | + +When `WmPaint` runs without double-buffering and `AllPaintingInWmPaint` is set, it creates a `PaintEventArgs` with an `HDC` and `DrawingEventFlags.SaveState`: + +```csharp +pevent = new PaintEventArgs(dc, clip, paintBackground ? DrawingEventFlags.SaveState : default); +``` + +The expected call sequence is: + +1. `PaintWithErrorHandling(pevent, PaintLayerBackground)` → `OnPaintBackground` → internal code accesses `GraphicsInternal` → `SaveStateIfNeeded` callback fires → `_savedGraphicsState` is set. +2. `pevent.ResetGraphics()` → finds `_savedGraphicsState != null` → restores state → sets it to `null`. + +However, if user code in `OnPaintBackground` (or any downstream virtual) accesses `e.Graphics` (the public property) **before** any internal WinForms code has accessed `e.GraphicsInternal`, the `Graphics` object is created via the overload **without** the `SaveStateIfNeeded` callback. `_savedGraphicsState` remains `null`. + +When `WmPaint` subsequently calls `pevent.ResetGraphics()`, the guard: + +```csharp +if (_event.Flags.HasFlag(DrawingEventFlags.SaveState) && graphics is not null) +{ + if (_savedGraphicsState is not null) // ← null because state was never saved + { + ... + } + else + { + Debug.Fail("Called ResetGraphics more than once?"); // ← fires → FailFast + } +} +``` + +…detects `_savedGraphicsState == null` and calls `Debug.Fail`, which in .NET 10 terminates the process via `FailFast`. + +The same class of bug also affects `PrintPaintEventArgs` (used in `WmPrintClient`), which is constructed with `DrawingEventFlags.SaveState` and calls `e.ResetGraphics()` from `OnPrint`. + +## Alternative Approaches Considered + +### Option A — Comment out `Debug.Fail` (rejected) + +A prior workaround (WI00857973) commented out the `Debug.Fail` line: + +```csharp +// commented out. Getting thrown a lot when we run net8.0-windows\CargoWiseOneAnyCpu.exe in debug mode. +// see: WI00857973 - Comment out Debug.Fail in PaintEventArgs in forked WinForms +//Debug.Fail("Called ResetGraphics more than once?"); +``` + +This only silences the crash. The underlying state is still broken: + +- `_savedGraphicsState` is still `null`, so `graphics.Restore(...)` is **silently skipped**. +- Any clip region or transform applied during `OnPaintBackground` **bleeds into `OnPaint`**, causing subtle rendering artifacts (wrong clipping, misaligned drawing). +- The `Debug.Fail` guard, which correctly detects real double-call bugs, is permanently disabled. + +| | Comment out `Debug.Fail` | Our fix (Option B) | +|---|---|---| +| Crash fixed | ✓ | ✓ | +| Graphics state correctly restored | ✗ — silently skipped | ✓ | +| Root cause addressed | ✗ | ✓ | +| `Debug.Fail` still guards real double-call bugs | ✗ — removed | ✓ | +| Risk of rendering artifacts | Yes (clip/transform leaks) | None | + +> Note: the `Debug.Fail` message "Called ResetGraphics more than once?" is misleading — the real bug is that it was called *before* state had been saved, not that it was called twice. Commenting it out treats the symptom while allowing incorrect rendering to proceed silently. + +### A note on test design + +`Debug.Fail` is decorated with `[Conditional("DEBUG")]` and is therefore a compile-time no-op in release/test builds. A test that simply asserts "no exception was thrown" when showing and refreshing a control will pass even **without** the fix, because the assertion never executes. + +The correct approach is to assert the **observable behavioural invariant** that the fix preserves: a clip region applied in `OnPaintBackground` via the public `Graphics` property must not be visible in `OnPaint`. This is always detectable via `Graphics.IsVisible(centre)` regardless of build configuration. + +### Option B — Save state in the `Graphics` getter (chosen) + +## Solution + +The fix is in [`PaintEventArgs.Graphics`](System.Windows.Forms/System/Windows/Forms/Rendering/PaintEventArgs.cs): + +The public `Graphics` property is changed from an expression-bodied passthrough to a full property that detects first-time `Graphics` creation and calls `SaveStateIfNeeded` in that case — matching the behaviour already present in `GraphicsInternal`: + +```csharp +public Graphics Graphics +{ + get + { + // When PaintEventArgs is created with an HDC and DrawingEventFlags.SaveState, + // SaveStateIfNeeded is normally called lazily via GraphicsInternal on first access. + // If user code accesses this public Graphics property before any GraphicsInternal + // call, the Graphics object gets created without SaveStateIfNeeded being invoked, + // leaving _savedGraphicsState as null. ResetGraphics() would then incorrectly + // trigger a Debug.Fail. We detect first-time creation here and save the state. + bool willBeCreated = _event.GetGraphics(create: false) is null; + Graphics g = _event.Graphics; + + if (willBeCreated) + { + SaveStateIfNeeded(g); + } + + return g; + } +} +``` + +### Why this is safe + +| Scenario | `willBeCreated` | Effect | +|---|---|---| +| `Graphics`-based constructor (double-buffered path) | `false` — graphics already exists from ctor | No-op; state was already saved in ctor via `SaveStateIfNeeded(graphics)`. | +| HDC path, `GraphicsInternal` accessed first (normal path) | `false` — callback already ran | No-op; `_savedGraphicsState` is already set. | +| HDC path, public `Graphics` accessed first (the bug) | `true` — graphics is about to be created | State saved immediately after creation. `ResetGraphics()` succeeds. ✓ | +| `SaveState` flag not set | No save attempted regardless | `SaveStateIfNeeded` is a no-op when `SaveState` is not in `Flags`. | + +No double-save can occur because `willBeCreated` is only `true` on the very first call that creates the object; all subsequent calls return `false`. diff --git a/src/System.Windows.Forms/System/Windows/Forms/Rendering/PaintEventArgs.cs b/src/System.Windows.Forms/System/Windows/Forms/Rendering/PaintEventArgs.cs index bbdecb9c2af..dd3694c3240 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Rendering/PaintEventArgs.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Rendering/PaintEventArgs.cs @@ -82,7 +82,27 @@ internal PaintEventArgs( /// /// Gets the object used to paint. /// - public Graphics Graphics => _event.Graphics; + public Graphics Graphics + { + get + { + // When PaintEventArgs is created with an HDC and DrawingEventFlags.SaveState, + // SaveStateIfNeeded is normally called lazily via GraphicsInternal on first access. + // If user code accesses this public Graphics property before any GraphicsInternal + // call, the Graphics object gets created without SaveStateIfNeeded being invoked, + // leaving _savedGraphicsState as null. ResetGraphics() would then incorrectly + // trigger a Debug.Fail. We detect first-time creation here and save the state. + bool willBeCreated = _event.GetGraphics(create: false) is null; + Graphics g = _event.Graphics; + + if (willBeCreated) + { + SaveStateIfNeeded(g); + } + + return g; + } + } /// /// Disposes of the resources (other than memory) used by the . From be654c136723923bdb9c80cc50cba421e0fd8965 Mon Sep 17 00:00:00 2001 From: "sam.peng" Date: Fri, 27 Mar 2026 19:42:14 +0800 Subject: [PATCH 3/3] Implement Anchor Layout High-DPI Regression Fixes - Added a new button in MainForm to demonstrate the AnchorLayoutHighDpiRegression. - Refactored Program.cs to set the main method as private and removed unnecessary comments regarding high DPI settings. - Introduced AnchorLayoutHighDpiRegressionTests to validate the behavior of bottom/right-anchored controls in high DPI scenarios. - Created a detailed markdown document outlining the high-DPI regression issues and proposed solutions. - Added compatibility logic in DefaultLayout.AnchorLayoutCompat.cs to handle stale positive anchors and stretch-anchor recovery. - Updated DefaultLayout.cs to integrate the new compatibility checks for anchor information during layout calculations. --- .../AnchorLayoutHighDpiRegressionDemoForm.cs | 304 ++++++++++++ .../MainForm.Designer.cs | 26 +- .../MainForm.cs | 6 + .../Program.cs | 15 +- .../AnchorLayoutHighDpiRegressionTests.cs | 454 ++++++++++++++++++ .../PaintEventArgsResetGraphicsTests.cs | 6 +- .../anchor-layout-highDpi-regression.md | 358 ++++++++++++++ .../DefaultLayout.AnchorLayoutCompat.cs | 177 +++++++ .../Windows/Forms/Layout/DefaultLayout.cs | 6 +- 9 files changed, 1333 insertions(+), 19 deletions(-) create mode 100644 src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/AnchorLayoutHighDpiRegressionDemoForm.cs create mode 100644 src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Layout/AnchorLayoutHighDpiRegressionTests.cs create mode 100644 src/System.Windows.Forms.Legacy/anchor-layout-highDpi-regression.md create mode 100644 src/System.Windows.Forms/System/Windows/Forms/Layout/DefaultLayout.AnchorLayoutCompat.cs diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/AnchorLayoutHighDpiRegressionDemoForm.cs b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/AnchorLayoutHighDpiRegressionDemoForm.cs new file mode 100644 index 00000000000..a86ce829d64 --- /dev/null +++ b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/AnchorLayoutHighDpiRegressionDemoForm.cs @@ -0,0 +1,304 @@ +using System.Drawing; +#if NET +using System.Runtime.InteropServices; +#endif +using System.Windows.Forms; + +namespace Demo; + +/// +/// Demo form that reproduces the anchor-layout high-DPI regression (WI00955507). +/// See anchor-layout-highDpi-regression.md for the full analysis. +/// +/// +/// +/// The regression manifests when ScaleHelper.IsScalingRequirementMet returns +/// — i.e. when at least one monitor is at a DPI scale above 100%. +/// The demo application must run with HighDpiMode.SystemAware (see Program.cs). +/// +/// +/// Symptom: OtherReferenceTextBox is displaced below — or entirely outside — the +/// MiscGroupBox client area after the form is shown. The title bar reports the +/// layout outcome once the form becomes visible. +/// +/// +/// Fix: add "System.Windows.Forms.AnchorLayoutV2": true to the app's +/// runtimeconfig.json to enable the V2 deferred-anchor path. +/// +/// +/// Observed platform behavior (counter-intuitive): this demo fails on +/// .NET Framework 4.8 even at 100% DPI (96 dpi), while it passes on the WTG +/// .NET 10 fork. The forced transient MiscGroupBox height of 50 px means +/// the captured Bottom anchor offset is always a large positive value +/// (e.g. +230 px at 96 dpi), which displaces OtherReferenceTextBox far below +/// the group box regardless of DPI scale. .NET Framework has no mechanism to correct +/// a stale positive anchor. The WTG fork's Solution 3 repair in +/// DefaultLayout.AnchorLayoutCompat.cs detects the positive offset via +/// ShouldRefreshAnchorInfoForStalePositiveAnchors and recomputes it against the +/// stable parent DisplayRectangle on the next layout pass, which is why the +/// demo passes on .NET 10 even before any DPI scaling is involved. +/// +/// +public class AnchorLayoutHighDpiRegressionDemoForm : Form +{ +#if NET + private static readonly string s_frameworkDescription = RuntimeInformation.FrameworkDescription; +#endif + private static readonly int s_scaleDpi = GetScaleDpi(); + private static readonly Size s_miscGroupBoxSize = ScaledSize(324, 240); + private static readonly int s_expectedBottomMargin = Scale(10); + + private readonly GroupBox _miscGroupBox; + private readonly TextBox _otherReferenceTextBox; + private readonly CheckBox _goodsServicesIndicatorCheckBox; + private readonly CheckBox _royaltyPaymentIndicatorCheckBox; + private readonly TextBox _diagnosticsTextBox; + + // Captured in OnLoad (before the form becomes visible) to mirror the + // "before-show" diagnostics logged by DefaultLayoutTest. + private Rectangle _miscGroupBoxClientAtAnchorCapture; + private Rectangle _otherRefBoundsBeforeShow; + private Rectangle _miscGroupBoxClientBeforeShow; + + public AnchorLayoutHighDpiRegressionDemoForm() + { + Font = new Font(new FontFamily("Microsoft Sans Serif"), 8.25f); + + _diagnosticsTextBox = new TextBox + { + Name = "DiagnosticsTextBox", + Dock = DockStyle.Bottom, + Multiline = true, + ReadOnly = true, + ScrollBars = ScrollBars.Vertical, + Height = Scale(96), + TabStop = false + }; + + // Mirror the hierarchy and initialization order from the CargoWise repro: + // TabControl → TabPage → GroupBox, with the GroupBox already parented into the + // tab hierarchy before its bottom-anchored child is added. + TabControl invoiceTabControl = new() + { + Dock = DockStyle.Fill, + Name = "InvoiceTabControl", + MinimumSize = ScaledSize(480, 470), + Size = ScaledSize(480, 480) + }; + + TabPage otherInfoTabPage = new() + { + Text = "Other Info", + Name = "OtherInfoTabPage", + Location = ScaledPoint(4, 23), + Padding = new Padding(Scale(3)), + Size = ScaledSize(480, 323) + }; + + // The CargoWise host uses a custom group box with hidden-caption semantics. + // Using ClientRectangle as DisplayRectangle keeps the reduced repro aligned + // with the authoritative layout math. + _miscGroupBox = new CargoWiseLikeGroupBox + { + Name = "MiscGroupBox", + Anchor = AnchorStyles.Top | AnchorStyles.Bottom | AnchorStyles.Left, + Location = ScaledPoint(7, 3), + Size = s_miscGroupBoxSize, + TabStop = false + }; + + otherInfoTabPage.Controls.Add(_miscGroupBox); + invoiceTabControl.Controls.Add(otherInfoTabPage); + + Label lastPortDateLabel = new() + { + Text = "Date of Direct Shipment:", + Location = ScaledPoint(16, 75), + AutoSize = true + }; + + TextBox lastPortDateTextBox = new() + { + Name = "LastPortDateTextBox", + Location = ScaledPoint(133, 71), + Size = ScaledSize(183, 20), + TabIndex = 2 + }; + + Label conditionsLabel = new() + { + Text = "Conditions:", + Location = ScaledPoint(16, 101), + AutoSize = true + }; + + TextBox conditionsOfSaleTextBox = new() + { + Name = "ConditionsOfSaleTextBox", + Location = ScaledPoint(133, 97), + Size = ScaledSize(183, 20), + TabIndex = 3 + }; + + Label termsLabel = new() + { + Text = "Terms:", + Location = ScaledPoint(16, 127), + AutoSize = true + }; + + TextBox termsOfPaymentTextBox = new() + { + Name = "TermsOfPaymentTextBox", + Location = ScaledPoint(133, 123), + Size = ScaledSize(183, 20), + TabIndex = 4 + }; + + _goodsServicesIndicatorCheckBox = new CheckBox + { + Name = "ServicesIndCheckBox", + Text = "Goods/Services Indicator", + Location = ScaledPoint(133, 149), + AutoSize = true, + TabIndex = 5, + UseVisualStyleBackColor = true + }; + + _royaltyPaymentIndicatorCheckBox = new CheckBox + { + Name = "RoyaltyIndCheckBox", + Text = "Royalty Payment Indicator", + Location = ScaledPoint(133, 169), + AutoSize = true, + TabIndex = 6, + UseVisualStyleBackColor = true + }; + + _otherReferenceTextBox = new TextBox + { + Name = "OtherReferenceTextBox", + Anchor = AnchorStyles.Bottom | AnchorStyles.Left | AnchorStyles.Right, + Location = ScaledPoint(6, 142), + Multiline = true, + ScrollBars = ScrollBars.Vertical, + Size = ScaledSize(310, 88), + TabIndex = 7 + }; + + _miscGroupBox.Controls.Add(lastPortDateLabel); + _miscGroupBox.Controls.Add(lastPortDateTextBox); + _miscGroupBox.Controls.Add(conditionsLabel); + _miscGroupBox.Controls.Add(conditionsOfSaleTextBox); + _miscGroupBox.Controls.Add(termsLabel); + _miscGroupBox.Controls.Add(termsOfPaymentTextBox); + _miscGroupBox.Controls.Add(_goodsServicesIndicatorCheckBox); + _miscGroupBox.Controls.Add(_royaltyPaymentIndicatorCheckBox); + + _miscGroupBoxClientAtAnchorCapture = _miscGroupBox.ClientRectangle; + _miscGroupBox.Controls.Add(_otherReferenceTextBox); + + Controls.Add(invoiceTabControl); + invoiceTabControl.SelectedTab = otherInfoTabPage; + + Controls.Add(_diagnosticsTextBox); + + ClientSize = ScaledSize(500, 560); + Name = nameof(AnchorLayoutHighDpiRegressionDemoForm); + Text = "Anchor Layout High-DPI Regression Demo"; + StartPosition = FormStartPosition.CenterScreen; + } + + /// + protected override void OnLoad(EventArgs e) + { + base.OnLoad(e); + + // Capture the pre-show layout state. This mirrors the "before-show" logging in + // DefaultLayoutTest and is the first place where the anchor-offset bug is visible: + // if OtherReferenceTextBox.Bounds is already outside MiscGroupBox.ClientRectangle + // here, the anchor info was captured with a wrong transient parent height. + _otherRefBoundsBeforeShow = _otherReferenceTextBox.Bounds; + _miscGroupBoxClientBeforeShow = _miscGroupBox.ClientRectangle; + } + + /// + protected override void OnShown(EventArgs e) + { + base.OnShown(e); + LogLayoutOutcome(); + } + + private void LogLayoutOutcome() + { + Rectangle groupBoxClientAfterShow = _miscGroupBox.ClientRectangle; + Rectangle otherRefBoundsAfterShow = _otherReferenceTextBox.Bounds; + + bool withinGroupBox = groupBoxClientAfterShow.Contains(otherRefBoundsAfterShow); + bool overlapsServices = otherRefBoundsAfterShow.IntersectsWith(_goodsServicesIndicatorCheckBox.Bounds); + bool overlapsRoyalty = otherRefBoundsAfterShow.IntersectsWith(_royaltyPaymentIndicatorCheckBox.Bounds); + bool belowIndicators = otherRefBoundsAfterShow.Top >= _royaltyPaymentIndicatorCheckBox.Bounds.Bottom; + int expectedY = groupBoxClientAfterShow.Height - s_expectedBottomMargin - otherRefBoundsAfterShow.Height; + bool alignedToExpectedBottomMargin = otherRefBoundsAfterShow.Y == expectedY; + + bool pass = withinGroupBox + && !overlapsServices + && !overlapsRoyalty + && belowIndicators + && alignedToExpectedBottomMargin; + + using Graphics g = CreateGraphics(); + int dpi = (int)g.DpiX; + + string dpiNote = dpi == 96 ? " (DPI=100%: set a monitor >100% to see FAIL)" : string.Empty; + +#if NET + string runtimeInfo = $"TFM={s_frameworkDescription} | HighDpiMode={Application.HighDpiMode} | ScaleDpi={s_scaleDpi} | DeviceDpi={DeviceDpi} | GraphicsDpi={dpi}"; +#else + string runtimeInfo = $"TFM=.NET Framework | ScaleDpi={s_scaleDpi} | GraphicsDpi={dpi}"; +#endif + + Text = $"{(pass ? "PASS" : "FAIL")}{dpiNote} — DPI={dpi}"; + _diagnosticsTextBox.Text = + $"ANCHOR-CAPTURE: GroupBox={_miscGroupBoxClientAtAnchorCapture}{Environment.NewLine}" + + $"BEFORE-SHOW: GroupBox={_miscGroupBoxClientBeforeShow} OtherRef={_otherRefBoundsBeforeShow}{Environment.NewLine}" + + $"AFTER-SHOW: GroupBox={groupBoxClientAfterShow} OtherRef={otherRefBoundsAfterShow}{Environment.NewLine}" + + $"EXPECT: BottomMargin={s_expectedBottomMargin} ExpectedY={expectedY} BelowIndicators={belowIndicators}{Environment.NewLine}" + + runtimeInfo; + } + + private sealed class CargoWiseLikeGroupBox : GroupBox + { + public override Rectangle DisplayRectangle => ClientRectangle; + } + + // Scale a logical (96-dpi) pixel value to device pixels, matching the behaviour of + // ControlDpiScalingHelper.NewScaledPoint/NewScaledSize in the CargoWise codebase. + private static int GetScaleDpi() + { +#if NET + try + { + return (int)GetDpiForSystem(); + } + catch (EntryPointNotFoundException) + { + } +#endif + + using Graphics g = Graphics.FromHwnd(IntPtr.Zero); + + return (int)Math.Round(g.DpiX); + } + + private static int Scale(int value) => (int)Math.Round(value * s_scaleDpi / 96.0); + +#if NET + [DllImport("user32.dll")] + private static extern uint GetDpiForSystem(); +#endif + + private static Point ScaledPoint(int x, int y) => new(Scale(x), Scale(y)); + + private static Size ScaledSize(int w, int h) => new(Scale(w), Scale(h)); +} diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/MainForm.Designer.cs b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/MainForm.Designer.cs index 793b6f22c80..03a0ec0fcbd 100644 --- a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/MainForm.Designer.cs +++ b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/MainForm.Designer.cs @@ -19,6 +19,8 @@ partial class MainForm private Label _dataGridDescriptionLabel = null!; private Button _statusBarButton = null!; private Label _statusBarDescriptionLabel = null!; + private Button _anchorLayoutHighDpiRegressionButton = null!; + private Label _anchorLayoutHighDpiRegressionDescriptionLabel = null!; protected override void Dispose(bool disposing) { @@ -43,6 +45,8 @@ private void InitializeComponent() _dataGridDescriptionLabel = new Label(); _statusBarButton = new Button(); _statusBarDescriptionLabel = new Label(); + _anchorLayoutHighDpiRegressionButton = new Button(); + _anchorLayoutHighDpiRegressionDescriptionLabel = new Label(); SuspendLayout(); // // _titleLabel @@ -136,11 +140,31 @@ private void InitializeComponent() _statusBarDescriptionLabel.Text = "Simple text mode, panel layout, owner-draw rendering, border styles, and sizing grip behavior."; _statusBarDescriptionLabel.TextAlign = ContentAlignment.TopCenter; // + // _anchorLayoutHighDpiRegressionButton + // + _anchorLayoutHighDpiRegressionButton.Location = new Point(210, 378); + _anchorLayoutHighDpiRegressionButton.Name = "_anchorLayoutHighDpiRegressionButton"; + _anchorLayoutHighDpiRegressionButton.Size = new Size(300, 60); + _anchorLayoutHighDpiRegressionButton.TabIndex = 10; + _anchorLayoutHighDpiRegressionButton.Text = "Anchor Layout"; + _anchorLayoutHighDpiRegressionButton.Click += AnchorLayoutHighDpiRegressionButton_Click; + // + // _anchorLayoutHighDpiRegressionDescriptionLabel + // + _anchorLayoutHighDpiRegressionDescriptionLabel.Location = new Point(210, 442); + _anchorLayoutHighDpiRegressionDescriptionLabel.Name = "_anchorLayoutHighDpiRegressionDescriptionLabel"; + _anchorLayoutHighDpiRegressionDescriptionLabel.Size = new Size(300, 40); + _anchorLayoutHighDpiRegressionDescriptionLabel.TabIndex = 11; + _anchorLayoutHighDpiRegressionDescriptionLabel.Text = "Launches the dedicated high-DPI anchor-layout regression repro in a separate process."; + _anchorLayoutHighDpiRegressionDescriptionLabel.TextAlign = ContentAlignment.TopCenter; + // // MainForm // AutoScaleDimensions = new SizeF(7F, 15F); AutoScaleMode = AutoScaleMode.Font; - ClientSize = new Size(720, 380); + ClientSize = new Size(720, 500); + Controls.Add(_anchorLayoutHighDpiRegressionDescriptionLabel); + Controls.Add(_anchorLayoutHighDpiRegressionButton); Controls.Add(_statusBarDescriptionLabel); Controls.Add(_statusBarButton); Controls.Add(_toolBarLabel); diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/MainForm.cs b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/MainForm.cs index 593b78b5c41..83dd13d17d0 100644 --- a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/MainForm.cs +++ b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/MainForm.cs @@ -32,4 +32,10 @@ private void StatusBarButton_Click(object? sender, EventArgs e) StatusBarForm form = new(); form.Show(this); } + + private void AnchorLayoutHighDpiRegressionButton_Click(object? sender, EventArgs e) + { + AnchorLayoutHighDpiRegressionDemoForm form = new(); + form.Show(this); + } } diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/Program.cs b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/Program.cs index 5cae1e14eeb..75e855007cd 100644 --- a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/Program.cs +++ b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Demo/Program.cs @@ -10,29 +10,16 @@ internal static class Program /// The main entry point for the application. /// [STAThread] - static void Main() + private static void Main() { // To customize application configuration such as set high DPI settings or default font, // see https://aka.ms/applicationconfiguration. Application.EnableVisualStyles(); Application.SetCompatibleTextRenderingDefault(false); - // Set High DPI mode to DpiUnaware, as currently there are some scaling issues when setting to other values - // https://github.com/WiseTechGlobal/Modernization.Content/blob/main/Controls/work-items/Difference%20display%20between%20migrated%20forms%20and%20original%20forms.md #if NET Application.SetHighDpiMode(HighDpiMode.DpiUnaware); #endif - //ApplicationConfiguration.Initialize(); Application.Run(new MainForm()); } } } - -#if NETFRAMEWORK -namespace System.Runtime.Versioning -{ - class SupportedOSPlatformAttribute : Attribute - { - public SupportedOSPlatformAttribute(string platformName) { } - } -} -#endif diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Layout/AnchorLayoutHighDpiRegressionTests.cs b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Layout/AnchorLayoutHighDpiRegressionTests.cs new file mode 100644 index 00000000000..e4fd9938dc2 --- /dev/null +++ b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Layout/AnchorLayoutHighDpiRegressionTests.cs @@ -0,0 +1,454 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Drawing; +using System.Reflection; +using System.Windows.Forms.Layout; +using System.Windows.Forms.Primitives; + + +namespace System.Windows.Forms.Legacy.Tests; + +/// +/// Issue-specific regression tests for WI00955507. +/// +/// +/// +/// These tests stay focused on the real CargoWise regression shape: a bottom/right-anchored +/// text box inside a group box inside a tab page, under high DPI, with transient parent +/// geometry during anchor capture. +/// +/// +/// See anchor-layout-highDpi-regression.md for the full analysis. +/// +/// +public class AnchorLayoutHighDpiRegressionTests +{ + private const string AnchorLayoutV2SwitchName = "System.Windows.Forms.AnchorLayoutV2"; + private static readonly int s_scaleDpi = GetScaleDpi(); + + private readonly ITestOutputHelper _output; + + public AnchorLayoutHighDpiRegressionTests(ITestOutputHelper output) + { + _output = output; + } + + /// + /// Sets the AnchorLayoutV2 AppContext switch and resets the + /// cached field so the new value is + /// actually observed — without relying on TestSwitch.LocalAppContext.DisableCaching + /// in a runtimeconfig file. + /// + private static void SetAnchorLayoutV2Switch(bool value) + { + // LocalAppContextSwitches caches switch values in a private static int field + // (0 = uncached, 1 = true, -1 = false). Reset it to 0 so the next access + // re-reads the value from AppContext rather than short-circuiting from cache. + typeof(LocalAppContextSwitches) + .GetField("s_anchorLayoutV2", BindingFlags.NonPublic | BindingFlags.Static)! + .SetValue(null, 0); + + AppContext.SetSwitch(AnchorLayoutV2SwitchName, value); + } + + private static int ScaleLogicalPixels(int value, float dpi) => (int)Math.Round(value * dpi / 96f); + + private static int ScaleLogicalPixels(int value) => (int)Math.Round(value * s_scaleDpi / 96f); + + private static Point ScaledPoint(int x, int y) => new(ScaleLogicalPixels(x), ScaleLogicalPixels(y)); + + private static Size ScaledSize(int width, int height) => new(ScaleLogicalPixels(width), ScaleLogicalPixels(height)); + + private static int GetScaleDpi() + { + using Graphics graphics = Graphics.FromHwnd(IntPtr.Zero); + + return (int)Math.Round(graphics.DpiX); + } + + /// + /// Integration-level regression: replicates the OtherReferenceTextBox scenario from + /// DefaultLayoutTest.cs (WI00955507) using standard WinForms controls. + /// A bottom/right-anchored inside a inside a + /// must remain fully contained within the group box after the form + /// is shown and layout is complete. + /// + [StaFact] + public void BottomRightAnchoredTextBoxInsideGroupBoxInsideTabPage_WithAnchorLayoutV2_RemainsWithinGroupBoxAfterFormIsShown() + { + SetAnchorLayoutV2Switch(true); + try + { + // Sizes chosen so that the textbox's design-time Bottom does NOT exceed the + // group box's final client height, which avoids a permanent deferral. + // This mirrors the net48 geometry from the CargoWise investigation. + using Form form = new() { ClientSize = new Size(400, 430) }; + using TabControl tabControl = new() { Dock = DockStyle.Fill }; + using TabPage tabPage = new(); + using GroupBox groupBox = new() + { + Location = new Point(7, 3), + Size = new Size(324, 240), + Anchor = AnchorStyles.Top | AnchorStyles.Bottom | AnchorStyles.Left + }; + using TextBox otherRef = new() + { + // Anchor = Bottom | Left | Right — must float to the bottom of the group box. + Anchor = AnchorStyles.Bottom | AnchorStyles.Left | AnchorStyles.Right, + Location = new Point(6, 142), + Size = new Size(310, 88), + Multiline = true + }; + + groupBox.Controls.Add(otherRef); + tabPage.Controls.Add(groupBox); + tabControl.TabPages.Add(tabPage); + tabControl.SelectedTab = tabPage; + form.Controls.Add(tabControl); + + form.Show(); + Application.DoEvents(); + form.PerformLayout(); + Application.DoEvents(); + form.Close(); + + Assert.True( + groupBox.ClientRectangle.Contains(otherRef.Bounds), + $"OtherRef should be fully inside GroupBox. " + + $"GroupBox.ClientRectangle={groupBox.ClientRectangle}, OtherRef.Bounds={otherRef.Bounds}"); + + Assert.True(otherRef.Visible, "OtherRef should be visible after form is shown."); + } + finally + { + SetAnchorLayoutV2Switch(false); + } + } + + /// + /// Mirrors the authoritative CargoWise initialization order under the V1 path using + /// standard WinForms controls plus a GroupBox whose + /// matches the hidden-caption semantics of the production host. + /// + [StaFact] + public void BottomRightAnchoredTextBoxInsideGroupBoxInsideTabPage_V1Path_CargoWiseGeometry_ShouldRemainWithinGroupBoxAfterFormIsShown() + { + bool setHighDpiModeResult = Application.SetHighDpiMode(HighDpiMode.SystemAware); + _output.WriteLine($"{nameof(AnchorLayoutHighDpiRegressionTests)}: SetHighDpiMode(SystemAware)={setHighDpiModeResult}, HighDpiMode={Application.HighDpiMode}"); + + using Form form = new() { ClientSize = ScaledSize(500, 560) }; + using TabControl tabControl = new() + { + Dock = DockStyle.Fill, + MinimumSize = ScaledSize(480, 470), + Size = ScaledSize(480, 480) + }; + using TabPage tabPage = new() + { + Location = ScaledPoint(4, 23), + Padding = new Padding(ScaleLogicalPixels(3)), + Size = ScaledSize(480, 323) + }; + using CargoWiseLikeGroupBox groupBox = new(); + using TextBox lastPortDate = new() + { + Location = ScaledPoint(133, 71), + Size = ScaledSize(183, 20), + TabIndex = 2 + }; + using TextBox conditionsOfSale = new() + { + Location = ScaledPoint(133, 97), + Size = ScaledSize(183, 20), + TabIndex = 3 + }; + using TextBox termsOfPayment = new() + { + Location = ScaledPoint(133, 123), + Size = ScaledSize(183, 20), + TabIndex = 4 + }; + using CheckBox servicesIndicator = new() + { + Text = "Goods/Services Indicator", + Location = ScaledPoint(133, 149), + AutoSize = true + }; + using CheckBox royaltyIndicator = new() + { + Text = "Royalty Payment Indicator", + Location = ScaledPoint(133, 169), + AutoSize = true + }; + using TextBox otherRef = new() + { + Anchor = AnchorStyles.Bottom | AnchorStyles.Left | AnchorStyles.Right, + Location = ScaledPoint(6, 142), + Size = ScaledSize(310, 88), + Multiline = true, + ScrollBars = ScrollBars.Vertical, + TabIndex = 7 + }; + + groupBox.Anchor = AnchorStyles.Top | AnchorStyles.Bottom | AnchorStyles.Left; + groupBox.Location = ScaledPoint(7, 3); + groupBox.Size = ScaledSize(324, 240); + groupBox.TabStop = false; + + tabPage.Controls.Add(groupBox); + tabControl.TabPages.Add(tabPage); + + groupBox.Controls.Add(lastPortDate); + groupBox.Controls.Add(conditionsOfSale); + groupBox.Controls.Add(termsOfPayment); + groupBox.Controls.Add(servicesIndicator); + groupBox.Controls.Add(royaltyIndicator); + groupBox.Controls.Add(otherRef); + + tabControl.SelectedTab = tabPage; + form.Controls.Add(tabControl); + + form.Show(); + Application.DoEvents(); + form.PerformLayout(); + Application.DoEvents(); + + using Graphics graphics = form.CreateGraphics(); + _output.WriteLine($"{nameof(AnchorLayoutHighDpiRegressionTests)}: GraphicsDpiX={graphics.DpiX}, GraphicsDpiY={graphics.DpiY}"); + + int expectedY = groupBox.ClientRectangle.Height - ScaleLogicalPixels(10, graphics.DpiY) - otherRef.Bounds.Height; + _output.WriteLine($"Post-show: GroupBox.ClientRectangle={groupBox.ClientRectangle}, GroupBox.DisplayRectangle={groupBox.DisplayRectangle}, OtherRef.Bounds={otherRef.Bounds}, ExpectedY={expectedY}, ServicesIndicator={servicesIndicator.Bounds}, RoyaltyIndicator={royaltyIndicator.Bounds}"); + + Assert.True( + groupBox.ClientRectangle.Contains(otherRef.Bounds), + $"CargoWise geometry: OtherRef should be fully inside GroupBox. " + + $"GroupBox.ClientRectangle={groupBox.ClientRectangle}, OtherRef.Bounds={otherRef.Bounds}"); + + Assert.True( + groupBox.DisplayRectangle.Contains(otherRef.Bounds), + $"CargoWise geometry: OtherRef should be fully inside GroupBox display area. " + + $"GroupBox.DisplayRectangle={groupBox.DisplayRectangle}, OtherRef.Bounds={otherRef.Bounds}"); + + Assert.Equal(expectedY, otherRef.Location.Y); + + Assert.False( + otherRef.Bounds.IntersectsWith(servicesIndicator.Bounds), + $"CargoWise geometry: OtherRef should not overlap ServicesIndicator. " + + $"ServicesIndicator={servicesIndicator.Bounds}, OtherRef={otherRef.Bounds}"); + + Assert.False( + otherRef.Bounds.IntersectsWith(royaltyIndicator.Bounds), + $"CargoWise geometry: OtherRef should not overlap RoyaltyIndicator. " + + $"RoyaltyIndicator={royaltyIndicator.Bounds}, OtherRef={otherRef.Bounds}"); + + Assert.True( + otherRef.Bounds.Top >= royaltyIndicator.Bounds.Bottom, + $"CargoWise geometry: OtherRef should be below RoyaltyIndicator. " + + $"RoyaltyIndicator={royaltyIndicator.Bounds}, OtherRef={otherRef.Bounds}"); + + Assert.Equal(ScaleLogicalPixels(6, graphics.DpiX), otherRef.Location.X); + + form.Close(); + } + + [StaFact] + public void ShouldRefreshAnchorInfoForStalePositiveAnchors_BottomAnchoredTrailingAxis_ReturnsTrue() + { + DefaultLayout.AnchorInfo anchorInfo = CreateAnchorInfo( + left: 10, + top: 70, + right: -190, + bottom: 170, + displayRectangle: new Rectangle(0, 0, 300, 100)); + Rectangle bounds = new(10, 120, 100, 50); + Rectangle currentDisplayRectangle = new(0, 0, 300, 200); + + bool shouldRefresh = ShouldRefreshAnchorInfoForStalePositiveAnchors( + anchorInfo, + bounds, + currentDisplayRectangle, + AnchorStyles.Bottom | AnchorStyles.Left | AnchorStyles.Right); + + Assert.True( + shouldRefresh, + $"A trailing-only bottom anchor with a positive cached bottom offset should be refreshed. " + + $"Bounds={bounds}, DisplayRectangle={currentDisplayRectangle}, CachedBottom={anchorInfo.Bottom}"); + } + + [StaFact] + public void ShouldRefreshAnchorInfoForStalePositiveAnchors_StretchAnchoredVerticalAxis_ReturnsFalse() + { + DefaultLayout.AnchorInfo anchorInfo = CreateAnchorInfo( + left: 8, + top: 0, + right: 656, + bottom: 6, + displayRectangle: new Rectangle(6, 6, 788, 474)); + Rectangle bounds = new(14, 6, 648, 766); + Rectangle currentDisplayRectangle = new(6, 6, 772, 760); + + bool shouldRefresh = ShouldRefreshAnchorInfoForStalePositiveAnchors( + anchorInfo, + bounds, + currentDisplayRectangle, + AnchorStyles.Top | AnchorStyles.Bottom | AnchorStyles.Left); + + Assert.False( + shouldRefresh, + $"A stretch-anchored vertical axis should not be treated as a stale positive bottom anchor. " + + $"Bounds={bounds}, DisplayRectangle={currentDisplayRectangle}, CachedBottom={anchorInfo.Bottom}"); + } + + [StaFact] + public void ShouldRefreshAnchorInfoForStalePositiveAnchors_RightAnchoredTrailingAxis_ReturnsTrue() + { + // Symmetric horizontal-axis case: Right without Left, stale positive right offset. + // Captured right = 140 against a parent width of 100 (transient), parent has since grown to 300. + DefaultLayout.AnchorInfo anchorInfo = CreateAnchorInfo( + left: 190, + top: 10, + right: 140, + bottom: -40, + displayRectangle: new Rectangle(0, 0, 100, 200)); + Rectangle bounds = new(190, 10, 90, 40); + Rectangle currentDisplayRectangle = new(0, 0, 300, 200); + + bool shouldRefresh = ShouldRefreshAnchorInfoForStalePositiveAnchors( + anchorInfo, + bounds, + currentDisplayRectangle, + AnchorStyles.Right | AnchorStyles.Top | AnchorStyles.Bottom); + + Assert.True( + shouldRefresh, + $"A trailing-only right anchor with a positive cached right offset should be refreshed. " + + $"Bounds={bounds}, DisplayRectangle={currentDisplayRectangle}, CachedRight={anchorInfo.Right}"); + } + + /// + /// Fast local regression for the current CargoWise-only issue: a stretch-anchored parent is + /// recovered once from its original captured display rectangle, then is incorrectly eligible + /// for a second stale-positive refresh that overwrites the recovered anchors. + /// + [StaFact] + public void StretchAnchoredGroupBox_RecoveredStretchAnchor_ShouldNotTriggerFollowUpPositiveRefresh() + { + using StretchTestContainer parent = new() + { + Bounds = new Rectangle(8, 40, 784, 772), + SimulatedDisplayRectangle = new Rectangle(6, 6, 772, 760) + }; + using CargoWiseLikeGroupBox groupBox = new() + { + Anchor = AnchorStyles.Top | AnchorStyles.Bottom | AnchorStyles.Left, + Bounds = new Rectangle(14, 6, 648, 100) + }; + + parent.Controls.Add(groupBox); + + SetSpecifiedBounds(parent, new Rectangle(8, 40, 784, 772)); + SetSpecifiedBounds(groupBox, new Rectangle(14, 6, 648, 480)); + + DefaultLayout.AnchorInfo anchorInfo = CreateAnchorInfo(8, 0, 656, -374, new Rectangle(6, 6, 788, 474)); + Rectangle currentDisplayRectangle = parent.DisplayRectangle; + + RefreshAnchorInfoForDisplayRectangleGrowth(groupBox, anchorInfo, currentDisplayRectangle, groupBox.Anchor, isStretchAnchorRefresh: true); + + Rectangle recoveredBounds = ComputeAnchoredBounds(anchorInfo, currentDisplayRectangle, groupBox.Anchor); + groupBox.Bounds = recoveredBounds; + SetSpecifiedBounds(groupBox, new Rectangle(14, 6, 648, 480)); + + bool shouldRefreshAgain = ShouldRefreshAnchorInfoForStalePositiveAnchors(anchorInfo, recoveredBounds, currentDisplayRectangle, groupBox.Anchor); + + _output.WriteLine($"Recovered stretch anchor bounds={recoveredBounds}, shouldRefreshAgain={shouldRefreshAgain}, recoveredBottom={anchorInfo.Bottom}, recoveredDisplayRect={anchorInfo.DisplayRectangle}"); + + Assert.False( + shouldRefreshAgain, + $"Recovered stretch anchor should not trigger a follow-up stale-positive refresh. " + + $"RecoveredBounds={recoveredBounds}, RecoveredBottom={anchorInfo.Bottom}, RecoveredDisplayRect={anchorInfo.DisplayRectangle}"); + } + + private static DefaultLayout.AnchorInfo CreateAnchorInfo(int left, int top, int right, int bottom, Rectangle displayRectangle) => + new() + { + Left = left, + Top = top, + Right = right, + Bottom = bottom, + DisplayRectangle = displayRectangle + }; + + private static void SetSpecifiedBounds(Control control, Rectangle bounds) => + CommonProperties.UpdateSpecifiedBounds(control, bounds.X, bounds.Y, bounds.Width, bounds.Height); + + private static bool ShouldRefreshAnchorInfoForStalePositiveAnchors(DefaultLayout.AnchorInfo anchorInfo, Rectangle bounds, Rectangle displayRectangle, AnchorStyles anchor) => + DefaultLayout.ShouldRefreshAnchorInfoForStalePositiveAnchors(anchorInfo, bounds, displayRectangle, anchor); + + private static void RefreshAnchorInfoForDisplayRectangleGrowth(Control control, DefaultLayout.AnchorInfo anchorInfo, Rectangle displayRectangle, AnchorStyles anchor, bool isStretchAnchorRefresh) => + DefaultLayout.RefreshAnchorInfoForDisplayRectangleGrowth(control, anchorInfo, displayRectangle, anchor, isStretchAnchorRefresh); + + private static Rectangle ComputeAnchoredBounds(DefaultLayout.AnchorInfo anchorInfo, Rectangle displayRectangle, AnchorStyles anchor) + { + int left = anchorInfo.Left + displayRectangle.X; + int top = anchorInfo.Top + displayRectangle.Y; + int right = anchorInfo.Right + displayRectangle.X; + int bottom = anchorInfo.Bottom + displayRectangle.Y; + + if ((anchor & AnchorStyles.Right) != AnchorStyles.None) + { + right += displayRectangle.Width; + + if ((anchor & AnchorStyles.Left) == AnchorStyles.None) + { + left += displayRectangle.Width; + } + } + else if ((anchor & AnchorStyles.Left) == AnchorStyles.None) + { + int center = displayRectangle.Width / 2; + right += center; + left += center; + } + + if ((anchor & AnchorStyles.Bottom) != AnchorStyles.None) + { + bottom += displayRectangle.Height; + + if ((anchor & AnchorStyles.Top) == AnchorStyles.None) + { + top += displayRectangle.Height; + } + } + else if ((anchor & AnchorStyles.Top) == AnchorStyles.None) + { + int center = displayRectangle.Height / 2; + bottom += center; + top += center; + } + + if (right < left) + { + right = left; + } + + if (bottom < top) + { + bottom = top; + } + + return new Rectangle(left, top, right - left, bottom - top); + } + + private sealed class CargoWiseLikeGroupBox : GroupBox + { + public override Rectangle DisplayRectangle => ClientRectangle; + } + + private sealed class StretchTestContainer : Panel + { + public Rectangle SimulatedDisplayRectangle { get; set; } + + public override Rectangle DisplayRectangle => SimulatedDisplayRectangle; + } +} diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Rendering/PaintEventArgsResetGraphicsTests.cs b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Rendering/PaintEventArgsResetGraphicsTests.cs index 7d7cbc5de70..c48191e1f25 100644 --- a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Rendering/PaintEventArgsResetGraphicsTests.cs +++ b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/Rendering/PaintEventArgsResetGraphicsTests.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Drawing; @@ -41,7 +41,7 @@ public class PaintEventArgsResetGraphicsTests private static readonly Rectangle s_backgroundClip = new(0, 0, 10, 10); /// - /// A control that: + /// A control that: /// /// In : accesses the public Graphics /// property first (triggering the bug path), then applies a small clip in the top-left corner. @@ -99,7 +99,7 @@ protected override void OnPaint(PaintEventArgs e) /// /// This test catches the bug in all build configurations. Without the fix, /// ResetGraphics() silently skips graphics.Restore() (because - /// _savedGraphicsState is null), and the background clip bleeds into + /// _savedGraphicsState is ), and the background clip bleeds into /// OnPaint, causing CentrePointVisibleInOnPaint to be . /// /// diff --git a/src/System.Windows.Forms.Legacy/anchor-layout-highDpi-regression.md b/src/System.Windows.Forms.Legacy/anchor-layout-highDpi-regression.md new file mode 100644 index 00000000000..e8c2a57409e --- /dev/null +++ b/src/System.Windows.Forms.Legacy/anchor-layout-highDpi-regression.md @@ -0,0 +1,358 @@ +# Anchor Layout High-DPI Regression: .NET Framework vs .NET 8+ + +Controls anchored to `Bottom` or `Right` can appear displaced or invisible in .NET 8+ on any +monitor scaled above 100% DPI, while the same code is correct under .NET Framework 4.8. + +The bug lives entirely in the **V1 anchor path** (`UpdateAnchorInfo` plus the compat repair in +`ComputeAnchoredBounds`) and does **not** affect `AnchorLayoutV2`. + +Two separate V1 compat failures exist in the .NET 10 path: + +- A **stale positive anchor capture** for controls anchored only to trailing edges (`Bottom` + without `Top`, or `Right` without `Left`). +- A **stretch-anchor recovery double-refresh** where a stretch-anchored parent is recovered from + the wrong baseline and then immediately re-qualifies for the stale-positive repair on the same + axis, collapsing the recovered height. + +`AnchorLayoutV2` eliminates the root cause (see [Solution 1](#solution-1--enable-anchorlayoutv2-recommended)). +The **fork compat repair** resolves both failures at the framework level without requiring an application +change (see [Solution 3](#solution-3--additive-layout-side-repair-current-fork-implementation)). + +--- + +## Problem Description + +### Symptoms + +On any machine where at least one monitor is scaled above 100% DPI (e.g. 200%), controls with +`Anchor` involving `Bottom` or `Right` inside deeply nested containers (e.g. `GroupBox` → +`TabPage` → `TabControl`) are positioned far outside their parent's client area when the form +becomes visible — or are completely invisible. + +Characteristic signs: + +| Observation | Detail | +|---|---| +| Control appears below visible area | `Bounds.Bottom` greatly exceeds `ClientRectangle.Height` | +| Problem is DPI-dependent | Invisible at 100% DPI, severe at 200%+ | +| Problem is already present before `Show()` | The bad position is set during form construction | +| The stale-anchor bug is .NET 8+ specific | The bad positive anchor capture does not occur under .NET Framework 4.8 | + +### Reproduction + +The displacement is proportional to the DPI scale factor. At 200% DPI, the child control's +coordinates are already DPI-scaled at the time of anchor capture, but the parent's +`DisplayRectangle` is still at a transient construction-time size. For example: + +``` +parent ClientRectangle.Height = 100 ← transient height during construction +control bounds (DPI-scaled) = {Y=284, Height=176} +bottomOffset = (284 + 176) − 100 = +360 ← positive: stale capture detected +→ final Y = finalParentHeight + 360 − 176 ← control far below visible area +``` + +The transient parent height occurs when a child control is added to a container **after** that +container has been inserted into a deeper hierarchy, but before any top-level form is attached. +At that moment the layout engine assigns a temporary, too-small height to the container, and the +V1 path captures the anchor offset against that height. + +--- + +## Root Cause + +### The Stable Behavior in .NET Framework 4.8 + +In .NET Framework, the `EnableAnchorLayoutHighDpiImprovements` switch defaults to `false`. The +legacy `ComputeAnchoredBounds` path always stores `Right` and `Bottom` as **negative distances** +from the parent's edges: + +``` +anchorInfo.Right = elementBounds.Right - parentWidth // e.g. −20 (20 px from right edge) +anchorInfo.Bottom = elementBounds.Bottom - parentHeight // e.g. −12 (12 px from bottom edge) +``` + +These negative offsets are stable regardless of when they are captured and are simply added to +the live `displayRect` dimensions at layout time, placing the control correctly. + +### The Broken Path in .NET 8+ + +`EnableAnchorLayoutHighDpiImprovements` no longer exists. The DPI-aware branch is now gated on +`ScaleHelper.IsScalingRequirementMet`, which returns `true` whenever any monitor is above 96 dpi +(100%) or the process is per-monitor-aware: + +```csharp +// ScaleHelper.cs +internal static bool IsScalingRequirementMet => IsScalingRequired || s_processPerMonitorAware; +internal static bool IsScalingRequired => InitialSystemDpi != OneHundredPercentLogicalDpi; +``` + +When `IsScalingRequirementMet` is `true`, `UpdateAnchorInfo` (V1) takes a special branch intended +to rescue controls that were pushed off-screen by a DPI-scale event: + +```csharp +if (IsAnchored(anchor, AnchorStyles.Right)) +{ + if (ScaleHelper.IsScalingRequirementMet + && (anchorInfo.Right - parentWidth > 0) // control appears beyond right edge + && (oldAnchorInfo.Right < 0)) // but had a valid negative anchor before + { + // Preserve old anchor to avoid losing the control off the right edge. + anchorInfo.Right = oldAnchorInfo.Right; + } + else + { + anchorInfo.Right -= parentWidth; // standard negative-distance calculation + } +} +``` + +**The guard fires incorrectly during initial form construction.** When a child control is added +while its parent already has a transient (too-small) `DisplayRectangle`, these conditions hold: + +1. The parent height is transiently small (e.g. `100`). +2. The control's `Y` coordinate is already DPI-scaled (e.g. `284` at 200%). +3. `anchorInfo.Bottom - parentHeight = (284 + 176) − 100 = +360 > 0` — the guard fires. +4. `oldAnchorInfo` is also invalid, so the preserved offset is wrong. + +On the next layout pass, this wrong positive offset is applied against the final (larger) parent +height, displacing the control far outside the visible area. + +### What the Compat Repair Fixes + +The compat repair in `ComputeAnchoredBounds` detects a stale positive trailing anchor and +recomputes it against a stable baseline. After the repair, the stale positive `Bottom` (or +`Right`) value is converted back into a stable negative margin. The control is then placed +correctly relative to the parent's final size. + +### The Stretch-Anchor Double-Refresh + +A second failure can occur on the same layout pass when a container is itself stretch-anchored +(`Top | Bottom` or `Left | Right`): + +1. The stretch anchor is recovered against the growth in the parent display rectangle. +2. After recovery, the `Bottom` (or `Right`) value is positive by design for a stretch-anchored + control — satisfying the stale-positive check. +3. A second refresh would then overwrite the recovered stretch anchors, shrinking the container + back down. + +This is not a separate layout system defect. It is the result of the compat repair firing twice +with different semantics on the same axis. The fix evaluates the stretch check first and +short-circuits the positive check when the stretch path fires (see Solution 3). + +### Why High DPI Amplifies the Error + +At 200% DPI all coordinates are approximately doubled. A moderate anchor offset error (e.g. +180) +becomes a large one (+360), pushing the control well outside any visible area. At 100% DPI the +same error is small enough to be nearly invisible. + +### Contributing Factors Summary + +| Factor | Role | +|---|---| +| `ScaleHelper.IsScalingRequirementMet` | Activates the DPI-aware branch for any non-100% DPI monitor | +| `UpdateAnchorInfo` V1 guard condition | Fires incorrectly during construction against a transient parent size | +| `ControlDpiScalingHelper` coordinate scaling | Multiplies all coordinates by the DPI factor, amplifying the captured error | +| Stretch-anchor follow-up refresh | Reapplies stale-positive logic on an axis already recovered as a stretch anchor, shrinking the parent again | + +--- + +## Solutions + +Three independent approaches exist, ordered from most to least recommended. + +### Decision Guide + +``` +Are you on .NET 8+ and can change the app configuration? + └─ Yes → Solution 1 (AnchorLayoutV2) — eliminates the root cause, zero code change +Are you unable to change configuration (e.g. third-party host)? + └─ Solution 3 (fork additive repair) — no app change required, smallest source delta +Do you need a workaround with no source changes at all? + └─ Solution 2 (DpiUnaware) — loses DPI sharpness on scaled monitors +``` + +--- + +### Solution 1 — Enable `AnchorLayoutV2` ✅ Recommended + +`AnchorLayoutV2` (introduced in .NET 8, opt-in) replaces the over-eager V1 capture with a +**deferred model**: anchor offsets are not committed until the parent's layout is resumed and its +`DisplayRectangle` is stable. This eliminates the transient-capture problem at the source. + +Enable it in `runtimeconfig.template.json` (or `runtimeconfig.json`): + +```json +{ + "configProperties": { + "System.Windows.Forms.AnchorLayoutV2": true + } +} +``` + +| Attribute | Value | +|---|---| +| Risk | Low — opt-in, no source change required | +| Scope | Per-application configuration | +| Verified | Yes — eliminates the transient-capture path entirely | +| Limitation | Requires the application to control its own `runtimeconfig` | + +--- + +### Solution 2 — Use `DpiUnaware` or `DpiUnawareGdiScaled` + +Setting the DPI mode to `DpiUnaware` (or `DpiUnawareGdiScaled`) makes +`IsScalingRequirementMet` return `false`, so .NET 8 follows the same stable anchor path as +.NET Framework. + +```csharp +Application.SetHighDpiMode(HighDpiMode.DpiUnaware); +// or +Application.SetHighDpiMode(HighDpiMode.DpiUnawareGdiScaled); +``` + +| Attribute | Value | +|---|---| +| Risk | Low — well-understood mode | +| Scope | Per-application startup | +| Verified | Yes — sidesteps the broken branch entirely | +| Limitation | UI appears blurry on scaled monitors (GDI bitmap stretching) | + +--- + +### Solution 3 — Additive Layout-Side Repair ⭐ Current Fork Implementation + +This is the **current implementation** in the WiseTech Global WinForms fork. It preserves the +original `UpdateAnchorInfo` behavior entirely and **adds a compat repair step** inside +`ComputeAnchoredBounds` that handles both stale positive trailing anchors and stale stretch-anchor +recovery when the parent's `DisplayRectangle` has grown to a stable size. + +**Key design principle:** additive and non-breaking — the existing V1 behavior is unchanged. + +The single call site in `DefaultLayout.cs`, inside `ComputeAnchoredBounds`, is: + +```csharp +// DefaultLayout.cs — ComputeAnchoredBounds +AnchorInfo layout = GetAnchorInfo(element)!; + +// ... read left/top/right/bottom from layout ... + +AnchorStyles anchor = TryRefreshAnchorInfoForDisplayRectangleGrowth(element, layout, displayRect); +``` + +All compat logic lives in `DefaultLayout.AnchorLayoutCompat.cs` (a partial class file) to keep the +change isolated from the main layout file: + +```csharp +// DefaultLayout.AnchorLayoutCompat.cs +private static AnchorStyles TryRefreshAnchorInfoForDisplayRectangleGrowth( + IArrangedElement element, AnchorInfo anchorInfo, Rectangle displayRect) +{ + AnchorStyles anchor = GetAnchor(element); + Rectangle bounds = GetCachedBounds(element); + + // Stretch check must run first. If it triggers, the positive check is skipped so that + // the recovered stretch anchors are not immediately overwritten by a second refresh. + bool shouldRefreshStretchAnchors = + ShouldRefreshAnchorInfoForStaleStretchAnchors(element, anchorInfo, bounds, displayRect, anchor); + + if (!shouldRefreshStretchAnchors + && !ShouldRefreshAnchorInfoForStalePositiveAnchors(anchorInfo, bounds, displayRect, anchor)) + { + return anchor; + } + + RefreshAnchorInfoForDisplayRectangleGrowth(element, anchorInfo, displayRect, anchor, shouldRefreshStretchAnchors); + + return anchor; +} +``` + +**How the repair works:** + +- `ShouldRefreshAnchorInfoForStalePositiveAnchors(AnchorInfo, Rectangle bounds, Rectangle displayRect, AnchorStyles)` + targets controls anchored only to trailing edges (`Right` without `Left`, `Bottom` without + `Top`). A positive trailing offset on such an axis — combined with evidence that the parent's + `displayRect` has grown past the recorded `anchorInfo.DisplayRectangle` — is the signature of a + stale transient capture. +- `ShouldRefreshAnchorInfoForStaleStretchAnchors(IArrangedElement, AnchorInfo, Rectangle bounds, Rectangle displayRect, AnchorStyles)` + covers the complementary case: a stretch-anchored control (`Left | Right` or `Top | Bottom`) + whose actual size is smaller than the size predicted by the specified bounds and the growth in + the parent display rectangle. It uses `GetDisplayRectangleForSpecifiedContainerBounds` to + reconstruct a stable reference rectangle from the container's `SpecifiedBounds`. +- The **stretch check runs before the positive check** (short-circuit `&&`). This is the key + guard against the double-refresh: once the stretch path fires for an axis, the positive path + cannot fire for the same axis in the same layout pass. +- `RefreshAnchorInfoForDisplayRectangleGrowth` delegates to `ResetAnchorInfo`, passing either the + original captured `DisplayRectangle` (stretch recovery) or a rectangle derived from the + container's `SpecifiedBounds` (trailing-edge recovery), then recomputes all four anchor values + so the trailing offsets become stable negative distances again. + +| Attribute | Value | +|---|---| +| Risk | Low — additive only, no existing behavior changed | +| Scope | Framework source change (no app change required) | +| Verified | Stale-anchor and stretch-anchor regression tests pass on `net10.0-windows` at 200% DPI | +| Limitation | Only targets the transient-capture class of failure; does not address other DPI layout edge cases | + +--- + +## Relationship Between Paths + +``` +.NET Framework 4.8 + └─ EnableAnchorLayoutHighDpiImprovements = false (disabled by default) + └─ Always uses legacy ComputeAnchoredBounds — stable, no DPI-aware guard + +.NET 8+ (default, DPI > 100%) + └─ IsScalingRequirementMet = true + └─ V1 UpdateAnchorInfo with DPI-aware guard — fires spuriously during construction + ├─ Solution 2: make IsScalingRequirementMet false (DpiUnaware) + └─ Solution 3: additive layout-side repair in ComputeAnchoredBounds ← current fork + +.NET 8+ with AnchorLayoutV2 = true + └─ UpdateAnchorInfoV2 — defers capture until parent layout is stable + └─ Solution 1: root-cause fix, no source change needed +``` + +--- + +## Verification + +### Manual Testing + +Requires a machine with at least one monitor set to 200% DPI: + +1. Launch the application and open a form with bottom/right-anchored controls inside nested containers. +2. **Without any fix:** the control is far below its parent or invisible. +3. **With Solution 1** (`AnchorLayoutV2`): control is correctly positioned inside the parent's client area. +4. **With Solution 2** (`DpiUnaware`): control is correctly positioned, but UI may appear blurry. +5. **With Solution 3** (fork repair): control is correctly positioned; test host requires no config change. + +### Automated Test Criteria + +For a bottom/right-anchored control inside a nested container at 200% DPI: + +- The control's `Bounds` are fully contained within the parent's `ClientRectangle` after `Show()`. +- The control does not overlap sibling controls that were correctly sized. +- For stretch-anchored containers: the container's height matches the size implied by the + specified bounds and the display-rectangle growth — it is not collapsed by a second refresh. + +### Fork Regression Tests + +| Test | Status | +|---|---| +| `...V1Path_ShouldRemainWithinGroupBoxAfterFormIsShown` | ✅ trailing-edge stale capture (Solution 3) | +| `...WithAnchorLayoutV2_RemainsWithinGroupBoxAfterFormIsShown` | ✅ V2 path unaffected | +| `...StretchAnchoredGroupBox_RecoveredStretchAnchor_ShouldNotTriggerFollowUpPositiveRefresh` | ✅ stretch double-refresh guard | + +No changes to `Control.cs` are required — the fix is entirely within `DefaultLayout.cs` and +`DefaultLayout.AnchorLayoutCompat.cs`. + +--- + +## References + +- [Anchor layout changes in .NET 8.0](https://github.com/dotnet/winforms/blob/main/docs/design/anchor-layout-changes-in-net80.md) +- `src/System.Windows.Forms/System/Windows/Forms/Layout/DefaultLayout.cs` — `UpdateAnchorInfo`, `ComputeAnchoredBounds`, `UpdateAnchorInfoV2` +- `src/System.Windows.Forms/System/Windows/Forms/Layout/DefaultLayout.AnchorLayoutCompat.cs` — `TryRefreshAnchorInfoForDisplayRectangleGrowth`, `ShouldRefreshAnchorInfoForStalePositiveAnchors`, `ShouldRefreshAnchorInfoForStaleStretchAnchors`, `RefreshAnchorInfoForDisplayRectangleGrowth`, `ResetAnchorInfo` +- `src/System.Windows.Forms.Primitives/src/System/Windows/Forms/Internals/ScaleHelper.cs` — `IsScalingRequirementMet` diff --git a/src/System.Windows.Forms/System/Windows/Forms/Layout/DefaultLayout.AnchorLayoutCompat.cs b/src/System.Windows.Forms/System/Windows/Forms/Layout/DefaultLayout.AnchorLayoutCompat.cs new file mode 100644 index 00000000000..3705afb9830 --- /dev/null +++ b/src/System.Windows.Forms/System/Windows/Forms/Layout/DefaultLayout.AnchorLayoutCompat.cs @@ -0,0 +1,177 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Drawing; + +namespace System.Windows.Forms.Layout; + +internal partial class DefaultLayout +{ + private static AnchorStyles TryRefreshAnchorInfoForDisplayRectangleGrowth(IArrangedElement element, AnchorInfo anchorInfo, Rectangle displayRect) + { + AnchorStyles anchor = GetAnchor(element); + Rectangle bounds = GetCachedBounds(element); + bool shouldRefreshStretchAnchors = ShouldRefreshAnchorInfoForStaleStretchAnchors(element, anchorInfo, bounds, displayRect, anchor); + + if (!shouldRefreshStretchAnchors + && !ShouldRefreshAnchorInfoForStalePositiveAnchors(anchorInfo, bounds, displayRect, anchor)) + { + return anchor; + } + + RefreshAnchorInfoForDisplayRectangleGrowth(element, anchorInfo, displayRect, anchor, shouldRefreshStretchAnchors); + + return anchor; + } + + internal static bool ShouldRefreshAnchorInfoForStalePositiveAnchors(AnchorInfo anchorInfo, Rectangle bounds, Rectangle displayRect, AnchorStyles anchor) + { + bool hasStaleRightAnchor = IsAnchored(anchor, AnchorStyles.Right) + && !IsAnchored(anchor, AnchorStyles.Left) + && anchorInfo.Right > 0 + && (bounds.Right - displayRect.X <= displayRect.Width + || displayRect.Width > anchorInfo.DisplayRectangle.Width); + + bool hasStaleBottomAnchor = IsAnchored(anchor, AnchorStyles.Bottom) + && !IsAnchored(anchor, AnchorStyles.Top) + && anchorInfo.Bottom > 0 + && (bounds.Bottom - displayRect.Y <= displayRect.Height + || displayRect.Height > anchorInfo.DisplayRectangle.Height); + + return hasStaleRightAnchor || hasStaleBottomAnchor; + } + + private static bool ShouldRefreshAnchorInfoForStaleStretchAnchors(IArrangedElement element, AnchorInfo anchorInfo, Rectangle bounds, Rectangle displayRect, AnchorStyles anchor) + { + if (!ScaleHelper.IsScalingRequirementMet || element.Container is not { } container) + { + return false; + } + + Rectangle specifiedContainerBounds = CommonProperties.GetSpecifiedBounds(container); + Rectangle specifiedElementBounds = CommonProperties.GetSpecifiedBounds(element); + + if (!HasValidSpecifiedBounds(specifiedContainerBounds, specifiedElementBounds)) + { + return false; + } + + if (!HasDisplayRectangleGrowth(anchorInfo.DisplayRectangle, displayRect)) + { + return false; + } + + Rectangle specifiedDisplayRect = GetDisplayRectangleForSpecifiedContainerBounds(container, displayRect, specifiedContainerBounds); + bool hasStaleHorizontalStretchAnchor = IsAnchored(anchor, AnchorStyles.Left) + && IsAnchored(anchor, AnchorStyles.Right) + && anchorInfo.Right < 0 + && IsSmallerThanExpectedStretchedSize(bounds.Width, specifiedElementBounds.Width, displayRect.Width, specifiedDisplayRect.Width); + + bool hasStaleVerticalStretchAnchor = IsAnchored(anchor, AnchorStyles.Top) + && IsAnchored(anchor, AnchorStyles.Bottom) + && anchorInfo.Bottom < 0 + && IsSmallerThanExpectedStretchedSize(bounds.Height, specifiedElementBounds.Height, displayRect.Height, specifiedDisplayRect.Height); + + return hasStaleHorizontalStretchAnchor || hasStaleVerticalStretchAnchor; + } + + internal static void RefreshAnchorInfoForDisplayRectangleGrowth(IArrangedElement element, AnchorInfo anchorInfo, Rectangle displayRect, AnchorStyles anchor, bool isStretchAnchorRefresh) + { + Rectangle elementBounds = GetCachedBounds(element); + Rectangle anchorDisplayRect = displayRect; + Rectangle originalDisplayRect = anchorInfo.DisplayRectangle; + + if (element.Container is { } container) + { + Rectangle specifiedContainerBounds = CommonProperties.GetSpecifiedBounds(container); + Rectangle specifiedElementBounds = CommonProperties.GetSpecifiedBounds(element); + + if (HasValidSpecifiedBounds(specifiedContainerBounds, specifiedElementBounds)) + { + elementBounds = specifiedElementBounds; + anchorDisplayRect = isStretchAnchorRefresh + ? originalDisplayRect + : GetDisplayRectangleForSpecifiedContainerBounds(container, displayRect, specifiedContainerBounds); + } + } + + ResetAnchorInfo(anchorInfo, elementBounds, anchorDisplayRect, anchor); + } + + private static void ResetAnchorInfo(AnchorInfo anchorInfo, Rectangle elementBounds, Rectangle displayRect, AnchorStyles anchor) + { + anchorInfo.DisplayRectangle = displayRect; + anchorInfo.Left = elementBounds.Left - displayRect.X; + anchorInfo.Top = elementBounds.Top - displayRect.Y; + anchorInfo.Right = elementBounds.Right - displayRect.X; + anchorInfo.Bottom = elementBounds.Bottom - displayRect.Y; + + if (IsAnchored(anchor, AnchorStyles.Right)) + { + anchorInfo.Right -= displayRect.Width; + + if (!IsAnchored(anchor, AnchorStyles.Left)) + { + anchorInfo.Left -= displayRect.Width; + } + } + else if (!IsAnchored(anchor, AnchorStyles.Left)) + { + anchorInfo.Right -= displayRect.Width / 2; + anchorInfo.Left -= displayRect.Width / 2; + } + + if (IsAnchored(anchor, AnchorStyles.Bottom)) + { + anchorInfo.Bottom -= displayRect.Height; + + if (!IsAnchored(anchor, AnchorStyles.Top)) + { + anchorInfo.Top -= displayRect.Height; + } + } + else if (!IsAnchored(anchor, AnchorStyles.Top)) + { + anchorInfo.Bottom -= displayRect.Height / 2; + anchorInfo.Top -= displayRect.Height / 2; + } + } + + private static bool HasValidSpecifiedBounds(Rectangle specifiedContainerBounds, Rectangle specifiedElementBounds) + { + return specifiedContainerBounds.Width > 0 + && specifiedContainerBounds.Height > 0 + && specifiedElementBounds.Width > 0 + && specifiedElementBounds.Height > 0; + } + + private static bool HasDisplayRectangleGrowth(Rectangle originalDisplayRect, Rectangle currentDisplayRect) + { + return currentDisplayRect.Width > originalDisplayRect.Width + || currentDisplayRect.Height > originalDisplayRect.Height; + } + + private static bool IsSmallerThanExpectedStretchedSize(int actualSize, int specifiedSize, int currentDisplaySize, int specifiedDisplaySize) + { + if (specifiedSize <= 0 || currentDisplaySize <= 0 || specifiedDisplaySize <= 0) + { + return false; + } + + int expectedSize = specifiedSize + Math.Max(0, currentDisplaySize - specifiedDisplaySize); + + // Allow 1-pixel tolerance for integer rounding. + return actualSize + 1 < expectedSize; + } + + private static Rectangle GetDisplayRectangleForSpecifiedContainerBounds(IArrangedElement container, Rectangle displayRect, Rectangle specifiedContainerBounds) + { + int nonClientWidth = Math.Max(0, container.Bounds.Width - displayRect.Width); + int nonClientHeight = Math.Max(0, container.Bounds.Height - displayRect.Height); + + int displayWidth = Math.Max(0, specifiedContainerBounds.Width - nonClientWidth); + int displayHeight = Math.Max(0, specifiedContainerBounds.Height - nonClientHeight); + + return new Rectangle(displayRect.X, displayRect.Y, displayWidth, displayHeight); + } +} diff --git a/src/System.Windows.Forms/System/Windows/Forms/Layout/DefaultLayout.cs b/src/System.Windows.Forms/System/Windows/Forms/Layout/DefaultLayout.cs index adbbb880085..b0b05183a93 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/Layout/DefaultLayout.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/Layout/DefaultLayout.cs @@ -242,7 +242,7 @@ private static Rectangle ComputeAnchoredBounds(IArrangedElement element, Rectang int right = layout.Right + displayRect.X; int bottom = layout.Bottom + displayRect.Y; - AnchorStyles anchor = GetAnchor(element); + AnchorStyles anchor = TryRefreshAnchorInfoForDisplayRectangleGrowth(element, layout, displayRect); if (IsAnchored(anchor, AnchorStyles.Right)) { @@ -748,6 +748,10 @@ private static void UpdateAnchorInfo(IArrangedElement element) int parentWidth = parentDisplayRect.Width; int parentHeight = parentDisplayRect.Height; + // Keep the display rectangle that produced these V1 anchors so the compat refresh path + // can detect when the parent later grows and the stored positive right/bottom anchors are stale. + anchorInfo.DisplayRectangle = parentDisplayRect; + // The anchors is relative to the parent DisplayRectangle, so offset the anchors // by the DisplayRect origin anchorInfo.Left -= parentDisplayRect.X;