NetTopologySuite / NetTopologySuite.IO.ShapeFile

The ShapeFile IO module for NTS.
35 stars 25 forks source link

Max "M" value is always set to double.PositiveInfinity #74

Closed jarobbins closed 2 years ago

jarobbins commented 3 years ago

In ShapeHandler.WriteZM:

double minM = double.PositiveInfinity; double maxM = double.PositiveInfinity; foreach (double m in mValues) { if (minM > m) minM = m; if (maxM < m) maxM = m; }

file.Write(minM); file.Write(maxM); for (int i = 0; i < count; i++) file.Write(mValues[i]);

The initial maxM value should be double.NegativeInfinity.

I'll have a pull request available ASAP.

DGuidi commented 3 years ago

can you push an unit test that shows the fixed behavior? in any case this looks like a bug. anyway the error lines in the bounding box read, that I think is not used to do anything useful.

DGuidi commented 3 years ago

not the best test in the world, tomorrow I will try to push to the PR, if possible...

[NtsIssueNumber(74)]
class Issue74Fixture
{
    [Test, Description("Max 'M' value is always set to double.PositiveInfinity")]
    public void Test()
    {
        var instance = NtsGeometryServices.Instance;
        var factory = instance.CreateGeometryFactory();
        var p1 = factory.CreatePoint(new CoordinateZM(-1, -1, -1, -1));
        var p2 = factory.CreatePoint(new CoordinateZM(2, 2, 2, 2));
        var points = factory.CreateMultiPoint(new[] { p1, p2 });
        var handler = new MultiPointHandler(ShapeGeometryType.MultiPointZM);
        byte[] bytes;
        using (var stream = new MemoryStream())
        {
            using var writer = new BinaryWriter(stream);
            handler.Write(points, writer, factory);
            bytes = stream.ToArray();
        }

        using var reader = new BinaryReader(new MemoryStream(bytes));
        Assert.AreEqual((int)ShapeGeometryType.MultiPointZM, reader.ReadInt32());
        reader.ReadDouble(); // Ignore MinX
        reader.ReadDouble(); // Ignore MinY
        reader.ReadDouble(); // Ignore MaxX
        reader.ReadDouble(); // Ignore MaxX
        Assert.AreEqual(points.NumGeometries, reader.ReadInt32());
        reader.ReadDouble(); // Ignore p1.X
        reader.ReadDouble(); // Ignore p1.Y
        reader.ReadDouble(); // Ignore p2.X
        reader.ReadDouble(); // Ignore p2.Y
        Assert.AreEqual(p1.Z, reader.ReadDouble()); // MinZ
        Assert.AreEqual(p2.Z, reader.ReadDouble()); // MaxZ
        reader.ReadDouble(); // Ignore p1.Z
        reader.ReadDouble(); // Ignore p2.Z
        Assert.AreEqual(p1.M, reader.ReadDouble()); // MinM
        Assert.AreEqual(p2.M, reader.ReadDouble()); // MaxM
        reader.ReadDouble(); // Ignore p1.M
        reader.ReadDouble(); // Ignore p2.M
    }
}
DGuidi commented 3 years ago

@jarobbins I'm unable to push directly to your repository (seems reasonable, probably I should fork your fork and ask for a PR to your PR code...), can you add the attached unit test to the solution? thx

using NetTopologySuite.Geometries;
using NetTopologySuite.IO.Handlers;
using NUnit.Framework;
using System.IO;

namespace NetTopologySuite.IO.ShapeFile.Test
{
    [NtsIssueNumber(74)]
    public class Issue74Fixture
    {
        [Test, Description("Max 'M' value is always set to double.PositiveInfinity")]
        public void ShapeHandler_correctly_writes_BBOX_info()
        {
            var factory = GeometryFactory.Default;
            var pMin = factory.CreatePoint(new CoordinateZM(-1, -1, -1, -1));
            var pMax = factory.CreatePoint(new CoordinateZM(2, 2, 2, 2));
            var coll = factory.CreateMultiPoint(new[] { pMin, pMax });
            var handler = new MultiPointHandler(ShapeGeometryType.MultiPointZM);
            byte[] bytes;
            using (var stream = new MemoryStream())
            {
                using var writer = new BinaryWriter(stream);
                handler.Write(coll, writer, factory);
                bytes = stream.ToArray();
            }

            using var reader = new BinaryReader(new MemoryStream(bytes));
            Assert.AreEqual((int)ShapeGeometryType.MultiPointZM, reader.ReadInt32());
            Assert.AreEqual(pMin.X, reader.ReadDouble()); // MinX
            Assert.AreEqual(pMin.Y, reader.ReadDouble()); // MinY
            Assert.AreEqual(pMax.X, reader.ReadDouble()); // MaxX
            Assert.AreEqual(pMax.Y, reader.ReadDouble()); // MaxY
            Assert.AreEqual(coll.NumGeometries, reader.ReadInt32());
            for (int i = 0; i < 4; i++)
            {
                // Skip XY values
                reader.ReadDouble();
            }
            Assert.AreEqual(pMin.Z, reader.ReadDouble()); // MinZ
            Assert.AreEqual(pMax.Z, reader.ReadDouble()); // MaxZ
            for (int i = 0; i < 2; i++)
            {
                // Skip Z values
                reader.ReadDouble();
            }
            Assert.AreEqual(pMin.M, reader.ReadDouble()); // MinM
            Assert.AreEqual(pMax.M, reader.ReadDouble()); // MaxM
            for (int i = 0; i < 2; i++)
            {
                // Skip M values
                reader.ReadDouble();
            }
        }
    }
}

Issue74Fixture.zip

DGuidi commented 2 years ago

@jarobbins can you add also the unit test to the repository?

jarobbins commented 2 years ago

@DGuidi I missed your messages back in July. I added your test & pushed to the PR branch.