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

Ring orientation fix with new deegree release in GmlGeoX v1 #96

Closed
carlospzurita opened this issue Aug 21, 2020 · 3 comments
Closed

Ring orientation fix with new deegree release in GmlGeoX v1 #96

carlospzurita opened this issue Aug 21, 2020 · 3 comments
Labels
EIP-approved EIP approved by the Steering Group Impl. EIP has been implemented and is ready for the next release. Doc might be incomplete (temp. label)
Milestone

Comments

@carlospzurita
Copy link

carlospzurita commented Aug 21, 2020

Background and Motivation:

In the PR deegree/deegree3#1034 on the deegree repository, two auxiliar methods have been added to check the ring orientation: insInterior and isExterior, taking into account if the patch being validated is clockwise or not. This has been included on the release 3.4.12 that was integrated in the ETF in PR etf-validator/legacy-etf-gmlgeox#25.

Proposed change

The next step would be to include this methods on the etf-gmlgeox module (version 1), in the class nl.vrom.roo.validator.core.dom4j.handlers.GMLValidationEventHandler. The proposal is to modify the current methods exteriorRingOrientation and interiorRingOrientation to make use of these methods, instead of checking directly if the geometry is clockwise and relying on deegree for this check

boolean exteriorRingOrientation( ExteriorRingOrientation evt ) {
    PolygonPatch patch = evt.getPatch();
    boolean isExterior = evt.isExterior();
  
    if(!isExterior) {
        String errMessage = ValidatorMessageBundle.getMessage(
                "validator.core.validation.geometry.exteriorRingCW",
                this.currentGmlId,
                this.currentOnderdeelName,
                ValidationUtil.getAffectedCoordinates(patch.getExteriorRing(), 60));
        this.validatorContext.addError(errMessage);
    }

    return isExterior;
}

boolean interiorRingOrientation( InteriorRingOrientation evt ) {
    PolygonPatch patch = evt.getPatch();
    boolean isInterior = evt.isInterior();

    if(!isInterior) {
        int idx = evt.getRindIdx();
        String errMessage = ValidatorMessageBundle.getMessage(
                "validator.core.validation.geometry.interiorRingCCW",
                this.currentGmlId,
                this.currentOnderdeelName,
                ValidationUtil.getAffectedCoordinates(patch.getInteriorRings().get(idx), 60),
                idx);
        this.validatorContext.addError(errMessage);
    }

    return isInterior;
}

Alternatives

Funding

This development is funded by JRC

Additional information

This development is intended for the currents INSPIRE datasets ETS that use BaseX. Given that the module is moving past this in the new ETF release, it was agreed that this will be kept on a separate "legacy" version of etf-gmlgeox

@MarcoMinghini MarcoMinghini added the EIP-draft Announcement. Will not be discussed further as long as the necessary information is available. label Sep 7, 2020
@jonherrmann
Copy link
Contributor

jonherrmann commented Sep 9, 2020

Please open a Pull Request. EIPs are not the right place to review code.

At the moment we are not using GmlGeoX version 1 and I will not run any manual tests with it. So please make sure that your PR code is covered by unit tests.

@cportele cportele added EIP-approved EIP approved by the Steering Group and removed EIP-draft Announcement. Will not be discussed further as long as the necessary information is available. labels Dec 1, 2020
@MarcoMinghini MarcoMinghini changed the title Ring orientation fix with new deegree release Ring orientation fix with new deegree release in GmlGeoX v1 Dec 8, 2020
@jonherrmann jonherrmann added the ? Unknown status or information missing in the EIP (temp. label) label May 23, 2022
@jonherrmann
Copy link
Contributor

@jonherrmann jonherrmann added Impl. EIP has been implemented and is ready for the next release. Doc might be incomplete (temp. label) and removed ? Unknown status or information missing in the EIP (temp. label) labels May 24, 2022
@jonherrmann
Copy link
Contributor

Implemented in Version 2.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EIP-approved EIP approved by the Steering Group Impl. EIP has been implemented and is ready for the next release. Doc might be incomplete (temp. label)
Projects
None yet
Development

No branches or pull requests

4 participants