<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 18, 2023 at 5:24 PM Daniel Baston <<a href="mailto:dbaston@gmail.com">dbaston@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>I noticed that the OverlayNG coverage union returns an empty polygon if the inputs do not form a coverage, whereas the older algorithm produces an error. I think it would be a good idea to apply an area post-check to the NG version so that we retain the error. </div></div></blockquote><div><br></div><div>Seems reasonable.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>I have also noticed cases (TIGER county boundaries) where the NG algorithm takes about 2x as long as the old algorithm but performs better than the old algorithm if the inputs are pre-sorted (as they are in the old algorithm). Maybe the same sorting should be added to the NG version.</div></div></blockquote><div><br></div><div>Any theories about why sorting improves the performance?   </div><div><br></div><div>Perhaps it's because of the BoundaryChainNoder, which adds segments to an std::unordered_set<Segment, Segment::HashCode>.  Segments are removed from the set when duplicates occur.  If the segments inserted have spatial coherency, that would tend to reduce the dynamic size of the unorderedset, which perhaps improves performance? </div><div><br></div><div>I also note that using the OverlayNG logic for CoverageUnion is a bit of overkill, since the topology of the unioned coverage is much simpler than a general topological situation.  At some point it would be good to see if a dedicated implementation for building the union coverage topology would be faster.  </div></div></div>