Repository navigation
Chart text keeps its size at high Windows scaling (#6678) - #6687
Conversation
|
Note (non-blocking): the new
Same story for the new Everything else checks out: all three |
There was a problem hiding this comment.
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 aWpfPlotin the repo areChartRenderHelper.ClearChart(ChartRenderHelper.cs:36-37) and the twoCorrelatedTimelineLanesControl.ClearChart(Lite:766-767, Darling:628-629), each followed byChartDisplayScale.Apply(chart). - VERIFIED — "Every caller ... hands Plot.GetCoordinates a DEVICE pixel (mouse x GetDpi)" — grepped every
GetCoordinates/GetPixelcall site (ChartHoverHelper,CorrelatedCrosshairManager.cs:265-267, both apps' heatmaps, both apps'CorrelatedTimelineLanesControl); all build the pixel aspos * 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.csitself. - 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/ChartAxisTitlehave no reference anywhere underLite/; onlyDarling/.../*.xamlbindui: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
- PerformanceMonitor.Ui/ChartHoverHelper.cs:217-222 & 356-358 —
GetNearestSeries/OnMouseMove's mouse→device-pixel hit-test, unchanged by this PR but now load-bearing onPlot.GetCoordinatescorrectly un-scaling a device pixel whenScaleFactor != 1— UNCOVERED:The_hover_finds_the_point_under_a_device_pixel_at_every_scalingbuilds its pixel viaPlot.GetPixel, notmousePos * dpi.DpiScaleX, so (per the PR body itself) it "passes both ways" and would not fail if that assumption were wrong. - 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 (ChartClickIsolateTestsonly exercisesNextIsolate/ResolveSeriesVisual/RestoreSeriesVisual, neverTryLegendHitTest/HandleLeftClick). - PerformanceMonitor.Ui/ChartAxisTitle.cs:94 —
MinPlotHeightraised to exactly 200, the inclusive upper bound of the pre-existing, unmodifiedMinimumPlotHeight_IsTallEnoughForTicksassertion (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/ChartLayoutrow-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
ChartDisplayScaleTestsassert real behavior (Plot.ScaleFactorvalues,ScaledFigureRectdimensions, 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.
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)inRenderManager.Render), and that stays 1.WpfPlot.DisplayScaleis 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
PerformanceMonitor.Ui/ChartDisplayScale.cs, shared by the Viewer and Lite.Apply(chart, dpiScale)setsPlot.ScaleFactor(andDisplayScale) from the control's DPI scale.Register()(called from bothApp.OnStartup) adds a class handler onWpfPlotSizeChanged, so every chart gets it when first laid out, and subscribes the chart's windowDpiChanged(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 threeReset()sites (ChartRenderHelper.ClearChartand the twoCorrelatedTimelineLanesControl.ClearChart) callApplyafter it.Hit-testing (no double conversion)
Every caller (
ChartHoverHelper,CorrelatedCrosshairManager, the lanes, both heatmaps in both apps) handsPlot.GetCoordinatesa DEVICE pixel (mouse xGetDpi), andGetCoordinatesdivides byScaleFactoritself,GetPixelmultiplies by it. So they stay correct unchanged. One caller did mix units:ChartHoverHelper.TryLegendHitTestcompares the click with the legend panel's layout rectangles, which are in scaled units. It now divides the click byScaleFactorfirst. The hover's "within 80 px" threshold stays in device pixels, as before.LaneAxisAlignermeasures atActualHeight * DisplayScale, which is now the real factor, and the gutter it measures and theMinimumSizeit sets are both in scaled units, so it stays consistent.#6674 axis-title fit (changed)
ChartAxisTitle.FitusesScaledFigureRect.Height, so it follows the scale. But #6674 was checked on this 250% laptop while the text was drawn at 40%, soReservedHeight = 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.ReservedHeight60 to 112 andChartLayout.MinPlotHeight150 to 200 (still insideViewerShortPlotTests' 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_CONFIGset in the Viewer's own process)Before = Release build with the one
ScaleFactorline disabled; after = this branch. FolderC:\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.ReservedHeightchange.Tests
ChartDisplayScaleTestsinDarling.TestsandLite.Tests(8 each): the scale follows the DPI scale, back to 1.0 after a DPI change, unknown DPI means 1,Reset()thenApplyrestores 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.ScaleFactorassignment 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).*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.pre-push.sh(brief: CI decides). Not rebuilt after the finalmerge origin/dev(merge brought only unrelated files).ChartDisplayScale,ChartHoverHelperandChartRenderHelperwith the Viewer.Considered, not added
Applytest 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.