#13492 - circle/ellipse should only select on their outline, not thei…#76
Merged
Merged
Conversation
Zeljko-Predjeskovic
force-pushed
the
feature/circle-ellipse-hittest
branch
from
July 16, 2026 08:41
7b31b6b to
e1d0106
Compare
Zeljko-Predjeskovic
changed the title
gentledepp
requested changes
Jul 16, 2026
gentledepp
left a comment
gentledepp
left a comment
Owner
There was a problem hiding this comment.
Please ensure that the hit-testing is "only" checking the lines only in case the shape (whatever it is) has no background (background and I guess fill-mode must be respected?) Ask claude ;-)
gentledepp
approved these changes
Jul 16, 2026
gentledepp
left a comment
gentledepp
left a comment
Owner
There was a problem hiding this comment.
Seems ok. Please test this in the svg editor playground before merging!
… bounding box Unfilled circles and ellipses now hit-test against their actual outline (approximated as a 36-segment polygon) instead of always matching anywhere inside their bounding box. Also adds a stroke-width-based "fat finger" tolerance to the existing border-only hit test for rectangle/polygon/polyline/path/line, so tapping anywhere within a thick border's visible band counts as a hit. Known follow-ups (see PR discussion): - ScaleTolerance's rotation-invariance assumption (documented in code) - circle/ellipse outline approximation error grows with radius/zoom - this only covers circle/ellipse + stroke tolerance for the straight-edged shapes; arrow and free-draw shapes are not addressed, so a uniform all-shapes path-based hit-testing pass is still needed as a separate PR
… shapes
Replaces the "any fill color set => whole bounding box counts as a hit"
shortcut with a real fill-aware check for rectangle/polygon/polyline/path
(and circle/ellipse, via the shared elliptical-outline helper):
- A tap now only hits the interior when the shape has a *visible*
background (HasVisibleFill: a fill colour is set AND fill-opacity > 0),
so a fill-opacity:0 shape correctly behaves like it has no background.
- The interior check is a real point-in-polygon test honouring the
shape's fill-rule (nonzero vs evenodd), so concave notches and
self-intersecting shapes (e.g. a pentagram) are handled correctly -
not just "inside the bounding box".
- The border ("fat finger" stroke tolerance) check still always applies,
filled or not.
Known limitations, measured and pinned by tests rather than assumed:
- SvgPath's interior test flattens all subpaths into one vertex ring
(PathData.GetLines() has no subpath-break awareness), so a
multi-contour path (e.g. a "donut") isn't evaluated as independent
contours - for a simple two-square donut this can even miss the real
filled body between the contours, not just fail to carve out the hole.
A real fix needs GetLines() to preserve subpath breaks. Out of scope here.
- The interior test for circle/ellipse reuses the existing 36-segment
polygon approximation of the curve, so the same chord/sagitta
approximation error that was previously only a border-hit caveat now
also applies near the border of a filled circle/ellipse.
- ScaleTolerance's rotation-invariance assumption and the arrow-marker
hit-testing gap remain open, unchanged from before - tracked separately.
… hit target
Corrects the previous commit's HasVisibleFill (fill-opacity > 0) check:
a fill-opacity:0 shape still has a fill colour set, so its interior must
stay a valid hit target - the same way WPF's Background="Transparent"
is still clickable while Background="{x:Null}" is not. Only a shape
with no fill colour at all (fill:none / unset) should skip the interior
test and fall through to border-only hit testing.
Removes the now-redundant HasVisibleFill wrapper in favor of calling
HasFill() directly, and replaces the test that encoded the old (wrong)
behavior with two tests: fill-opacity doesn't affect the interior hit
either way, and fill:none correctly has no interior hit target.
Zeljko-Predjeskovic
force-pushed
the
feature/circle-ellipse-hittest
branch
from
July 17, 2026 07:27
a8fd299 to
f7a0ed3
Compare
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
2 participants
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.
…r bounding box
Unfilled circles and ellipses now hit-test against their actual outline (approximated as a 36-segment polygon) instead of always matching anywhere inside their bounding box. Also adds a stroke-width-based "fat finger" tolerance to the existing border-only hit test for rectangle/polygon/polyline/path/line, so tapping anywhere within a thick border's visible band counts as a hit.
Known follow-ups (see PR discussion):