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

GeometryValidator-Different results than OGC Cite tests for left-handed CRS #1480

Closed
jenriquesoriano opened this issue Mar 28, 2023 · 5 comments
Labels
question user question TMC discussion to be discussed by technical management committee members

Comments

@jenriquesoriano
Copy link

Dear deegree community,

we have noticed an issue with the validation of the geometry's orientation results provided by deegree. This is the same issue to which we have contributed some years ago. deegree is reused by the open source ETF validation framework, which is in turn used by the INSPIRE Reference Validator and the latter is the source application where we spotted the issue.

Test results for geometry orientation using deegree and the OGC TEAM Engine are different for the same geometry and declared CRS.
After some investigation, it seems that this issue is related with the coordinates treatment, in particular the algorithm that decides if a CRS is left-handed or right-handed and how to deal with the coordinates.

For this CRS: http://www.opengis.net/def/crs/EPSG/0/3035, the obtained results are as follows:

imagen

We have tested it in our own application, but we have created this piece of code to help reproduce the issue (thanks to the source code in the initial comment.

import org.deegree.cs.coordinatesystems.ICRS;
import org.deegree.cs.persistence.CRSManager;
import org.deegree.geometry.Geometry;
import org.deegree.geometry.GeometryFactory;
import org.deegree.geometry.primitive.Ring;
import org.deegree.geometry.standard.points.PackedPoints;
import org.deegree.geometry.validation.GeometryValidationEventHandler;
import org.deegree.geometry.validation.GeometryValidator;
import org.deegree.geometry.validation.event.ExteriorRingOrientation;
import org.deegree.geometry.validation.event.GeometryValidationEvent;
import org.deegree.geometry.validation.event.InteriorRingOrientation;

import java.io.File;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class GeometryValidatorTest implements GeometryValidationEventHandler {

    private final List<String> messages = new ArrayList<>();

    public static void main(String[] args) {

        GeometryFactory geomFactory = new GeometryFactory();

        /*
         * Create a geometry with a left-handed CRS. Coordinates are given as
         * pairs of lat/long values.
         */
        CRSManager crsManager =new CRSManager();
        crsManager.init(new File("/home/manuelaromero/Inspire/IJ/untitled1/src/resources/srsconfig/"));

        ICRS crs = CRSManager.getCRSRef("http://www.opengis.net/def/crs/EPSG/0/3035");
        Ring exterior = geomFactory.createLinearRing("lr_ext", crs,
            //clockwise
            new PackedPoints(crs, new double[] { 2180200, 700120, 2180220, 700120, 2180220, 700100,
                2180200, 700100, 2180200, 700120 }, 2));
            //anti
            //new PackedPoints(crs, new double[] { 0, 120, 0, 100, 20, 100,
            //   20, 120, 0, 120 }, 2));
        Ring interior = geomFactory.createLinearRing("lr_int", crs,
            //anti
            new PackedPoints(crs,
                new double[] { 218005, 700105, 218010, 700105, 218010, 700110, 218005, 700110, 218005, 700105 },
                2));
        //clock
                //new double[] { 5, 105, 5, 110, 10, 110, 10, 105, 5, 105 },
                //2));

        /*
         * NOTE: Using 'epsg:4326' as CRS would result in the right-handed
         * version of EPSG 4326, also called not axis aware on
         * https://github.com/deegree/deegree3/wiki/Axis-order-handling. The
         * coordinates of the following rings are given as pairs of long/lat
         * values. The orientation of the boundaries of the resulting polygon
         * would be valid.
         */
        // ICRS crs = CRSManager.getCRSRef("EPSG:4326");
        // Ring exterior = geomFactory.createLinearRing("lr_ext", crs,
        // new PackedPoints(crs,
        // new double[] { 120, 0, 100, 10, -100, 10, -120, 10, -120, -10, 120, 0
        // },
        // 2));
        // Ring interior = geomFactory.createLinearRing("lr_int", crs,
        // new PackedPoints(crs,
        // new double[] { 100, 0, 0, -5, -100, 0, 0, 10, 100, 0 }, 2));

        Geometry geom = geomFactory.createPolygon("testgeometry", crs, exterior,
            new ArrayList<Ring>(Collections.singletonList(interior)));

        GeometryValidatorTest eventHandler = new GeometryValidatorTest();

        GeometryValidator validator = new GeometryValidator(eventHandler);

        validator.validateGeometry(geom);
        if (eventHandler.messages.isEmpty()) {
            System.out.println("Orientation is valid.");
        } else {
            System.out.println("Orientation is invalid:");
            for (String msg : eventHandler.messages) {
                System.out.println(msg);
            }
        }
    }

    protected boolean exteriorRingOrientation(ExteriorRingOrientation evt) {

        System.out.println(evt.isClockwise());
        if (evt.isExterior()) {
            this.messages.add("Exterior ring valid");
            return true;
        } else {
            this.messages.add("Exterior ring not valid");
            return false;
        }
    }

    protected boolean interiorRingOrientation(InteriorRingOrientation evt) {
        System.out.println(evt.isClockwise());

        if (evt.isInterior()) {
            this.messages.add("Interior ring valid");
            return false;
        } else {
            this.messages.add("Interior ring not valid");
            return true;
        }
    }

    @Override
    public boolean fireEvent(GeometryValidationEvent event) {
        if (event instanceof ExteriorRingOrientation) {
            return exteriorRingOrientation((ExteriorRingOrientation) event);
        } else if (event instanceof InteriorRingOrientation) {
            return interiorRingOrientation((InteriorRingOrientation) event);
        } else {
            return false;
        }
    }
}

We have investigated the issue and it seems to us that it is related to the ExteriorRingOrientation.java class or to how a CRS is considered right-handed or left-handed in the AbstractGeometryValidationEvent.java class.

In the ExteriorRingOrientation.java class the method that would be related to this matter would be is Exterior():

public boolean isExterior() {
        boolean isExterior = !isClockwise;

        if ( isLeftHanded( patch.getExteriorRing().getCoordinateSystem() ) ) {
            isExterior = !isExterior;
        }

        return isExterior;
    }

Or in the AbstractGeometryValidationEvent.java class the method would be isLeftHand():

protected boolean isLeftHanded( ICRS crs ) {
        // get number of dimensions (it should be 2)
        if ( crs.getDimension() == 2 ) {
            int axis1 = crs.getAxis()[0].getOrientation();
            int axis2 = crs.getAxis()[1].getOrientation();

            // check if CRS is left handed
            if ( axis1 == Axis.AO_EAST || axis1 == Axis.AO_WEST ) {
                if ( axis1 == Axis.AO_EAST && ( axis2 == Axis.AO_SOUTH || axis2 == Axis.AO_DOWN ) ) {
                    return true;
                }
                else if ( axis1 == Axis.AO_WEST && ( axis2 == Axis.AO_NORTH || axis2 == Axis.AO_UP ) ) {
                    return true;
                }
            }
            else {
                if ( ( axis1 == Axis.AO_SOUTH || axis1 == Axis.AO_DOWN ) && axis2 == Axis.AO_WEST ) {
                    return true;
                }
                else if ( ( axis1 == Axis.AO_NORTH || axis1 == Axis.AO_UP ) && axis2 == Axis.AO_EAST ) {
                    return true;
                }
            }
        }

        // return false in any other case
        return false;
    }

It may also be that the order of the axis in certain systems when reading them is not the same as the one described in the CRS, causing a different treatment of their coordinates than the one that should be used.

Thus, as the results provided by deegree are different for this left-handed CRSs compared with the OGC TEAM Engine, what would be the correct output so they would both be the same? @tfr42 @jechterhoff

Thanks for your attention.

Best,

@fernandocuevasgdtel
Copy link

As proposals to solve this issue, we may suggest different approaches to obtain the same results than CITE, considering:

a) Revise the source code at ExteriorRingOrientation.java class by modifying the condition applied to left-handed CRSs

