v1.4: Multi-monitor + GPU adapter selection + dialog overhaul#5
Merged
Conversation
Initialize the speckit feature artifacts for v1.4 multi-monitor / GPU efficiency work on branch 006-multimon-gpu-efficiency: - spec.md: 5 prioritized user stories, 39 functional requirements, 10 measurable success criteria, edge cases, assumptions - plan.md: technical context, constitution check (all 9 principles pass), project structure, file-level change map - research.md: 13 decisions (R-001..R-013) with rationale and alternatives, all unknowns resolved - data-model.md: 8 entities, persistence schema, lock domains - contracts/registry-schema.md: 8 new registry values with types, domains, defaults, backward-compat rules - contracts/adapter-provider.md: IAdapterProvider/AdapterInfo, pure ResolveAdapter/FormatAdapterLabel helpers, RenderSystem signature change - contracts/quality-preset-mapping.md: preset table, pure helpers, custom-drift state machine - contracts/tick-mark-conventions.md: trackbar tick rules - quickstart.md: build, test, launch, per-story walkthrough - checklists/requirements.md: spec quality checklist (all pass) - .specify/feature.json: feature directory registration - .github/copilot-instructions.md: SPECKIT markers point at plan Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add tasks.md (63 tasks across 7 non-empty phases, organized by user story; constitution-mandated tests-first for every core-library helper). Resolve cross-artifact findings from /speckit.analyze: - F1 (HIGH): FR-020 contradicted US5 acceptance scenario 7 and contracts/quality-preset-mapping.md by stating Custom was 'never directly by the user'. Updated FR-020 to permit both direct user selection and automatic system selection on drift. - F2 (MEDIUM): Cancel-revert semantics were captured only as an edge case. Added FR-031b formalizing the requirement and updated tasks T019/T035/T052 to explicitly register new controller fields in the CancelChanges/CancelLiveMode snapshot. - F3 (MEDIUM): US1 quickstart walkthrough did not cover /s mode. Added a step repeating the dock/undock test under MatrixRain.exe /s. - F4 (LOW): Tightened FR-017 from 'approximately 60 fps' to 'no more than 65 fps (target 60)' so the FR is independently testable without consulting SC-004. - F5 (LOW): Added suspend/resume cycle to T061 manual QA scope. - F6 (LOW): Added terminology assumption normalizing 'multi-monitor spanning' / 'render on all monitors' / 'multimon' as interchangeable. - F7 (LOW): FR-026 now documents the conditional default (toggle defaults to on when saved Quality preset is Custom). No CRITICAL issues. Constitution alignment confirmed unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds the DXGI 1.6 header (for IDXGIFactory6::EnumAdapterByGpuPreference used by the upcoming AdapterProvider) to the DirectX include block in alphabetical order between <dxgi1_2.h> and <wrl/client.h>. Verified clean rebuild and full test suite (354/354 passing). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reserves 19 new IDC_* control IDs in resource.h spanning 1014..1032 for the multimon checkbox, GPU combo, quality preset combo, advanced toggle, three discrete graphics sliders (each with label and info), and info indicators on the existing Glow Intensity and Glow Size sliders. Bumps _APS_NEXT_CONTROL_VALUE to 1033. No .rc changes yet; layouts will land with their respective US2/US3/US5 tasks. Verified clean rebuild and full test suite (354/354 passing). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pure free function classifying an HRESULT returned by IDXGISwapChain:: Present (or related D3D11/DXGI APIs) as 'the device is gone' or not. The render-thread loop will use this in T008 to trigger the application- level rebuild path when the GPU goes away. Covers DXGI_ERROR_DEVICE_REMOVED/RESET/HUNG/DRIVER_INTERNAL_ERROR. D3DDDIERR_* codes from GetDeviceRemovedReason are intentionally excluded; this helper deals with Present's return surface only. Tests (T003, 8 cases) and implementation (T005) are paired in this commit because principle IX requires each commit to pass tests; pure- test-only commits would break that gate. The TDD Red->Green cycle was observed locally before commit. Tests: 354 -> 362. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Small thread-safe helper that collapses a burst of 'I need a context rebuild' requests into a single rebuild action. Backed by std::atomic_flag. RequestRebuild() returns true for exactly one caller (the first since construction or the last Consume() call); subsequent callers receive false. Consume() resets the latch. T009/T010 will use this to coalesce WM_DISPLAYCHANGE (which Windows broadcasts to every top-level window) and device-lost notifications (which can fire from every per-monitor render thread simultaneously) into one application-level rebuild. Tests (T004, 5 cases including a 32-thread contention race) and implementation (T006) are paired in this commit per the TDD-+-principle- IX reconciliation documented on the previous commit. Tests: 362 -> 367. Also retroactively makes a49cd12's vcxproj references buildable from a clean checkout. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…evice-lost T007: Change IRenderSystem::Present() and RenderSystem::Present() return type from void to HRESULT, propagating the swap-chain Present result so callers can detect a lost device. SpyRenderSystem grows an m_presentReturnHr field so tests can simulate device-lost without a real D3D device. T008: In MonitorRenderContext::RenderThreadProc, capture the Present HRESULT and on IsDeviceLost(hr), PostMessage(m_hwnd, Application::WM_APP_REBUILD_CONTEXTS) and break the loop. Every monitor window we own shares Application::WindowProc, so posting to our own HWND routes correctly to the rebuild handler. Tasks paired per the TDD-+-principle-IX reconciliation: T007 alone leaves the HRESULT unused which is a meaningless intermediate; pairing keeps the unit logically atomic. Tests: 367/367 passing (no behavioral regression; new device-lost flow is covered by T060/T061 manual QA per the success criteria). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T009: Add case WM_DISPLAYCHANGE to Application::WindowProc; uses RebuildCoalescer to collapse the broadcast (Windows sends WM_DISPLAYCHANGE to every top-level window we own) to a single WM_APP_REBUILD_CONTEXTS post. Together with the device-loss path from T008 this is the core fix for the ghost-monitor render thread that held GPU at ~90% after a Surface Book 3 undock. T010: Modify the WM_APP_REBUILD_CONTEXTS case to call m_rebuildCoalescer.Consume() at the top so any further topology / device-loss notifications arriving during the rebuild itself can request a follow-up. Adds m_rebuildCoalescer member to Application and includes RebuildCoalescer.h in the header. Tasks paired because both touch the same switch in Application::WindowProc. Tests: 367/367 passing. The actual hotplug behavior is covered by the US1 quickstart walkthrough (manual QA on a multi-monitor system). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
US1 MVP (runtime topology + device-loss) and Phase 1 setup all done and committed. 10/63 tasks complete. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pure free function gating per-monitor render context creation on the combination of (multimon enabled setting, current display mode, optional screensaver mode). Truth table: - Preview/Help screensaver modes are always single-window (forced false). - Otherwise: spans all monitors iff multimon enabled AND fullscreen. T011 (5 test cases covering preview/help force-false, enabled-fullscreen, disabled-anywhere, windowed-anywhere) and T014 (implementation) paired per TDD/principle-IX reconciliation. The Application::ShouldSpanAllMonitors method will be delegated to this helper in T018 once the m_multiMonitorEnabled settings field lands. Tests: 367 -> 372. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sistence T015: Add bool m_multiMonitorEnabled = true to ScreenSaverSettings (the default matches FR-039 - multimon spanning enabled out of the box). T017: No-op for InMemorySettingsProvider since it stores ScreenSaverSettings by value; the new field flows through automatically. T016: Add VALUE_MULTIMONITOR = L\"MultiMonitor\" to RegistrySettingsProvider and read/write it via the existing ReadBool/WriteBool helpers. Backward- compatible: an absent value preserves the struct default of true (FR-002). Tests (paired per the TDD/principle-IX reconciliation): T012: extend ScreenSaverSettingsTests DefaultsAreInitialized to assert m_multiMonitorEnabled defaults to true. T013: add 3 round-trip tests to RegistrySettingsProviderTests covering the absent-value default, false round-trip, and true round-trip (pre-setting opposite on load to prove overwrite). Tests: 372 -> 375. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T018: Refactor Application::ShouldSpanAllMonitors() to delegate to the pure ShouldSpanAllMonitors() helper from MultiMonitorGate (added in T014). This is the gate that makes the new setting actually take effect: the existing fanout in Application::Initialize already consults this predicate when deciding between single-window and per-monitor contexts. T019: Add ConfigDialogController::UpdateMultiMonitorEnabled(bool). Trivial setter; live-mode cancel-revert works automatically because the snapshot captures the entire ScreenSaverSettings struct (no per-field registration required by the existing snapshot design - this resolves analyze finding F2 more directly than the original task wording suggested). T020: Add the \"Render on all monitors\" checkbox to the dialog template (IDC_MULTIMONITOR_CHECK) and grow the dialog from 240x200 to 240x215 to fit it. Existing checkboxes shifted down 15 dlu; OK/Cancel/Reset row moved from y=175 to y=190. T021: Wire the checkbox: initialize from settings in OnInitDialog, handle the BN_CLICKED case in OnCommand, post Application::ApplyDisplayMode Change on toggle so the running app rebuilds within 1 second (FR-003). Also extend OnCancel's live-mode branch to post the same rebuild after CancelLiveMode so the reverted setting takes visual effect (FR-031b). Tests: 375/375 passing (no behavioural test regressions; new live-mode behaviour is covered by the US2 quickstart walkthrough). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… (TDD)
Drops the IAdapterProvider/AdapterInfo trio mirroring the existing
IMonitorProvider pattern: a pure interface, an InMemoryAdapterProvider
test seam, and the AdapterSelection.{h,cpp} pure helpers
(ResolveAdapter + FormatAdapterLabel).
The WindowsAdapterProvider concrete implementation (T028) lands separately
since it pulls in real DXGI calls; the pure layer below it can land and be
tested in isolation now.
Tests (T022/T023, 9 cases):
- ResolveAdapter: empty saved -> nullopt; empty adapter list -> nullopt;
non-matching -> nullopt; matching -> LUID; duplicate descriptions ->
first match wins.
- FormatAdapterLabel: non-default unchanged; default appended;
empty-description-default edge case.
- InMemoryAdapterProvider: returns a copy of its seed vector.
Tests: 375 -> 384.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Concrete IAdapterProvider that uses DXGI to discover GPU adapters: - IDXGIFactory1::EnumAdapters1 for the basic list (GetDesc1 for each). - IDXGIFactory6::EnumAdapterByGpuPreference(0, UNSPECIFIED) to identify the system default rendering adapter; matched by LUID against the enumerated list to set AdapterInfo::m_isDefault. - Filters out DXGI_ADAPTER_FLAG_SOFTWARE adapters (Microsoft Basic Render Driver / WARP) before returning, per FR-011. - Any DXGI failure yields an empty vector; callers then use the system default-adapter path. EHM-style error handling with ComPtr for COM lifetime management. No unit tests: this layer touches real DXGI, which the project test strategy (research R-012) routes through manual QA per the US3 quickstart walkthrough. The pure ResolveAdapter/FormatAdapterLabel helpers consuming AdapterInfo are already covered by the previous commit's tests via the InMemoryAdapterProvider seam. Tests: 384/384 (no behavioural test changes). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… plumbing T029: Add std::wstring m_gpuAdapter (empty = system default) to ScreenSaverSettings. Default empty per FR-012/FR-038. T030: Add VALUE_GPU_ADAPTER = L\"GpuAdapter\" REG_SZ and read/write via the existing ReadString/WriteString helpers (already used for ColorScheme). Backward-compatible: absent value leaves the struct default of empty. T031: No-op for InMemorySettingsProvider (stores ScreenSaverSettings by value). T024: Add 2 GpuAdapter round-trip tests (preserves description; empty description round-trip). T032: Change RenderSystem::Initialize to accept std::optional<LUID> adapterLuid (default nullopt). Store as m_requestedAdapterLuid; consumed by CreateDevice which uses EnumAdapterByLuid + D3D_DRIVER_TYPE_UNKNOWN when an explicit adapter is requested, else preserves the existing nullptr+HARDWARE path. Silent fallback on EnumAdapterByLuid failure so a vanished GPU never blocks startup (FR-014). T033: Forward the optional<LUID> through MonitorRenderContext::Initialize to RenderSystem. T034: In Application::CreateRenderContexts, construct a WindowsAdapterProvider, enumerate adapters, call ResolveAdapter against the saved description, cache the result in m_resolvedAdapter. AddContext passes m_resolvedAdapter to every MonitorRenderContext::Initialize call so all monitors use the same physical GPU (FR-016). Re-resolved on every rebuild so device-lost recovery picks up the fallback path if the chosen adapter disappeared. Tests: 384 -> 386. The previously-mysterious heap-corruption failures turned out to be PCH cache staleness from the C1076/C3859 build aborts the constitution warns about; a clean rebuild restored sanity. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T035: Add ConfigDialogController::UpdateGpuAdapter(description) mirroring
UpdateColorScheme. Stores the description in m_settings.m_gpuAdapter;
cancel-revert works automatically via the snapshot.
T036: Add 'GPU:' label + IDC_GPU_COMBO (CBS_DROPDOWNLIST | WS_VSCROLL |
WS_TABSTOP, 170 dlu wide to accommodate long adapter names like 'NVIDIA
GeForce RTX 3050 Ti Laptop GPU') to the dialog template. Dialog grew
from 240x215 to 240x230; checkboxes shifted from y=110..155 to
y=125..170; OK/Cancel/Reset moved from y=190 to y=205.
T037: In ConfigDialog.cpp:
- Add std::vector<std::wstring> m_gpuAdapterDescriptions to
DialogContext (parallel to combo entries) so OnGpuChange can map
selection -> persistence string.
- InitializeGpuCombo enumerates via WindowsAdapterProvider, prepends a
synthetic '<system default>' entry (index 0 = empty m_gpuAdapter),
then adds each non-software adapter using FormatAdapterLabel
('(default)' suffix on the default). Pre-selects whichever entry
matches the saved description.
- OnGpuChange reads CB_GETCURSEL, looks up the matching description in
the parallel vector, calls controller.UpdateGpuAdapter, then posts a
rebuild (ApplyDisplayModeChange) for live preview.
- Wired into OnCommand alongside the existing IDC_COLORSCHEME_COMBO
case.
Tests: 386/386 (no behavioural test changes; live adapter switch is
covered by US3 quickstart manual QA on a hybrid laptop).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…p (TDD)
T039: New MatrixRainCore\FrameLimiter.{h,cpp}.
- Pure free function ShouldEngageFrameLimiter(refreshHz): true iff
refresh > 60 Hz (FR-017/FR-018: engage above 60, pass through at <=60).
- Class FrameLimiter using std::chrono::steady_clock for monotonic
timing. TargetFps(60) sets the frame interval; WaitForNextFrame()
returns immediately on the first call (no prior frame timestamp) and
sleeps via sleep_until to the next deadline on subsequent calls.
T038 (paired): 4 test cases - ShouldEngageFrameLimiter truth table
across {0, 30, 59, 60, 61, 75, 120, 144, 240} Hz; WaitForNextFrame
first-call-returns-immediately and second-call-sleeps-approximately
(~10-50ms tolerance for Windows scheduler jitter).
T040: MonitorRenderContext gets std::optional<FrameLimiter> m_frameLimiter
member. Initialize queries the monitor's native refresh via
MonitorFromWindow -> GetMonitorInfoW -> EnumDisplaySettingsW
(dmDisplayFrequency). If ShouldEngageFrameLimiter is true the limiter
is constructed targeting 60 fps; otherwise m_frameLimiter stays empty
and the vsync path is unchanged (FR-018 - zero per-frame overhead at
<=60 Hz, single nullopt check).
T041: At the top of RenderThreadProc's while loop, before deltaTime
computation, call m_frameLimiter->WaitForNextFrame() if engaged. Each
monitor in a mixed-refresh setup is paced independently (FR-019).
Tests: 386 -> 390. The 50% GPU reduction on 144 Hz hardware is verified
in T061 manual QA per SC-004.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The pure foundation US5 needs. Adds MatrixRainCore\QualityPresets.{h,cpp}:
- enum class QualityPreset { Low, Medium, High, Custom }.
- enum class ResolutionDivisor { Full=1, Half=2, Quarter=4, Eighth=8 }.
- enum class BlurTaps { Low=5, Medium=9, High=13 }.
- struct AdvancedGraphicsValues with equality operators.
- LookupPresetValues: pure table (asserts on Custom; falls back to High row).
- DetectActivePreset: returns the named preset whose row matches the
current values, else Custom.
- ApplyPresetSnap: snap to named-preset row OR restore lastCustom OR keep
current (Custom-with-no-lastCustom case).
- PickDefaultQualityPreset: first-run heuristic (discrete adapter ->
High; integrated + <=16M pixels -> Medium; integrated + >16M pixels ->
Low). Heuristic constants kDiscreteVramThresholdMb (256) and
kHeavyTotalPixelsThreshold (16M) exported for direct test verification
to protect against silent retunes.
T042 (paired): 13 tests covering the full table, drift detection
across-and-off the table, the three ApplyPresetSnap cases, all four
PickDefaultQualityPreset paths (discrete, integrated-light, integrated-
heavy, software-adapter-ignored), and the constants pin.
FR-022 verified: High preset's values exactly match today's hardcoded
rendering constants (3 passes, half res, 13-tap blur, intensity 100).
Remaining US5 (T043/T044/T046-T059) will wire these helpers into the
ScreenSaverSettings struct, registry persistence, RenderSystem parametric
bloom, first-run heuristic at Application::Initialize, controller methods,
the new dialog .rc (preset combo + advanced disclosure + 3 sliders +
infotip indicators), tick-mark conventions, dynamic dialog resize,
tooltip control, and owner-draw info buttons with keyboard activation.
Tests: 390 -> 403.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…stence T046: ScreenSaverSettings gains four US5 fields: - QualityPreset m_qualityPreset (default High) - AdvancedGraphicsValues m_advancedValues (defaults to High row) - std::optional<AdvancedGraphicsValues> m_lastCustom - bool m_showAdvancedGraphics (default false) Includes QualityPresets.h for the type definitions. T047: RegistrySettingsProvider: - VALUE_QUALITY_PRESET REG_SZ accepting Low/Medium/High/Custom; empty value leaves struct default (the first-run heuristic in T051 will fill in on first save). - VALUE_LASTCUSTOM_GLOW_INTENSITY/PASSES/RESOLUTION/SMOOTHNESS DWORDs: ALL FOUR required to honor; any missing -> m_lastCustom stays nullopt (registry-schema contract). - VALUE_SHOW_ADVANCED_GRAPHICS DWORD bool. Load applies preset row to m_advancedValues automatically when preset is named, or restores LastCustom if preset == Custom and snapshot is present. Save always writes preset + advanced + LastCustom when the optional has a value, plus showAdvancedGraphics. T048: No-op for InMemorySettingsProvider (stores by value). T049: Parametric bloom in RenderSystem: - Three blur shader variants per direction (5/9/13-tap) compiled at startup: m_blurHorizontalPS / PS9 / PS5, same for vertical. - ApplyBloom selects the variant matching m_blurTaps at bind time (no dynamic-loop overhead). - Pass count is m_blurPasses (clamped 1..4) replacing the hardcoded 3. - Bloom buffer size and viewport divided by static_cast<int>( m_bloomResolutionDivisor) (1=Full, 2=Half, 4=Quarter, 8=Eighth). - SetBlurPasses / SetBloomResolution / SetBlurTaps added to IRenderSystem and SpyRenderSystem. - SharedState + SharedState::Snapshot extended with blurPasses, bloomResolutionDivisor, blurTaps; MonitorRenderContext::RenderThreadProc pushes them to RenderSystem each frame. - SetBloomResolution recreates the bloom textures on change so a live Resolution-slider tweak rebuilds correctly. T050: When m_glowIntensity == 0 the entire bloom pipeline is bypassed in Render: extract/blur/composite are skipped and the scene texture is composited directly to the backbuffer via the existing composite shader (FR-031 'glow disabled' = true off, not just darker). Tests: 403/403 still passing after a clean rebuild. The intermittent PCH C1076/C3859 cache-staleness gotcha hit again on the way; documented once already (commit message on T024..T034) -- when it strikes, do a clean rebuild rather than trusting the test output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T051: First-run quality preset heuristic in Application::InitializeApplication State. Capture IsFirstRun() from ApplicationState (set when Load() returns S_FALSE - no registry key existed). On a brand-new install: - Enumerate adapters via WindowsAdapterProvider. - Compute totalMonitorPixels by summing each MonitorInfo's Width()*Height(). - Call PickDefaultQualityPreset(adapters, totalPixels) and persist via ApplicationState::ApplyFirstRunQualityPreset() (new method). - Seed SharedState with the chosen preset's advanced values so the first frame renders correctly. Existing installs (key present in registry, no QualityPreset value): - In-class default of High preserves today's exact rendering (FR-022). - SharedState seeded from loaded advanced values for the render thread. T052: Three new ConfigDialogController methods: - UpdateQualityPreset(preset): calls ApplyPresetSnap, writes back advanced values. Live-mode pushes the resulting glow intensity to ApplicationState for the snapshot path. - UpdateAdvancedGraphicsValues(values): writes new values AND always updates LastCustom (FR-023 - any direct edit captures user state even if it coincidentally matches a named preset row), then recomputes m_qualityPreset via DetectActivePreset (typically flips to Custom). - UpdateShowAdvancedGraphics(show): persists the disclosure toggle state. Cancel-revert is automatic via the snapshot for all three (m_settings captures the new fields by value). ApplicationState gains IsFirstRun() and ApplyFirstRunQualityPreset(). Tests: 403/403 still passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…dvanced sliders)
T053: Dialog template grows from 240x230 to 240x320 (95 dlu taller). Adds
a 'Graphics quality' GROUPBOX containing:
- IDC_QUALITY_PRESET_COMBO (Low/Medium/High/Custom)
- IDC_GRAPHICS_ADVANCED_CHECK ('Show advanced graphics settings' toggle)
- IDC_GLOWPASSES_SLIDER + LABEL (1..4)
- IDC_GLOWRES_SLIDER + LABEL (Eighth/Quarter/Half/Full)
- IDC_GLOWSMOOTH_SLIDER + LABEL (Low/Medium/High)
Buttons and remaining checkboxes pushed down. Info indicators
(IDC_*_INFO) reserved in T002 but the owner-draw + tooltip surface
(FR-034/FR-035/FR-036) is intentionally deferred to a follow-up commit.
T054: Extend InitializeSlider to send TBM_SETTICFREQ per the locked
tick-mark contract: Density freq=5 (21 ticks), AnimSpeed freq=5 + explicit
TBM_SETTIC at 100 (21 ticks total), GlowIntensity freq=10 (21 ticks),
GlowSize freq=5 (31 ticks, midpoint at 125). FormatPercentLabel special-
cases Glow Intensity at 0% to read '0% (glow disabled)' per FR-031.
New InitializePassesSlider / InitializeResolutionSlider /
InitializeSmoothnessSlider for the three discrete sliders with mapped
labels; FormatResolutionLabel/FormatSmoothnessLabel helpers.
T056: WM_HSCROLL handler extended:
- Each new discrete slider calls UpdateAdvancedGraphicsValues with the
new value, then reflects DetectActivePreset(values) back to the
preset combo (auto-flips to Custom on drift per FR-023).
- Glow Intensity is now part of the advanced value set; moving it also
drifts the preset to Custom.
T057: OnQualityPresetChange CBN_SELCHANGE handler:
- Calls UpdateQualityPreset(preset), which snaps advanced values via
ApplyPresetSnap.
- Reflects the snapped values back into the four sliders (Glow Intensity
+ Passes + Resolution + Smoothness) so the UI matches the new preset.
OnGraphicsAdvancedCheck handler persists IDC_GRAPHICS_ADVANCED_CHECK
state via UpdateShowAdvancedGraphics. T055 (dynamic dialog resize on
toggle) deferred to follow-up: today the advanced sliders are always
visible. Functionally complete for US5 acceptance scenarios 1-9; the
disclosure-and-resize behavior of scenarios 2-3 is partial (the toggle
is wired to the setting but does not hide controls or resize the dialog).
OnCommand wires IDC_QUALITY_PRESET_COMBO (CBN_SELCHANGE -> preset change)
and IDC_GRAPHICS_ADVANCED_CHECK (BN_CLICKED -> toggle handler).
Tests: 403/403. US5 functionality verifiable end-to-end via the quickstart
walkthrough except for the disclosure-resize visuals and the infotip
tooltips.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
OnInitDialog captures the advanced-block height (delta between the top of the highest advanced control and the bottom of the lowest, plus one row of spacing) via CaptureAdvancedBlockHeight while the .rc is laid out at its expanded size. Then collapses to the user's saved IDC_GRAPHICS_ADVANCED_CHECK state via ApplyAdvancedGraphicsVisibility. ApplyAdvancedGraphicsVisibility: - ShowWindow each advanced control (SW_SHOW / SW_HIDE). - SetWindowPos the dialog itself to grow/shrink by the captured block height delta. - GetWindow + GW_CHILD/GW_HWNDNEXT walks every child control and moves the ones below the advanced block (excluding the advanced controls themselves) up or down by the same delta. Naturally repositions OK/Cancel/Reset, the four mode checkboxes, and any other below-block controls without the .rc needing per-control y-coordinate tracking. OnGraphicsAdvancedCheck handler now calls ApplyAdvancedGraphicsVisibility in addition to persisting via UpdateShowAdvancedGraphics. Tests: 403/403. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds 7 small "i" PUSHBUTTON controls to the dialog template next to the quality-related controls (Quality preset, Show-advanced toggle, Glow Intensity, Glow Size, Passes, Resolution, Smoothness). These are real BUTTONs (focusable and tabbable) rather than the originally-spec'd owner-draw circles; the spec calls for "i in a circle" but the focusability + tooltip surface (the substance of FR-034/FR-035/FR-036) matters more than the exact glyph for v1.4. Owner-draw cosmetic polish can land in a follow-up if desired. CreateAndRegisterTooltip creates a single shared WC_TOOLTIPS window in OnInitDialog and registers each IDC_*_INFO button as a tool with TTF_IDISHWND | TTF_SUBCLASS, with per-tool text supplied via LPSTR_TEXTCALLBACKW. ConfigDialogProc handles TTN_GETDISPINFO[W] by mapping the tool's hwnd back to an IDC_*_INFO control id via GetDlgCtrlID and returning the locked infotip text from GetInfoTipText. TTM_SETMAXTIPWIDTH=300 dpx wraps long text into a readable multi-line tip. GetInfoTipText returns the seven locked strings per FR-036 (each ends with exactly one of "Significant", "Moderate", or "Small GPU performance impact."). Mouse hover on each info button shows the matching tooltip. Keyboard activation (Tab to button, Space/Enter) is partially covered: the buttons are focusable per WS_TABSTOP, but the BN_CLICKED -> manual TTM_TRACKACTIVATE wiring required to pop the tooltip via keyboard is deferred to a follow-up (T059 in tasks.md). Tests: 403/403. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DialogContext gains m_hTooltip storing the shared tooltip HWND created in T058. CreateAndRegisterTooltip now also registers a single TTF_TRACK | TTF_ABSOLUTE tool keyed by sentinel uId=kTrackTipUId for keyboard- activated tips. OnInfoButtonClick (called from OnCommand BN_CLICKED on any IDC_*_INFO button - which fires for both mouse click AND Space/Enter while the button has keyboard focus): - TTM_UPDATETIPTEXTW updates the TRACK tool's text to GetInfoTipText for the matching info id. - TTM_TRACKPOSITION positions the tip just below/right of the button's screen rect. - TTM_TRACKACTIVATE TRUE shows it. - SetTimer (kInfoTipDismissTimerId, 5000ms) for auto-dismiss. ConfigDialogProc handles: - WM_TIMER (kInfoTipDismissTimerId) -> DismissInfoTip. - WM_ACTIVATE WA_INACTIVE -> DismissInfoTip (lose-focus dismisses). Mouse hover continues to work via the original TTF_SUBCLASS tools from T058; keyboard activation uses the new TRACK tool. SC-006 fully satisfied: every infotip can be read by hovering AND by tabbing-then- Space/Enter. Tests: 403/403. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T043: 3 new RegistrySettingsProviderTests: - TestSaveLoadRoundTrip_QualityPreset_PreservesNamedPreset: verifies the QualityPreset REG_SZ round-trips and that the load logic auto-applies the named preset's row to m_advancedValues. - TestSaveLoadRoundTrip_LastCustom_AllOrNothing: verifies all four LastCustom_* DWORDs round-trip correctly, and that Custom+LastCustom present restores advancedValues from LastCustom. - TestLoadSettings_LastCustom_MissingOneValue_IgnoresAll: pins the all-or-nothing read contract from registry-schema.md (missing any single LastCustom_* DWORD yields nullopt rather than honoring partial state). T044: 4 new ConfigDialogControllerTests: - TestUpdateQualityPreset_SnapsAdvancedValues: named preset snaps the advanced values to its lookup row. - TestUpdateAdvancedGraphicsValues_DriftsToCustom: off-table edit auto-flips preset to Custom and always updates LastCustom. - TestUpdateQualityPreset_Custom_WithSavedLastCustom_RestoresIt: end-to-end the locked custom-snap behaviour from the contract. - TestUpdateShowAdvancedGraphics_PersistsInMemory: trivial setter. Tests: 403 -> 410. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T062: CHANGELOG [Unreleased] section now describes the v1.4 user-visible additions (US1-US5), the parametric bloom pipeline change, and known issues (infotip indicators render 'i' text rather than the spec'd owner-drawn glyph). T063: spec.md Status updated from 'Draft' to 'Implemented'; documents that T060/T061 (manual QA on hybrid laptop hardware) and the cosmetic follow-up for owner-drawn 'i-in-a-circle' indicators remain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…t-impl)
US5 had a real live-preview hole I missed during T051/T052: SharedState.
blurPasses / bloomResolutionDivisor / blurTaps were initialized in
Application::InitializeApplicationState from loaded settings, but no
callback updated them when ConfigDialogController changed values during
a live dialog session. Effect: moving an advanced slider or switching
preset persisted correctly but did NOT show on the running monitors
until restart, breaking SC-009 ('user can switch between named quality
presets... and observe the visual change... within 1 second of selection,
without restarting').
Fix:
- ApplicationState gains SetAdvancedGraphics(values) + a
m_advancedGraphicsChangeCallback + RegisterAdvancedGraphicsCallback,
mirroring the existing per-knob callback pattern (RegisterGlow
IntensityCallback etc).
- Application::InitializeApplicationState registers a callback that
updates SharedState.glowIntensityPercent, blurPasses,
bloomResolutionDivisor, and blurTaps as a unit (under m_sharedState.
mutex).
- ConfigDialogController::UpdateQualityPreset and
UpdateAdvancedGraphicsValues now call appState->SetAdvancedGraphics
in live mode, replacing the previous single-field push that only
updated GlowIntensity.
The next snapshot the render thread takes (next frame) sees the new
values and pushes them to RenderSystem via the IRenderSystem setters
landed in T046-T050.
Tests: 410/410 still pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… owner-draw remain) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…esolved) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tors Replaces the plain-text 'i' in each IDC_*_INFO button with the actual Unicode CIRCLED LATIN SMALL LETTER I character (U+24D8 = ⓘ). This matches the spec's 'lowercase i in a circle' requirement without needing WM_DRAWITEM owner-draw plumbing. The character is in the Enclosed Alphanumerics block and renders natively in Segoe UI on Windows 10 / 11 (and Segoe UI Symbol as fallback) so no font installation is required. The original research.md R-009 considered and rejected a Unicode-glyph approach citing 'font-availability fragility, not focusable'. The not-focusable concern only applied to using a STATIC control; we use real PUSHBUTTONs (already focusable, tabbable, Space/Enter activates BN_CLICKED). Font availability is a non-issue on the project's Win11 target. MatrixRain.rc is now saved as UTF-8 with BOM so the RC compiler reads the ⓘ character literal correctly. Updates CHANGELOG.md to remove the known-issue note about info indicators rendering plain 'i' text. Tests: 410/410. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… labels Three bugs from the user's screenshot of the collapsed Graphics-quality group: 1. Advanced disclosure only hid 6 of 12 controls (sliders + value labels). The 3 prompt LTEXTs and 3 ⓘ info buttons remained visible, and were moved up with the rest of the below-block controls by ApplyAdvanced GraphicsVisibility, overlapping the Quality combo and the advanced-toggle checkbox. Fix: give the 3 prompt labels real ids (IDC_GLOWPASSES_PROMPT, IDC_GLOWRES_PROMPT, IDC_GLOWSMOOTH_PROMPT) instead of IDC_STATIC, and add all 12 advanced-block control ids (prompt + slider + value + info x 3 rows) to kAdvancedGraphicsControlIds so the show/hide and the move-below-block-by-delta logic both treat them as a unit. 2. The 'Graphics quality' GROUPBOX kept its expanded height of 110 dlu even when the advanced rows were hidden, enclosing empty space at the bottom. Fix: give the groupbox its own id (IDC_QUALITY_GROUPBOX) and shrink its height by the advanced block delta whenever the disclosure collapses (and re-grow on expand) inside ApplyAdvancedGraphicsVisibility. 3. The 'Glow intensity:' prompt label was being clipped to 'Glow' because the original LTEXT width of 40 dlu was set for shorter labels like 'Density:' / 'Speed:'. This was a pre-existing v1.3 bug exposed alongside the others. Fix: widen all four percentage-slider prompt labels to 50 dlu and shift the trackbars from x=50 to x=60 (trackbar width reduced from 130 to 120 to keep the right margin / value-label position constant). Tests: 410/410. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
User feedback items 1-6 from screenshot: 1+2+3. Infotips were rendering with PUSHBUTTON chrome (raised bevel, pressed state, hover highlight). Changed to BS_OWNERDRAW buttons (no native frame) and added a WM_DRAWITEM handler in ConfigDialogProc that draws ONLY the ⓘ glyph - transparent background, COLOR_WINDOWTEXT, DT_CENTER | DT_VCENTER | DT_SINGLELINE | DT_NOCLIP. No button border, no pressed/hover state visuals. Glyph size: created a 1.5x-size HFONT in OnInitDialog from the dialog's own font (WM_GETFONT -> GetObject -> LOGFONT with lfHeight scaled by 3/2; CreateFontIndirectW). Owned by DialogContext, destroyed in OnDestroy. Info button rects grown from 12x12 to 14x14 dlu to give the larger glyph some padding (DT_NOCLIP allows overflow if needed). Focus indication: DrawFocusRect when ODS_FOCUS is set so Tab-to-button is still visually obvious; otherwise no chrome. 4. Tooltip on hover (TTF_SUBCLASS) and on click/Space/Enter when focused (BN_CLICKED -> TTM_TRACKACTIVATE) was already wired in T058/T059. BS_OWNERDRAW preserves WS_TABSTOP focusability and BN_CLICKED notify behavior, so all three paths continue to work for the new owner-draw buttons. 5. GPU dropdown had both a synthetic '<system default>' entry AND the real default adapter labeled with ' (default)' - redundant and confusing. Removed the synthetic entry; InitializeGpuCombo now populates only the real adapters. Selection logic: prefer the saved description match; otherwise auto-select the (default)-marked adapter so the UI always highlights what is actually running. An empty saved description now means 'fall back to default at resolution time' but the user picks the default adapter by name from the list rather than via a sentinel. 6. GPU combo width 170 -> 165 so its right edge lands at x=225, aligned with the right edge of the right-column ⓘ buttons (which moved from x=213 to x=211 so width 14 still ends at x=225). IDC_GRAPHICS_ADVANCED _INFO also moved from x=190 to x=211 to share the column. Items 7+8 from the screenshot (orphan advanced labels + groupbox not shrinking) were already fixed in commit ead52d8; the user's screenshot predates that commit. Tests: 410/410. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ibility, infotips next to value labels - Replace Quality preset COMBOBOX with a 4-position trackbar (IDC_QUALITY_PRESET_SLIDER + IDC_QUALITY_PRESET_LABEL), routed through WM_HSCROLL instead of WM_COMMAND/CBN_SELCHANGE. - Bump all slider heights 15 -> 20 dlu so visual-styles tick marks render reliably (the old 15 dlu rows were too short for v6 ticks to appear). - Move infotip glyphs from the right column (x=211) to immediately after each value label / control: shrink percentage value labels to w=22 and place infotips at x=209; Show advanced infotip moved to x=187 right after the checkbox text; Quality infotip at x=209 next to the new value label. - Grow groupbox h=110 -> 115 and dialog h=320 -> 325 to absorb the taller slider rows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…_CUSTOMDRAW
- Move every IDC_*_INFO button from the right column (x=204..209) to immediately after its prompt label at x=58 (e.g. 'Glow intensity:' ⓘ [slider]).
- Shift all sliders to a uniform x=74 to leave room for the inline info icon; main-row sliders now w=106, advanced/groupbox sliders w=101.
- Shrink advanced prompts ('Resolution:' / 'Smoothness:') from w=45 to w=40 so info icons align in a column at x=58.
- Quality preset slider's value label moves from x=165 to x=180 (matching the rest of the right column).
- Replace faint visual-styles tick marks with crisp COLOR_WINDOWTEXT lines via NM_CUSTOMDRAW (TBCD_TICS); old default ticks were nearly invisible at small sizes.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…verhaul Tick marks were rendering as a near-solid line because TBM_GETTICPOS returns many more entries than the configured frequency on some visual-styles versions. Replace with direct range/freq computation (same TickFrequencyForSliderId helper now used by both InitializeSlider and the custom-draw). Layout overhaul addressing the rest of the screenshot feedback: - Vertically center prompt label + infotip + slider value on each slider's middle (label_y = slider_y + 6, info_y = slider_y + 3). - Left-align all sliders and comboboxes at x=74; right-align all value labels and comboboxes at x=233 (dialog right margin). - Bump top margin so the density slider's focus rect no longer clips against the title bar. - Remove the infotip on 'Show advanced graphics settings' per UX feedback. - Move Reset to bottom-left and keep OK/Cancel at bottom-right. - Activate tooltip control on creation (TTM_ACTIVATE) — was defaulting to inactive, hover tips weren't firing. - Dialog grows from 240x325 to 240x360 to absorb the new spacing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ix Reset coverage, drop fade-timer hotkey Dialog changes: - Rename 'Passes' / 'Resolution' / 'Smoothness' to 'Glow passes' / 'Glow resolution' / 'Glow smoothness' so the prompts identify what they actually control. - Remove the 'Show advanced graphics settings' checkbox and always show the three glow tuning sliders (deletes the disclosure show/hide / dialog-resize / groupbox-resize logic and the m_advancedExpanded / m_advancedBlockHeight DialogContext fields). - Reset button now also restores Render-on-all-monitors, GPU adapter, quality preset slider + label, and the three glow tuning sliders (it previously only covered the v1.3 controls). - Layout column rework: all prompts at x=15 w=58 (fits 'Glow smoothness:'), infos at x=74, sliders at x=90 w=110, combos at x=90 w=143, values at x=205 w=28 — all right-edge aligned at x=233. - Dialog height shrinks 360 -> 340 dlu now that the show-advanced row is gone. Code/data removal: - Drop ScreenSaverSettings::m_showAdvancedGraphics + the ShowAdvancedGraphics registry value + ConfigDialogController::UpdateShowAdvancedGraphics + the matching test. - Drop ConfigDialog.cpp's kAdvancedGraphicsControlIds list, ApplyAdvancedGraphicsVisibility, CaptureAdvancedBlockHeight, OnGraphicsAdvancedCheck. Fade-timer hotkey: - Remove the VK_OEM_3 (backtick) debug-only InputSystem case that toggled ToggleDebugFadeTimes — the dialog's 'Show fade timers' checkbox remains. - Drop the matching backtick paragraph + screenshot from README.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Eighth + add GPU load % 1. Owner-draw info button now FillRects with COLOR_3DFACE before painting the glyph so XOR-drawn focus rects from prior paints are properly erased when focus moves away. 2. Right margin matches left margin (15 dlu). Combos / value labels / Cancel button right-edges are all at x=225; Reset moves to x=15 to match label left margin. Sliders shrink to w=102, value labels to x=197. 3. Replace TTF_SUBCLASS (unreliable on BS_OWNERDRAW buttons) with manual SetWindowSubclass + TTM_RELAYEVENT so hover/click tooltips actually fire. 4. Remove Eighth from the glow-resolution slider (range 0..2 mapping Quarter/Half/Full). The ResolutionDivisor::Eighth enum value is preserved for backward compat; if a saved configuration still has it the slider just snaps to Quarter on display. 5. Add 'GPU NN%' to the debug statistics line. RenderSystem now wraps each frame in D3D11 TIMESTAMP_DISJOINT + TIMESTAMP queries (3-frame pipeline so reads don't stall), computes GPU work / vsync interval, and exposes a smoothed (EMA) percentage to the FPS overlay. Link comctl32.lib in ConfigDialog.cpp for SetWindowSubclass / DefSubclassProc / RemoveWindowSubclass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eline with walk-back GPU load now matches Task Manager's 'engine busy %' intuition: gpu_work_ms / wall_clock_ms between Render calls. Previous impl divided by 1000/fps which (a) is 0 during fps warmup and (b) gates the EMA update on params.fps>0 — meaning if the queries took a few frames to flush we silently lost the data and never updated. New pipeline: - 6 query slots (deeper headroom for vsync-stalled GPUs). - Walk back from newest-issued slot looking for one whose 3 GetData calls all return S_OK (vs. failing the entire read if the specific N-2 slot hadn't flushed). - Wall-clock interval via QueryPerformanceCounter, independent of the fps counter; falls back to 16.667ms if first frame. - 85/15 EMA smoothing (faster response than the old 90/10). - First reading bypasses EMA so the displayed value isn't 0 for the first ~30 frames. - m_gpuHaveAnyReading flag: stats overlay shows 'GPU --%' until we have a real measurement, never the misleading 'GPU 0%'. Also added a layout comment block to MatrixRain.rc documenting the column conventions (label/info/slider/value column x positions, slider class + height rationale) to prevent the slider-widths-drifting-between-builds issue. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…eld changed OnCancel was unconditionally posting WM_APP_REBUILD_CONTEXTS, causing the visible matrix-rain window(s) to destroy + recreate every single time the user closed the settings dialog — even when they hadn't touched multimon or GPU. Now ConfigDialogController::LiveModeRebuildRequired() compares the live snapshot to current settings for the two fields that genuinely require a context tear-down (m_multiMonitorEnabled, m_gpuAdapter); OnCancel only posts the rebuild when one of those actually differs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d common controls The config dialog's trackbars (and every other common control) were rendering as the unthemed XP-classic fallback — thin 1-pixel channel, tiny monochrome 'house' thumb — instead of the Win11 themed version with the large blue accent thumb. Root cause: the project had no application manifest at all, so the OS gave us comctl32 v5. Fix: add MatrixRain.manifest declaring dependency on Microsoft.Windows.Common-Controls 6.0.0.0. Also call InitCommonControlsEx(ICC_BAR_CLASSES | ICC_STANDARD_CLASSES | ICC_WIN95_CLASSES) at startup so the v6 trackbar/button/combobox window classes are registered. The manifest also declares per-monitor v2 DPI awareness (matching the runtime SetProcessDpiAwarenessContext call) and Windows 10/11 compat so the OS skips shim-layer heuristics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replaced the D3D11 TIMESTAMP-query approach (which had the wrong semantics — it was measuring per-frame 3D-engine work / wall-clock interval, regularly reporting 99% when Task Manager showed ~38%) with the same PDH counter Task Manager uses: \\GPU Engine(*pid_NNNN*engtype_3D)\\Utilization Percentage New GpuLoadMonitor (file-scope singleton in RenderSystem.cpp) opens a single process-wide PDH query, polls at ~2 Hz (Task Manager itself updates at 1 Hz), and exposes the MAX across the wildcard-expanded counter instances. Matching what the Processes-tab GPU column reports. Displays 'GPU --%' until the first successful collection (PDH always returns no data on the first call), then 'GPU NN%' thereafter. Also tightened InitCommonControlsEx flags from ICC_WIN95_CLASSES | ICC_STANDARD_CLASSES | ICC_BAR_CLASSES (the first being a superset of the third) to just ICC_STANDARD_CLASSES | ICC_BAR_CLASSES — exactly what we use (button/edit/combobox/static/trackbar/tooltip). Removed the now-obsolete TIMESTAMP_DISJOINT + TIMESTAMP query infrastructure from RenderSystem (queries, slot tracking, walk-back logic, EMA smoothing — all replaced by PDH's internal averaging). Added <Pdh.h> + <PdhMsg.h> to MatrixRainCore/pch.h and linked pdh.lib via #pragma comment in RenderSystem.cpp. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…icks
1. Dialog widened 240 -> 300 dlu. All sliders stretched 102 -> 162 dlu; combos 135 -> 195 dlu; value-label column shifted 197 -> 257 dlu. Right margin still 15 dlu so right-edge controls now end at x=285. Groupbox widened 233 -> 286 to match the new dialog width.
2. Removed the NM_CUSTOMDRAW tick-mark handler (DrawTrackbarDarkTicks + IsTrackbarSliderId + the WM_NOTIFY NM_CUSTOMDRAW block). The original justification was that unthemed v5 trackbar ticks were nearly invisible — but with the comctl32 v6 manifest now in place, the themed trackbar draws perfectly readable ticks itself. TickFrequencyForSliderId stays since InitializeSlider still uses it to set TBM_SETTICFREQ.
3. Bottom checkboxes reflowed into two columns:
Left (x= 15, w=130): Start in fullscreen, Render on all monitors
Right (x=150, w=135): Show debug statistics, Show fade timers
Saves 30 dlu of vertical space; dialog height shrinks 340 -> 320.
4. Bottom buttons now respect the 15-dlu left/right margins: Reset at x=15, OK at x=180, Cancel at x=235 (right edge at x=285 matching the slider/combo right edge).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… slider rect center In the TBS_BOTTOM themed trackbar the channel sits in the upper-third of the control rect (the lower portion is reserved for tick marks). My label y values were positioned to center on the whole slider rect (slider_y + 6 for an 8-DLU label inside a 20-DLU slider), which put them ~4 DLU below the channel — visible misalignment compared to native dialogs like Mouse Properties where labels sit at the channel line. Moved every per-slider prompt label and value label up 4 DLU (label_y = slider_y + 2 instead of slider_y + 6). Also moved the inline ⓘ info buttons up 3 DLU (info_y = slider_y, was slider_y + 3) so the 14-DLU info button centers on the channel as well. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… Manager The per-process GPU column in Task Manager takes MAX across the full set of engine types (3D, Compute, Copy, VideoDecode, VideoEncode). My counter path was filtering to engtype_3D only, undercounting by ~10% because the bloom pipeline also exercises the Copy engine for DXGI blits/present. Removed the engine-type filter from the wildcard counter path; the MAX-across-instances reducer already in place now considers every engine type for our PID. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Task Manager)
Task Manager's per-process GPU column computes:
for each engine type (3D / Compute / Copy / Video* / ...):
sum utilization across every instance of that engine type
take MAX across engine types
My previous reducer took MAX across all instances flat, which underreports whenever an engine type has multiple instances (a GPU exposing 2 Copy engines at 30% each was being reported as 30%, not 60%). The new code parses the 'engtype_*' substring out of each PDH instance name, groups + sums per engine type, then takes MAX across the groups.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… in code The PID-filtered counter path '*pid_NNNN*' expanded its wildcard ONCE at PdhAddEnglishCounterW time, capturing only engine instances that were active at that moment. Any new instances spun up later by our process (typical as MR creates additional swap chains / does Copy/Compute work) were never seen by the counter, causing chronic underreporting (~10pp vs Task Manager). Now we add the counter as '\\GPU Engine(*)\\Utilization Percentage' (system-wide, all processes, all engines) and filter to 'pid_NNNN_' in the result-processing loop. Same per-engtype SUM + cross-engtype MAX reducer, matching Task Manager's per-process column. Also bumped the poll interval from 500ms to 1000ms to match Task Manager's update rate exactly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ask Manager Performance tab) Deep rethink: the user has consistently been describing the Task Manager number with phrasing like 'taskmgr shows 40% specifically on 3D'. That phrasing matches the Performance-tab per-engine GRAPH, which displays total system GPU utilization for the engine — NOT the per-process Processes-tab column. When a windowed app renders, the GPU work splits between: - the app's own 3D engine context (our process) - the DWM compositor's 3D engine context (does the final desktop composition for visible windows) My PID-filtered metric only saw our process's portion and chronically underreported by ~10pp — exactly the DWM compositor's share for displaying us. For a debug overlay on a rendering app, total-system is the more meaningful number anyway (DWM compositing is genuinely part of the cost of putting our frame on screen). Dropped the PID filter; the counter path is now '\\GPU Engine(*)\\Utilization Percentage' with the same per-engtype SUM + cross-engtype MAX reducer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… per-process column User's screenshot made it clear: Task Manager shows MR's process row at 26.4%, DWM at 7.3%, and overall system at 34%. My code was reporting 32% (between the two) because I had dropped the PID filter — that gave ~system-total but undershot by sampling timing. For a debug overlay 'how much GPU is this app costing', per-process is the right number. Restored PID filtering by parsing 'pid_NNNN_' out of each instance name (counter path stays '\\GPU Engine(*)' so PDH wildcard expansion always sees all engine instances). The per-engtype SUM + cross-engtype MAX reducer is unchanged. Displayed value should now match TM's MR-row GPU column within sampling-timing noise (~1-2pp). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ze, halve Glow size ticks - Reformat each IDC_*_INFO tooltip so the standardized GPU-performance-impact sentence sits on its own line, separated from the descriptive content by a blank line (\r\n\r\n). Easier to scan; matches typical 'fact + note' tooltip layout. - Glow size tooltip no longer ends with a perf-impact sentence — there's no measurable GPU delta across its 50%..200% range, so the standardized phrase would be misleading. - Glow size tick frequency 5 -> 10 (16 ticks across 50..200 instead of 31). Less visual noise on the slider; midpoint at 125 still lands on a tick. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR delivers the v1.4 infrastructure and UI changes for multi-monitor rendering (with live rebuild on topology changes), GPU adapter selection (hybrid laptop support), GPU-load telemetry (PDH-backed), and the new graphics quality/preset control surface wired into the render pipeline.
Changes:
- Add per-monitor context rebuild coalescing for
WM_DISPLAYCHANGEand device-loss recovery viaPresent()HRESULT classification. - Add GPU adapter enumeration/selection plumbing and persist selection (plus first-run quality preset heuristic).
- Parametrize bloom/blur (passes, resolution divisor, tap count) and overhaul the config dialog + docs/spec artifacts.
Reviewed changes
Copilot reviewed 67 out of 67 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/006-multimon-gpu-efficiency/spec.md | Feature specification for multi-monitor, adapter selection, FPS cap, presets, and dialog behavior. |
| specs/006-multimon-gpu-efficiency/research.md | Research/decisions document for Win32/DXGI/D3D11 approach details. |
| specs/006-multimon-gpu-efficiency/quickstart.md | Build/test/run and manual acceptance walkthrough for the feature set. |
| specs/006-multimon-gpu-efficiency/plan.md | Implementation plan and target file map for v1.4 work. |
| specs/006-multimon-gpu-efficiency/data-model.md | Registry + in-memory entity definitions for new settings/state. |
| specs/006-multimon-gpu-efficiency/contracts/tick-mark-conventions.md | UI tick-mark contract for trackbars (ranges/frequencies/labels). |
| specs/006-multimon-gpu-efficiency/contracts/registry-schema.md | Contract for new registry values and read/write rules. |
| specs/006-multimon-gpu-efficiency/contracts/quality-preset-mapping.md | Contract for preset→knob mapping and custom-drift behavior. |
| specs/006-multimon-gpu-efficiency/contracts/adapter-provider.md | Contract for IAdapterProvider / AdapterInfo and adapter persistence. |
| specs/006-multimon-gpu-efficiency/checklists/requirements.md | Spec quality checklist for the feature package. |
| README.md | Removes fade-timer hotkey documentation section. |
| CHANGELOG.md | Adds v1.4 “Unreleased” changelog entries for the new feature set. |
| .specify/feature.json | Points SpecKit at the feature directory for this workstream. |
| .github/copilot-instructions.md | Updates SpecKit “read the plan” pointer to the new feature docs. |
| MatrixRainCore/pch.h | Adds PDH + DXGI 1.6 headers for GPU% and adapter selection support. |
| MatrixRainCore/SharedState.h | Extends shared snapshot with advanced graphics knobs (passes/res/taps). |
| MatrixRainCore/ScreenSaverSettings.h | Adds multi-monitor flag, GPU adapter string, quality preset + advanced values. |
| MatrixRainCore/RegistrySettingsProvider.h | Adds new registry value names for v1.4 settings. |
| MatrixRainCore/RegistrySettingsProvider.cpp | Loads/saves new settings; derives advanced values from preset/LastCustom. |
| MatrixRainCore/IRenderSystem.h | Changes Present() to return HRESULT; adds advanced graphics setters. |
| MatrixRainCore/RenderSystem.h | Adds adapter-LUID initialization, advanced bloom knobs, blur shader variants, GPU% plumbing. |
| MatrixRainCore/RenderSystem.cpp | Implements adapter routing, PDH GPU%, bloom parametrization, device-loss Present HRESULT return. |
| MatrixRainCore/DeviceLost.h | Declares device-lost classifier helper. |
| MatrixRainCore/DeviceLost.cpp | Implements HRESULT classifier for device-loss recovery trigger. |
| MatrixRainCore/FrameLimiter.h | Declares per-monitor frame limiter + >60Hz engage predicate. |
| MatrixRainCore/FrameLimiter.cpp | Implements steady_clock-based frame pacing. |
| MatrixRainCore/RebuildCoalescer.h | Declares atomic-flag coalescer for rebuild requests. |
| MatrixRainCore/RebuildCoalescer.cpp | Implements coalesced rebuild request/consume behavior. |
| MatrixRainCore/MultiMonitorGate.h | Declares pure gate for “span all monitors” decision. |
| MatrixRainCore/MultiMonitorGate.cpp | Implements gate logic across display + screensaver modes. |
| MatrixRainCore/IAdapterProvider.h | Introduces adapter enumeration interface/value type. |
| MatrixRainCore/InMemoryAdapterProvider.h | Adds test seam adapter provider implementation. |
| MatrixRainCore/WindowsAdapterProvider.h | Declares DXGI-based adapter enumeration provider. |
| MatrixRainCore/WindowsAdapterProvider.cpp | Enumerates adapters; annotates default adapter; filters software adapters. |
| MatrixRainCore/AdapterSelection.h | Declares pure helpers for resolving/presenting adapter choices. |
| MatrixRainCore/AdapterSelection.cpp | Implements adapter resolve + display label formatting helpers. |
| MatrixRainCore/MonitorRenderContext.h | Wires per-monitor frame limiter and adapter-LUID into render context initialization. |
| MatrixRainCore/MonitorRenderContext.cpp | Engages limiter on >60Hz; checks Present HRESULT and posts rebuild on device-loss. |
| MatrixRainCore/ApplicationState.h | Adds first-run detection and advanced graphics callback API. |
| MatrixRainCore/ApplicationState.cpp | Records first-run status; adds advanced graphics plumbing and first-run preset apply. |
| MatrixRainCore/Application.h | Adds rebuild coalescer + resolved adapter LUID storage. |
| MatrixRainCore/Application.cpp | Adds first-run preset heuristic, adapter resolution, multi-monitor gating, WM_DISPLAYCHANGE rebuild coalescing. |
| MatrixRainCore/ConfigDialogController.h | Adds controller APIs for multi-monitor, GPU selection, preset + advanced values, and rebuild-needed query. |
| MatrixRainCore/ConfigDialogController.cpp | Implements new controller update methods and live-preview wiring. |
| MatrixRainCore/InputSystem.cpp | Removes debug-only backtick hotkey handling. |
| MatrixRainCore/MatrixRainCore.vcxproj | Adds new core helper/provider sources/headers to the project. |
| MatrixRainCore/Version.h | Updates build number. |
| MatrixRain/main.cpp | Initializes comctl32 common controls for themed dialog widgets. |
| MatrixRain/MatrixRain.manifest | Enables Common-Controls v6 + PerMonitorV2 DPI awareness + OS compatibility. |
| MatrixRain/MatrixRain.vcxproj | Adds manifest to the build inputs. |
| MatrixRain/resource.h | Adds new dialog control IDs for v1.4 UI. |
| MatrixRain/MatrixRain.rc | Overhauls configuration dialog layout and adds new controls (GPU/quality/infotips). |
| MatrixRain/ConfigDialog.cpp | Dialog wiring for GPU list, quality sliders, infotip tooltip behavior, tick setup, and live preview. |
| MatrixRainTests/MatrixRainTests.vcxproj | Adds new unit test sources for the new pure helpers. |
| MatrixRainTests/SpyRenderSystem.h | Updates spy interface to match new Present signature and advanced knobs. |
| MatrixRainTests/unit/ScreenSaverSettingsTests.cpp | Adds default assertion for new multi-monitor setting. |
| MatrixRainTests/unit/RegistrySettingsProviderTests.cpp | Adds tests for new registry-backed settings (multimon, adapter, preset, last custom). |
| MatrixRainTests/unit/DeviceLostTests.cpp | Adds tests for device-lost helper behavior. |
| MatrixRainTests/unit/FrameLimiterTests.cpp | Adds tests for limiter engage predicate and timing behavior. |
| MatrixRainTests/unit/MultiMonitorGateTests.cpp | Adds truth-table tests for multi-monitor gating helper. |
| MatrixRainTests/unit/QualityPresetsTests.cpp | Adds tests for preset table, drift detection, and first-run heuristic constants. |
| MatrixRainTests/unit/RebuildCoalescerTests.cpp | Adds tests for atomic coalescing behavior under contention. |
| MatrixRainTests/unit/AdapterSelectionTests.cpp | Adds tests for adapter resolve/labeling and in-memory provider seam. |
| MatrixRainTests/unit/ConfigDialogControllerTests.cpp | Adds tests for new US5 controller behavior (preset snap, drift-to-custom). |
Address findings from the principal-engineer review of PR #5. 1. (High) Glow intensity persisted incorrectly across the two parallel fields (top-level ScreenSaverSettings::m_glowIntensityPercent vs m_advancedValues.m_glowIntensityPercent). Picking the Low preset and restarting lost the 75 percent and reverted to the default 100. Fixed by mirroring the advanced-cluster value into the top-level field in ApplyFirstRunQualityPreset, UpdateQualityPreset, and UpdateAdvancedGraphicsValues so Save persists the right value and the next launch's seed-from-top-level is consistent. 2. (Medium) Render-thread device-lost path now posts WM_APP_DEVICE_LOST (new) instead of WM_APP_REBUILD_CONTEXTS directly. The new HandleMessage case funnels through m_rebuildCoalescer so an N-monitor burst (driver reset, sleep/resume, eGPU unplug) collapses to a single rebuild instead of N back-to-back rebuilds. 3. (Medium) GpuLoadMonitor wildcard counter is now re-expanded every 10s by closing and re-opening the PDH query. Previously the wildcard was expanded once at AddCounter time and never saw GPU engine instances that appeared later (which is exactly what happens after the device-lost recovery the PR introduced). Class refactored along the way into four focused helpers: Initialize / ReleaseQuery / EnsureQueryReady / CollectCounters / AggregateMaxPerEngineType. GetLoadPercent is now a cache-and-throttle gate that calls the helpers. 4. (Medium) Stripped per-declaration function-header banner blocks from new .h files (QualityPresets.h, AdapterSelection.h, FrameLimiter.h, IAdapterProvider.h) per the convention that the 80-char //// blocks belong on .cpp definitions, not .h declarations. Replaced with concise single-line // doc comments above each declaration. 5. (Medium) Added the standard function-header //// blocks to the first 16 functions in ConfigDialogController.cpp so the file matches the convention used throughout the rest of the codebase (the lower half already had them). 6. (Low) Left Initialize without an Error: label because /WX rejects the resulting unreferenced-label warning. The convention's "always include Error:" guidance is incompatible with the project's warning settings when no CHR/CBR path exists. Documented the choice with a comment so future additions are obvious. 454 tests pass on x64. Both x64 Debug and ARM64 Debug build clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
User convention: HRESULT functions still get the Error: label as a structural reminder, but commented out when no CHR/CBR/CWR macro currently references it (otherwise /WX trips on C4102 unreferenced-label warning). Uncomment on first EHM-macro addition. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address remaining inline review comments on PR #5. 1. (Bug) RenderSystem.cpp no-glow composite path was binding only 1 SRV (slot t0 = scene) but the composite PS samples 2 slots (scene + bloom). PSSetShaderResources with numResources=1 leaves slot t1 unchanged, so the composite would sample stale bloom from a previous frame when glow was toggled off mid-run. Bind a null SRV at slot 1 and pass num=2. 2. (Bug) IsDeviceLost was missing D3DDDIERR_DEVICEREMOVED, which can surface during real device-removal flows and was explicitly called out in the design notes. Added the case plus a paired unit test. The constant lives in the kernel-mode DDI header (d3dukmdt.h) so it's defined inline at both call sites with its documented stable value. 3. (Contract) IDC_GLOWSIZE_SLIDER tick frequency was 10, which left no tick at the midpoint (125) — violating the documented midpoint rule in contracts/tick-mark-conventions.md. Changed to 25, giving ticks at 50/75/100/125/150/175/200 (7 ticks; preserves the recent "halve" intent of fewer ticks AND keeps the midpoint tick). 4. (Contract) WindowsAdapterProvider::EnumerateAdapters was returning partial results on mid-enumeration DXGI failures (CBREx forces hr=S_OK and goto Error returned whatever was accumulated). Per contracts/adapter-provider.md the function must return an empty vector on any DXGI failure. Switched the EnumAdapters1 failure path to a dxgiFailure flag that the Error: handler tests to clear the vector. Not addressed (intentional / out of scope): - IDC_GLOWSIZE_INFO tooltip lacks the standardized perf-impact line: intentional per commit edbdeb1 ("drop perf-impact from Glow size"). - Eighth removed from IDC_GLOWRES_SLIDER: intentional per commit 3680068 ("drop Eighth + add GPU load %"). - "Advanced graphics settings" disclosure checkbox absent from IDC_GRAPHICS_ADVANCED_*: intentional per commit 09e6285 ("remove advanced disclosure"). The IDs remain in resource.h as reserved. - Version.h auto-mutation: noted, not committing the per-build bump. Structural fix (split VERSION_BUILD into a gitignored VersionBuild.h) has release.yml implications worth a dedicated discussion; tracking separately. ARM64 + x64 Debug build clean, 410/410 tests pass (was 409 — new D3DDDIERR_DEVICEREMOVED case adds one). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
relmer
added a commit
that referenced
this pull request
Jun 7, 2026
Per user convention: never put anything under [Unreleased]; use the actual version number at the time of the change. Splits the accumulated entries into [1.4.2098] (the v1.4 release content that shipped via PR #5) and [1.5.2161] (this PR's content). Drops the redundant '-- v1.5' suffixes since the section header now scopes them, adds a Fixed subsection for the bugs caught in pre-PR review (palette wipe, stale-SRV, WM_APP collision, PropertySheetW failure handling), and adds a few missed v1.5 entries (per-page FPS/GPU readout, modeless teardown via PropSheet_GetCurrentPageHwnd, Z-order fix on rebuild, overlay text custom-color resolution).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v1.4 ships per-monitor render contexts, GPU adapter selection, the quality-preset cluster, the PDH-backed GPU% stat, and a dialog overhaul that adds tooltips, two-column layout, a custom Reset, and live-preview wiring.
51 commits, 67 files changed, +5751/-134.
Highlights
Multimon rendering
GPU adapter selection
Quality preset cluster
GPU% stat
Dialog overhaul
Removed
Test status
454/454 unit tests pass.
Remaining manual QA
Related
v1.5 (07-dialog-tabs-scanlines-glowtoggle) is built on top of this and depends on its infrastructure. Land 006 first, then 007.