Merge branch 'dotnet:main' into master

Author: PWagner1

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

.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<SystemVisualSettingsChangedEventArgs>? 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<SystemVisualSettingsChangedEventArgs>? 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.