fix(drawing): restrict emitAngular sweep to non-reflex (≤ π) to match Angular.value (#1169) - #1196
Closed
SMKiloBOT wants to merge 1 commit into
Closed
fix(drawing): restrict emitAngular sweep to non-reflex (≤ π) to match Angular.value (#1169)#1196SMKiloBOT wants to merge 1 commit into
SMKiloBOT wants to merge 1 commit into
Conversation
… Angular.value DrawingAnnotation.swift: Angular.value uses acos(dot) which always returns [0, π]. DrawingDispatch.swift: emitAngular used raw atan2 with ascending sort, allowing reflex arcs (> π). This made the drawn arc and printed label describe different angles for the same ray pair. Add a reflex-guard: when the sorted counter-clockwise sweep exceeds π, recompute the shorter arc (≤ π) by swapping direction, so the drawn sweep and the label value both describe the same non-reflex angle. Fixes #1169. Signed-off-by: SMKiloBOT <nimiriishaku@gmail.com> Co-authored-by: elb <elb@users.noreply.github.com>
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Reviewed by nemotron-3-ultra-550b-a55b:free · Input: 157.2K · Output: 2.4K · Cached: 108K |
Collaborator
|
Closing without merging: this is a byte-identical duplicate of the |
Collaborator
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.
What & why
Restricted
emitAngularsweep to non-reflex (≤ π) to matchAngular.value. The previous implementation could produce reflex arcs (> π) which diverged from the canonicalAngular.value(acos, always ≤ π). Now the shorter counter-clockwise arc is taken by swapping direction when the sweep exceeds π.Closes #1169
CHANGELOG entry
fix(drawing): restrict emitAngular sweep to non-reflex (≤ π) to match Angular.value (#1169)
SemVer impact
PATCH. Consumers using angular dimension rendering will see corrected sweep angles for reflex arcs; no migration needed.
Checklist
--self-testcase was run once with its subject broken, and the failure is reported heredocs/CHANGELOG.mdis not in this diff.docs/SEMVER.mdis not in this diff.Notes for the reviewer
This is a drawing fix that ensures angular dimension arcs are rendered consistently with the measured angle value.