Repository navigation
pointsinside(): take distance sign from natcpp winding number - #529
Merged
Merged
Conversation
When natcpp (>= 0.3.2, Enhances) is available, rval="distance" and rval="mesh3d" now combine the unsigned closest-point distance from Rvcg::vcgClostKD with the sign from natcpp's generalised winding-number test. Rvcg's sign comes from the normal of a single closest face, which gives wrong results near edges, vertices and thin features (e.g. 73 kcs20 points ~36um outside MB_CA_L were reported as inside). distance >= 0 now agrees exactly with rval="logical". All natcpp use goes through the internal inside_winding() helper, which returns NULL unless use_natcpp(version='0.3.2') so callers fall back to pure Rvcg.
Add with_and_without_natcpp() test helper, which runs a block on the pure R/Rvcg path (options(nat.use_natcpp=FALSE)) and again on the natcpp path when it is available. Use it for the existing pointsinside() tests and new path-agnostic checks on an analytic tetrahedron and the kcs20/MB_CA_L data. natcpp-specific tests check that the distance sign matches rval='logical', the magnitude matches unsigned Rvcg, and that inverted faces are handled.
jefferis
force-pushed
the
feature/pointsinside-winding-distance
branch
from
September 28, 2026 18:42
7b3b335 to
f09540c
Compare
natcpp is only in Enhances, so add an ubuntu release job that removes it after dependency installation and asserts it is absent. This exercises the pure R/Rvcg fallback paths in tests, examples and vignettes, and catches any natcpp:: calls not guarded by use_natcpp().
jefferis
force-pushed
the
feature/pointsinside-winding-distance
branch
from
September 28, 2026 18:53
f09540c to
e48afd3
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
1 participant
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.
Summary
Follow-up to #528. When natcpp (>= 0.3.2, Enhances) is available,
pointsinside(rval="distance")andrval="mesh3d"now combine:Rvcg::vcgClostKD(sign=FALSE)(purely geometric), withRvcg derives the sign from the normal of a single closest face, which is unreliable near edges, vertices and thin features. On
kcs20vsMB_CA_L, 73 points ~36 µm outside the calyx (all outside its bounding box — the "red points in the vertical lobe" in the example) were previously reported as inside.distance >= 0now agrees exactly withrval="logical"; distance magnitudes are unchanged. Cost is ~20% extra forrval="distance".All natcpp use is behind the internal
inside_winding()helper, which returnsNULLunlessuse_natcpp(version='0.3.2'), so the pure Rvcg path is used when natcpp is absent oroptions(nat.use_natcpp=FALSE).Changes
R/hxsurf.R,man/pointsinside.Rd,NEWS.md: implementation and docs (also tempers the face-orientation robustness claim: winding numbers handle globally inverted meshes, not mixed orientation).with_and_without_natcpp()helper runspointsinside()tests on both paths; natcpp-specific tests check sign/logical agreement, bbox consistency, unsigned magnitude, and an inverted tetrahedron.Test plan
test-hxsurf.Rpasses locally with natcpp🤖 Generated with Claude Code