Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wip/redo everything #69

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

KubaSzostak
Copy link
Member

There are two parts of library. NetTopologySuite.IO.Esri.Core and NetTopologySuite.IO.Esri. The former provides forward-only readers and writers for Esri shapefiles. It is vanilla .NET Standard 2.0 library without any dependencies. The latter is is full featured NTS library. It provides unified access to shapefile triplet (SHP, SHX, DBF) through wrapping Core classes.

Writing features to a shapefile

var features = new List<Feature>();
for (int i = 1; i < 5; i++)
{
    var attributes = new AttributesTable();
    attributes.Add("date", new DateTime(2000, 1, i + 1));
    attributes.Add("float", i * 0.1);
    attributes.Add("int", i);
    attributes.Add("logical", i % 2 == 0);
    attributes.Add("text", i.ToString("0.00"));

    var lineCoords = new List<CoordinateZ>();
    lineCoords.Add(new CoordinateZ(i, i + 1, i));
    lineCoords.Add(new CoordinateZ(i, i, i));
    lineCoords.Add(new CoordinateZ(i + 1, i, i));
    var line = new LineString(lineCoords.ToArray());

    var feature = new Feature(line, attributes);
    features.Add(feature);
}

features.SaveToShapefile(shpPath);

Reading features from a shapefile

foreach (var feature in ShapefileReader.ReadAll(shpPath))
{
    Console.WriteLine("Record ID: " + feature.Attributes["Id"]);
    foreach (var attrName in feature.Attributes.GetNames())
    {
        Console.WriteLine($"  {attrName}: {feature.Attributes[attrName]}");
    }
    Console.WriteLine($"  SHAPE: {feature.Geometry}");
}

Reading a SHP file geometries

foreach (var geom in ShapefileReader.ReadAllGeometries(shpPath))
{
    Console.WriteLine(geom);
}

Performance
The core part of the library was designed with performance in mind. There is a lot of other optimizations, to name a few of them:

  • Using structs over classes
    for storing ShpCoordinates.
  • Using the ShpShapeBuilder which under the hood is a buffer with smart resizing capabilities.
  • Using dedicated BinaryBuffer class which avoids file I/O operations
    by reading/writing a whole shape record data at once instead of reading/writing
    every single coordinate one by one. Again - without resource costly memory realocating.
  • The BinaryBuffer have also custom bit-converter functions with support
    for big-endian and little-endian byte order. It's implementation avoids realocating
    new byte[8] array again and again, any time you want to write single coordinate.
    This avoid bottleneck GC and much faster than
    BitConverter .

Encoding

The .NET Framework supports a large number of character encodings and code pages.
On the other hand, .NET Core only supports
limited list of encodings.
To retrieve an encoding that is present in the .NET Framework on the Windows
desktop but not in .NET Core, you need to do the following:

  1. Add to your project reference to to the System.Text.Encoding.CodePages.dll.
  2. Put the following line somewhere in your code:
    Encoding.RegisterProvider(CodePagesEncodingProvider.Instance);

@CLAassistant
Copy link

CLAassistant commented Feb 21, 2021

CLA assistant check
All committers have signed the CLA.

@FObermaier
Copy link
Member

@KubaSzostak , thanks for your valueable input, I assume your PR is ready for review?

@KubaSzostak
Copy link
Member Author

Yes. It's ready for review.

@kdethlefs
Copy link

Any update on this? It's been since Feb 2021.

@kdethlefs
Copy link

Still curious if we have any update on this? @FObermaier what the next step to get this merged in?

Copy link
Member

@FObermaier FObermaier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution, sorry it has taken so long.
If you don't feel like adopt my suggestions, I'll see when I can find some time to do that.

/// <summary>
/// Base class for reading a shapefile.
/// </summary>
public abstract class ShapefileReader : Shapefile, IEnumerable<Feature>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class needs a GeometryFactory instance member that is accessible by the derived classes.
Instead of instantiating geometries with their constructors (e.g. new Point(x, y, z, m) geometries should be instantiated using the appropriate GeometryFactory.Create... method.

}


private static CoordinateSequence CreateCoordinateSequence(int size, bool hasZ, bool hasM)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function needs a CoordinateSequenceFactory parameter that is to be used for creating coordinate sequences.
Using GeometryFactory.Default.CoordinateSequenceFactory is not encouraged.

/// <returns>Shapefile reader.</returns>
public static ShapefileReader Open(string shpPath, Encoding encoding = null)
{
shpPath = Path.ChangeExtension(shpPath, ".shp");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some logic to query SRID from accompanied *.prj file would be nice. Use it to set up GeometryFactory for ShapefileReaders. If not just set 0.

int srid = GetSRID(System.IO.Path.ChangeExtension(shpPath, "prj");
var factory = NtsGeometryServices.Instance.CreateGeometryFactory(srid);

}


public static void SetCoordinates(this CoordinateSequence sequence, int index, ShpCoordinates coords, bool hasZ, bool hasM)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method needs a PrecisionModel parameter to make x- and y-coordinates precise

            sequence.SetX(index, pm.MakePrecise(coords.X));
            sequence.SetY(index, pm.MakePrecise(coords.Y));

{
public interface IShapefileFeature : IFeature
{
long FeatureId { get; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why long, isn't it uint

@@ -5,8 +5,4 @@ indent_style = space
indent_size = 4
trim_trailing_whitespace = true
insert_final_newline = true
dotnet_style_predefined_type_for_locals_parameters_members = true:error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FObermaier it's ok to remove these lines?

@DGuidi
Copy link
Contributor

DGuidi commented Jan 21, 2022

not a big deal, but maybe we cah restore the original solution name NetTopologySuite.IO.Shapefile.sln instead of NetTopologySuite.IO.Esri.sln?

@DGuidi
Copy link
Contributor

DGuidi commented Jan 21, 2022

Thanks for the contribution. A totally minor issue: I see a lot of warnings and messages due to empty newline, whitespaces and unnecessary usings. some of then can be simply fixed using this default (I think) tool
image

@DGuidi
Copy link
Contributor

DGuidi commented Jan 25, 2022

@KubaSzostak Just to let you know that this test (from #80) fails also with "ESRI" lib

        /// <summary>
        /// <see href="https://github.com/NetTopologySuite/NetTopologySuite.IO.ShapeFile/issues/70"/>
        /// </summary>
        [Test]
        public void TestReadPolygonWithWrongShellOrientation()
        {
            /*
             * The shell_bad_ccw.shp contains a single polygon, with:
             *  - a shell CCW-oriented (like a hole from ESRI specs
             *  - a hole CW-oriented (like a shell from ESRI specs)
             */
            string filePath = Path.Combine(
                CommonHelpers.TestShapefilesDirectory,
                "shell_bad_ccw.shp");
            Assert.That(File.Exists(filePath), Is.True);
            var geoms = ShapefileReader.ReadAllGeometries(filePath);
            Assert.That(geoms, Is.Not.Null);
            Assert.That(geoms.Length, Is.EqualTo(1));
            var geom = geoms[0];
            Assert.That(geom, Is.Not.Null);
            Assert.That(geom.IsValid, Is.True);
            Assert.That(geom.NumGeometries, Is.EqualTo(1));
            Assert.That(geom, Is.InstanceOf<Polygon>());
            var poly = (Polygon)geom.GetGeometryN(0);
            Assert.That(poly.Shell, Is.Not.Null);
            Assert.That(poly.Holes, Is.Not.Null);
            Assert.That(poly.Holes.Length, Is.EqualTo(1));
        }

see also attached data shell_bad_ccw.zip.

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.

None yet

6 participants