-
Notifications
You must be signed in to change notification settings - Fork 25
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
base: develop
Are you sure you want to change the base?
Wip/redo everything #69
Conversation
…ring instead of MultiPolygon/MultiLineString.
@KubaSzostak , thanks for your valueable input, I assume your PR is ready for review? |
Yes. It's ready for review. |
Any update on this? It's been since Feb 2021. |
Still curious if we have any update on this? @FObermaier what the next step to get this merged in? |
There was a problem hiding this 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> |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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"); |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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; } |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
not a big deal, but maybe we cah restore the original solution name |
@KubaSzostak Just to let you know that this test (from #80) fails also with "ESRI" lib
see also attached data shell_bad_ccw.zip. |
There are two parts of library.
NetTopologySuite.IO.Esri.Core
andNetTopologySuite.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
Reading features from a shapefile
Reading a SHP file geometries
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:
for storing
ShpCoordinates
.ShpShapeBuilder
which under the hood is a buffer with smart resizing capabilities.BinaryBuffer
class which avoids file I/O operationsby 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.
BinaryBuffer
have also custom bit-converter functions with supportfor 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:
Encoding.RegisterProvider(CodePagesEncodingProvider.Instance);