b) Revise the source code at AbstractGeometryValidationEvent.java class, modifying the method isLeftHand() in order to apply special conditions to adapt it to the behaviour of CITE.

c) Revise the definition of the CRS in order to adapt it to left-handed/right-handed definition of of CITE.

@MarcoMinghini
Copy link

The problem reported is becoming quite critical for the INSPIRE community and we need to implement a solution for it in the INSPIRE Reference Validator, which is in turn based on the ETF validation tool, which uses deegree.

I would like to clarify that we can take care of the pull request, but we would need some feedback from the deegree community/PSC on which approach (among the proposed ones, or a new one) would be best.

Thank you.

@tfr42 tfr42 added question user question TMC discussion to be discussed by technical management committee members labels Apr 17, 2023
@tfr42
Copy link
Member

tfr42 commented Apr 17, 2023

@jenriquesoriano Thank you for sharing your findings. You mentioned that OGC TEAM Engine is reporting the correct results as expected for EPSG:3035. Do I read the entries in the column "Team Engine" correctly? Are you aware that similar geometries in other left-handed CRS are also affected?

To fix this issue I do propose to prepare a pull request wich contains:

And to discuss the proposed approaches I invite you all @MarcoMinghini @jenriquesoriano @fernandocuevasgdtel to one of our next TMC meetings: next meeting is Wednesday 2023-04-19 at 1700-1800 CEST. See https://www.deegree.org/community/ and https://github.com/deegree/deegree3/blob/main/CONTRIB.md#irc-channel

Any contribition is welcome.

@jenriquesoriano
Copy link
Author

@tfr42 . Thanks for your feedback. We will attend to the next TMC meeting on Wednesday 2023-04-19 at 1700-1800 CEST.
We will progress as you request so we ellaborate the case and propose a possible solution.

@tfr42
Copy link
Member

tfr42 commented Oct 17, 2023

Fixed with #1556 and #1489.

@tfr42 tfr42 closed this as completed Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question user question TMC discussion to be discussed by technical management committee members
Projects
None yet
Development

No branches or pull requests

4 participants