Skip to content

Chart text keeps its size at high Windows scaling (#6678) - #6687

Merged
erikdarlingdata merged 3 commits into
devfrom
fix/6678-chart-text-dpi
Oct 24, 2026
Merged

erikdarlingdata merged 3 commits into
devfrom
fix/6678-chart-text-dpi

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 24, 2026 •

Copy link
Copy Markdown
Owner

Fixes #6678.

Claude posting for Erik Darling.

Cause

ScottPlot.WPF 5.1.59 paints into a device-pixel canvas. The only scale it applies is Plot.ScaleFactor (canvas.Scale(ScaleFactor) in RenderManager.Render), and that stays 1. WpfPlot.DisplayScale is read once in the constructor, before the control has a window, and the render never reads it. So a 13 point label was 13 device pixels: 40% of its size at 250%.

Fix

  • New PerformanceMonitor.Ui/ChartDisplayScale.cs, shared by the Viewer and Lite. Apply(chart, dpiScale) sets Plot.ScaleFactor (and DisplayScale) from the control's DPI scale. Register() (called from both App.OnStartup) adds a class handler on WpfPlot SizeChanged, so every chart gets it when first laid out, and subscribes the chart's window DpiChanged (weakly held, so charts on hidden tabs follow too) for a move between monitors.
  • WpfPlot.Reset() swaps in a plot at scale 1, so the three Reset() sites (ChartRenderHelper.ClearChart and the two CorrelatedTimelineLanesControl.ClearChart) call Apply after it.
  • Library setting used, no per-font scaling.

Hit-testing (no double conversion)

Every caller (ChartHoverHelper, CorrelatedCrosshairManager, the lanes, both heatmaps in both apps) hands Plot.GetCoordinates a DEVICE pixel (mouse x GetDpi), and GetCoordinates divides by ScaleFactor itself, GetPixel multiplies by it. So they stay correct unchanged. One caller did mix units: ChartHoverHelper.TryLegendHitTest compares the click with the legend panel's layout rectangles, which are in scaled units. It now divides the click by ScaleFactor first. The hover's "within 80 px" threshold stays in device pixels, as before.
LaneAxisAligner measures at ActualHeight * DisplayScale, which is now the real factor, and the gutter it measures and the MinimumSize it sets are both in scaled units, so it stays consistent.

#6674 axis-title fit (changed)

ChartAxisTitle.Fit uses ScaledFigureRect.Height, so it follows the scale. But #6674 was checked on this 250% laptop while the text was drawn at 40%, so ReservedHeight = 60 (the bottom ticks, legend and padding) was far too small for text at its real size: at the 150 floor the plot was about 38 DIP tall and "Blocking events (blocked-process-report)" was cut mid-word. I measured the real stack on the shots: about 112 DIP. ReservedHeight 60 to 112 and ChartLayout.MinPlotHeight 150 to 200 (still inside ViewerShortPlotTests' 140-200 range), so a row's plot is about 90 DIP and titles shrink or end in an ellipsis instead of being cut. Visible effect: at 1366x768 the stacked chart tabs scroll earlier. Lite does not use these row floors.

Screenshots (Viewer, this laptop at 250%, 1366x768 DIP, seeded throwaway store, DARLING_CONFIG set in the Viewer's own process)

Before = Release build with the one ScaleFactor line disabled; after = this branch. Folder C:\Users\edarl\AppData\Local\Temp\claude\c--GitHub-PerformanceMonitor\6c96b8f0-077c-40a9-9846-851157a73b54\scratchpad\wave-312\shots-6678\:
250pct-before-{overview,blocking-trends,tempdb,fileio-latency,queries-trends,query-heatmap}.png, 250pct-after- the same six, 250pct-after-hover-blocking-tooltip.png, 250pct-after-hover-heatmap-pick.png.

  • After: tick labels, legends and axis titles are the size of the window's own text on every chart; before they are about 40%.
  • Tooltip: hovering the Lock wait line at x between 21:00 and 00:00 gives "LCK_M_X, 50 ms/sec, 21:23:00", under the cursor.
  • Heatmap pick: hovering the 10-100ms band at the 22:00 column gives "22:00:00 | 10-100ms | 3 queries", the cell under the cursor.
  • All six "after" files were taken after the ReservedHeight change.

Tests

  • New ChartDisplayScaleTests in Darling.Tests and Lite.Tests (8 each): the scale follows the DPI scale, back to 1.0 after a DPI change, unknown DPI means 1, Reset() then Apply restores it, the figure lays out at 1000/2.5 = 400 by 500/2.5 = 200 units at 2.5, and the hover finds each point from its device pixel at 1.0 and 2.5.
  • Revert-proved: with the ScaleFactor assignment commented out, 3 of the 8 fail (the scale rule, reset, layout size); the hover cases pass both ways by design (they guard against a double conversion).
  • Darling.Tests: ChartDisplayScale, *Census*, *Ratchet*, *Xaml*, ViewerShortPlot*, hover and lane-aligner classes: 537 run, 0 failed, 1 skipped (live test needing the rig). Lite.Tests: ChartDisplayScale, Census, Ratchet, Xaml: 40 run, 0 failed.
  • Not run: full suite and pre-push.sh (brief: CI decides). Not rebuilt after the final merge origin/dev (merge brought only unrelated files).
  • Lite was not run: the brief forbids starting it on real data and I did not set up a throwaway Lite folder; Lite is proven by its test and by sharing ChartDisplayScale, ChartHoverHelper and ChartRenderHelper with the Viewer.

Considered, not added

  • Scaling the hover's 80 px tolerance by the DPI scale: unchanged from before, no wrong result.
  • A per-monitor-move shot: DPI-change path covered by the Apply test only (one monitor here).

CHANGELOG entry

Chart text keeps its size at high Windows scaling ([#6678]) - On a high-DPI screen (for example 250%) the Viewer's and Lite's chart tick labels, legends and axis titles were drawn at about 40% of their intended size; they now scale with the window, including after moving to a monitor with another scaling, and the Viewer's stacked chart rows keep a taller minimum so axis titles fit.

@erikdarlingdata
erikdarlingdata marked this pull request as ready for review October 24, 2026 01:50
@claude

claude Bot commented Oct 24, 2026

Copy link
Copy Markdown

Note (non-blocking): the new ChartHoverHelper hover/click tests don't exercise the actual production mouse→pixel path.

OnMouseMove/GetNearestSeries (PerformanceMonitor.Ui/ChartHoverHelper.cs:217-222, 356-358) compute the hover's hit-test pixel as mousePos * dpi.DpiScaleX/Y, unchanged by this PR, and the whole fix hinges on Plot.GetCoordinates/GetPixel dividing/multiplying that device pixel by ScaleFactor internally (asserted in the ChartDisplayScale XML remarks and in the PR body's "Hit-testing" section).

The_hover_finds_the_point_under_a_device_pixel_at_every_scaling (ChartDisplayScaleTests.cs, both apps) is named for exactly that path, but it builds its pixel with chart.Plot.GetPixel(coords) rather than mousePos * dpi.DpiScaleX, so it round-trips through GetPixel/GetCoordinates self-consistently regardless of which pixel convention they use — it would pass even if the real device-pixel assumption were wrong. The PR body says as much itself ("the hover cases pass both ways by design"), so this is a known, accepted gap rather than something hidden — flagging only because a live check at a scaled DPI (mouse actually driving a tooltip, not GetPixel driving it) is the only thing that would actually prove the claim.

Same story for the new clickPixel.Divide(ScaleFactor) in TryLegendHitTest (ChartHoverHelper.cs:507): no test (new or existing — ChartClickIsolateTests only covers the pure static helpers, never a simulated click) exercises the legend hit-test at all, scaled or not.

Everything else checks out: all three WpfPlot.Reset() sites get the post-reset Apply, Register() follows the established AccessibleNames class-handler pattern, LaneAxisAligner.Wpf.cs's pre-existing ActualHeight * DisplayScale comment (which assumed DisplayScale already reflected the real DPI factor) is now actually true instead of silently stuck at 1, and ReservedHeight/MinPlotHeight land inside the pre-existing ViewerShortPlotTests bounds (200 is exactly the upper bound of InRange(140, 200) — worth knowing if that constant ever needs to grow again).

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: LGTM

Claims

  • VERIFIED — "WpfPlot.Reset() swaps in a plot at scale 1, so the three Reset() sites ... call Apply after it" — confirmed all three (and only three) .Reset() calls on a WpfPlot in the repo are ChartRenderHelper.ClearChart (ChartRenderHelper.cs:36-37) and the two CorrelatedTimelineLanesControl.ClearChart (Lite:766-767, Darling:628-629), each followed by ChartDisplayScale.Apply(chart).
  • VERIFIED — "Every caller ... hands Plot.GetCoordinates a DEVICE pixel (mouse x GetDpi)" — grepped every GetCoordinates/GetPixel call site (ChartHoverHelper, CorrelatedCrosshairManager.cs:265-267, both apps' heatmaps, both apps' CorrelatedTimelineLanesControl); all build the pixel as pos * dpi.DpiScaleX/Y, none in scaled units.
  • VERIFIED — "One caller did mix units: ChartHoverHelper.TryLegendHitTest ... now divides the click by ScaleFactor first" — ChartHoverHelper.cs:507, the only new pixel-space adjustment outside ChartDisplayScale.cs itself.
  • VERIFIED — "ChartLayout.MinPlotHeight 150 to 200 (still inside ViewerShortPlotTests' 140-200 range)" — Darling/Darling.Tests/ViewerShortPlotTests.cs:111 asserts InRange(ChartLayout.MinPlotHeight, 140, 200); 200 is the exact upper bound.
  • VERIFIED — "Lite does not use these row floors" — ChartLayout/ChartAxisTitle have no reference anywhere under Lite/; only Darling/.../*.xaml bind ui:ChartLayout.MinPlotHeight.
  • UNVERIFIED — "ScottPlot.WPF 5.1.59 paints into a device-pixel canvas... canvas.Scale(ScaleFactor) in RenderManager.Render... GetCoordinates divides by ScaleFactor itself, GetPixel multiplies by it" — a claim about the ScottPlot.WPF 5.1.59 library's own source; no package source or network access from this read-only tree to confirm directly. The repo's own tests (ScaledFigureRect math, hover-at-2.5x) are consistent with it but don't independently prove the GetCoordinates/GetPixel internals.
  • UNVERIFIED — the cited test-run counts ("537 run, 0 failed, 1 skipped" / "40 run, 0 failed") and the revert-proof ("3 of the 8 fail... the hover cases pass both ways by design") — live build/test results, not reproducible from a diff read.
  • UNVERIFIED — the screenshot-based visual claims (40%→real-size text, tooltip/heatmap pick wording) — external image files, not in the diff.

Riskiest lines

  1. PerformanceMonitor.Ui/ChartHoverHelper.cs:217-222 & 356-358 — GetNearestSeries/OnMouseMove's mouse→device-pixel hit-test, unchanged by this PR but now load-bearing on Plot.GetCoordinates correctly un-scaling a device pixel when ScaleFactor != 1 — UNCOVERED: The_hover_finds_the_point_under_a_device_pixel_at_every_scaling builds its pixel via Plot.GetPixel, not mousePos * dpi.DpiScaleX, so (per the PR body itself) it "passes both ways" and would not fail if that assumption were wrong.
  2. PerformanceMonitor.Ui/ChartHoverHelper.cs:507 — new clickPixel.Divide(ScaleFactor) before the legend-panel rect test — UNCOVERED: no test, old or new, simulates an actual legend click (ChartClickIsolateTests only exercises NextIsolate/ResolveSeriesVisual/RestoreSeriesVisual, never TryLegendHitTest/HandleLeftClick).
  3. PerformanceMonitor.Ui/ChartAxisTitle.cs:94 — MinPlotHeight raised to exactly 200, the inclusive upper bound of the pre-existing, unmodified MinimumPlotHeight_IsTallEnoughForTicks assertion (ViewerShortPlotTests.cs:111) — not a defect today, but any future increase will fail that test and the coupling isn't obvious from either file alone.

Traps near this diff

  • "Lite and Darling ship in parity... a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding" — does not apply: every change (ChartDisplayScale.Register, the ClearChart Apply call, the new ChartDisplayScaleTests file) is duplicated identically in Lite and Darling (Lite/App.xaml.cs:607, Darling/.../App.xaml.cs:153; Lite.Tests/ChartDisplayScaleTests.cs and Darling/Darling.Tests/ChartDisplayScaleTests.cs are near-identical). The one asymmetry (ChartAxisTitle/ChartLayout row-floor constants, Darling-only) is pre-existing (from #6652) and the PR body states the reason ("Lite does not use these row floors"), not something this diff introduced unevenly.
  • "Ruling (#5833): tests are load-bearing... never answer a trap with a test that reads source text or asserts how code is spelled" — does not apply: the new ChartDisplayScaleTests assert real behavior (Plot.ScaleFactor values, ScaledFigureRect dimensions, hover text content), not source shape.

Divergence (blind read vs PR body)

None of substance — the PR body's "Hit-testing (no double conversion)" section independently names the same coverage limitation I found in Phase 1 ("the hover cases pass both ways by design"), so this is agreement rather than a hidden gap. The body is otherwise an accurate account of the diff: the Cause/Fix/Hit-testing/#6674 sections all match what's in the tree.

Reviewer checklist

  • Verify interactively on an actual high-DPI (e.g. 250%) monitor that chart hover tooltips and legend-key clicks land on the correct point/key, not just that chart text is the right size — the included tests do not exercise the real mouse-device-pixel path for either.

@erikdarlingdata
erikdarlingdata enabled auto-merge (squash) October 24, 2026 02:12
@erikdarlingdata
erikdarlingdata merged commit d659bef into dev Oct 24, 2026
31 of 32 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/6678-chart-text-dpi branch October 24, 2026 03:34
@github-actions github-actions Bot mentioned this pull request Oct 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant