From f302f2d825ef7125bd97d06986aa2948cc162c7c Mon Sep 17 00:00:00 2001 From: MoreDwg Date: Sun, 30 Aug 2026 10:13:50 +0700 Subject: [PATCH 1/3] fix: the 1/64 threshold in GetArbitraryAxis is integer division and is always zero The arbitrary axis algorithm picks its reference axis from the world Y when the normal is close to the world Z, and from the world Z otherwise. The threshold was written as (1 / 64), which in C# is integer division between two ints and evaluates to 0, so the condition could never hold and the Y branch was dead code. Every normal then took the Z branch. That is correct for a genuinely tilted normal, but for one that means +Z while carrying the rounding dust a real file records - (-3.7e-13, 8.8e-14, 1) is taken verbatim from an architectural drawing - the cross product with Z is on the order of 1e-13, and normalising it returns a direction decided entirely by that dust. The resulting frame has nothing to do with the entity. The visible effect is that block references land in the wrong place: measured across eighteen drawings, 169 inserts whose normal was +Z to within a rounding error were being positioned by a garbage rotation, which is enough to leave a drawing's computed extents tens of times too large. Neither GetArbitraryAxis nor Matrix4 had any test coverage. The new tests pin the identity and negated-Z cases, the near-Z case above, and that either side of the threshold still yields a frame whose Z is the normal. Co-Authored-By: Claude Opus 5 (1M context) --- CSMath.Tests/Matrix4Tests.cs | 52 ++++++++++++++++++++++++++++++++++++ CSMath/Matrix4.cs | 6 ++++- 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 CSMath.Tests/Matrix4Tests.cs diff --git a/CSMath.Tests/Matrix4Tests.cs b/CSMath.Tests/Matrix4Tests.cs new file mode 100644 index 0000000..dd661a0 --- /dev/null +++ b/CSMath.Tests/Matrix4Tests.cs @@ -0,0 +1,52 @@ +using Xunit; + +namespace CSMath.Tests; + +public class Matrix4Tests +{ + [Fact] + public void GetArbitraryAxisReturnsIdentityForTheZAxis() + { + Assert.Equal(Matrix4.Identity, Matrix4.GetArbitraryAxis(XYZ.AxisZ)); + } + + [Fact] + public void GetArbitraryAxisMirrorsXAndZForANegatedZAxis() + { + Matrix4 m = Matrix4.GetArbitraryAxis(-XYZ.AxisZ); + + AssertUtils.AreEqual(new XYZ(-1, 2, -3), m * new XYZ(1, 2, 3)); + } + + [Fact] + public void GetArbitraryAxisHandlesANormalThatIsNearlyButNotExactlyTheZAxis() + { + //What a real drawing stores: a normal that means "+Z" but carries the dust of whatever + //arithmetic produced it. The arbitrary axis algorithm switches to the Y reference below a + //1/64 threshold precisely for this case - written as integer division that threshold is + //zero, the branch never runs, and the cross product with Z is then so close to the zero + //vector that normalising it yields a direction unrelated to the entity. + XYZ almostZ = new XYZ(-3.707652143685886E-13, 8.818399612355268E-14, 1); + + Matrix4 m = Matrix4.GetArbitraryAxis(almostZ); + + XYZ point = new XYZ(3952258.943164733, -9072802.879860947, 0); + + AssertUtils.AreEqual(point, m * point); + } + + [Fact] + public void GetArbitraryAxisUsesTheYReferenceBelowTheThreshold() + { + //Just inside the threshold the X axis comes from Y x N, just outside it from Z x N. Both + //have to produce a right handed frame whose Z is the normal itself. + foreach (double tilt in new[] { 1.0 / 128.0, 1.0 / 32.0 }) + { + XYZ normal = new XYZ(tilt, 0, 1).Normalize(); + + Matrix4 m = Matrix4.GetArbitraryAxis(normal); + + AssertUtils.AreEqual(normal, m * XYZ.AxisZ); + } + } +} diff --git a/CSMath/Matrix4.cs b/CSMath/Matrix4.cs index acb2797..d16ff0e 100644 --- a/CSMath/Matrix4.cs +++ b/CSMath/Matrix4.cs @@ -434,7 +434,11 @@ public static Matrix4 GetArbitraryAxis(XYZ zaxis) } XYZ xaxis; - if (Math.Abs(zaxis.X) < (1 / 64) && Math.Abs(zaxis.Y) < (1 / 64)) + //1/64 is integer division and is therefore zero, which made this branch unreachable and sent + //every normal that is near +Z - but not exactly it, which is what a real drawing stores - + //down the Wz path, where the cross product is near zero and normalises to a direction that + //has nothing to do with the entity. + if (Math.Abs(zaxis.X) < (1.0 / 64.0) && Math.Abs(zaxis.Y) < (1.0 / 64.0)) { xaxis = XYZ.Cross(XYZ.AxisY, zaxis); } From b988dea347970b7b0879112fae462ef9bff9bb7b Mon Sep 17 00:00:00 2001 From: MoreDwg Date: Sun, 30 Aug 2026 18:14:40 +0700 Subject: [PATCH 2/3] fix: complete partial stream reads safely --- CSUtilities.Tests/IO/StreamIOTests.cs | 67 +++++++++++++++- CSUtilities/IO/StreamIO.cs | 108 ++++++++++++++++++-------- 2 files changed, 142 insertions(+), 33 deletions(-) diff --git a/CSUtilities.Tests/IO/StreamIOTests.cs b/CSUtilities.Tests/IO/StreamIOTests.cs index 27b98fd..b23c817 100644 --- a/CSUtilities.Tests/IO/StreamIOTests.cs +++ b/CSUtilities.Tests/IO/StreamIOTests.cs @@ -9,6 +9,42 @@ namespace CSUtilities.Tests.IO; public class StreamIOTests : IDisposable { + private sealed class PartialReadMemoryStream : MemoryStream + { + public PartialReadMemoryStream(byte[] data) : base(data) { } + + public override int Read(byte[] buffer, int offset, int count) + { + return base.Read(buffer, offset, Math.Min(count, 2)); + } + + public override Task ReadAsync(byte[] buffer, int offset, int count, System.Threading.CancellationToken cancellationToken) + { + return base.ReadAsync(buffer, offset, Math.Min(count, 2), cancellationToken); + } + } + + private sealed class NonSeekableStream : Stream + { + private readonly Stream _inner; + + public NonSeekableStream(byte[] data) + { + this._inner = new MemoryStream(data); + } + + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => throw new NotSupportedException(); + public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); } + public override void Flush() { } + public override int Read(byte[] buffer, int offset, int count) => this._inner.Read(buffer, offset, Math.Min(count, 2)); + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } + private readonly MemoryStream _memoryStream; private readonly StreamIO _streamIO; @@ -41,6 +77,35 @@ public void WriteAndReadBytes_WorksCorrectly() Assert.Equal(data, _streamIO.ReadBytes(data.Length)); } + [Fact] + public void ConstructorBuffersANonSeekablePartialReadStream() + { + byte[] data = { 1, 2, 3, 4, 5, 6 }; + using NonSeekableStream source = new NonSeekableStream(data); + using StreamIO stream = new StreamIO(source); + + Assert.True(stream.Stream.CanSeek); + Assert.Equal(data, stream.ReadBytes(data.Length)); + } + + [Fact] + public void ReadBytesCompletesPartialReads() + { + byte[] data = { 1, 2, 3, 4, 5, 6 }; + using StreamIO stream = new StreamIO(new PartialReadMemoryStream(data)); + + Assert.Equal(data, stream.ReadBytes(data.Length)); + } + + [Fact] + public async Task ReadBytesAsyncCompletesPartialReads() + { + byte[] data = { 1, 2, 3, 4, 5, 6 }; + using StreamIO stream = new StreamIO(new PartialReadMemoryStream(data)); + + Assert.Equal(data, await stream.ReadBytesAsync(data.Length)); + } + [Fact] public void WriteAndReadString_WorksCorrectly() { @@ -206,4 +271,4 @@ public void ReadBytes_ThrowsOnEndOfStream() { Assert.Throws(() => _streamIO.ReadBytes(1)); } -} \ No newline at end of file +} diff --git a/CSUtilities/IO/StreamIO.cs b/CSUtilities/IO/StreamIO.cs index fbf56b2..6e06a19 100644 --- a/CSUtilities/IO/StreamIO.cs +++ b/CSUtilities/IO/StreamIO.cs @@ -59,17 +59,36 @@ public StreamIO(string filename, FileMode mode, FileAccess access) /// public StreamIO(Stream stream, bool createCopy, bool resetPosition) { - long position = stream.Position; + if (stream == null) + { + throw new ArgumentNullException(nameof(stream)); + } - //Check if supports seeking + //Readers require seeking. A non-seekable input therefore has to be buffered from its + //current position; querying Position or Length first would itself throw. if (!stream.CanSeek || createCopy) { - stream.Position = 0; - //Create a copy of the stream to allow seeking - byte[] buffer = new byte[stream.Length]; - stream.Read(buffer, 0, buffer.Length); - _stream = (Stream)new MemoryStream(buffer); - stream.Position = position; + MemoryStream copy = new MemoryStream(); + if (stream.CanSeek) + { + long position = stream.Position; + try + { + stream.Position = 0; + stream.CopyTo(copy); + } + finally + { + stream.Position = position; + } + } + else + { + stream.CopyTo(copy); + } + + copy.Position = 0; + _stream = copy; } else { @@ -116,14 +135,15 @@ public byte[] GetBytes(int offset, int length) //Set the position to the begining this.Position = offset; - byte[] buffer = this.ReadBytes(length); - //if (this.m_stream.Read(buffer, offset, length) < length) - // throw new EndOfStreamException(); - - //Reset the position - this.Position = save; - - return buffer; + try + { + return this.ReadBytes(length); + } + finally + { + //A failed read must not leave this utility at the requested offset. + this.Position = save; + } } public async Task GetBytesAsync(int offset, int length) @@ -136,14 +156,14 @@ public async Task GetBytesAsync(int offset, int length) //Set the position to the begining this.Position = offset; - byte[] buffer = await this.ReadBytesAsync(length); - //if (this.m_stream.Read(buffer, offset, length) < length) - // throw new EndOfStreamException(); - - //Reset the position - this.Position = save; - - return buffer; + try + { + return await this.ReadBytesAsync(length); + } + finally + { + this.Position = save; + } } /// @@ -163,9 +183,15 @@ public byte LookByte() /// public byte[] LookBytes(int count) { - byte[] bs = this.ReadBytes(count); - this.Position -= count; - return bs; + long position = this.Position; + try + { + return this.ReadBytes(count); + } + finally + { + this.Position = position; + } } /// @@ -214,8 +240,17 @@ public virtual byte[] ReadBytes(int length) byte[] buffer = new byte[length]; - if (this._stream.Read(buffer, 0, length) < length) - throw new EndOfStreamException(); + int offset = 0; + while (offset < length) + { + int read = this._stream.Read(buffer, offset, length - offset); + if (read == 0) + { + throw new EndOfStreamException(); + } + + offset += read; + } return buffer; } @@ -227,8 +262,17 @@ public virtual async Task ReadBytesAsync(int length, CancellationToken c byte[] buffer = new byte[length]; - if (await this._stream.ReadAsync(buffer, 0, length, cancellationToken) < length) - throw new EndOfStreamException(); + int offset = 0; + while (offset < length) + { + int read = await this._stream.ReadAsync(buffer, offset, length - offset, cancellationToken); + if (read == 0) + { + throw new EndOfStreamException(); + } + + offset += read; + } return buffer; } @@ -520,4 +564,4 @@ public void Dispose() { _stream.Dispose(); } -} \ No newline at end of file +} From c07ed09353296a2f9a72ae8eae6c219391d6d831 Mon Sep 17 00:00:00 2001 From: MoreDwg Date: Mon, 7 Sep 2026 12:19:19 +0700 Subject: [PATCH 3/3] Make RadToDeg honour the absolute flag it already accepted The parameter was in the signature and never read, so every conversion was normalised. That silently destroys a full sweep: an angle pair of -PI to PI comes back as 180 and 180 - the same number twice, with the 360 degrees between them gone. ACadSharp writes hatch boundary arc and ellipse angles through this method, and a circular boundary is stored exactly that way. Measured on real drawings: twenty-five boundaries across four of them. AutoCAD 2027 opens the resulting file with no audit error at all and simply has no fill where the circle was - its extents come back as a single point. Co-Authored-By: Claude Opus 5 (1M context) --- CSMath/MathHelper.cs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/CSMath/MathHelper.cs b/CSMath/MathHelper.cs index 980e5d0..332d4d4 100644 --- a/CSMath/MathHelper.cs +++ b/CSMath/MathHelper.cs @@ -1,4 +1,4 @@ -using System; +using System; namespace CSMath; @@ -339,12 +339,21 @@ public static double NormalizeAngleRadians(double angle) /// Convert a value from radian to degree. /// /// Value in radians - /// Calculates the negative values in a 0-360 range. + /// + /// Normalises the result into a 0-360 range. Pass to keep the converted + /// value as it is, in the -360 to 360 range the caller gave it in. + /// /// The degree value. + /// + /// The parameter used to be accepted and ignored, so the result was always normalised. That + /// loses a full sweep: an angle pair of -PI to PI, which is how a circular hatch boundary is + /// stored, came back as 180 and 180 - the same number twice, with the 360 degrees between them + /// gone. Measured on real drawings, twenty-five boundaries in four of them. + /// public static double RadToDeg(double value, bool absolute = true) { var result = value * RadToDegFactor; - return NormalizeAngle(result); + return absolute ? NormalizeAngle(result) : result; } ///