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

Harmonized dimension estimation in OracleGeometryWriter #14

Closed
wants to merge 5 commits into from

Conversation

hlaebe
Copy link
Contributor

@hlaebe hlaebe commented Aug 25, 2023

There was an inconsistency in getting the correct dimension from th e geomtry to be transformed to SdoGeometry. One was based on the Coordinate property and one on the coordinatesequence dimension. It seems that internally all coordinates are 3D now, even for 2D (Z is NaN then). This lead to incorrect SdoGeometry having ElemInfo telling 2D and an ordinate array containing 3 ordinates (including NaN).

The pull request fixes this by defining the dimension prior to conversion based on coordinate property and storing it in private variable in the Writer instance.

fixes #11

@hlaebe
Copy link
Contributor Author

hlaebe commented Mar 6, 2024

@airbreather @FObermaier Any reasons why this PR doesn't make it to production ? BR, Holger

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.

You are right that the computation of the geometry dimension is not consistent.
A better fix would be to compute the dimension based on CoordinateSequence, too.

Sth along these lines:

private class SpatialDimensionDeterminator : IEntireCoordinateSequenceFilter {
    public void Filter(CoordinateSequence sequence) {
        NumSpatialDimensions = sequence.Dimension - sequence.Measures;
        Done = true;
    }
    public int NumSpatialDimensions { get; private set; } = 2;
    public bool Done { get; private set; }
    public bool GeometryChanged => false;
}

private static int Dimension(Geometry geom) {
    var sdd = new SpatialDimensionDeterminator();
    geom.Apply(sdd);
    return sdd.NumSpatialDimensions
}

@@ -40,6 +41,8 @@ public SdoGeometry Write(Geometry geometry)
return null;
}

this.dimension = Dimension(geometry);
Copy link
Member

Choose a reason for hiding this comment

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

I don't like this as it messes things up in a re-entrant scenario. Make dimension a local variable and pass it to the private specific Write functions.

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.

2 participants