MCIndexSegmentSetMutualIntersector: Allow usage from multiple threads

dbaston · dbaston · commit 09bdab48665c · 2023-02-15T21:37:47.000-05:00
diff --git a/include/geos/index/chain/MonotoneChain.h b/include/geos/index/chain/MonotoneChain.h
@@ -96,13 +96,12 @@ class GEOS_DLL MonotoneChain {
     ///   Ownership left to caller, this class holds a reference.
     ///
     MonotoneChain(const geom::CoordinateSequence& pts,
-                  std::size_t start, std::size_t end, void* context);
+                  std::size_t start, std::size_t end, void* context, double expansionDistance);
 
     ~MonotoneChain() = default;
 
     /// Returned envelope is owned by this class
     const geom::Envelope& getEnvelope() const;
-    const geom::Envelope& getEnvelope(double expansionDistance) const;
 
     size_t
     getStartIndex() const
diff --git a/include/geos/index/chain/MonotoneChainBuilder.h b/include/geos/index/chain/MonotoneChainBuilder.h
@@ -58,6 +58,7 @@ class GEOS_DLL MonotoneChainBuilder {
      */
     static void getChains(const geom::CoordinateSequence* pts,
                           void* context,
+                          double expansionDistance,
                           std::vector<MonotoneChain>& mcList);
 
     /**
diff --git a/include/geos/noding/FastSegmentSetIntersectionFinder.h b/include/geos/noding/FastSegmentSetIntersectionFinder.h
@@ -49,8 +49,7 @@ namespace noding { // geos::noding
  */
 class FastSegmentSetIntersectionFinder {
 private:
-    std::unique_ptr<MCIndexSegmentSetMutualIntersector> segSetMutInt;
-    std::unique_ptr<geos::algorithm::LineIntersector> lineIntersector;
+    MCIndexSegmentSetMutualIntersector segSetMutInt;
 
 protected:
 public:
@@ -67,7 +66,7 @@ class FastSegmentSetIntersectionFinder {
     const SegmentSetMutualIntersector*
     getSegmentSetIntersector() const
     {
-        return segSetMutInt.get();
+        return &segSetMutInt;
     }
 
     bool intersects(SegmentString::ConstVect* segStrings);
diff --git a/include/geos/noding/MCIndexSegmentSetMutualIntersector.h b/include/geos/noding/MCIndexSegmentSetMutualIntersector.h
@@ -29,8 +29,6 @@ class SegmentIntersector;
 }
 }
 
-//using namespace geos::index::strtree;
-
 namespace geos {
 namespace noding { // geos::noding
 
@@ -45,12 +43,7 @@ class MCIndexSegmentSetMutualIntersector : public SegmentSetMutualIntersector {
 public:
 
     MCIndexSegmentSetMutualIntersector(double p_tolerance)
-        : monoChains()
-        , indexCounter(0)
-        , processCounter(0)
-        , nOverlaps(0)
-        , overlapTolerance(p_tolerance)
-        , indexBuilt(false)
+        : overlapTolerance(p_tolerance)
     {}
 
     MCIndexSegmentSetMutualIntersector()
@@ -68,9 +61,10 @@ class MCIndexSegmentSetMutualIntersector : public SegmentSetMutualIntersector {
 
     void setBaseSegments(SegmentString::ConstVect* segStrings) override;
 
-    // NOTE: re-populates the MonotoneChain vector with newly created chains
     void process(SegmentString::ConstVect* segStrings) override;
 
+    void process(SegmentString::ConstVect* segStrings, SegmentIntersector* si);
+
     class SegmentOverlapAction : public index::chain::MonotoneChainOverlapAction {
     private:
         SegmentIntersector& si;
@@ -98,31 +92,25 @@ class MCIndexSegmentSetMutualIntersector : public SegmentSetMutualIntersector {
 private:
 
     typedef std::vector<index::chain::MonotoneChain> MonoChains;
-    MonoChains monoChains;
 
     /*
      * The index::SpatialIndex used should be something that supports
      * envelope (range) queries efficiently (such as a index::quadtree::Quadtree
      * or index::strtree::STRtree).
      */
     index::strtree::TemplateSTRtree<const index::chain::MonotoneChain*> index;
-    int indexCounter;
-    int processCounter;
-    // statistics
-    int nOverlaps;
-    double overlapTolerance;
+
+    const double overlapTolerance;
 
     /* memory management helper, holds MonotoneChain objects used
      * in the SpatialIndex. It's cleared when the SpatialIndex is
      */
-    bool indexBuilt;
+    std::once_flag indexBuilt;
     MonoChains indexChains;
 
-    void addToIndex(SegmentString* segStr);
-
-    void intersectChains();
+    void intersectChains(const MonoChains& chains, SegmentIntersector& segmentIntersector);
 
-    void addToMonoChains(SegmentString* segStr);
+    void addChains(const SegmentString* segStr, MonoChains& chains) const;
 
 };
 
diff --git a/include/geos/noding/SegmentIntersectionDetector.h b/include/geos/noding/SegmentIntersectionDetector.h
@@ -40,7 +40,7 @@ namespace noding { // geos::noding
  */
 class SegmentIntersectionDetector : public SegmentIntersector {
 private:
-    algorithm::LineIntersector* li;
+    algorithm::LineIntersector li;
 
     bool findProper;
     bool findAllTypes;
@@ -54,9 +54,8 @@ class SegmentIntersectionDetector : public SegmentIntersector {
 
 protected:
 public:
-    SegmentIntersectionDetector(algorithm::LineIntersector* p_li)
+    SegmentIntersectionDetector()
         :
-        li(p_li),
         findProper(false),
         findAllTypes(false),
         _hasIntersection(false),
@@ -68,7 +67,6 @@ class SegmentIntersectionDetector : public SegmentIntersector {
 
     ~SegmentIntersectionDetector() override
     {
-        //delete intPt;
         delete intSegments;
     }
 
diff --git a/src/geom/prep/AbstractPreparedPolygonContains.cpp b/src/geom/prep/AbstractPreparedPolygonContains.cpp
@@ -83,9 +83,7 @@ AbstractPreparedPolygonContains::findAndClassifyIntersections(const geom::Geomet
     noding::SegmentString::ConstVect lineSegStr;
     noding::SegmentStringUtil::extractSegmentStrings(geom, lineSegStr);
 
-    algorithm::LineIntersector li;
-
-    noding::SegmentIntersectionDetector intDetector(&li);
+    noding::SegmentIntersectionDetector intDetector;
 
     intDetector.setFindAllIntersectionTypes(true);
     prepPoly->getIntersectionFinder()->intersects(&lineSegStr, &intDetector);
diff --git a/src/index/chain/MonotoneChain.cpp b/src/index/chain/MonotoneChain.cpp
@@ -32,29 +32,22 @@ namespace index { // geos.index
 namespace chain { // geos.index.chain
 
 MonotoneChain::MonotoneChain(const geom::CoordinateSequence& newPts,
-                             std::size_t nstart, std::size_t nend, void* nContext)
+                             std::size_t nstart, std::size_t nend, void* nContext,
+                             double expansionDistance)
     : pts(&newPts)
     , context(nContext)
     , start(nstart)
     , end(nend)
-    , env()
-{}
-
-const Envelope&
-MonotoneChain::getEnvelope() const
+    , env(pts->getAt<CoordinateXY>(start), pts->getAt<CoordinateXY>(end))
 {
-    return getEnvelope(0.0);
+    if (expansionDistance > 0.0) {
+        env.expandBy(expansionDistance);
+    }
 }
 
 const Envelope&
-MonotoneChain::getEnvelope(double expansionDistance) const
+MonotoneChain::getEnvelope() const
 {
-    if (env.isNull()) {
-        env.init(pts->getAt<CoordinateXY>(start), pts->getAt<CoordinateXY>(end));
-        if (expansionDistance > 0.0) {
-            env.expandBy(expansionDistance);
-        }
-    }
     return env;
 }
 
diff --git a/src/index/chain/MonotoneChainBuilder.cpp b/src/index/chain/MonotoneChainBuilder.cpp
@@ -48,13 +48,14 @@ namespace chain { // geos.index.chain
  */
 class ChainBuilder : public CoordinateFilter {
 public:
-    ChainBuilder(const CoordinateSequence* pts, void* context, std::vector<MonotoneChain> & list) :
+    ChainBuilder(const CoordinateSequence* pts, void* context, double expansionDistance, std::vector<MonotoneChain> & list) :
      m_prev(nullptr),
      m_i(0),
      m_quadrant(-1),
      m_start(0),
      m_seq(pts),
      m_context(context),
+     m_distance(expansionDistance),
      m_list(list) {}
 
     void filter_ro(const CoordinateXY* c) override {
@@ -72,7 +73,7 @@ class ChainBuilder : public CoordinateFilter {
     void finishChain() {
         if ( m_i == 0 ) return;
         std::size_t chainEnd = m_i - 1;
-        m_list.emplace_back(*m_seq, m_start, chainEnd, m_context);
+        m_list.emplace_back(*m_seq, m_start, chainEnd, m_context, m_distance);
         m_start = chainEnd;
     }
 
@@ -99,15 +100,17 @@ class ChainBuilder : public CoordinateFilter {
     std::size_t m_start;
     const CoordinateSequence* m_seq;
     void* m_context;
+    double m_distance;
     std::vector<MonotoneChain>& m_list;
 };
 
 
 /* static public */
 void
 MonotoneChainBuilder::getChains(const CoordinateSequence* pts, void* context,
+                                double expansionDistance,
                                 std::vector<MonotoneChain>& mcList) {
-    ChainBuilder builder(pts, context, mcList);
+    ChainBuilder builder(pts, context, expansionDistance, mcList);
     pts->apply_ro(&builder);
     builder.finish();
 }
diff --git a/src/noding/FastSegmentSetIntersectionFinder.cpp b/src/noding/FastSegmentSetIntersectionFinder.cpp
@@ -39,17 +39,15 @@ namespace noding { // geos::noding
  */
 FastSegmentSetIntersectionFinder::
 FastSegmentSetIntersectionFinder(noding::SegmentString::ConstVect* baseSegStrings)
-    :	segSetMutInt(new MCIndexSegmentSetMutualIntersector()),
-      lineIntersector(new algorithm::LineIntersector())
 {
-    segSetMutInt->setBaseSegments(baseSegStrings);
+    segSetMutInt.setBaseSegments(baseSegStrings);
 }
 
 bool
 FastSegmentSetIntersectionFinder::
 intersects(noding::SegmentString::ConstVect* segStrings)
 {
-    SegmentIntersectionDetector intFinder(lineIntersector.get());
+    SegmentIntersectionDetector intFinder;
 
     return this->intersects(segStrings, &intFinder);
 }
@@ -59,8 +57,7 @@ FastSegmentSetIntersectionFinder::
 intersects(noding::SegmentString::ConstVect* segStrings,
            SegmentIntersectionDetector* intDetector)
 {
-    segSetMutInt->setSegmentIntersector(intDetector);
-    segSetMutInt->process(segStrings);
+    segSetMutInt.process(segStrings, intDetector);
 
     return intDetector->hasIntersection();
 }
diff --git a/src/noding/MCIndexNoder.cpp b/src/noding/MCIndexNoder.cpp
@@ -51,7 +51,7 @@ MCIndexNoder::computeNodes(SegmentString::NonConstVect* inputSegStrings)
 
     if (!indexBuilt) {
         for(const auto& mc : monoChains) {
-            index.insert(mc.getEnvelope(overlapTolerance), &mc);
+            index.insert(mc.getEnvelope(), &mc);
         }
         indexBuilt = true;
     }
@@ -84,7 +84,7 @@ MCIndexNoder::add(SegmentString* segStr)
 
     // segChains will contain newly allocated MonotoneChain objects
     MonotoneChainBuilder::getChains(segStr->getCoordinates(),
-                                    segStr, monoChains);
+                                    segStr, overlapTolerance, monoChains);
 
 }
 
diff --git a/src/noding/MCIndexSegmentSetMutualIntersector.cpp b/src/noding/MCIndexSegmentSetMutualIntersector.cpp
@@ -13,14 +13,11 @@
  **********************************************************************/
 
 #include <geos/noding/MCIndexSegmentSetMutualIntersector.h>
-#include <geos/noding/SegmentSetMutualIntersector.h>
 #include <geos/noding/SegmentString.h>
 #include <geos/noding/SegmentIntersector.h>
-#include <geos/index/SpatialIndex.h>
 #include <geos/index/chain/MonotoneChain.h>
 #include <geos/index/chain/MonotoneChainBuilder.h>
 #include <geos/index/chain/MonotoneChainOverlapAction.h>
-#include <geos/index/strtree/SimpleSTRtree.h>
 
 // std
 #include <cstddef>
@@ -33,37 +30,26 @@ namespace noding { // geos::noding
 
 /*private*/
 void
-MCIndexSegmentSetMutualIntersector::addToIndex(SegmentString* segStr)
-{
-    MonotoneChainBuilder::getChains(segStr->getCoordinates(),
-                                    segStr, indexChains);
-
-}
-
-
-/*private*/
-void
-MCIndexSegmentSetMutualIntersector::addToMonoChains(SegmentString* segStr)
+MCIndexSegmentSetMutualIntersector::addChains(const SegmentString* segStr, MonoChains& chains) const
 {
     if (segStr->size() == 0)
         return;
     MonotoneChainBuilder::getChains(segStr->getCoordinates(),
-                                    segStr, monoChains);
+                                    (void*) segStr, overlapTolerance, chains);
 }
 
 
 /*private*/
 void
-MCIndexSegmentSetMutualIntersector::intersectChains()
+MCIndexSegmentSetMutualIntersector::intersectChains(const MonoChains& chains, SegmentIntersector& segmentIntersector)
 {
-    MCIndexSegmentSetMutualIntersector::SegmentOverlapAction overlapAction(*segInt);
+    MCIndexSegmentSetMutualIntersector::SegmentOverlapAction overlapAction(segmentIntersector);
 
-    for(auto& queryChain : monoChains) {
-        index.query(queryChain.getEnvelope(overlapTolerance), [&queryChain, &overlapAction, this](const MonotoneChain* testChain) -> bool {
+    for(auto& queryChain : chains) {
+        index.query(queryChain.getEnvelope(), [&queryChain, &overlapAction, &segmentIntersector, this](const MonotoneChain* testChain) -> bool {
             queryChain.computeOverlaps(testChain, overlapTolerance, &overlapAction);
-            nOverlaps++;
 
-            return !segInt->isDone(); // abort early if segInt->isDone()
+            return !segmentIntersector.isDone(); // abort early if segmentIntersector->isDone()
         });
     }
 }
@@ -73,37 +59,38 @@ MCIndexSegmentSetMutualIntersector::intersectChains()
 void
 MCIndexSegmentSetMutualIntersector::setBaseSegments(SegmentString::ConstVect* segStrings)
 {
-    // NOTE - mloskot: const qualifier is removed silently, dirty.
-
     for(const SegmentString* css: *segStrings) {
-        if (css->size() == 0)
-            continue;
-        SegmentString* ss = const_cast<SegmentString*>(css);
-        addToIndex(ss);
+        addChains(css, indexChains);
     }
 }
 
 /*public*/
 void
 MCIndexSegmentSetMutualIntersector::process(SegmentString::ConstVect* segStrings)
 {
-    if (!indexBuilt) {
+    process(segStrings, segInt);
+}
+
+/*public*/
+void
+MCIndexSegmentSetMutualIntersector::process(SegmentString::ConstVect* segStrings,
+                                            SegmentIntersector* segmentIntersector)
+{
+    std::call_once(indexBuilt, [this]() {
         for (auto& mc: indexChains) {
-            index.insert(&(mc.getEnvelope(overlapTolerance)), &mc);
+            index.insert(&(mc.getEnvelope()), &mc);
         }
-        indexBuilt = true;
-    }
-
-    // Reset counters for new inputs
-    monoChains.clear();
-    processCounter = indexCounter + 1;
-    nOverlaps = 0;
+    });
 
+    // TODO: Rework MonotoneChain extraction to take a callback, so we can pass the chains
+    // to intersectChains as they are identified.
+    MonoChains monoChains;
     for(const SegmentString* css: *segStrings) {
         SegmentString* ss = const_cast<SegmentString*>(css);
-        addToMonoChains(ss);
+        addChains(ss, monoChains);
     }
-    intersectChains();
+
+    intersectChains(monoChains, *segmentIntersector);
 }
 
 
diff --git a/src/noding/SegmentIntersectionDetector.cpp b/src/noding/SegmentIntersectionDetector.cpp
diff --git a/src/operation/buffer/SegmentMCIndex.cpp b/src/operation/buffer/SegmentMCIndex.cpp
diff --git a/tests/unit/geom/prep/PreparedGeometryTest.cpp b/tests/unit/geom/prep/PreparedGeometryTest.cpp
diff --git a/tests/unit/index/chain/MonotoneChainBuilderTest.cpp b/tests/unit/index/chain/MonotoneChainBuilderTest.cpp
diff --git a/tests/unit/operation/relate/RelateOpTest.cpp b/tests/unit/operation/relate/RelateOpTest.cpp

Original file line number	Diff line number	Diff line change
`@@ -39,17 +39,15 @@ namespace noding { // geos::noding`
`39`	`39`	`*/`
`40`	`40`	`FastSegmentSetIntersectionFinder::`
`41`	`41`	`FastSegmentSetIntersectionFinder(noding::SegmentString::ConstVect* baseSegStrings)`
`42`		`- : segSetMutInt(new MCIndexSegmentSetMutualIntersector()),`
`43`		`- lineIntersector(new algorithm::LineIntersector())`
`44`	`42`	`{`
`45`		`- segSetMutInt->setBaseSegments(baseSegStrings);`
	`43`	`+ segSetMutInt.setBaseSegments(baseSegStrings);`
`46`	`44`	`}`
`47`	`45`
`48`	`46`	`bool`
`49`	`47`	`FastSegmentSetIntersectionFinder::`
`50`	`48`	`intersects(noding::SegmentString::ConstVect* segStrings)`
`51`	`49`	`{`
`52`		`- SegmentIntersectionDetector intFinder(lineIntersector.get());`
	`50`	`+ SegmentIntersectionDetector intFinder;`
`53`	`51`
`54`	`52`	`return this->intersects(segStrings, &intFinder);`
`55`	`53`	`}`
`@@ -59,8 +57,7 @@ FastSegmentSetIntersectionFinder::`
`59`	`57`	`intersects(noding::SegmentString::ConstVect* segStrings,`
`60`	`58`	`SegmentIntersectionDetector* intDetector)`
`61`	`59`	`{`
`62`		`- segSetMutInt->setSegmentIntersector(intDetector);`
`63`		`- segSetMutInt->process(segStrings);`
	`60`	`+ segSetMutInt.process(segStrings, intDetector);`
`64`	`61`
`65`	`62`	`return intDetector->hasIntersection();`
`66`	`63`	`}`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ MCIndexNoder::computeNodes(SegmentString::NonConstVect* inputSegStrings)`
`51`	`51`
`52`	`52`	`if (!indexBuilt) {`
`53`	`53`	`for(const auto& mc : monoChains) {`
`54`		`- index.insert(mc.getEnvelope(overlapTolerance), &mc);`
	`54`	`+ index.insert(mc.getEnvelope(), &mc);`
`55`	`55`	`}`
`56`	`56`	`indexBuilt = true;`
`57`	`57`	`}`
`@@ -84,7 +84,7 @@ MCIndexNoder::add(SegmentString* segStr)`
`84`	`84`
`85`	`85`	`// segChains will contain newly allocated MonotoneChain objects`
`86`	`86`	`MonotoneChainBuilder::getChains(segStr->getCoordinates(),`
`87`		`- segStr, monoChains);`
	`87`	`+ segStr, overlapTolerance, monoChains);`
`88`	`88`
`89`	`89`	`}`
`90`	`90`