Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make prepared geometry (more?) thread-safe #826

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion benchmarks/index/chain/MonotoneChainBuilderPerfTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ static void BM_MonotoneChainBuilder(benchmark::State& state) {

for (auto _ : state) {
std::vector<MonotoneChain> chains;
MonotoneChainBuilder::getChains(&cs, nullptr, chains);
MonotoneChainBuilder::getChains(&cs, nullptr, 0, chains);
}
}

Expand Down
4 changes: 2 additions & 2 deletions benchmarks/index/chain/MonotoneChainPerfTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ static void BM_MonotoneChainOverlaps(benchmark::State& state) {
prev = c;
}

MonotoneChain mc1(cs1, 0, cs1.size(), nullptr);
MonotoneChain mc2(cs2, 0, cs1.size(), nullptr);
MonotoneChain mc1(cs1, 0, cs1.size(), nullptr, 0.0);
MonotoneChain mc2(cs2, 0, cs1.size(), nullptr, 0.0);

struct EmptyOverlapAction : public MonotoneChainOverlapAction {
virtual void
Expand Down
7 changes: 5 additions & 2 deletions include/geos/geom/prep/PreparedLineString.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,13 @@ namespace prep { // geos::geom::prep
*/
class PreparedLineString : public BasicPreparedGeometry {
private:
std::unique_ptr<noding::FastSegmentSetIntersectionFinder> segIntFinder;
mutable std::unique_ptr<noding::FastSegmentSetIntersectionFinder> segIntFinder;
mutable noding::SegmentString::ConstVect segStrings;
mutable std::unique_ptr<operation::distance::IndexedFacetDistance> indexedDistance;

mutable std::once_flag segIntFinderFlag;
mutable std::once_flag indexedDistanceFlag;

protected:
public:
PreparedLineString(const Geometry* geom)
Expand All @@ -54,7 +57,7 @@ class PreparedLineString : public BasicPreparedGeometry {

~PreparedLineString() override;

noding::FastSegmentSetIntersectionFinder* getIntersectionFinder();
noding::FastSegmentSetIntersectionFinder* getIntersectionFinder() const;

bool intersects(const geom::Geometry* g) const override;
std::unique_ptr<geom::CoordinateSequence> nearestPoints(const geom::Geometry* g) const override;
Expand Down
7 changes: 2 additions & 5 deletions include/geos/geom/prep/PreparedLineStringIntersects.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,7 @@ class PreparedLineStringIntersects {
return op.intersects(geom);
}

/**
* \todo FIXME - mloskot: Why not taking linestring through const reference?
*/
PreparedLineStringIntersects(PreparedLineString& prep)
PreparedLineStringIntersects(const PreparedLineString& prep)
: prepLine(prep)
{ }

Expand All @@ -74,7 +71,7 @@ class PreparedLineStringIntersects {
bool intersects(const geom::Geometry* g) const;

protected:
PreparedLineString& prepLine;
const PreparedLineString& prepLine;

/**
* Tests whether any representative point of the test Geometry intersects
Expand Down
10 changes: 8 additions & 2 deletions include/geos/geom/prep/PreparedPolygon.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,19 @@ namespace prep { // geos::geom::prep
*/
class PreparedPolygon : public BasicPreparedGeometry {
private:
bool isRectangle;
mutable std::unique_ptr<noding::FastSegmentSetIntersectionFinder> segIntFinder;
mutable std::unique_ptr<algorithm::locate::PointOnGeometryLocator> ptOnGeomLoc;
mutable std::unique_ptr<algorithm::locate::PointOnGeometryLocator> indexedPtOnGeomLoc;
mutable noding::SegmentString::ConstVect segStrings;
mutable std::unique_ptr<operation::distance::IndexedFacetDistance> indexedDistance;

mutable noding::SegmentString::ConstVect segStrings;

mutable std::once_flag segIntFinderFlag;
mutable std::once_flag ptOnGeomLocFlag;
mutable std::once_flag indexedPtOnGeomLocFlag;
mutable std::once_flag indexedDistanceFlag;
bool isRectangle;

protected:
public:
PreparedPolygon(const geom::Geometry* geom);
Expand Down
5 changes: 3 additions & 2 deletions include/geos/index/chain/MonotoneChain.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,15 @@ class GEOS_DLL MonotoneChain {
/// @param context
/// Ownership left to caller, this class holds a reference.
///
/// @param expansionDistance distance by which envelope should be expanded
///
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
Expand Down
1 change: 1 addition & 0 deletions include/geos/index/chain/MonotoneChainBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class GEOS_DLL MonotoneChainBuilder {
*/
static void getChains(const geom::CoordinateSequence* pts,
void* context,
double expansionDistance,
std::vector<MonotoneChain>& mcList);

/**
Expand Down
24 changes: 11 additions & 13 deletions include/geos/index/strtree/TemplateSTRtree.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ class TemplateSTRtreeImpl {

template<typename ItemDistance>
ItemType nearestNeighbour(const BoundsType& env, const ItemType& item, ItemDistance& itemDist) {
build();
buildIfNeeded();

if (getRoot() == nullptr) {
return nullptr;
Expand Down Expand Up @@ -273,9 +273,7 @@ class TemplateSTRtreeImpl {
// false values will be taken as a signal to stop the query.
template<typename Visitor>
void query(const BoundsType& queryEnv, Visitor &&visitor) {
if (!built()) {
build();
}
buildIfNeeded();

if (root && root->boundsIntersect(queryEnv)) {
if (root->isLeaf()) {
Expand All @@ -294,9 +292,7 @@ class TemplateSTRtreeImpl {
// false values will be taken as a signal to stop the query.
template<typename Visitor>
void queryPairs(Visitor&& visitor) {
if (!built()) {
build();
}
buildIfNeeded();

if (numItems < 2) {
return;
Expand All @@ -318,7 +314,7 @@ class TemplateSTRtreeImpl {
* Returns a depth-first iterator over all items in the tree.
*/
Items items() {
build();
buildIfNeeded();
return Items(*this);
}

Expand All @@ -341,7 +337,7 @@ class TemplateSTRtreeImpl {
/// @{

bool remove(const BoundsType& itemEnv, const ItemType& item) {
build();
buildIfNeeded();

if (root == nullptr) {
return false;
Expand Down Expand Up @@ -369,16 +365,18 @@ class TemplateSTRtreeImpl {

/** Determine whether the tree has been built, and no more items may be added. */
const Node* getRoot() {
build();
buildIfNeeded();
return root;
}

/// @}

/** Build the tree if it has not already been built. */
void build() {
std::lock_guard<std::mutex> lock(lock_);
void buildIfNeeded() {
std::call_once(builtFlag, [this]() { build(); });
}

void build() {
if (built()) {
return;
}
Expand Down Expand Up @@ -410,7 +408,7 @@ class TemplateSTRtreeImpl {
}

protected:
std::mutex lock_;
std::once_flag builtFlag;
NodeList nodes; //**< a list of all leaf and branch nodes in the tree. */
Node* root; //**< a pointer to the root node, if the tree has been built. */
size_t nodeCapacity; //*< maximum number of children of each node */
Expand Down
20 changes: 11 additions & 9 deletions include/geos/math/DD.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@

#pragma once

#include <geos/export.h>

#include <cmath>

namespace geos {
Expand All @@ -115,36 +117,36 @@ class GEOS_DLL DD {


public:
DD(double p_hi, double p_lo) : hi(p_hi), lo(p_lo) {};
DD(double x) : hi(x), lo(0.0) {};
DD() : hi(0.0), lo(0.0) {};
constexpr DD(double p_hi, double p_lo) : hi(p_hi), lo(p_lo) {};
constexpr DD(double x) : hi(x), lo(0.0) {};
constexpr DD() : hi(0.0), lo(0.0) {};

bool operator==(const DD &rhs) const
constexpr bool operator==(const DD &rhs) const
{
return hi == rhs.hi && lo == rhs.lo;
}

bool operator!=(const DD &rhs) const
constexpr bool operator!=(const DD &rhs) const
{
return hi != rhs.hi || lo != rhs.lo;
}

bool operator<(const DD &rhs) const
constexpr bool operator<(const DD &rhs) const
{
return (hi < rhs.hi) || (hi == rhs.hi && lo < rhs.lo);
}

bool operator<=(const DD &rhs) const
constexpr bool operator<=(const DD &rhs) const
{
return (hi < rhs.hi) || (hi == rhs.hi && lo <= rhs.lo);
}

bool operator>(const DD &rhs) const
constexpr bool operator>(const DD &rhs) const
{
return (hi > rhs.hi) || (hi == rhs.hi && lo > rhs.lo);
}

bool operator>=(const DD &rhs) const
constexpr bool operator>=(const DD &rhs) const
{
return (hi > rhs.hi) || (hi == rhs.hi && lo >= rhs.lo);
}
Expand Down
5 changes: 2 additions & 3 deletions include/geos/noding/FastSegmentSetIntersectionFinder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -67,7 +66,7 @@ class FastSegmentSetIntersectionFinder {
const SegmentSetMutualIntersector*
getSegmentSetIntersector() const
{
return segSetMutInt.get();
return &segSetMutInt;
}

bool intersects(SegmentString::ConstVect* segStrings);
Expand Down
28 changes: 8 additions & 20 deletions include/geos/noding/MCIndexSegmentSetMutualIntersector.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ class SegmentIntersector;
}
}

//using namespace geos::index::strtree;

namespace geos {
namespace noding { // geos::noding

Expand All @@ -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()
Expand All @@ -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;
Expand Down Expand Up @@ -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;

};

Expand Down
6 changes: 2 additions & 4 deletions include/geos/noding/SegmentIntersectionDetector.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace noding { // geos::noding
*/
class SegmentIntersectionDetector : public SegmentIntersector {
private:
algorithm::LineIntersector* li;
algorithm::LineIntersector li;

bool findProper;
bool findAllTypes;
Expand All @@ -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),
Expand All @@ -68,7 +67,6 @@ class SegmentIntersectionDetector : public SegmentIntersector {

~SegmentIntersectionDetector() override
{
//delete intPt;
delete intSegments;
}

Expand Down
3 changes: 2 additions & 1 deletion src/algorithm/CGAlgorithmsDD.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ namespace {
inline int
OrientationDD(const DD &dd)
{
static DD const zero(0.0);
constexpr const DD zero(0.0);

if(dd < zero) {
return CGAlgorithmsDD::RIGHT;
}
Expand Down
5 changes: 1 addition & 4 deletions src/algorithm/locate/IndexedPointInAreaLocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,12 @@ IndexedPointInAreaLocator::buildIndex(const geom::Geometry& g)
IndexedPointInAreaLocator::IndexedPointInAreaLocator(const geom::Geometry& g)
: areaGeom(g)
{
buildIndex(areaGeom);
}

geom::Location
IndexedPointInAreaLocator::locate(const geom::CoordinateXY* /*const*/ p)
{
if (index == nullptr) {
buildIndex(areaGeom);
}

algorithm::RayCrossingCounter rcc(*p);

index->query(p->y, p->y, [&rcc](const SegmentView& ls) {
Expand Down