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

Fixes incorrect result for polygon in left-handed CRS in GeometryValidator #1034

Merged
merged 2 commits into from
Mar 20, 2020
Merged

Fixes incorrect result for polygon in left-handed CRS in GeometryValidator #1034

merged 2 commits into from
Mar 20, 2020

Conversation

carlospzurita
Copy link
Contributor

@carlospzurita carlospzurita commented Feb 10, 2020

Added fix for #886 methods hasLeftHandedSrs, isInterior and isExterior in InteriorRingOriention and ExteriorRingOrientation respectively. The solution implemented checks the orientation of the axis for the declared Polygon patch to decide the left-handness, and uses this to check if the ring is exterior or interior.

…erior in ExteriorRingOrientation and InteriorRingOrientation from validation package
@carlospzurita carlospzurita changed the title Fixes issue #886 Fixes issue https://github.com/deegree/deegree3/issues/886 Feb 10, 2020
@carlospzurita carlospzurita changed the title Fixes issue https://github.com/deegree/deegree3/issues/886 Fixes issue #886 Feb 10, 2020
@carlospzurita
Copy link
Contributor Author

We are seeing some errors related to the build process in Travis and Jenkins. We have looking into the logs and have seen some errors that also appeared in other PR. Is there any issue with the continuous integration pipeline?

@copierrj
Copy link
Member

Travis is currently broken (= fails even when local builds and Jenkins do succeed). This happens often and we are currently unsure what to do about this.

Unfortunately there is also a known regression (#1033) in master. We are currently working on that.

@copierrj
Copy link
Member

copierrj commented Feb 21, 2020

Regarding both hasLeftHandedSrs() methods: these two are almost identical. Please create an appropriate helper method in AbstractGeometryValidationEvent in order to avoid too much code duplication.

In order to safeguard against future regressions we would like some unittest to be included.

…teriorRingOrientation and ExteriorRingOrientation classes.

Added test for ring orientation using left-handed and right-handed CRS.
@carlospzurita
Copy link
Contributor Author

We added the modifications indicated, both in the AbstractGeometryValidationEvent and in the GeometryValidatorTest.

The test for validation is not fully complete, given that in order to maintain the compatibility with other clients, we did not include the left-handed and right-handed in the validation process for geometry orientation.

@tfr42 tfr42 added the bug error issue and bug (fix) label Mar 6, 2020
@carlospzurita
Copy link
Contributor Author

We see that there are some errors in the CI processes, but we are not sure if this is related to our own code uploaded, the tests provided or other unrelated issue. Can you provide us with some feedback about this? Thank you

@carlospzurita
Copy link
Contributor Author

Would it be useful for the integration of the pull request if we attend the TMC meeting on 20/03/2020? We can provide any feedback that you need for the solution implemented or the tests included.

@copierrj
Copy link
Member

We've looked at the latest changes you made and we are happy with the result.

@MarcoMinghini
Copy link

@tfr42 do you know when the next deegree version including this fix will be released?

@kalxas
Copy link

kalxas commented Oct 7, 2021

Has this fix been released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug error issue and bug (fix)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants