Skip to content

Preserve full-sweep HATCH boundary angles - #1237

Merged
DomCR merged 4 commits into
DomCR:masterfrom
mediummandoo:fix/dxf-hatch-full-sweep
Sep 10, 2026
Merged

DomCR merged 4 commits into
DomCR:masterfrom
mediummandoo:fix/dxf-hatch-full-sweep

Conversation

@mediummandoo

@mediummandoo mediummandoo commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Description

Preserve full-sweep HATCH arc and ellipse boundaries while keeping DXF angles in -360..360. Converting negative angles to positive equivalents can collapse a sweep such as -90..270 to equal endpoints.

Both edge types use MathHelper.RadToDeg(..., normalize: true, absolute: false). A HATCH-only adjustment preserves full sweeps whose normalized endpoints would otherwise coincide: 180..540 is written as -180..180, not 180..180 or an angle above 360. No common utility or other entity conversions are changed.

Tasks done in this PR

  • Move the file-generation fixture into WriterSingleObjectTests.SingleCaseGenerator and register it in Data.
  • Cover full, partial, and zero sweeps, signed angle limits, and direction flags in DxfWriterSingleObjectTests.
  • Include the negative-angle arc and above-range full ellipse in the shared generation fixture.

Related Issues / Pull Requests

Notes for reviewer

net9.0 writer tests: 859/859 passed with the repository's CI settings:

LOCAL_ENV=false SAVE_OUTPUT_IN_STREAM=true SAVE_PREVIEW=false SELF_CHECK_OUTPUT=false \
dotnet test src/ACadSharp.Tests/ACadSharp.Tests.csproj --framework net9.0 \
  --filter 'FullyQualifiedName~DxfWriterSingleObjectTests|FullyQualifiedName~DwgWriterSingleObjectTests'

The 12 angle cases assert round-trip results independently of SELF_CHECK_OUTPUT. The shared fixture also passes all 11 DXF/DWG file-generation and re-read cases with self-check enabled. net48 builds successfully. AutoCAD display behavior has not been checked locally.

@DomCR

DomCR commented Sep 2, 2026

Copy link
Copy Markdown
Owner

The methods that you are replacing do the same as the change, it seems a redundant change.

@DomCR DomCR closed this Sep 2, 2026
@mediummandoo

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing. I checked the exact CSUtilities revision pinned by ACadSharp (fdf1403ede6e3376a0baa1deac166cbe4d21262e). MathHelper.RadToDeg is not equivalent to direct multiplication: it always calls NormalizeAngle(result), and its absolute parameter is currently unused.

For this full sweep:

  • RadToDeg(-π/2) produces 270
  • RadToDeg(3π/2) also produces 270
  • direct conversion produces -90 and 270, preserving the 360-degree span

The DXF reader only converts those values back with DegToRad, so it cannot recover the lost sweep. This is the case covered by the added arc and ellipse regression test.

Would you consider reopening this PR? If you prefer, I can instead fix the unused absolute behavior in CSUtilities and update ACadSharp to use RadToDeg(value, false).

@DomCR

DomCR commented Sep 2, 2026

Copy link
Copy Markdown
Owner

I see, the issue then is in CSUtilities, it should allow the option for non absolute values in range of -360 to 360, reopening the PR.

@DomCR DomCR reopened this Sep 2, 2026
redbluevn pushed a commit to redbluevn/ACadSharpNew that referenced this pull request Sep 7, 2026
Group codes 50 and 51 of a hatch boundary arc or ellipse were each converted with
MathHelper.RadToDeg, which normalises. A circular boundary is stored as a full sweep -
-PI to PI, or PI to 3*PI - so both endpoints normalised to the same number, 180 and
180, and the 360 degrees between them were gone. The reader only calls DegToRad, so
nothing downstream can get the sweep back: the boundary, and the fill with it,
disappears from the file.

Both endpoints now convert without normalising, through the absolute flag that
CSUtilities accepted and ignored until the companion commit there.

AutoCAD 2027 was asked, on a minted fixture rather than a client drawing, because
writing one of those to DXF makes a 54 MB file that accoreconsole refuses either way:

  written the old way   AUDIT opened, 0 errors, extents (60,100)..(60,100) - a point
  written this way      AUDIT opened, 0 errors, extents (60,60)..(140,140) - the circle

Note the audit column. AutoCAD never complains about the old file; it simply has no
hatch in it. That is why this went unnoticed while every gate here stayed green.

The PI..3*PI shape writes 180 and 540, outside the -360..360 range the maintainer
named in DomCR#1237, so that case was put to AutoCAD too: opened, 0 errors,
and the extents are the whole circle.

Measured on the client corpus: twenty-five such boundaries across four drawings out of
13,371 arc and 338 ellipse boundary edges. A DXF round trip of one of them returned
three arcs with a zero sweep before this change and none after.

Three regression tests, all three red when either half of the fix is reverted.
Suite 2954/0, no in-scope loss, twenty AUDIT channels clean, extents 21 of 21,
corpus gate green with the DXF channel holding its baseline.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@DomCR DomCR added the bug Something isn't working label Sep 9, 2026
@DomCR

DomCR commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Hi @mediummandoo,

CSUtilities has been updated.

Comment thread src/ACadSharp.Tests/IO/DXF/DxfHatchWriterTests.cs Outdated
Comment thread src/ACadSharp/IO/DXF/DxfStreamWriter/DxfSectionWriterBase.Entities.cs Outdated
@DomCR

DomCR commented Sep 10, 2026

Copy link
Copy Markdown
Owner

One last change and is good to go!

@DomCR
DomCR merged commit 53bb495 into DomCR:master Sep 10, 2026
5 checks passed
@mediummandoo
mediummandoo deleted the fix/dxf-hatch-full-sweep branch September 12, 2026 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants