[GitHub] [lucene-solr] nknize commented on issue #608: LUCENE-8620: LatLonShape contains

[GitBox <gi...@apache.org>]

nknize commented on issue #608: LUCENE-8620: LatLonShape contains
URL: https://github.com/apache/lucene-solr/pull/608#issuecomment-476690169
 
 
   This is coming along nicely and I like the overall approach.  A few general thoughts:
   
   1. Would it make sense to split this PR into a couple separate issues? Maybe create a PR for the Tessellator logic that labels the Polygon edges along with the changes to `LatLonShape.encodeTriangle`? I think that will help make the changes more incremental and make it easier to review?
   2. I'd also like to make sure we get the `componentRelateTriangle` and `componentRelate` logic tight in [LUCENE-8736](https://issues.apache.org/jira/browse/LUCENE-8736) first, and probably the Polygon quantization issues corrected, before fully merging this PR. IMHO it makes sense to ensure the polygon relation logic is fully sound before adding all of the `CONTAINS` changes?

----------------------------------------------------------------
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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org