Thanks for the detailed review, Jason. As a matter of fact, I made some
more commits to address the issues you noticed:
- The check for node equality in insertExact (was insertNew) remains, since
now the findBestMatch logic is optimized out if the tolerance is 0
- short-circuiting boolean operators are now used to short-circuit the
checks on whether to increment an existing node
Hopefully this can get ported to NTS soon so you can use it.
On Wed, Jul 15, 2015 at 6:59 AM, Pepper, Jason <[email protected]> wrote:
> I am actually using the .Net port of the library, so I can’t test this.
>
>
>
> But there are a couple of changes I would recommend. The test to see if
> the point is already equal to a node (KdTree.java lines 211 - 221) can be
> removed from KdNode.insertNew since you would have already tested that with
> findBestMatchNode. Also, you can make lines 183 and 185 into “else if” to
> short circuit the logic if one of them is true. Other than that, it looks
> like it will work fine. If you really want to optimize it, a single
> traversal through the tree would be the way to go.
>
>
>
> *From:* Martin Davis [mailto:[email protected]]
> *Sent:* Tuesday, July 14, 2015 7:30 PM
> *To:* Pepper, Jason
> *Cc:* [email protected]
> *Subject:* Re: [Jts-topo-suite-user] KdTree Matching Bug
>
>
>
> A fix for this is now in SVN:
>
>
>
> https://sourceforge.net/p/jts-topo-suite/code/1002/
>
>
>
> Would be great if you can try this out and confirm it works in your
> situation. Feedback on any performance degradation would be good too.
>
>
>
> On Mon, Jul 6, 2015 at 11:36 AM, Pepper, Jason <[email protected]>
> wrote:
>
> The kd-tree does not correctly find existing nodes. It ignores branches
> that may contain matches within the tree’s threshold.
>
>
>
> To demonstrate this, create a kd-tree with a threshold of 1.0. Then add
> the following points (0.0, 0.0), (0.1, 1.0), and (-0.1, 1.0). The third
> point should be matched to the second point. However, the code ignores the
> right child of the root node and does not find the match.
>
>
>
> Jason Pepper
>
>
>
>
> ------------------------------
>
> *TERRALIGN CONFIDENTIAL:* This email and its attachments contain
> confidential and proprietary information of The TerrAlign Group, Inc..
> Disclosure is strictly prohibited.
>
>
>
>
>
> ------------------------------------------------------------------------------
> Don't Limit Your Business. Reach for the Cloud.
> GigeNET's Cloud Solutions provide you with the tools and support that
> you need to offload your IT needs and focus on growing your business.
> Configured For All Businesses. Start Your Cloud Today.
> https://www.gigenetcloud.com/
> _______________________________________________
> Jts-topo-suite-user mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/jts-topo-suite-user
>
>
>
> ------------------------------
> *TERRALIGN CONFIDENTIAL:* This email and its attachments contain
> confidential and proprietary information of The TerrAlign Group, Inc..
> Disclosure is strictly prohibited.
>
>
------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
Jts-topo-suite-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/jts-topo-suite-user
