Fix preview controls not responding on scaled displays - #698
Open
rmbakker88 wants to merge 2 commits into
Open
Conversation
DrawOverlay receives an SKImageInfo already divided by uiScale and draws onto a canvas scaled by the same factor, so every hit region it stores is in scaled coordinates. The mouse handlers compared those regions against e.Location, which is in device pixels, so at any display scaling above 100% each region sat towards the top left of the control it was drawn in, by a factor of uiScale. At 125% the right chevron is drawn at the right edge but its clickable area ends at 80% of the width, so clicking it did nothing while clicking empty space well to its left advanced the preview. The play/pause button and the carousel indicators were offset the same way, and the arrows stopped responding below 80% of the height. The left chevron kept working only because its region starts at x = 0 and still overlapped where the chevron is drawn. Convert the mouse position into the same scaled space before hit testing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes theme preview overlay hit-testing on high-DPI / scaled displays by converting mouse coordinates into the same logical (scaled) coordinate space used when laying out overlay hit regions.
Changes:
- Add a helper to convert device-pixel mouse coordinates to overlay (logical) coordinates.
- Update click and hover hit-testing to use the converted coordinates for all overlay controls.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+140
to
+144
| private Point ToOverlayPoint(Point location) | ||
| { | ||
| float scale = DeviceDpi / 96f; | ||
| return new Point((int)(location.X / scale), (int)(location.Y / scale)); | ||
| } |
Reuse the uiScale field instead of recomputing the scale from DeviceDpi. The hit regions are laid out during the last render, so deriving the mouse conversion from the same field keeps the two matched if the control's DPI changes before it is redrawn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Fixes #697
On a display scaled above 100%, the right chevron in the theme preview does nothing, the play/pause button does nothing, and arrow clicks in the lower part of the pane are ignored. Clicking empty image area to the left of the chevron advances the preview instead.
OnPaintSurfacescales the canvas byuiScaleand handsDrawOverlayanSKImageInfoalready divided by that factor, so every hit region the renderer stores is in scaled coordinates.OnMouseClickandOnMouseMovecompared them againste.Location, which is in device pixels, so at 125% every region sat 20% toward the top left of where it was drawn.The left chevron kept working only because
LeftArrowRectstarts atx = 0and still overlapped where the chevron is drawn — which is why this looks like a right-arrow-only bug.This converts the mouse position into the same scaled space before hit testing, which fixes the arrows, the play/pause button, the download message and the carousel indicators in one place.
Note on when this appeared
This is not in 5.7.0 —
v5.7.0:src/Skia/ThemePreviewer.cshas nouiScale,canvas.ScaleorlogicalInfo, so the overlay was drawn unscaled and the regions lined up. The scaling came in with 86eccca ("Fix small text in theme preview for high DPI") three weeks after the tag, and the hit testing was not updated alongside it. So this only affectsmain, and would otherwise ship with the next release.Testing
dotnet test --filter "type!=system"— 7 passed, unchanged frommain.