Fix the sampled domain of a periodic spline (bounding box reached the origin) - #1225
Open
redbluevn wants to merge 1 commit into
Open
Fix the sampled domain of a periodic spline (bounding box reached the origin)#1225redbluevn wants to merge 1 commit into
redbluevn wants to merge 1 commit into
Conversation
This was referenced Aug 30, 2026
getStartAndEndKnots tested IsClosed before IsPeriodic. A periodic spline forms a closed loop and so carries the closed flag as well, which meant the periodic branch never ran and the domain was taken as knots[0]..knots[last]. For an unclamped knot vector the domain starts at knots[degree]. Evaluating below it makes every basis function vanish, so the first sampled point came back as the zero vector - and BoundingBox.FromPoints then stretched the box from the spline all the way to the origin. A drawing measured here has a periodic spline about 59 by 74 units across, sitting four million units from the origin. Its bounding box was reported as (0, -9264141) to (4021541, 0). It sits inside a block, so every reference to that block inherited the same box. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
redbluevn
force-pushed
the
moredwg/pr73-periodic-spline-domain
branch
from
September 5, 2026 07:41
4a13687 to
bd05822
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
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.
The problem
getStartAndEndKnotstestsIsClosedbeforeIsPeriodic:A periodic spline forms a closed loop and therefore carries the closed flag too, so the periodic branch never runs and the domain is taken as
knots[0] .. knots[last].For an unclamped knot vector the domain starts at
knots[degree]. Evaluating below it makes every basis function vanish, andPolygonalVertexesreturns the zero vector for that sample.What it costs
Measured on a periodic spline out of a real drawing — degree 3, 7 control points, 11 knots spanning 0..2,
IsClosed = IsPeriodic = true:A spline about 59 by 74 units across reports a box four million by nine million, because one sample came back as
(0,0,0)andBoundingBox.FromPointshonoured it. It lives inside a block, so every reference to that block inherited the box, and the drawing's extents with it.After the fix,
zeros=0and the box is the spline's own(4021482.2, -9264138.8) .. (4021541.2, -9264067.2).The fix
Ask about periodic first. The knot vector decides the domain; the closed flag does not override it.
Tests
New
GetBoundingBoxOfAPeriodicSplineStaysOnTheCurvebuilds a periodic spline away from the origin and asserts the box stays within the convex hull of the control points — a property every B-spline has, and one the previous behaviour violated. It fails against the current implementation.dotnet teston this branch: 2314 passed / 17 failed.masterat 592d70a on this machine is 2313 passed / 17 failed — the same failures (SingleMLeader, GeoData, InsertWithSpatialFilter), plus the one test this PR adds.Related
Found while reconciling computed extents with the extents AutoCAD reports for eighteen architectural drawings, alongside #1223 and #1224.