You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2020/02/26 11:08:33 UTC
[GitHub] [lucene-solr] iverase opened a new pull request #1290: LUCENE-9251:
Filter equal edges with different value on isEdgeFromPolygon
iverase opened a new pull request #1290: LUCENE-9251: Filter equal edges with different value on isEdgeFromPolygon
URL: https://github.com/apache/lucene-solr/pull/1290
See https://issues.apache.org/jira/browse/LUCENE-9251
Unfortunately, I haven't been able to produce a meaningful unit test as I can only reproduce the issue with the complex polygon provided by the user. Still I think the change make sense.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] iverase commented on a change in pull request #1290:
LUCENE-9251: Filter equal edges with different value on isEdgeFromPolygon
Posted by GitBox <gi...@apache.org>.
iverase commented on a change in pull request #1290: LUCENE-9251: Filter equal edges with different value on isEdgeFromPolygon
URL: https://github.com/apache/lucene-solr/pull/1290#discussion_r386049672
##########
File path: lucene/core/src/java/org/apache/lucene/geo/Tessellator.java
##########
@@ -915,13 +914,14 @@ private static final Node filterPoints(final Node start, Node end) {
continueIteration = false;
nextNode = node.next;
prevNode = node.previous;
- //We can filter points when they are the same, if not and they are co-linear we can only
- //remove it if both edges have the same value in .isNextEdgeFromPolygon
- if (isVertexEquals(node, nextNode) ||
- (prevNode.isNextEdgeFromPolygon == node.isNextEdgeFromPolygon &&
+ // we can filter points when:
+ if (isVertexEquals(node, nextNode) || // 1. they are the same,
+ // isVertexEquals(prevNode, nextNode) || // 2.- each one starts and ends in each other
Review comment:
ups... that was a left over while testing
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] nknize commented on a change in pull request #1290:
LUCENE-9251: Filter equal edges with different value on isEdgeFromPolygon
Posted by GitBox <gi...@apache.org>.
nknize commented on a change in pull request #1290: LUCENE-9251: Filter equal edges with different value on isEdgeFromPolygon
URL: https://github.com/apache/lucene-solr/pull/1290#discussion_r386674661
##########
File path: lucene/core/src/test/org/apache/lucene/geo/TestTessellator.java
##########
@@ -561,6 +561,18 @@ public void testComplexPolygon39() throws Exception {
checkPolygon(wkt);
}
+ @Nightly
+ public void testComplexPolygon40() throws Exception {
+ String wkt = GeoTestUtil.readShape("lucene-9251.wkt.gz");
Review comment:
nice!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] iverase merged pull request #1290: LUCENE-9251:
Filter equal edges with different value on isEdgeFromPolygon
Posted by GitBox <gi...@apache.org>.
iverase merged pull request #1290: LUCENE-9251: Filter equal edges with different value on isEdgeFromPolygon
URL: https://github.com/apache/lucene-solr/pull/1290
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] nknize commented on a change in pull request #1290:
LUCENE-9251: Filter equal edges with different value on isEdgeFromPolygon
Posted by GitBox <gi...@apache.org>.
nknize commented on a change in pull request #1290: LUCENE-9251: Filter equal edges with different value on isEdgeFromPolygon
URL: https://github.com/apache/lucene-solr/pull/1290#discussion_r385902441
##########
File path: lucene/core/src/java/org/apache/lucene/geo/Tessellator.java
##########
@@ -915,13 +914,14 @@ private static final Node filterPoints(final Node start, Node end) {
continueIteration = false;
nextNode = node.next;
prevNode = node.previous;
- //We can filter points when they are the same, if not and they are co-linear we can only
- //remove it if both edges have the same value in .isNextEdgeFromPolygon
- if (isVertexEquals(node, nextNode) ||
- (prevNode.isNextEdgeFromPolygon == node.isNextEdgeFromPolygon &&
+ // we can filter points when:
+ if (isVertexEquals(node, nextNode) || // 1. they are the same,
+ // isVertexEquals(prevNode, nextNode) || // 2.- each one starts and ends in each other
Review comment:
nit: remove comment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] iverase commented on issue #1290: LUCENE-9251: Filter
equal edges with different value on isEdgeFromPolygon
Posted by GitBox <gi...@apache.org>.
iverase commented on issue #1290: LUCENE-9251: Filter equal edges with different value on isEdgeFromPolygon
URL: https://github.com/apache/lucene-solr/pull/1290#issuecomment-593473631
test added
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] iverase commented on issue #1290: LUCENE-9251: Filter
equal edges with different value on isEdgeFromPolygon
Posted by GitBox <gi...@apache.org>.
iverase commented on issue #1290: LUCENE-9251: Filter equal edges with different value on isEdgeFromPolygon
URL: https://github.com/apache/lucene-solr/pull/1290#issuecomment-592985468
I agree but the issue is that the polygon is too big to add it to the unit test and there is no really framework to read it from a file. My suggestion is to push this change as it is and open a Lucene issue to be able to test this kind go big polygons. Probably adding it to the test framework? wdyt?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org