Skip to content

Fix hatch boundary bounding boxes: a weight is not a height, and a bulge is not either - #1231

Open
redbluevn wants to merge 1 commit into
DomCR:masterfrom
redbluevn:moredwg/pr79-hatch-boundary-boxes
Open

redbluevn wants to merge 1 commit into
DomCR:masterfrom
redbluevn:moredwg/pr79-hatch-boundary-boxes

Conversation

@redbluevn

Copy link
Copy Markdown

Two edge types, the same mistake, and both are documented against themselves a few lines above the fault.

Spline edge

/// Position values are only X and Y, Z represents the weight.
public List<XYZ> ControlPoints { get; private set; }
...
public override BoundingBox GetBoundingBox()
{
    return BoundingBox.FromPoints(this.ControlPoints);
}

So the box's Z range is a range of weights.

Polyline edge

/// The vertex bulge is stored in the Z component.
public List<XYZ> Vertices { get; private set; }
...
public override BoundingBox GetBoundingBox()
{
    return BoundingBox.FromPoints(this.Vertices);
}

So the box's Z range is a range of bulges.

A hatch boundary is planar; both boxes are flat. And a hatch lives inside a block as often as not, so the false height propagates: the block inherits it, and every INSERT of that block inherits it again.

The polyline edge also lost real geometry

A bulged segment bows outside its own chord. The bulges were not merely misplaced into Z — they were never used as geometry at all, so the box was too small in X and Y as well. Each bulged segment is now measured through Arc, which is how the rest of the library measures an arc.

A repeated vertex that still carries a bulge is skipped: it leaves no chord for an arc to span, and Arc refuses the zero radius its own maths then produces (Circle.Radius's setter throws).

Evidence

Measured over eighteen architectural drawings: 17,315 polyline boundary edges, of which 2,302 carry a bulge and 2,106 were reporting a false Z range. After the fix, zero.

The drawings' computed extents still agree with AutoCAD's on all eighteen, so the larger X and Y boxes are geometry AutoCAD had already accounted for — the fix does not overshoot.

Tests

HatchSplineBoundingBoxTests (new file) pins the spline case, including that the box is unchanged whatever the weights are. HatchTests.APolylineBoundaryIsBoxedFlatAndFollowsItsBulge pins the polyline case: a bulge of 1 over a chord of 10 is a semicircle of radius 5, so the box must be flat in Z and reach 5 off the chord — taken from the two vertices alone it would be flat in Y too.

dotnet test: 2319 passed / 17 failed, against master at 592d70a on this machine at 2313 / 17 — the same pre-existing failures, plus the tests added here.

Related

#1223-#1230, DomCR/CSUtilities#23 — all found by comparing computed extents against the extents AutoCAD reports.

…s geometry

Two edge types, the same mistake, both documented against themselves.

Spline edge: "Position values are only X and Y, Z represents the weight" - the
remark sits on ControlPoints. GetBoundingBox handed them to FromPoints as though
they were points, so the box's Z range was a range of WEIGHTS.

Polyline edge: "The vertex bulge is stored in the Z component" - the remark sits
on Vertices. Same call, so the box's Z range was a range of BULGES.

A hatch boundary is planar, so both boxes are flat. And a hatch lives inside a
block as often as not, so a false height propagated: the block inherited it, and
every INSERT of that block inherited it again.

The polyline edge lost something else as well. A bulged segment bows outside its
own chord, so a box taken from the vertices alone is too small in X and Y - the
bulges were not merely misplaced into Z, they were never used as geometry at
all. Each one is now measured through Arc, which is how the rest of the library
measures an arc; a repeated vertex is skipped, since it leaves no chord for one
to span and Arc refuses the zero radius its own maths then produces.

Measured on eighteen architectural drawings: 17,315 polyline boundary edges, of
which 2,302 carry a bulge and 2,106 were reporting a false Z range. After: zero.
The drawings' extents still agree with AutoCAD's on all eighteen, so the larger
X and Y boxes are geometry AutoCAD had already accounted for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant