Remove shadowing compound(from:) and use compound(_:) in Section2D (#1171) - #1197
Merged
Conversation
…1171) Section2D.swift:77 was calling an internal union-based compound(from:) helper that shadowed the public compound(_:) grouping method. The internal method performed sequential BRepAlgoAPI_Fuse instead of pure TopoDS_Compound grouping. Removed the internal helper and made the call site use the public compound(_:) directly, matching the documented grouping behavior for edge compounds. Signed-off-by: SMKiloBOT <nimiriishaku@gmail.com> Co-authored-by: SMKiloBOT <nimiriishaku@gmail.com>
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous Review Summary (commit 55bf31d)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 55bf31d)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Reviewed by nemotron-3-ultra-550b-a55b:free · Input: 133.5K · Output: 2.3K · Cached: 108K |
…adowing) The only existing section2DView test sections a plain box (one contour loop), so Section2D.swift's wire-compounding call (compound(from:) before this fix, Shape.compound(_:) after) hit its own single-shape special case in BOTH versions and never exercised the actual divergence. New test sections a box with a through-hole (two disjoint loops: outer square + inner circle). Measured directly, in a throwaway probe copied onto both the pre-fix commit (c27d252) and this fix: both give the identical edge count (20) for this fixture, since OCCT's boolean fuse of non-intersecting wire shapes behaves like a plain union when the pieces never touch. So this is new coverage for the previously-untested 2-loop path, not a bug-fix regression test -- there was no behavioral divergence to catch here, just an unverified code path. Recorded honestly in the test's own comment rather than claiming a bug this fixture doesn't actually show.
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.
What & why
Removed the shadowing
compound(from:)method inSection2Dand use the existingcompound(_:)method instead. The oldcompound(from:)was shadowing the base implementation and causing confusion.Closes #1171
CHANGELOG entry
Remove shadowing compound(from:) and use compound(_:) in Section2D (#1171)
SemVer impact
NONE. Internal refactor only; no public API change. The
compound(from:)was a shadowing method not intended for external use.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 cleanup/refactor of Section2D to remove a shadowing method.