Repository navigation
Conversation
…dinates The same fault already fixed for LwPolyline and Circle, in three more places. TEXT's insertion point, SOLID's four corners, and a HATCH's boundary are all recorded in the entity's own object coordinate system, and each GetBoundingBox returned them untouched - so for the (0,0,-1) extrusion AutoCAD writes whenever geometry is mirrored, all three were boxed at the negated X of where they draw. The hatch also ignored Elevation entirely, boxing a boundary that sits on a plane at Z as though it sat at zero. A hatch's derived boundary holds whole entities, each with its own extrusion, so those are left alone rather than mapped a second time. ELLIPSE is deliberately NOT changed, and now has a test saying so: unlike a circle, an ellipse's centre and major axis are WCS in both formats, and CurveExtensions has a separate overload for it that correctly never builds the arbitrary-axis matrix. That test exists to stop the next reader "fixing" Ellipse the way Circle had to be fixed - the corpus this came from has 666 mirrored ellipses that are currently in the right place. One thing I could not settle: the sign of Elevation under a flipped normal. The fix maps (x, y, Elevation) through the arbitrary axis, which is the consistent reading, but I have no AutoCAD measurement pinning a mirrored hatch that also carries a non-zero elevation - the corpus has none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
redbluevn
force-pushed
the
moredwg/pr76-ocs-text-solid-hatch
branch
from
September 5, 2026 07:41
2e5cce1 to
323cef1
Compare
This branch has not been deployed
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 same fault as #1224, in three more places — and a test asserting that a fourth type must not be changed.
The three
TEXT's insertion point,SOLID's four corners and aHATCH's boundary are all recorded in the entity's own object coordinate system. EachGetBoundingBox()returned them untouched, so for the(0,0,-1)extrusion AutoCAD writes whenever geometry is mirrored, all three were boxed at the negated X of where they draw.The hatch also ignored
Elevationentirely, boxing a boundary that sits on a plane at Z as though it sat at zero.A hatch's derived boundary (
BoundaryPath.Entities) holds whole entities, each carrying its own extrusion, so those are left on their own path rather than mapped a second time.The one that must stay as it is
ELLIPSE is deliberately not changed, and this PR adds a test saying so.
Unlike a circle's, an ellipse's centre and major-axis endpoint are WCS in both DWG and DXF — this repo's own reader says so (
//Center 3BD 10 (WCS),//SM axis vec 3BD 11 ... (WCS)inDwgObjectReader.readEllipse, against a bare//Center 3BD 10for circle and arc). AndCurveExtensionshas twoPolygonalVertexesoverloads: the radius one (circle/arc) applies the arbitrary-axis matrix tocenter + offset, while the ellipse one — the one taking a major-axis point and a ratio — is pure WCS vector maths and never builds the matrix at all.So an ellipse is already right, and "fixing" it the way
Circlehad to be fixed would break it. The corpus this came from has 666 mirrored ellipses currently in the correct place.MirroredEllipseCentreIsAlreadyInWorldCoordinatesexists to stop that.What I could not settle
The sign of
Elevationunder a flipped normal. The fix maps(x, y, Elevation)through the arbitrary axis, which is the consistent reading, but I have no AutoCAD measurement pinning it: the eighteen drawings I have contain no mirrored hatch that also carries a non-zero elevation. For the overwhelmingly common+Zcase the mapping is the identity and usingElevationinstead of zero is unambiguously an improvement. Flagging it rather than quietly asserting it.Tests
New, each failing against the current code:
MirroredTextIsBoxedThroughItsNormal,MirroredSolidIsBoxedThroughItsNormal,MirroredHatchIsBoxedThroughItsNormal.SolidTestsis new and also fills in theGetBoundingBoxTestthe common base requires. Plus the ellipse guard described above, which passes both before and after — that is its point.dotnet test: 2329 passed / 17 failed, againstmasterat 592d70a on this machine at 2313 / 17 — the same pre-existing failures, plus the tests added here and the inherited ones the newSolidTestsclass brings with it.Related
#1223, #1224, #1225, #1226, #1227 and DomCR/CSUtilities#23.