diff --git a/.github/Feature-Prompts/Net11/01-B05-HighPrecisionTimer-Rework-WorkOrder.md b/.github/Feature-Prompts/Net11/01-B05-HighPrecisionTimer-Rework-WorkOrder.md new file mode 100644 index 00000000000..470d3584b5b --- /dev/null +++ b/.github/Feature-Prompts/Net11/01-B05-HighPrecisionTimer-Rework-WorkOrder.md @@ -0,0 +1,157 @@ +# Work Order: HighPrecisionTimer Rework (Animation Timing) + +**Branch:** `Net11/Integration-2` (KlausLoeffelmann/winforms) +**Files:** +- `src/System.Windows.Forms.Primitives/src/System/Windows/Forms/Animation/HighPrecisionTimer.cs` +- `src/System.Windows.Forms.Primitives/src/System/Windows/Forms/Animation/HighPrecisionTimerTick.cs` +- `src/System.Windows.Forms.Primitives/tests/UnitTests/System/Windows/Forms/Animation/HighPrecisionTimerTests.cs` +- Consumer: `src/System.Windows.Forms/System/Windows/Forms/Rendering/Animation/AnimationManager.cs` + +This work order is self-contained; it results from a code review of the current implementation. +Read the current sources first, verify each finding against the code as it stands (the branch may have +moved), then implement. + +**Keep as-is (explicitly not up for redesign):** the registration model — per-registration +`SynchronizationContext` capture, `InFlight` CAS-based frame coalescing with a `DroppedFrames` counter +surfaced in `HighPrecisionTimerTick`, and the id-based `TimerRegistration` disposable struct +(double-dispose safe, `default` safe). The *clock/pacing core* is what gets replaced. + +--- + +## Finding 1 (critical): fixed-cadence pacer + spin causes a sawtooth burning ~50% of a core + +Current design: `PeriodicTimer` at 14 ms (60 Hz path), then `SpinToTarget` spins to the 16.667 ms +frame target with `SpinOnce(sleep1Threshold: -1)` (never sleeps). + +`PeriodicTimer` fires on its own **fixed** cadence (14, 28, 42, 56, …) and does not re-phase per +`WaitForNextTickAsync` call, while frame targets are 16.67, 33.33, 50, 66.67, … The phase slips +2.67 ms per frame, so spin duration grows every frame — wake 28 → spin 5.3 ms; wake 42 → spin 8 ms; +wake 56 → spin 10.7 ms — until phases wrap and the sawtooth restarts. Average spin ≈ half a frame +≈ 8 ms of every 16.67 ms ⇒ ~50% of one core, continuously, for as long as **any** animation is +registered. Infinite cycles (e.g. a pulsing focus indicator) make this permanent. The 30 Hz path +(30 ms tick vs 33.33 ms frame) has the identical slip. + +**Required fix (architecture, not constant-tuning):** absolute schedule. Due times are +`epoch + frameIndex * period` against a single clock; wait on a mechanism that can hit them +(Finding 3), with at most a sub-millisecond residual spin. Overshoot must be amortized against the +absolute schedule (no `lastTick + period` relative scheduling — that accumulates and runs slow). + +## Finding 2: integer-millisecond arithmetic throughout + +`stopwatch.ElapsedMilliseconds` (truncated `long`) feeds `lastTickTimestamp`, `elapsed`, the spin +target, drift detection, and the `Timestamp`/`Elapsed` values delivered to consumers — ±1 ms +quantization (6% of a 16.667 ms budget) plus systematic truncation bias. Use `ElapsedTicks` / +`Elapsed.TotalMilliseconds` (double) end to end; `HighPrecisionTimerTick` fields stay `TimeSpan`, +constructed from ticks. + +## Finding 3: replace `timeBeginPeriod(1)` with a high-resolution waitable timer + +The code gates on `windows10.0.17134` — which `timeBeginPeriod` (ancient) does not need, but which is +exactly the build (1803) that introduced `CreateWaitableTimerExW` with +`CREATE_WAITABLE_TIMER_HIGH_RESOLUTION`. Use it: + +- absolute due times (negative-relative or absolute FILETIME) with sub-ms accuracy, +- no process-wide timer-resolution raise (current code holds 1 ms resolution for the entire lifetime + of any animation — a documented power/battery anti-pattern, and post-Win11 the effective resolution + changes for occluded windows, silently shifting the timing floor), +- composes directly with Finding 1's absolute schedule and eliminates the spin loop almost entirely. + +Fallback below 17134: keep a coarse path (30 Hz, plain waits, no `timeBeginPeriod`) — document that +sub-frame precision is not attempted there. Remove `TimeBeginPeriod`/`TimeEndPeriod` P/Invokes if no +longer referenced. + +## Finding 4: `Register`/`Unregister` vs `StopTimer` race strands registrations + +`Unregister` does `TryRemove` → `IsEmpty?` → `StopTimer()` without coordinating with `Register`. +Interleaving: A removes the last entry and observes empty; B adds a registration and `EnsureRunning` +sees `s_loopTask != null` (still running) and returns; A stops the timer. Result: live registration, +dead timer — animation frozen until an unrelated `Register` restarts the loop. +**Fix:** perform the emptiness check + stop decision under `s_lock` together with loop-state +transitions, or introduce a generation counter that `StopTimer` validates before actually stopping. + +## Finding 5: per-frame, per-registration closure allocations on the hot path + +`SyncContext.Post(_ => _ = InvokeCallbackAsync(registration, tick, cancellationToken), null)` +allocates closure + delegate per registration per frame (60 Hz × N renderers of steady GC pressure). +**Fix:** one cached `static SendOrPostCallback`; pass state via a per-registration state object — +`InFlight` guarantees exclusivity, so tick data can be written into a reusable per-registration slot +before posting. Target: zero allocations per frame in steady state. + +## Finding 6: drift `Debug.Assert` is an assert storm + +Drift >20% for 10 frames is normal under a debugger, breakpoints, or CI load. Once tripped, +`consecutiveDriftFrames` keeps incrementing, so the assert fires **every subsequent frame**. +**Fix:** replace with tracing/EventSource counters (drift, dropped frames, spin time). If any assert +remains, reset the counter after firing once. + +## Finding 7: lifecycle edges + +- `StopTimer` never observes `s_loopTask`; a stop/start pair can transiently run two loops, and a + stopping loop can dispatch one final frame with a canceled token. Decide and document: either join + the old loop (bounded) or make late dispatch provably benign. +- Post-unregister ticks can still be in flight toward a disposed consumer; `AnimationManager` / + `AnimatedControlRenderer` must tolerate late callbacks — add a test. +- `Reset()` (test hook) mutates state without locking — document the serialization requirement or + lock it. +- Dead code: `Registration.Id` is never read. +- `InvokeCallbackAsync` swallows non-OCE exceptions with `Debug.Fail` and keeps invoking the same + callback forever; consider auto-unregistering a registration after N consecutive faults. + +## Finding 8: single-SyncContext funnel in `AnimationManager` (design note) + +`HighPrecisionTimer` correctly captures a `SynchronizationContext` **per registration**, but +`AnimationManager` is a process-wide singleton with a single registration, so every animation in a +multi-message-loop application marshals to the first UI thread — and dies with it. Minimum: document +the constraint. Better: make the manager per-UI-thread (e.g. `[ThreadStatic]` instance keyed off the +message-loop thread), preserving one timer-registration-per-UI-thread. Also note the duplicate +timeline: the manager keeps its own `Stopwatch` instead of deriving progress from +`HighPrecisionTimerTick.Timestamp/Elapsed`; animation progress should use the tick's timeline so frame +coalescing (`DroppedFrames`) is accounted for consistently. + +## Finding 9: 60 Hz is a settled ceiling; the period is a pacer-owned runtime value for *downshift* + +**Decision (do not relitigate):** the timer targets a hard 60 Hz ceiling (30 Hz fallback). Do not +add refresh-rate matching or raise the cadence. Rationale, for the record: + +- Under DWM, windowed GDI/GDI+ apps do not tear (composition is tear-free from the redirection + surface); the only artifact of an unsynced 60 Hz timer is judder, which is imperceptible for this + content class (focus pulses, hover fades, toggle transitions — not motion/scrolling). +- GDI+ raster cost scales linearly with rate; driving N animated controls at 120–144 Hz multiplies + compute for no perceptible gain (e.g. batched MVVM-driven updates across many controls). +- A process-wide timer cannot refresh-match on mixed-rate multi-monitor setups ("the" refresh rate + is ill-defined), and `DwmFlush`-style vblank pacing blocks per frame, binds to one monitor, and + misbehaves under RDP — unfit for a process-wide UI timer by construction. + +**Required now (structural):** the frame period must be a runtime value owned by the pacer +(queryable/settable internally), not compile-time constants woven through the loop. The 30 Hz +fallback already makes the period variable; the forward-looking motivation is **downshifting**, not +matching: future power-driven reductions (30 Hz or full pause for occluded/minimized windows — where +Windows 11 timer coalescing already alters the effective cadence — and battery-saver scenarios) +must be addable without another rework. + +--- + +## Acceptance criteria + +1. Steady-state CPU of the timer loop with one registered infinite animation: **< 2% of one core** + (measure; the current implementation is the ~50% baseline per Finding 1). +2. No `timeBeginPeriod` while animations run (verify via `powercfg /energy` or timer-resolution + query on a Win10 1803+ box). +3. Frame delivery: mean interval within ±0.5 ms of target over a 10 s run on an idle machine at + 60 Hz; no monotonic slow drift (absolute-schedule check: 600th frame due time within one frame of + `epoch + 600 × period`). +4. Zero per-frame heap allocations in steady state (verify with an allocation-tracking test or + `GC.GetAllocatedBytesForCurrentThread` bracketing). +5. Race test for Finding 4: concurrent register/unregister stress leaves no registration without a + running loop. +6. Existing `HighPrecisionTimerTests` updated/extended accordingly; late-callback tolerance test for + consumers (Finding 7). +7. `HighPrecisionTimerTick` surface unchanged (internal consumers depend on it); all other churn is + internal to the pacer. + +## Constraints + +- Everything stays `internal`; no public API review implications. +- Follow repo conventions (`LibraryImport`, nullable enabled, existing XML-doc voice). +- Windows-only paths gated with `[SupportedOSPlatform]` / `OperatingSystem.IsWindowsVersionAtLeast` + as currently practiced in the file. diff --git a/.github/Feature-Prompts/Net11/02-B01-SystemVisualSettings-Implementation.md b/.github/Feature-Prompts/Net11/02-B01-SystemVisualSettings-Implementation.md new file mode 100644 index 00000000000..56070f96b3e --- /dev/null +++ b/.github/Feature-Prompts/Net11/02-B01-SystemVisualSettings-Implementation.md @@ -0,0 +1,158 @@ +# Work Order: SystemVisualSettings (Implementation) + +**Branch:** `Net11/Integration-2` (KlausLoeffelmann/winforms) +**Scope:** Code changes only. The GitHub proposal issue is handled by a separate work order +(`ApiReview-IssueUpdates.md`, Section 3) and run only after this implementation stands. +**Supersedes:** `Application.GetWindowsAccentColor` and the standalone accessibility text-size change +event currently on the branch (see item 6). + +--- + +## Background + +Windows delivers visual/accessibility setting changes through four channels — `WM_SETTINGCHANGE`, +`WM_DWMCOLORIZATIONCOLORCHANGED`, `WM_THEMECHANGED`, `WM_SYSCOLORCHANGE` — and today each consumer +normalizes that zoo individually. This work order introduces one typed, read-only snapshot plus one +unified change notification, with a **leak-free consumption path for controls** (virtual cascade, no +static-event subscription — the `SystemEvents.UserPreferenceChanged` leak class must be structurally +impossible for the default path). + +Deliberate non-goals, enforced by the type system: **no settable properties.** Perceptual adjustments +(contrast, color filtering, text scale, focus prominence, motion) are user-owned via Windows +accessibility settings; app-side theming is the business of control vendor partners. Renderers derive +output from this snapshot combined with `EffectiveVisualStylesMode`. + +--- + +## 1. New types (`System.Windows.Forms`) + +```csharp +public sealed class SystemVisualSettings +{ + public Color AccentColor { get; } + public float TextScaleFactor { get; } // 1.0–2.25, Windows a11y text scale + public bool HighContrastEnabled { get; } + public bool ClientAreaAnimationEnabled { get; } // SPI_GETCLIENTAREAANIMATION + public bool KeyboardCuesVisible { get; } // SPI_GETKEYBOARDCUES (system default) + public Size FocusBorderMetrics { get; } // SPI_GETFOCUSBORDERWIDTH/HEIGHT, pixels +} + +[Flags] +public enum SystemVisualSettingsCategories +{ + None = 0, + AccentColor = 1 << 0, + TextScale = 1 << 1, + HighContrast = 1 << 2, + Animations = 1 << 3, + KeyboardCues = 1 << 4, + FocusMetrics = 1 << 5 +} + +public class SystemVisualSettingsChangedEventArgs : EventArgs +{ + public SystemVisualSettings OldSettings { get; } + public SystemVisualSettings NewSettings { get; } + public SystemVisualSettingsCategories Changed { get; } +} +``` + +Immutable snapshot semantics — values must not shift under an event handler comparing old vs new. +XML remarks per the design notes: `HighContrastEnabled` cross-references the effective-mode clamp; +`KeyboardCuesVisible` documents the system-default vs per-window (`WM_UPDATEUISTATE`) distinction; +`FocusBorderMetrics` is documented as the baseline input for border/focus prominence in renderers +(scaled for DPI and `TextScaleFactor`), replacing fixed constants; the class remarks state the +no-app-side-overrides principle explicitly. + +## 2. `Application` surface + +```csharp +public static SystemVisualSettings SystemVisualSettings { get; } +public static event EventHandler? SystemVisualSettingsChanged; +``` + +Event XML remarks must state: (a) raised once per settings transition, normalized across the four +underlying messages; (b) handlers should early-out via `e.Changed`; (c) **audience is app-lifetime +consumers** (theming engines, services); components with shorter lifetime than the application must +unsubscribe; **controls and forms should use the `Control`-level virtual/instance event instead, +which requires no unsubscription** (see item 3). This positioning is the leak fix — make the +leak-free path the documented default. + +## 3. `Control`-level consumption (the leak-free path) + +```csharp +protected virtual void OnSystemVisualSettingsChanged(SystemVisualSettingsChangedEventArgs e); +public event EventHandler? SystemVisualSettingsChanged; +``` + +- Model the cascade on the existing `OnSystemColorsChanged` pattern in `Control.cs`: virtual + dispatch parent→children, instance event raised from within the virtual. No subscription to any + static event exists anywhere in this path — lifetime coupling is structural. +- Keep the cascade pattern-identical to `OnVisualStylesModeChanged` / `OnParentVisualStylesModeChanged` + (see the VisualStylesMode impact work order). A `HighContrast` category change resolves as an + effective-visual-styles-mode change for affected controls and must route through that machinery's + early-out/dispatch — no duplicate HC handling in this cascade. +- **Remove/replace the existing Form-level replicated text-size event**: it generalizes into this + cascade and disappears as a special case. Migrate in-box usages. +- Staleness rule (document in XML remarks, mirroring `OnSystemColorsChanged` folklore — this time + written down): the cascade only reaches parented controls; a control created but not yet parented + misses transitions and must re-query `Application.SystemVisualSettings` on handle creation / + `OnParentChanged`. + +## 4. Message plumbing and normalization + +Central internal tracker (e.g. `SystemVisualSettingsTracker`) holding the current snapshot: + +- Every **top-level** window already receives the four raw messages; handle them in the existing + top-level `WndProc` paths. +- On receipt: the window asks the tracker to re-query. The **first** arriver computes the diff + against the current snapshot, atomically swaps it (`Interlocked` reference swap), raises the static + `Application` event **once**, and cascades into its own tree. Subsequent top-levels re-query, see + no diff, raise nothing at Application level, but **still cascade into their own trees** using the + already-computed args. +- Threading falls out for free: each tree is notified on the thread owning its top-level — no + marshaling, correct for multi-message-loop applications. Do not centralize onto one thread. +- Coalesce message storms: a single user action can produce several of the four messages; debounce + within a message-pump iteration (re-query once per burst per top-level, not per message). + +## 5. In-box consumption cleanup + +- Audit in-box `Microsoft.Win32.SystemEvents` subscriptions (`UserPreferenceChanged` et al.) in + controls/renderers; migrate those covered by the new categories to the cascade. Document any that + must remain (categories outside this surface) — do not expand the snapshot to chase them in this + work order. +- `TextBoxBase` Net11 border rendering: consume `FocusBorderMetrics` + `TextScaleFactor` as the + border-prominence input where fixed constants are currently used (coordinate with the animation / + focus-indicator renderer as applicable). +- Animated renderers (`AnimatedControlRenderer` / `AnimationManager`): honor + `ClientAreaAnimationEnabled == false` by rendering final state immediately and suppressing + transitions; react to the `Animations` category change at runtime. + +## 6. Supersede the piecemeal APIs + +- `Application.GetWindowsAccentColor` → `Application.SystemVisualSettings.AccentColor`. The method + has not shipped stable: **remove it** on this branch (preferred) rather than obsoleting, to avoid + two sources of truth. If removal is blocked by preview-compat policy, `[Obsolete]` with pointer. +- The standalone text-size change event (Application- and/or Form-level) → `Changed.HasFlag(TextScale)` + on the unified event / cascade. Same removal-vs-obsolete decision, same preference. +- Migrate all in-box call sites. + +## 7. Tests + +- Snapshot immutability and correct SPI mapping per property (mock/native-shim as the repo's test + infra allows). +- Normalization: N top-level windows + one settings transition ⇒ exactly one Application-level raise; + every window's tree cascaded exactly once; delivery on each tree's own thread. +- Flags correctness per category, including multi-category transitions (HC toggle typically changes + colors + HC + metrics in one burst — must coalesce to one event with combined flags). +- Leak test: create/dispose forms subscribing to the **control-level** event in a loop; assert + collectability (`WeakReference`), proving no static rooting. Counter-test documenting that the + static event does root (expected, documented behavior). +- HC toggle end-to-end: cascade triggers effective-mode change path once, no duplicate layout. + +## Constraints + +- All new public surface XML-documented in the branch's voice; internal tracker fully internal. +- No settable members anywhere on the new types — if implementation pressure suggests one, stop and + flag rather than adding it. +- Windows-only P/Invoke via `LibraryImport`, gated per existing repo practice. diff --git a/.github/Feature-Prompts/Net11/03-B05-VisualStylesMode-ImpactApi-Implementation.md b/.github/Feature-Prompts/Net11/03-B05-VisualStylesMode-ImpactApi-Implementation.md new file mode 100644 index 00000000000..a1d56d1aa0b --- /dev/null +++ b/.github/Feature-Prompts/Net11/03-B05-VisualStylesMode-ImpactApi-Implementation.md @@ -0,0 +1,137 @@ +# Work Order: VisualStylesMode — Change-Impact API (Implementation) + +**Branch:** `Net11/Integration-2` (KlausLoeffelmann/winforms) +**Scope:** Code changes only. GitHub issue updates are handled by a separate work order +(`ApiReview-IssueUpdates.md`, Section 1) and run only after this implementation stands. +**Out of scope:** `HighPrecisionTimer` (separate work order), `SystemVisualSettings` (separate work order — +but see the composition note in item 4, the two cascades share a pattern). + +--- + +## Background + +The `VisualStylesMode` API is implemented on this branch. Two gaps motivate this change: + +1. **Correctness/ordering:** Base `Control.OnVisualStylesModeChanged` only invalidates, raises the + event, and cascades — no preferred-size cache invalidation, no layout transaction (unlike + `OnFontChanged`). Every metric-changing control reimplements a four-step protocol; `TextBoxBase` + currently runs `LayoutTransaction.DoLayoutIf(...)` **before** `UpdateStyles()`, so containers + measure against pre-transition metrics — a live mode switch clips/overlaps until controls are + recreated. +2. **Performance:** A top-level switch triggers one full parent layout per metric-affected control, + each against partially-transitioned state; repaint-only controls pay costs they don't need. + +Fix: a declarative impact model — derived controls state **what** changes, the base class owns +**how and in what order** to react, with one coalesced layout pass per container. + +--- + +## Changes + +### 1. `Control.cs` — visibility promotion + +`EffectiveVisualStylesMode`: `private protected` → `protected`. **Non-virtual** (deliberate — it is +the accessibility policy enforcement point; the customization hook is the already-virtual +`DefaultVisualStylesMode`). Extend XML docs: this is the value controls must honor for rendering; +reflects High Contrast (⇒ `Classic`) and `Disabled`; cross-reference `DefaultVisualStylesMode`. + +### 2. `Control.cs` — impact enum + virtual + +```csharp +protected enum VisualStylesModeChangeImpact +{ + None, // no rendering difference; skip all work + Repaint, // client-area rendering only; metrics identical + NonClientUpdate, // NC frame changes; style/frame update, no size change + Metrics // preferred size / layout metrics change; full invalidation + layout +} + +protected virtual VisualStylesModeChangeImpact GetVisualStylesModeChangeImpact( + VisualStylesMode oldMode, + VisualStylesMode newMode) + => VisualStylesModeChangeImpact.Repaint; +``` + +Parameters are **effective** modes. Document: implementations may consult instance state +(`Multiline`, `BorderStyle`, …) but the value must be stable for the duration of the change handling. + +### 3. `Control.cs` — setter early-out on effective equality + +In the `VisualStylesMode` setter and `OnParentVisualStylesModeChanged`: capture effective mode before +the change; if unchanged after, skip `OnVisualStylesModeChanged` entirely (no invalidate, no cascade +into that subtree). Preserve the existing shadowing behavior (child with explicit local value already +stops the ambient cascade) and add the effective-equality check on top. High Contrast active ⇒ raw +`Net11 → Latest` switch is a complete no-op. + +### 4. `Control.cs` — `OnVisualStylesModeChanged` becomes the dispatcher + +Preserve the disposal guard and public event raise. Then: + +- `impact = GetVisualStylesModeChangeImpact(oldEffective, newEffective)` — plumb old/new effective + values from the setter/cascade (least-invasive mechanism consistent with how `OnParentFontChanged` + plumbs state). +- `None` → raise event, skip rendering/layout work, still cascade (children's impact may differ). +- `Repaint` → `Invalidate()`. +- `NonClientUpdate` → `UpdateStyles()` + NC frame refresh (`SetWindowPos` + `FRAMECHANGED`, per the + branch's existing pattern), then `Invalidate()`. +- `Metrics` → in order: `CommonProperties.xClearPreferredSizeCache(this)` → `UpdateStyles()` / frame + refresh → layout deferred to the coalesced step: + +```csharp +if (ChildControls is { } children) +{ + using (new LayoutTransaction(this, this, PropertyNames.VisualStylesMode, resumeLayout: false)) + { + for (int i = 0; i < children.Count; i++) + { + children[i].OnParentVisualStylesModeChanged(e); + } + } +} + +LayoutTransaction.DoLayout(this, this, PropertyNames.VisualStylesMode); +``` + +Add `PropertyNames.VisualStylesMode` if missing. + +**Composition note:** the `SystemVisualSettings` work order introduces a structurally identical +parent→child cascade (`OnSystemVisualSettingsChanged`, modeled on `OnSystemColorsChanged`). Keep the +two cascades pattern-identical; a High Contrast toggle arriving via that path resolves here as an +effective-mode change and must take the same early-out/dispatch route — no duplicate handling. + +### 5. `TextBoxBase.cs` — adopt the model, delete the hand-rolled protocol + +- Override `GetVisualStylesModeChangeImpact`: crossing `Classic`/`Disabled` ↔ `>= Net11` ⇒ `Metrics` + (`PreferredHeight` selection and NC padding both change). Within `>= Net11` (`Net11 → Latest`) ⇒ + `Repaint` **unless** the branch's renderer shows metric differences between those modes — verify + against `PreferredHeightCore` and the NC padding tables; document the decision in XML remarks. +- Slim `OnVisualStylesModeChanged`: remove manual `xClearPreferredSizeCache` / `DoLayoutIf` / + `AdjustHeight` sequencing — base owns it. Keep control-specific work + (`_focusIndicatorRenderer?.Synchronize(...)`, `_triggerNewClientSizeRequest = false`), with the + ordering contract: latch reset **before** `base.OnVisualStylesModeChanged(e)` (which runs + `UpdateStyles()`); any residual `AdjustHeight` need goes **after** `base` — verify with the + live-switch repro (TableLayoutPanel, AutoSize rows, anchored single-line TextBoxes; runtime mode + switch; rows must resize without reopening the form). +- This closes the `DoLayoutIf(AutoSize: false)` hole: base-owned coalesced layout means + `AutoSize == false` TextBoxes no longer skip parent re-measurement. + +### 6. Documentation & tests + +- `docs/Net11Api_05_VisualStylesMode.HighRiskReview.md`: add the behavioral change (base + `OnVisualStylesModeChanged` now clears caches, updates styles, requests layout; overriders calling + `base` inherit this). If the branch's `WFCC`/`ComponentChange` infrastructure is present, annotate; + otherwise the high-risk doc entry suffices. +- `ControlTests.VisualStylesMode.cs`: + - effective-equality early-out (HC active ⇒ no event storm, no layout), + - default impact is `Repaint`, + - `Metrics` path clears preferred-size cache; exactly one layout per container for a multi-control + subtree switch (layout-count instrumentation or `LayoutEventArgs` assertion), + - `TextBoxBase` live-switch regression test: preferred-height change reflected in an AutoSize + `TableLayoutPanel` row without control recreation. + +## Constraints + +- No binary breaking changes (`protected` promotion + new `protected` members on unsealed public + class are additive). +- Do not change the `EffectiveVisualStylesMode` clamping logic. +- Match repo analyzers/style and the branch's XML-doc voice. diff --git a/.github/Feature-Prompts/Net11/Application.SystemTextAwareness/Application.SystemTextAwareness.md b/.github/Feature-Prompts/Net11/Application.SystemTextAwareness/Application.SystemTextAwareness.md new file mode 100644 index 00000000000..da7edcf854e --- /dev/null +++ b/.github/Feature-Prompts/Net11/Application.SystemTextAwareness/Application.SystemTextAwareness.md @@ -0,0 +1,180 @@ +# Task: Create a new API — `Application` system-text-size awareness — and the respective API proposal + +## What to do + +Create a **new API proposal / API review issue in the upstream `dotnet/winforms` repo** +(`origin` = my fork, `upstream` = the Microsoft repo where this lands). Write it per the +WinForms repo conventions and the relevant skills for authoring new-API issues. Apply the +skills; don't ask me for boilerplate. + +This proposal introduces **runtime awareness of the Windows Accessibility text-size +setting** at the `Application` level, plus a per-`Form` change notification. It is the +**foundation** proposal; a companion `TreeView.NodeLeading` proposal references this one. + +## Before you implement anything + +**Verify every premise below against current source before committing to the design.** +Verify, don't trust. If any premise is wrong, stop and tell me. Key files (VMR @ +`96982699e0dd8c046f397541dc0eb235ea8a4958`): + +- `src/winforms/src/System.Windows.Forms.Primitives/src/System/Windows/Forms/Internals/ScaleHelper.cs` +- `src/winforms/src/System.Windows.Forms/System/Windows/Forms/Application.cs` +- `src/winforms/src/System.Windows.Forms/System/Windows/Forms/Application.ThreadContext.cs` +- `src/winforms/src/System.Windows.Forms/System/Windows/Forms/Application.ParkingWindow.cs` +- `src/runtime/src/libraries/Microsoft.Win32.SystemEvents/...` (SystemEvents / UserPreferenceCategory) + +## Rationale — surface and complete an existing partial implementation + +This is **not** a net-new feature; it **finishes a half-built one**. WinForms already reads +the Accessibility text-size setting, but only once and only for the default font: + +`ScaleHelper.ScaleToSystemTextSize(Font?)` reads +`HKEY_CURRENT_USER\Software\Microsoft\Accessibility` → value `TextScaleFactor` +(REG_DWORD, clamped 100–225), and returns the font scaled by `TextScaleFactor / 100` +(or `null` if 100, if the font `IsSystemFont`, or if the OS is < Windows 10 1507). It is +documented in-source as the **Settings → Display → Make Text Bigger** setting. + +The gaps: (1) the value is **never surfaced** to developers; (2) it is read **once**, at +default-font construction, and the app **never reacts** when the user changes the setting at +runtime; (3) there is **no notification** mechanism. This proposal fills those three gaps. + +## Three-knob disambiguation (MANDATORY callout — reviewers will conflate these) + +Windows surfaces three *different* sizing mechanisms; the Settings UI even puts two on one +page (`System → Display → Custom scaling` shows a "Custom scaling 100–500%" box AND a +"Text size" link). The proposal MUST state plainly which one it targets: + +1. **Display / Custom scaling (100–500%)** → this is **DPI**. Already handled by + `HighDpiMode` and the DPI events (`WM_DPICHANGED`, `Control.DpiChanged*`). **NOT** this + proposal. +2. **Accessibility → Text size (100–225%)** → registry `TextScaleFactor` under + `HKCU\Software\Microsoft\Accessibility`; WinRT `UISettings.TextScaleFactor` / + `TextScaleFactorChanged`. **THIS is the target.** Independent of DPI. +3. **Legacy pre-Win10 per-element text sizing** (title bars/menus) → **removed** in Windows + 10 1703; the Accessibility slider replaced it. Mentioned only to close the loop. + +State explicitly that `Application.SystemTextSize` reflects **#2 only**, and is orthogonal to +DPI (#1). + +## Proposed API + +### `Application` (process-static) + +- **`public static double SystemTextSize { get; }`** — the current Accessibility text-scale + factor (1.0–2.25; i.e. `TextScaleFactor / 100`). **Live getter** — re-reads the value, does + not cache, because it is a system setting that changes at runtime. Well-defined regardless + of whether any `Form` exists or how many UI threads are running (it is process-global). +- **`public static SystemTextSizeAwareness SystemTextSizeAwareness { get; set; }`** — the + mode. **Enum, not bool**, deliberately, to reserve room for a future `Automatic`: + - `Unaware` (default) — no notification raised; fully back-compatible, nothing changes. + - `Notify` — raise change notifications (see below); the app decides how to respond. + - *(reserved, NOT implemented now: `Automatic` — framework re-flows for you. Reserving the + enum slot now avoids a future breaking bool→enum change, the same lesson `HighDpiMode` + learned.)* +- **`public static event EventHandler? SystemTextSizeChanged`** — fires once per process when + the setting changes (only when awareness is `Notify`). + +### `Form` (instance) + +- **`public event EventHandler? SystemTextSizeChanged`** — instance event, raised on each + top-level `Form` when the setting changes. +- **`protected virtual void OnSystemTextSizeChanged(EventArgs e)`** — overridable, fires the + instance event via the `EventHandlerList` pattern (`Events[s_systemTextSizeChangedEvent]`). + +## The trigger architecture (the leak-critical part — get this exactly right) + +A naive design — a static `Application.SystemTextSizeChanged` that `Form`s/`Control`s +subscribe to — **leaks**: the static event strongly roots every subscriber, so no `Form` +that subscribes is ever collected. Avoid this by **mirroring the DPI architecture**, where +`Control`/`Form` learn of DPI changes from their **own `WndProc`** (`WM_DPICHANGED` → +`OnDpiChanged` → instance event via `EventHandlerList`), **not** from a static subscription. + +Verified facts that constrain the design: + +- **`WM_SETTINGCHANGE` is broadcast** (`HWND_BROADCAST`) and delivered directly to top-level + windows' `WndProc`s — it bypasses the thread message queue, so **`IMessageFilter` does NOT + see it.** Do not use a message filter. +- **The WinForms parking window is message-only** (`CreateParams.Parent = HWND_MESSAGE`). + Message-only windows are **excluded from broadcasts**, so the parking window **cannot** + receive `WM_SETTINGCHANGE`. Do not use it. +- **`Application` has no `MainForm`.** The main form lives on `ApplicationContext` (per-run, + per-UI-thread), can be `null` (tray/loop-only apps), and is mutable (splash→main handoff). + So the main form is **not** a reliable receiver. Do not anchor the app-level event to it. +- **`SystemEvents` already owns a hidden top-level broadcast-receiving window** (its + `.NET-BroadcastEventWindow`), and exposes `UserPreferenceChanged` with a + `UserPreferenceCategory` (the relevant value is `Accessibility`, which is **coarse** — it + covers any accessibility change, so you must re-read `TextScaleFactor` and diff to confirm + it was text-scale). + +**Resulting design:** + +- **App-level:** `Application` makes **one internal, process-lifetime** subscription to + `SystemEvents.UserPreferenceChanged`, filters `Category == Accessibility`, re-reads + `TextScaleFactor`, diffs against the cached value, and if changed raises the static + `SystemTextSizeChanged`. This is a single framework-static→framework-static link — it does + **not** root any user object, so it is **not** the leak hazard. Reuses the existing hidden + broadcast window; **no new HWND** required. +- **Form-level:** each top-level `Form` handles `WM_SETTINGCHANGE` in its **own `WndProc`** + (it is a broadcast — every top-level window receives it), re-reads + diffs, and raises its + **instance** `SystemTextSizeChanged` via `OnSystemTextSizeChanged`. `Form`s do **not** + subscribe to `Application` — no rooting, lifetime = the window. + +Caveat to document: there is **no dedicated `WM_TEXTSCALECHANGED`** message. Both paths must +recognize a *relevant* change by re-reading `TextScaleFactor` and comparing, not by the +message alone. + +## Aids vs. leave-it-to-the-user + +**Notify-only. No automatic re-layout / font-rescaling aids in this proposal.** Reasons: +text scale interacts with `AutoScaleMode`, anchored/docked layout, and explicitly-set fonts +in app-specific ways; a generic "scale all fonts by the factor" helper breaks more than it +fixes. The reserved `Automatic` enum value is exactly where such behavior would live later. +`Notify` gives the developer the factor and the event; they decide. (Consistent with the +companion `NodeLeading` proposal's conservative-default philosophy.) + +## Why this matters across controls (evidence — include the matrix) + +Multiple text-measuring controls derive item/row/tile extents from a `Font` that today only +reacts to the text-size setting once at startup (via `ScaleToSystemTextSize` on the default +font) and never again. So **any single cached height scalar is wrong the moment text size +changes at runtime** — which is the core argument for runtime awareness: + +- **ListBox / ComboBox** — have `MeasureItem` + `OwnerDrawVariable` (a real per-item measure + hatch) and `ItemHeight`. Their gap is the legacy default base calc + the missing runtime + text-scale reaction — i.e. exactly this proposal. +- **TreeView** — has neither `MeasureItem` nor a wrapped native height API; only a uniform + native item height. Worst-positioned for a managed fix; addressed by the companion + `TreeView.NodeLeading` proposal, which depends on this one. +- **ListView (Details)** — no `MeasureItem`, no native row-height message; row height is + comctl-computed from `SmallImageList` + control font, while per-item/subitem fonts are + honored via `NM_CUSTOMDRAW` (`CDRF_NEWFONT`). Userland workarounds (phantom `SmallImageList`; + `LVS_OWNERDRAWFIXED` + one-shot reflected `WM_MEASUREITEM`; "inflate control font / shrink + item fonts") each have holes (header leak, set-once, exhaustive per-item font setting, + owner-draw-all). **No clean complete userland solution exists** — strengthening the case + that text-size reaction belongs in the framework. + +## XML doc requirements + +- Document that `SystemTextSize` is the **Accessibility text-size** factor (Settings → + Display → Make Text Bigger), **not** DPI/display scaling, and is process-global / live. +- Document the `Unaware`/`Notify` semantics and that `Automatic` is reserved for future use. +- Document the no-rooting design note on the static event (so consumers understand instance + vs. static). + +## Open questions for review + +- Should `SystemTextSize` be `double` (1.0–2.25) or expose the raw int percent (100–225)? +- Behavior on OS < Windows 10 1507 (where `ScaleToSystemTextSize` no-ops): `SystemTextSize` + returns 1.0 and no events fire? +- Whether to also expose the value/event on `Application` only, leaving `Form` consumers to + use their own `WndProc` override — or provide the `Form` instance event as proposed + (recommended, for parity with the DPI event model). + +## Output + +The upstream issue per the skills: summary; the "complete a partial implementation" +rationale; the mandatory three-knob disambiguation; the proposed API; the leak-safe trigger +architecture with the four verified constraints (broadcast vs. IMessageFilter, message-only +parking window, no MainForm on Application, SystemEvents reuse); Notify-only stance; the +cross-control matrix; XML-doc requirements; open questions. Flag anything the source +contradicts. diff --git a/.github/Feature-Prompts/Net11/Application/net11-VisualStylesMode/TextBoxBase-VisualStyles-WorkOrder.md b/.github/Feature-Prompts/Net11/Application/net11-VisualStylesMode/TextBoxBase-VisualStyles-WorkOrder.md new file mode 100644 index 00000000000..09894f9d23f --- /dev/null +++ b/.github/Feature-Prompts/Net11/Application/net11-VisualStylesMode/TextBoxBase-VisualStyles-WorkOrder.md @@ -0,0 +1,137 @@ +# Work Order — Port `TextBoxBase` NC-Painting + `VisualStylesMode` Chrome onto `VisualStylesNet11` + +**Target branch:** `KlausLoeffelmann/winforms` → `VisualStylesNet11` (current-`main`-based; modern path layout, no `/src/src/` doubling; `TextBoxBase.cs` ≈ 2130 lines). + +**Source of the original implementation (pinned):** `KlausLoeffelmann/winforms` @ `cf32e9c4efeba9d77e1a14025a8590b104e3c705` (old `/src/System.Windows.Forms/src/...` layout; `TextBoxBase.cs` ≈ 2562 lines). Relevant files: +- `.../Controls/TextBox/TextBoxBase.cs` — NC paint, NC calc, focus invalidation, `GetVisualStylesPadding`, helpers. +- `.../Controls/TextBox/TextBoxBase.NonClientBitmapCache.cs` — the offscreen cache class. +- `.../Controls/TextBox/TextBox.cs` — derived-level overrides (`CreateParams`, `WndProc`, `OnBackColorChanged`, `PRF_NONCLIENT` path). + +**Standing approval:** API review board sign-off from the .NET 9 cycle is still valid; DRI owns the call. This is a port + cleanup, **not** a redesign. Do not invent new public API beyond what the original exposed plus the `Padding` unshadowing called out below. + +**House style (apply throughout):** namespaces globally imported (no `using`/`imports` noise); C# 13/14; NRTs on; `var` only for long type names or when the type is obvious from the RHS, explicit type names for primitives; blank line between a new block and a following `return`; pattern matching / `is` / `and` / `or` / switch expressions preferred; collection expressions (`List x = [];`); expression-bodied members for single-line methods/read-only props formatted with the `=>` on the next line, 1-space-indented. Generate XML doc comments; use ``/``. + +--- + +## PROMPT 1 — Carry-over / Port + +> **Role:** You are porting a working-but-imperfect feature across a 3-year gap in the surrounding file. Faithfully reproduce the *mechanism*; do not improve, simplify, or "modernize" the algorithm except where this document explicitly says to. Where the surrounding `VisualStylesNet11` code has moved on, rebase onto the new shape rather than pasting. + +**Task.** Bring the non-client (NC) painting feature for `TextBoxBase` (and the `TextBox` derived touchpoints) from the pinned SHA `cf32e9c4…` onto `VisualStylesNet11`. The target branch currently has **none** of this work (no `WmNcPaint`/`WmNcCalcSize`/`OnNcPaint`/`GetVisualStylesPadding`/`NonClientBitmapCache`), but it **does** have the `VisualStylesMode` property infrastructure referenced in doc comments — wire into that, don't redeclare it. + +**Steps, in order:** + +1. **Fetch and diff the three source files** at SHA `cf32e9c4…` against their `VisualStylesNet11` counterparts. Produce a short inventory of every member you intend to add or modify, grouped by file, before writing any code. + +2. **Port `TextBoxBase.cs` members:** + - `WmNcPaint` / `OnNcPaint` (offscreen bitmap fill → AA rounded/single chrome → blit; `GetWindowDC`/`ReleaseDC` in `finally`). + - `WmNcCalcSize` (carves the padding band from `NCCALCSIZE_PARAMS->rgrc[0]`; gated on the `_triggerNewClientSizeRequest` latch). + - `InitializeClientArea` (the one-shot `SetWindowPos(SWP_FRAMECHANGED|NOMOVE|NOSIZE|NOZORDER|NOACTIVATE)` that provokes the single NC-calc). + - `GetVisualStylesPadding` / `GetScrollBarPadding` and the `VisualStyles{Fixed3D|FixedSingle|NoBorder}BorderPadding`, `BorderThickness` consts. + - The `WM_NCCALCSIZE` / `WM_NCPAINT` cases in `WndProc`. + - `OnGotFocus` / `OnLostFocus` / `OnSizeChanged` NC-frame invalidation (`RedrawWindow` with `RDW_FRAME|RDW_INVALIDATE`). + - `PreferredHeight` split (`PreferredHeightClassic` vs `PreferredHeightCore` selected by `VisualStylesMode`). + - Reconcile `GetPreferredSizeCore` with the modern branch already present on target. + +3. **Port `TextBoxBase.NonClientBitmapCache.cs`** — BUT this is a **decision point**, see Step 6. + +4. **Reconcile `TextBox.cs` (derived):** the target already has `CreateParams`, `WndProc`, `OnBackColorChanged` (special-casing `Fixed3D`), `OnGotFocus`, and a `WM_PRINTCLIENT`/`PRF_NONCLIENT` + `Application.RenderWithVisualStyles` path. Merge the NC behavior so the derived overrides cooperate with the new base NC painting (no double border draw, no fighting the `PRF_NONCLIENT` path). Call out every conflict you resolve. + +5. **Unshadow `Padding`.** On target it is still the neutered shadow (`[Browsable(false)]`, `EditorBrowsableState.Never`, `DesignerSerializationVisibility.Hidden`, `get/set => base.Padding`). Make it a real, browsable, serializable property that feeds the NC band via `GetVisualStylesPadding`. Preserve classic-mode behavior when `VisualStylesMode` is `Disabled`/`Classic`. Note the designer-serialization and back-compat implications in the PR description. + +6. **Retarget the rounded-rectangle helpers to the framework.** The original called fork-local `FillRoundedRectangle`/`DrawRoundedRectangle`. These now ship as **`System.Drawing.Graphics` instance methods** (`(Pen, Rectangle, Size)` + `RectangleF`/`SizeF` overloads; landed in the .NET 9 wave, current on target). **Delete the fork-local helpers and call the shipped methods.** ⚠️ Verify a **`FillRoundedRectangle(Brush, …)`** overload actually exists on target — the public ref only enumerates the `Draw`(`Pen`) overloads. If the `Fill`/`Brush` overload is absent, STOP and flag it; do not re-add a private helper without surfacing the gap. + +7. **Cache → `BufferedGraphics` (DECIDED — implement, do not re-evaluate).** The original used a hand-rolled per-instance `NonClientBitmapCache` (`CreateCompatibleBitmap` + `Image.FromHbitmap` + manual `DeleteObject`, `EnsureSize` realloc). Jeremy's late "use the existing cached bitmap" is confirmed to mean WinForms' own **`BufferedGraphics`/`BufferedGraphicsContext`** (the engine behind `OptimizedDoubleBuffer`; in `System.Drawing.Common` since .NET Framework 2.0, present on target). It is **not** `System.Drawing.Imaging.CachedBitmap` — that type is a read-only, device-dependent, blit-only frozen copy (no `Graphics`, translation-only, dies on bit-depth change) and cannot be a render target. **Delete `NonClientBitmapCache` entirely** (and its file `TextBoxBase.NonClientBitmapCache.cs`) and the `_cachedBitmap` field; replace with the shared buffer. Exact wiring: + - Inside `OnNcPaint`, get the shared context and allocate the buffer against the **window-DC `Graphics` already created in `WmNcPaint`**, sized to the window `bounds`: + `BufferedGraphicsContext context = BufferedGraphicsManager.Current;` + `using BufferedGraphics buffer = context.Allocate(graphics, bounds);` + `Graphics offscreenGraphics = buffer.Graphics;` + - **Do NOT `using`/dispose `buffer.Graphics`** — the `buffer` owns it; `using` the **buffer** only. (The original `using`-disposed its `GetNewGraphics()` because it owned that `Graphics`; that ownership is now the buffer's.) + - **All drawing into `offscreenGraphics` is unchanged** — the `FillRectangle(parentBackgroundBrush…)` corner-fill, the `BorderStyle` switch, the focus line: byte-for-byte identical, just a different `Graphics` target. + - **`ExcludeClip(clientBounds)` stays on the *target* `graphics`** (the window-DC one), exactly where it is now, set *before* the buffer draws. It governs where `Render()` may blit, protecting the client area — unchanged semantics. + - Replace the final blit `graphics.DrawImageUnscaled(offscreenBitmap, Point.Empty);` with **`buffer.Render();`** (no argument — it blits to the `graphics` captured at `Allocate` time). + - While here, fix the pre-existing **double-dispose** in `WmNcPaint`: it has both `using Graphics graphics = …` and an explicit `graphics.Dispose()` in `finally`. Drop the explicit `Dispose()`; keep the `using` (or keep explicit and drop `using` — one, not both). + - **Rationale to record in PR notes:** WinForms paints NC serially (one HWND at a time on the UI thread), so a single shared buffer suffices for any number of controls; the per-instance cache kept N resident GDI bitmaps to serve a one-deep queue. Steady-state allocation is unchanged (zero — shared buffer is reused when size fits); resident GDI memory drops from N× to 1×. The only cost is buffer-resize churn if controls of *wildly varying* sizes paint in a grow/shrink-alternating order — see smoke scenario 7 instrumentation. + +8. **Carve clamp + chrome degradation (settled design — implement exactly as stated, do NOT add a minimum size).** The original `WmNcCalcSize` does raw subtraction on `rgrc[0]` with **no clamp**, so a large `Padding` (made worse because `GetVisualStylesPadding(true)` *adds* the live scrollbar allowance from `GetScrollBarPadding` on top of the border padding) can drive the carved client rect to **zero or inverted**. Two separate fixes, and they are deliberately *not* a `MinimumSize`: + - **(8a) Never-invert clamp in `WmNcCalcSize`.** Floor each carved extent so the client rect can never invert: after the four adjustments, ensure `bottom >= top` and `right >= left` (e.g. clamp so the resulting client width/height is `Math.Max(0, …)`). A 0–1px client area is **acceptable and intended** — shipping multiline `TextBox` already shrinks to ~1px with scrollbars present, and we match that exactly. **Do NOT introduce a min-height/`MinimumSize`**, and do NOT make sizing behavior differ by `VisualStylesMode` (that would fracture the appearance-only contract of the opt-in). The clamp only prevents *underflow past zero*, which raw subtraction does and plain shrinking does not. + - **(8b) Paint-time chrome degradation in `OnNcPaint`.** The rounded `Fixed3D` chrome (15px radius) renders as a broken lozenge below roughly `2 × cornerRadius + BorderThickness` in height. When the available band/height is below that viable threshold, **fall back to the original/simple chrome render** (flat or single-style border) instead of the rounded path. This is a *rendering* fallback only — it does not change size or layout. Rationale on record: if the box is so small there's no usable client area, the control isn't usable anyway, so graceful visual degradation (not a size floor) is the correct response. + +9. **Build** `System.Windows.Forms` for the target TFM. Resolve all errors. Do not suppress new analyzer warnings without a one-line justification each. + +**Deliverable:** a single commit (or tight series) on `VisualStylesNet11` plus a PR description that lists: members added/modified per file, every `TextBox.cs` conflict resolved, confirmation that `NonClientBitmapCache` was removed and replaced by `BufferedGraphics` (Step 7) with the resize-churn rationale, the clamp/degradation (Step 8) confirmed as render-only with no size minimum, the `Padding` unshadowing implications, and any flagged gaps (Step 6 `Fill` overload). + +--- + +## PROMPT 2 — Critical Review (run AFTER Prompt 1, BEFORE smoke test) + +> **Role:** Adversarial reviewer. The author wants the issues a sharp WinForms maintainer would catch, not reassurance. Cite file + line for every finding. Classify each as **MUST-FIX**, **SHOULD-FIX**, or **PRESERVE (do not 'improve')**. + +Audit the ported code against this checklist. For each item, state the finding and the exact location. + +1. **DPI scaling of the corner radius.** The original hardcodes `const int cornerRadius = 15` and `BorderThickness = 1` in device-independent units, then uses them inside a DPI-scaled NC band, while `GetVisualStylesPadding` *does* take a DPI path (`_deviceDpi`). Confirm whether the radius/thickness now scale Per-Monitor-V2. If not → **MUST-FIX** (corners look proportionally too tight at 150/200%). + +2. **Full-frame NC repaint vs. partial `hrgnClip`.** `WmNcPaint` ignores the wParam clip region and repaints the whole frame. This is the **intended** fix for the offscreen-restore "dirty corners" artifact — verify it's preserved. But confirm `base.WndProc(ref m)` is still invoked with the original message and isn't double-painting the native border under the custom chrome. Classify the "ignore clip" behavior as **PRESERVE**. + +3. **Corner-blend source = `Parent?.BackColor ?? BackColor`.** This is the known ceiling: corners blend against the parent's flat back color, so they mismatch over a gradient/image/Mica/sibling. For the common case (solid form/panel) it's correct. **PRESERVE** — do not let it be "improved" into a fake general-case solution. Note it as a documented limitation only. + +4. **`WM_NCCALCSIZE` ↔ `Padding` round-trip + underflow.** Verify the band carved in `WmNcCalcSize` matches what `GetVisualStylesPadding(true)` reports and what `GetPreferredSizeCore`/`SizeFromClientSize` assume, for all three `BorderStyle` values × `Multiline` × scrollbars. Off-by-one here clips text or the caret. **Additionally** confirm the never-invert clamp (Prompt 1 Step 8a) is present and correct: with large `Padding` on a small multiline box *with both scrollbars*, the carved client rect must floor at 0, never invert. Remember the threshold is **border padding + live scrollbar padding** (`GetScrollBarPadding` reads `WS_HSCROLL`/`WS_VSCROLL`), so underflow hits sooner than the `Padding` value alone implies. Classify "0–1px client area is allowed, no min-size" as **PRESERVE** — do not let a reviewer or the agent add a `MinimumSize` floor. + +4b. **Chrome degradation below viable height.** Confirm `OnNcPaint` (Prompt 1 Step 8b) falls back to simple/flat chrome when height < ≈`2 × cornerRadius + BorderThickness`, instead of drawing a corrupted rounded rect. This is **render-only**; assert it does **not** alter size, layout, or `ClientSize`. Verify the fallback path itself is DPI-correct (the threshold scales with the radius, which per item 1 must scale). + +5. **`BorderStyle` fork + native edge suppression.** `Fixed3D` → rounded chrome, `FixedSingle` → single + underline, `None` → fill. Confirm the native `WS_EX_CLIENTEDGE`/`WS_BORDER` from `CreateParams` is suppressed when NC chrome is active, so the native edge isn't drawn under the custom one. + +6. **`VisualStylesMode` gating is total.** Every NC entry point (`WmNcPaint`, `WmNcCalcSize`, `InitializeClientArea`, the focus/size invalidations) must early-out to byte-for-byte classic behavior when `VisualStylesMode` is `Disabled`/`Classic`. One missing guard = a back-compat regression. Note the original's `OnLostFocus` was **missing** the guard that `OnGotFocus` had — verify the port fixed this asymmetry. + +7. **DC / GDI lifetime.** `GetWindowDC`→`ReleaseDC` in `finally`, and `Graphics.FromHdc`+`Dispose` ordering: correct **only** because `FromHdc` doesn't own the DC. **PRESERVE** — flag any "tidy into a single `using`" as a regression. Confirm the `WmNcPaint` **double-dispose** was fixed (it had both `using Graphics` and an explicit `graphics.Dispose()`). For the `BufferedGraphics` swap (Prompt 1 Step 7): verify the **buffer** is `using`-scoped but **`buffer.Graphics` is NOT separately disposed**; verify `Allocate` targets the window-DC `graphics` and `Render()` is called with no argument; confirm `NonClientBitmapCache` and the `_cachedBitmap` field are fully removed with no dangling refs. Audit for any HBITMAP/HDC leak in the new path (there should be none — the buffer owns it). + +8. **DPI-change without handle recreate.** `_triggerNewClientSizeRequest` is a one-shot latch reset on handle recreate. Does a DPI change that does *not* recreate the handle re-carve the band with new padding? If the band can go stale on monitor move → **MUST-FIX** or at least an explicit tracked issue. + +9. **Caret / IME / selection repaint.** The native `EDIT` invalidates aggressively. Confirm NC chrome doesn't go stale on caret blink/IME composition, and conversely that NC isn't thrashing-repainting on every caret tick. (Author never confirmed this was clean in the original.) + +10. **`TextBox.cs` derived reconciliation.** Verify the derived `WndProc`, `OnBackColorChanged` `Fixed3D` special-case, and the `PRF_NONCLIENT`/`Application.RenderWithVisualStyles` path don't conflict with base NC painting (double draw, wrong-mode paint). + +11. **Allocation churn.** Brushes/pens use cached scopes (`GetCachedSolidBrushScope`/`GetCachedPenScope`) — good; confirm preserved. Confirm the offscreen surface isn't reallocated per paint (only on size change). + +**Deliverable:** a findings list (file:line, severity, recommendation). MUST-FIX items get fixed in this pass; SHOULD-FIX either fixed or filed; PRESERVE items annotated in code with a brief `// Intentional:` comment so the next reader doesn't "fix" them. + +--- + +## PROMPT 3 — Smoke Test Harness + +> *("Smoke test" = the shallow "does it power on without catching fire" pass — from hardware bring-up, where first power-on literally checked for smoke — run before any deep/perf testing. Goal here: broad coverage that it comes up and behaves on the obvious axes, with the two known-fragile cases as explicit named tests.)* + +**Task.** Build a throwaway WinForms test app (separate project, not shipped) that exercises the ported feature across its permutation space and **specifically reproduces the two regressions this feature is prone to.** + +**Permutation grid** — generate a form populated with `TextBox`es (and at least one `RichTextBox`, since `TextBoxBase` is the shared base) covering the cross-product of: +- `BorderStyle`: `None` × `FixedSingle` × `Fixed3D` +- `Multiline`: `false` × `true` (+ `WordWrap` on/off for multiline) +- `Padding`: `Empty` × asymmetric (e.g. `2,6,2,6`) × large (`12`) +- Scrollbars: none × vertical × both +- `VisualStylesMode`: `Disabled`/`Classic` (must look exactly like today) × `Net10`+ (new chrome) +- Focused vs unfocused (drive focus programmatically to capture the adorner/underline) + +**Named, must-pass scenarios (the ones that silently regress):** + +1. **Offscreen-restore ("dirty corners").** Move the window partly off the left/top screen edge, then back. Assert the NC corner regions are repainted clean (no stale pixels). This is the artifact that drove the full-frame-repaint design. Automate the drag via `SetWindowPos`/`MoveWindow`; capture before/after. + +2. **Partial NC invalidation.** Trigger a partial `WM_NCPAINT` (e.g. overlap then reveal a sliver of the frame) and assert the whole chrome is coherent, not just the revealed strip. + +3. **Per-Monitor-V2 DPI.** Run DPI-aware; move forms between a 100% and a 150%/200% monitor (or fake via `LogicalToDeviceUnits`/DPI-changed messages). Assert corner radius, border thickness, and padding band all scale; assert no clipped text/caret. + +4. **Classic-mode parity.** With `VisualStylesMode = Disabled`, assert the control is pixel-identical to baseline `main` (native edge, no custom NC). A regression here is the back-compat line breaking. + +5. **`BorderStyle` switch at runtime** (`Fixed3D`↔`FixedSingle`↔`None`) and **`Padding` change at runtime** — assert the band re-carves and chrome redraws without artifacts (exercises the `_triggerNewClientSizeRequest` reset path). + +6. **Focus transitions** — tab through the grid; assert the focus underline (single) / 3D focus line (Fixed3D, shortened to clear the corner curve) appears/clears correctly. + +7. **Shrink-to-collapse (clamp + degradation).** Take a multiline `Fixed3D` box with large `Padding` (e.g. `12`) and **both** scrollbars visible, then programmatically drag/resize its height down toward 1px. Assert: (a) **no crash / no inverted client rect** handed to the native `EDIT` — the carve floors at 0 (Step 8a); (b) below ≈`2 × cornerRadius + thickness` the chrome **falls back to flat/simple render** rather than drawing a corrupted lozenge (Step 8b); (c) the control **still collapses** to ~1px exactly like classic multiline — assert it is **not** held open by any min-size (regression if a floor appeared). Repeat at 150%/200% DPI so the degradation threshold is verified scaled, not fixed at 96-dpi pixels. + +8. **BufferedGraphics allocation churn (perf sanity).** Build two forms: (a) **40 same-size** textboxes, (b) **40 wildly varying-size** textboxes (mix tiny and large), all `VisualStylesMode ≥ Net10`. Force a full repaint storm (invalidate all NC frames repeatedly; resize the form to cascade re-layout). Instrument the shared buffer: wrap/observe `BufferedGraphicsManager.Current` and count actual **bitmap (re)allocations** vs. reuses across the storm. Assert: case (a) allocates the buffer ≈once then reuses (steady-state alloc ≈ 0); case (b) may reallocate on grow but must **not** allocate-per-paint. Log alloc count per case. This empirically confirms the shared-buffer reasoning from Prompt 1 Step 7 and catches any accidental per-paint allocation regression. + +**Harness mechanics:** +- A "capture all" button that screenshots each form to disk per `VisualStylesMode`, for eyeball diffing classic-vs-modern and pre-vs-post-DPI. +- A console/log line per assertion (pass/fail) so it can run semi-automated. +- Keep it dependency-light: raw WinForms + `SetWindowPos`/`RedrawWindow` P/Invoke for the offscreen and invalidation drivers. + +**Deliverable:** the test project + a one-screen README naming the six scenarios and how to run them, plus a results log from one full run on the porter's machine (note DPI of monitors used). diff --git a/.github/Feature-Prompts/Net11/SuspendRelocationPaintingFlashPreventing/Create-Github-API-Proposal-Prompt.md b/.github/Feature-Prompts/Net11/SuspendRelocationPaintingFlashPreventing/Create-Github-API-Proposal-Prompt.md new file mode 100644 index 00000000000..ce3ad461cab --- /dev/null +++ b/.github/Feature-Prompts/Net11/SuspendRelocationPaintingFlashPreventing/Create-Github-API-Proposal-Prompt.md @@ -0,0 +1,120 @@ +# Copilot Prompt 1 — Author the WinForms API Suggestion + +## Your task + +Write a complete API suggestion for the `dotnet/winforms` repository, ready to be filed +as a GitHub issue with the `api-suggestion` label. The issue covers two related features +for reducing visible intermediate states while WinForms applications update their UI: + +1. Painting suspension with optional layout suspension across a control tree. +2. Deferred top-level form reveal. + +Use these issue sections: + +- `## Rationale` +- `## API Proposal` +- `## API Usage` +- `## Alternative Designs` +- `## Risks` +- `## Will this feature affect UI controls?` +- `### Status Checklist` + +## Sub-feature A — painting and layout suspension + +### Settled API surface + +```csharp +namespace System.Windows.Forms; + +public interface ISupportSuspendPainting +{ + void BeginSuspendPainting(); + void EndSuspendPainting(); +} + +public enum LayoutSuspendTraversal +{ + None = 0, + TopLevelOnly = 1, + Traverse = 2, +} + +public sealed class SuspendPaintingScope : IDisposable +{ + public SuspendPaintingScope(ISupportSuspendPainting? target); + public void Dispose(); +} + +public static class ControlMutationExtensions +{ + public static SuspendPaintingScope SuspendPainting( + this ISupportSuspendPainting target); + + public static SuspendPaintingScope SuspendPainting( + this ISupportSuspendPainting target, + LayoutSuspendTraversal layoutSuspendTraversal); + + public static SuspendPaintingScope SuspendPainting( + this ISupportSuspendPainting target, + Func suspendLayoutContainerFilter); +} +``` + +- `Control` implements `ISupportSuspendPainting` explicitly and exposes protected virtual + `BeginSuspendPaintingCore` / `EndSuspendPaintingCore` hooks. +- `ListView`, `ListBox`, `ComboBox`, `TreeView`, and `RichTextBox` route painting + suspension through their existing `BeginUpdate` / `EndUpdate` paths. +- `None` suspends painting only. +- `TopLevelOnly` suspends layout on the target `Control`. +- `Traverse` suspends layout on the target and all existing descendants. +- The predicate is evaluated for the target and every descendant. A `false` result skips + that node but does not prune traversal. +- Selected nodes are suspended even when they currently have no children. +- Layout-aware overloads require the target to derive from `Control`. +- The scope is a sealed class so it can span `await`. + +### Design considerations + +- Snapshot the selected controls when the scope starts. +- Suspend layout root-to-leaf and resume it deepest-first. +- Resume layout before ending painting so recursive invalidation occurs after layout. +- Keep disposal idempotent and preserve nesting through the existing ref counts. +- Explain that controls added after the snapshot are covered by their parent's suspended + layout but do not receive an independently balanced suspension. +- Discuss traversal cost for large control trees and exceptions thrown by predicates. +- Include invalid and non-`Control` target behavior in the proposal. + +## Sub-feature B — deferred form reveal + +Use the current `FormRevealMode` design from the tracked API proposal: + +```csharp +namespace System.Windows.Forms; + +public enum FormRevealMode +{ + Inherit = -1, + Classic = 0, + Deferred = 1, +} + +public partial class Form +{ + public virtual FormRevealMode FormRevealMode { get; set; } +} + +public partial class Application +{ + public static FormRevealMode DefaultFormRevealMode { get; } + public static void SetDefaultFormRevealMode(FormRevealMode mode); + public static bool IsFormRevealDeferred { get; } +} +``` + +Describe DWM cloaking, dark-mode-aware default resolution, designer serialization, +top-level-window limitations, and conservative fallback behavior on unsupported systems. + +## Filing instruction + +Produce a complete proposal rather than an implementation plan. Clearly distinguish fixed +API shape from implementation choices that remain open for review. diff --git a/.github/Feature-Prompts/Net11/SuspendRelocationPaintingFlashPreventing/SuspendRelocationAndPainting-API-Feature-Prompt.md b/.github/Feature-Prompts/Net11/SuspendRelocationPaintingFlashPreventing/SuspendRelocationAndPainting-API-Feature-Prompt.md new file mode 100644 index 00000000000..35d765ad3ce --- /dev/null +++ b/.github/Feature-Prompts/Net11/SuspendRelocationPaintingFlashPreventing/SuspendRelocationAndPainting-API-Feature-Prompt.md @@ -0,0 +1,76 @@ +# Copilot Prompt 2 — Implement the flicker-free UI mutation APIs + +## Prerequisite + +Read the current upstream API suggestion before implementation. Treat its latest +`API Proposal` section as the public contract and report any conflict instead of silently +choosing a different shape. + +## Scope + +### A — painting suspension with optional layout traversal + +- Implement `ISupportSuspendPainting` on `Control` with ref-counted + `BeginSuspendPaintingCore` / `EndSuspendPaintingCore` hooks. +- Route `ListView`, `ListBox`, `ComboBox`, `TreeView`, and `RichTextBox` through their + existing `BeginUpdate` / `EndUpdate` mechanisms. +- Keep `SuspendPaintingScope` as a sealed, idempotent `IDisposable` class so it can span + `await`. +- Provide these extension methods: + + ```csharp + public static SuspendPaintingScope SuspendPainting( + this ISupportSuspendPainting target); + + public static SuspendPaintingScope SuspendPainting( + this ISupportSuspendPainting target, + LayoutSuspendTraversal layoutSuspendTraversal); + + public static SuspendPaintingScope SuspendPainting( + this ISupportSuspendPainting target, + Func suspendLayoutContainerFilter); + ``` + +- `LayoutSuspendTraversal` has `None`, `TopLevelOnly`, and `Traverse`. +- Snapshot selected controls before suspension. Suspend root-to-leaf and resume + deepest-first. +- The predicate is evaluated for the target and every descendant. A `false` result skips + suspension for that node without pruning its descendants. +- Suspend selected controls even when they currently have no children. +- Validate layout-aware calls before beginning painting. A non-`Control` target is invalid. +- Resume all selected layout scopes before ending painting so the recursive invalidation + occurs after layout. + +### B — deferred form reveal + +- Implement the approved `FormRevealMode`, `Form.FormRevealMode`, and + `Application` configuration APIs. +- Preserve classic behavior when deferred reveal is not active. +- Cloak eligible top-level form handles and uncloak according to the timing strategy in the + proposal. +- Keep unsupported DWM and window configurations inert and safe. + +## Engineering requirements + +- Match the current repository language version, nullable annotations, code style, and + interop conventions. +- Add XML documentation for every public or protected API. +- Update `PublicAPI.Unshipped.txt`. +- Keep state lazy where existing `Control` property-store patterns apply. +- Do not change existing public `BeginUpdate` / `EndUpdate` behavior. + +## Tests + +- Painting ref-count balance, nesting, idempotent disposal, handle creation, and handle + recreation. +- `None`, `TopLevelOnly`, and `Traverse` layout selection. +- Predicate selection, continued traversal after a rejected node, and empty containers. +- Deepest-first layout resume and recursive invalidation after layout. +- Null, invalid-enum, and non-`Control` target failures without partial suspension. +- Existing native update paths for the selected built-in controls. +- Classic/deferred form reveal behavior and unsupported-OS fallback. + +## Deliverable + +Provide production code, focused tests, updated API tracking, and an updated API proposal. +Call out any behavior that the implementation proves unsafe or unnecessarily costly. diff --git a/.github/copilot/Async/invokeAsync_generate_test_instructions.md b/.github/Feature-Prompts/Net9/Async/invokeAsync_generate_test_instructions.md similarity index 100% rename from .github/copilot/Async/invokeAsync_generate_test_instructions.md rename to .github/Feature-Prompts/Net9/Async/invokeAsync_generate_test_instructions.md diff --git a/.github/copilot/GDI/DarkModeButtonRendererCodeGenerationInstructions.md b/.github/Feature-Prompts/Net9/GDI/DarkModeButtonRendererCodeGenerationInstructions.md similarity index 100% rename from .github/copilot/GDI/DarkModeButtonRendererCodeGenerationInstructions.md rename to .github/Feature-Prompts/Net9/GDI/DarkModeButtonRendererCodeGenerationInstructions.md