[geos-commits] r2476 - in trunk/source: algorithm
headers/geos/index headers/geos/index/bintree
headers/geos/index/chain index/bintree index/chain noding
svn_geos at osgeo.org
svn_geos at osgeo.org
Wed May 6 12:44:07 EDT 2009
Author: strk
Date: 2009-05-06 12:44:06 -0400 (Wed, 06 May 2009)
New Revision: 2476
Modified:
trunk/source/algorithm/MCPointInRing.cpp
trunk/source/headers/geos/index/SpatialIndex.h
trunk/source/headers/geos/index/bintree/Bintree.h
trunk/source/headers/geos/index/bintree/Root.h
trunk/source/headers/geos/index/chain/MonotoneChain.h
trunk/source/index/bintree/Bintree.cpp
trunk/source/index/chain/MonotoneChain.cpp
trunk/source/index/chain/MonotoneChainBuilder.cpp
trunk/source/index/chain/MonotoneChainOverlapAction.cpp
trunk/source/index/chain/MonotoneChainSelectAction.cpp
trunk/source/noding/MCIndexNoder.cpp
trunk/source/noding/MCIndexSegmentSetMutualIntersector.cpp
Log:
MonotoneChain const-correctness and interface cleanups, cascaded changes. Possibly discovered a leak in MCPointInRing algorithm, needs some unit testing.
Modified: trunk/source/algorithm/MCPointInRing.cpp
===================================================================
--- trunk/source/algorithm/MCPointInRing.cpp 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/algorithm/MCPointInRing.cpp 2009-05-06 16:44:06 UTC (rev 2476)
@@ -72,14 +72,29 @@
// Envelope *env=ring->getEnvelopeInternal();
tree=new bintree::Bintree();
pts=CoordinateSequence::removeRepeatedPoints(ring->getCoordinatesRO());
- vector<chain::MonotoneChain*> *mcList=chain::MonotoneChainBuilder::getChains(pts);
- for(int i=0;i<(int)mcList->size();i++) {
+
+ // NOTE: we take ownership of mcList and it's elements
+ vector<chain::MonotoneChain*> *mcList =
+ chain::MonotoneChainBuilder::getChains(pts);
+
+ for(size_t i=0, n=mcList->size(); i<n; ++i)
+ {
chain::MonotoneChain *mc=(*mcList)[i];
- Envelope *mcEnv=mc->getEnvelope();
- interval.min=mcEnv->getMinY();
- interval.max=mcEnv->getMaxY();
- tree->insert(&interval,mc);
+ const Envelope& mcEnv = mc->getEnvelope();
+ interval.min = mcEnv.getMinY();
+ interval.max = mcEnv.getMaxY();
+
+ // TODO: is 'mc' ownership transferred here ? (see below)
+ // by documentation SpatialIndex does NOT take
+ // ownership of the items, so unless we query it
+ // all later we've a leak problem here..
+ // Need a focused testcase.
+ //
+ tree->insert(&interval, mc);
}
+
+ // TODO: mcList elements ownership went to tree or what ?
+
delete mcList;
}
Modified: trunk/source/headers/geos/index/SpatialIndex.h
===================================================================
--- trunk/source/headers/geos/index/SpatialIndex.h 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/headers/geos/index/SpatialIndex.h 2009-05-06 16:44:06 UTC (rev 2476)
@@ -52,7 +52,13 @@
* Adds a spatial item with an extent specified by the given Envelope
* to the index
*
- * TODO: document ownership of 'itemEnv' and 'item'
+ * @param itemEnv
+ * Envelope of the item, ownership left to caller.
+ * TODO: Reference hold by this class ?
+ *
+ * @param item
+ * Opaque item, ownership left to caller.
+ * Reference hold by this class.
*/
virtual void insert(const geom::Envelope *itemEnv, void *item) = 0;
Modified: trunk/source/headers/geos/index/bintree/Bintree.h
===================================================================
--- trunk/source/headers/geos/index/bintree/Bintree.h 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/headers/geos/index/bintree/Bintree.h 2009-05-06 16:44:06 UTC (rev 2476)
@@ -52,7 +52,18 @@
public:
- static Interval* ensureExtent(Interval *itemInterval, double minExtent);
+ /**
+ * Ensure that the Interval for the inserted item has non-zero extents.
+ * Use the current minExtent to pad it, if necessary
+ *
+ * NOTE: in GEOS this function always return a newly allocated object
+ * with ownership transferred to caller. TODO: change this ?
+ *
+ * @param itemInterval
+ * Source interval, ownership left to caller, no references hold.
+ */
+ static Interval* ensureExtent(const Interval *itemInterval,
+ double minExtent);
Bintree();
@@ -64,7 +75,13 @@
int nodeSize();
- void insert(Interval *itemInterval,void* item);
+ /// @param itemInterval
+ /// Ownership left to caller, NO reference hold by this class.
+ ///
+ /// @param item
+ /// Ownership left to caller, reference kept by this class.
+ ///
+ void insert(Interval *itemInterval, void* item);
std::vector<void*>* iterator();
Modified: trunk/source/headers/geos/index/bintree/Root.h
===================================================================
--- trunk/source/headers/geos/index/bintree/Root.h 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/headers/geos/index/bintree/Root.h 2009-05-06 16:44:06 UTC (rev 2476)
@@ -55,7 +55,13 @@
~Root() {}
- void insert(Interval *itemInterval,void* item);
+ /// @param itemInterval
+ /// Ownership left to caller, references kept in this class.
+ ///
+ /// @param item
+ /// Ownership left to caller, references kept in this class.
+ ///
+ void insert(Interval *itemInterval, void* item);
protected:
Modified: trunk/source/headers/geos/index/chain/MonotoneChain.h
===================================================================
--- trunk/source/headers/geos/index/chain/MonotoneChain.h 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/headers/geos/index/chain/MonotoneChain.h 2009-05-06 16:44:06 UTC (rev 2476)
@@ -22,6 +22,8 @@
#include <geos/geom/Envelope.h> // for inline
+#include <memory> // for auto_ptr
+
// Forward declarations
namespace geos {
namespace geom {
@@ -84,30 +86,40 @@
{
public:
- MonotoneChain(const geom::CoordinateSequence* pts,
+ /// @param pts
+ /// Ownership left to caller, this class holds a reference.
+ ///
+ /// @param start
+ ///
+ /// @param end
+ ///
+ /// @param context
+ /// Ownership left to caller, this class holds a reference.
+ ///
+ MonotoneChain(const geom::CoordinateSequence& pts,
size_t start, size_t end, void* context);
~MonotoneChain();
/// Returned envelope is owned by this class
- geom::Envelope* getEnvelope();
+ const geom::Envelope& getEnvelope() const;
- size_t getStartIndex() { return start; }
+ size_t getStartIndex() const { return start; }
- size_t getEndIndex() { return end; }
+ size_t getEndIndex() const { return end; }
- /// Set given LineSegment with points of the segment starting
- /// at the given index.
- void getLineSegment(unsigned int index, geom::LineSegment *ls);
+ /** \brief
+ * Set given LineSegment with points of the segment starting
+ * at the given index.
+ */
+ void getLineSegment(unsigned int index, geom::LineSegment& ls) const;
/**
* Return the subsequence of coordinates forming this chain.
* Allocates a new CoordinateSequence to hold the Coordinates
*
- * TODO: return by auto_ptr
- *
*/
- geom::CoordinateSequence* getCoordinates();
+ std::auto_ptr<geom::CoordinateSequence> getCoordinates() const;
/**
* Determine all the line segments in the chain whose envelopes overlap
@@ -121,25 +133,25 @@
void setId(int nId) { id=nId; }
- inline int getId() { return id; }
+ inline int getId() const { return id; }
void* getContext() { return context; }
private:
void computeSelect(const geom::Envelope& searchEnv,
- unsigned int start0,
- unsigned int end0,
+ size_t start0,
+ size_t end0,
MonotoneChainSelectAction& mcs);
- void computeOverlaps(int start0, int end0, MonotoneChain* mc,
- int start1, int end1, MonotoneChainOverlapAction* mco);
+ void computeOverlaps(size_t start0, size_t end0, MonotoneChain& mc,
+ int start1, int end1, MonotoneChainOverlapAction& mco);
- /// Externally owned (could be a reference)
- const geom::CoordinateSequence* pts;
+ /// Externally owned
+ const geom::CoordinateSequence& pts;
/// Owned by this class, lazely created
- geom::Envelope* env;
+ mutable geom::Envelope* env;
/// user-defined information
void* context;
Modified: trunk/source/index/bintree/Bintree.cpp
===================================================================
--- trunk/source/index/bintree/Bintree.cpp 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/index/bintree/Bintree.cpp 2009-05-06 16:44:06 UTC (rev 2476)
@@ -25,23 +25,26 @@
using namespace std;
-/**
-* Ensure that the Interval for the inserted item has non-zero extents.
-* Use the current minExtent to pad it, if necessary
-*/
Interval*
-Bintree::ensureExtent(Interval *itemInterval, double minExtent)
+Bintree::ensureExtent(const Interval* itemInterval, double minExtent)
{
- double min=itemInterval->getMin();
- double max=itemInterval->getMax();
+ double min = itemInterval->getMin();
+ double max = itemInterval->getMax();
// has a non-zero extent
- if (min!=max) return new Interval(itemInterval);
+ if (min != max)
+ {
+ // GEOS forces a copy here to be predictable wrt
+ // memory management. May change in the future.
+ return new Interval(*itemInterval);
+ }
+
// pad extent
- if (min==max) {
- min=min-minExtent/2.0;
- max=min+minExtent/2.0;
+ if (min==max)
+ {
+ min = min-minExtent/2.0;
+ max = min+minExtent/2.0;
}
-// delete itemInterval;
+
return new Interval(min, max);
}
Modified: trunk/source/index/chain/MonotoneChain.cpp
===================================================================
--- trunk/source/index/chain/MonotoneChain.cpp 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/index/chain/MonotoneChain.cpp 2009-05-06 16:44:06 UTC (rev 2476)
@@ -31,9 +31,15 @@
namespace index { // geos.index
namespace chain { // geos.index.chain
-MonotoneChain::MonotoneChain(const geom::CoordinateSequence *newPts,
+MonotoneChain::MonotoneChain(const geom::CoordinateSequence& newPts,
size_t nstart, size_t nend, void* nContext)
-: pts(newPts), env(0), context(nContext), start(nstart), end(nend), id(-1)
+ :
+ pts(newPts),
+ env(0),
+ context(nContext),
+ start(nstart),
+ end(nend),
+ id(-1)
{
}
@@ -42,48 +48,44 @@
delete env;
}
-Envelope*
-MonotoneChain::getEnvelope()
+const Envelope&
+MonotoneChain::getEnvelope() const
{
if (0 == env)
{
- const Coordinate& p0 = pts->getAt(start);
- const Coordinate& p1 = pts->getAt(end);
+ const Coordinate& p0 = pts[start];
+ const Coordinate& p1 = pts[end];
env = new Envelope(p0, p1);
}
- return env;
+ return *env;
}
void
-MonotoneChain::getLineSegment(unsigned int index, LineSegment *ls)
+MonotoneChain::getLineSegment(unsigned int index, LineSegment& ls) const
{
- ls->p0=pts->getAt(index);
- ls->p1=pts->getAt(index+1);
+ ls.p0 = pts[index];
+ ls.p1 = pts[index+1];
}
-/**
-* Return the subsequence of coordinates forming this chain.
-* Allocates a new array to hold the Coordinates
-*/
-CoordinateSequence*
-MonotoneChain::getCoordinates()
+std::auto_ptr<CoordinateSequence>
+MonotoneChain::getCoordinates() const
{
- return pts->clone();
+ return std::auto_ptr<CoordinateSequence>(pts.clone());
}
void
MonotoneChain::select(const Envelope& searchEnv, MonotoneChainSelectAction& mcs)
{
- computeSelect(searchEnv,start,end,mcs);
+ computeSelect(searchEnv, start, end, mcs);
}
void
MonotoneChain::computeSelect(const Envelope& searchEnv,
- unsigned int start0, unsigned int end0,
+ size_t start0, size_t end0,
MonotoneChainSelectAction& mcs )
{
- const Coordinate& p0=pts->getAt(start0);
- const Coordinate& p1=pts->getAt(end0);
+ const Coordinate& p0=pts[start0];
+ const Coordinate& p1=pts[end0];
mcs.tempEnv1->init(p0,p1);
//Debug.println("trying:"+p0+p1+" [ "+start0+","+end0+" ]");
@@ -113,37 +115,42 @@
}
}
+/* public */
void
-MonotoneChain::computeOverlaps(MonotoneChain *mc, MonotoneChainOverlapAction *mco)
+MonotoneChain::computeOverlaps(MonotoneChain *mc,
+ MonotoneChainOverlapAction *mco)
{
- computeOverlaps(start,end,mc,mc->start,mc->end,mco);
+ computeOverlaps(start, end, *mc, mc->start, mc->end, *mco);
}
+/*private*/
void
-MonotoneChain::computeOverlaps(int start0, int end0, MonotoneChain *mc,
- int start1, int end1, MonotoneChainOverlapAction *mco)
+MonotoneChain::computeOverlaps(size_t start0, size_t end0,
+ MonotoneChain& mc,
+ int start1, int end1,
+ MonotoneChainOverlapAction& mco)
{
//Debug.println("computeIntersectsForChain:"+p00+p01+p10+p11);
// terminating condition for the recursion
if (end0-start0==1 && end1-start1==1)
{
- mco->overlap(this,start0,mc,start1);
+ mco.overlap(this, start0, &mc, start1);
return;
}
- const Coordinate& p00=pts->getAt(start0);
- const Coordinate& p01=pts->getAt(end0);
- const Coordinate& p10=mc->pts->getAt(start1);
- const Coordinate& p11=mc->pts->getAt(end1);
+ const Coordinate& p00 = pts[start0];
+ const Coordinate& p01 = pts[end0];
+ const Coordinate& p10 = mc.pts[start1];
+ const Coordinate& p11 = mc.pts[end1];
// nothing to do if the envelopes of these chains don't overlap
- mco->tempEnv1->init(p00,p01);
- mco->tempEnv2->init(p10,p11);
- if (!mco->tempEnv1->intersects(mco->tempEnv2)) return;
+ mco.tempEnv1->init(p00, p01);
+ mco.tempEnv2->init(p10, p11);
+ if (!mco.tempEnv1->intersects(mco.tempEnv2)) return;
// the chains overlap,so split each in half and iterate (binary search)
- int mid0=(start0+end0)/2;
- int mid1=(start1+end1)/2;
+ size_t mid0=(start0+end0)/2;
+ size_t mid1=(start1+end1)/2;
// Assert: mid != start or end (since we checked above for
// end-start <= 1)
@@ -151,17 +158,17 @@
if (start0<mid0)
{
if (start1<mid1)
- computeOverlaps(start0,mid0,mc,start1,mid1,mco);
+ computeOverlaps(start0, mid0, mc, start1, mid1, mco);
if (mid1<end1)
- computeOverlaps(start0,mid0,mc,mid1,end1,mco);
+ computeOverlaps(start0, mid0, mc, mid1, end1, mco);
}
if (mid0<end0)
{
if (start1<mid1)
- computeOverlaps(mid0,end0,mc,start1,mid1,mco);
+ computeOverlaps(mid0, end0, mc, start1, mid1, mco);
if (mid1<end1)
- computeOverlaps(mid0,end0,mc,mid1,end1,mco);
+ computeOverlaps(mid0, end0, mc, mid1, end1, mco);
}
}
Modified: trunk/source/index/chain/MonotoneChainBuilder.cpp
===================================================================
--- trunk/source/index/chain/MonotoneChainBuilder.cpp 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/index/chain/MonotoneChainBuilder.cpp 2009-05-06 16:44:06 UTC (rev 2476)
@@ -63,7 +63,7 @@
size_t n = nindexes - 1;
for(size_t i = 0; i < n; i++)
{
- MonotoneChain* mc = new MonotoneChain(pts, startIndex[i], startIndex[i+1], context);
+ MonotoneChain* mc = new MonotoneChain(*pts, startIndex[i], startIndex[i+1], context);
mcList.push_back(mc);
}
}
Modified: trunk/source/index/chain/MonotoneChainOverlapAction.cpp
===================================================================
--- trunk/source/index/chain/MonotoneChainOverlapAction.cpp 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/index/chain/MonotoneChainOverlapAction.cpp 2009-05-06 16:44:06 UTC (rev 2476)
@@ -41,16 +41,13 @@
}
-/**
- * This function can be overridden if the original chains are needed
- */
void
MonotoneChainOverlapAction::overlap(MonotoneChain *mc1, int start1,
MonotoneChain *mc2, int start2)
{
- mc1->getLineSegment(start1,overlapSeg1);
- mc2->getLineSegment(start2,overlapSeg2);
- overlap(overlapSeg1,overlapSeg2);
+ mc1->getLineSegment(start1, *overlapSeg1);
+ mc2->getLineSegment(start2, *overlapSeg2);
+ overlap(overlapSeg1, overlapSeg2);
}
} // namespace geos.index.chain
Modified: trunk/source/index/chain/MonotoneChainSelectAction.cpp
===================================================================
--- trunk/source/index/chain/MonotoneChainSelectAction.cpp 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/index/chain/MonotoneChainSelectAction.cpp 2009-05-06 16:44:06 UTC (rev 2476)
@@ -39,7 +39,7 @@
void
MonotoneChainSelectAction::select(MonotoneChain& mc, unsigned int start)
{
- mc.getLineSegment(start, selectedSegment);
+ mc.getLineSegment(start, *selectedSegment);
select(selectedSegment);
}
Modified: trunk/source/noding/MCIndexNoder.cpp
===================================================================
--- trunk/source/noding/MCIndexNoder.cpp 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/noding/MCIndexNoder.cpp 2009-05-06 16:44:06 UTC (rev 2476)
@@ -72,7 +72,7 @@
MonotoneChain* queryChain = *i;
assert(queryChain);
vector<void*> overlapChains;
- index.query(queryChain->getEnvelope(), overlapChains);
+ index.query(&(queryChain->getEnvelope()), overlapChains);
for (vector<void*>::iterator
j=overlapChains.begin(), jEnd=overlapChains.end();
j != jEnd;
@@ -117,7 +117,7 @@
assert(mc);
mc->setId(idCounter++);
- index.insert(mc->getEnvelope(), mc);
+ index.insert(&(mc->getEnvelope()), mc);
// MonotoneChain objects deletion delegated to destructor
monoChains.push_back(mc);
Modified: trunk/source/noding/MCIndexSegmentSetMutualIntersector.cpp
===================================================================
--- trunk/source/noding/MCIndexSegmentSetMutualIntersector.cpp 2009-05-06 16:36:14 UTC (rev 2475)
+++ trunk/source/noding/MCIndexSegmentSetMutualIntersector.cpp 2009-05-06 16:44:06 UTC (rev 2476)
@@ -45,7 +45,7 @@
{
MonotoneChain * mc = (*segChains)[i];
mc->setId(indexCounter++);
- index->insert(mc->getEnvelope(), mc);
+ index->insert(&(mc->getEnvelope()), mc);
}
}
@@ -59,7 +59,7 @@
MonotoneChain * queryChain = (MonotoneChain *)((*monoChains)[i]);
std::vector<void*> overlapChains;
- index->query( queryChain->getEnvelope(), overlapChains);
+ index->query( &(queryChain->getEnvelope()), overlapChains);
for (std::size_t j = 0, nj = overlapChains.size(); j < nj; j++)
{
More information about the geos-commits
mailing list