[geos-devel] Problems creating polygons with holes

strk strk at keybit.net
Tue Jun 29 18:47:54 EDT 2004


MultiPolygon constructor (as of GEOS version 1.0) take
ownership of Geometry * in the passed vector.
This might be your problem.
Martin Davis suggests users learn this, so here is an hint:
	geom_copy = geom->clone()
GEOS-CVS changes this, deep copying passed geometries.
Next GEOS release might change the whole copying mechanism,
but we are currently discussing how.

--strk;

On Tue, Jun 29, 2004 at 08:53:22AM +0100, Darren Carman wrote:
> I changed the polygons and holes vectors to be vector <LinearRing *>, and changed the createPolygon calls to (LinearRing *). This got me past the seg fault. 
> 
> I now seg fault after creating the MultiPolygon and trying the toString function.
> 
> What type should I be using for this (I have tried MultiPolygon, LinearRing and Geometry with no luck) or should polygons be a different type (didnt work for Geometry or LinearRing vectors)?
> 
> >From Sub-problem1&2:
> Thanks for that. In the actual code it is accounted for, but as I only had 1 hole in this example I didnt bother changing it.
> 
> Thanks,
> Darren
> 
> -----Original Message-----
> From: strk [mailto:strk at keybit.net]
> Sent: 28 June 2004 21:37
> To: Darren Carman; GEOS Development List
> Cc: Paul Ramsey; Yury A. Bychkov
> Subject: Re: [geos-devel] Problems creating polygons with holes
> 
> 
> Ok. I've found the bug.
> It is in your code (and in GEOS not handling it).
> 
> Main problem: you push a Polygon in the holes vector.
> This - although defined as a Geometry vector - is indeed a LinearRing
> vector, failing to enforce this typing does result in segfault
> (not checked in Polygon.cpp).
> 
> Sub-problem1: your holes vector is an automatic variable, so it
> is deleted when the code exits the block it is defined in.
> The Polygon constructor copy the vector pointer and will delete it
> at Polygon destructio time (I'd change this).
> 
> Sub-problem2: if I understood your code you are pushing in the
> holes vector the geometries you previously created, with no cloning.
> Again the Polygon constructor will take ownership of them (which should
> be LinearRing) so you should ->clone() them before adding to the vector.
> 
> My proposal: make Polygon contructor deep copy shell and holes checking
> for holes type to be LinearRing.
> 
> A question: why isn't holes a vector <LinearRing *> * ?
> 
> A note: I did not update CVS... I'd like to hear about my proposal first.
> 
> --strk;
> 
> On Mon, Jun 28, 2004 at 09:58:54PM +0200, strk wrote:
> > I could not find the problem.
> > Here is a reduced code exploiting the error.
> > I'll keep inspecting this...
> > 
> > BTW: I don't feel comportable in Polygon constructor taking
> > ownership of passed arguments, what people think about modifying
> > this as well ?
> > 
> > --strk;
> > 
> > //--------------------------8<-------------------
> > 
> > #include <geos/io.h>
> > #include <geos/geom.h>
> > #include <geos/util.h>
> > 
> > using namespace std;
> > using namespace geos;
> > 
> > int main(int argc, char *argv[]) {
> >         Coordinate c;
> >         CoordinateList *cl1 = CoordinateListFactory::internalFactory->createCoordinateList();
> >         CoordinateList *cl2 = CoordinateListFactory::internalFactory->createCoordinateList();
> > 
> >         c.x = 339252; c.y = 1019302; cl1->add(c);
> >         c.x = 339252; c.y = 1029077; cl1->add(c);
> >         c.x = 348752; c.y = 1029077; cl1->add(c);
> >         c.x = 346102; c.y = 1019653; cl1->add(c);
> >         c.x = 339252; c.y = 1019302; cl1->add(c);
> > 
> >         c.x = 340000; c.y = 1020000; cl2->add(c);
> >         c.x = 340000; c.y = 1021000; cl2->add(c);
> >         c.x = 341000; c.y = 1021000; cl2->add(c);
> >         c.x = 341000; c.y = 1020000; cl2->add(c);
> >         c.x = 340000; c.y = 1020000; cl2->add(c);
> > 
> >         try {
> >                 GeometryFactory *gf=new GeometryFactory(new PrecisionModel(),0);
> > 
> >                 Geometry *geom1=gf->createPolygon(gf->createLinearRing(cl1),NULL);
> >                 cout<<"geom1: "<<geom1->toString()<<endl;
> >                 Geometry *geom2=gf->createPolygon(gf->createLinearRing(cl2),NULL);
> >                 cout<<"geom2: "<<geom2->toString()<<endl;
> >                 vector<Geometry*>*holes = new vector<Geometry*>(1);
> >                 (*holes)[0] = geom2->clone();
> >                 Geometry *geom3=gf->createPolygon(gf->createLinearRing(cl1),holes);
> >                 cout<<"geom3: "<<geom3->toString()<<endl;
> >                 delete cl1;
> >                 delete cl2;
> >         } catch (GEOSException *ge) {
> >                 cout << "ERROR: " << ge->toString() << endl;
> >         } catch (...) {
> >                 cout << "generic ERROR" << endl;
> >         }
> > }
> > 
> > //--------------------------8<-------------------
> > 
> > 
> > On Mon, Jun 28, 2004 at 11:47:17AM +0100, Darren Carman wrote:
> > > #include <geos/io.h>
> > > #include <geos/geom.h>
> > > 
> > > using namespace std;
> > > using namespace geos;
> > > 
> > > typedef struct boundaryPt {
> > >   double x;
> > >   double y;
> > > };
> > > typedef vector<boundaryPt>    boundary;
> > > typedef vector<boundary>      boundaryList;
> > > typedef vector<boundaryList>  polygonList;
> > > 
> > > polygonList       ptPolygons;
> > > 
> > > int main(int argc, char *argv[]) {
> > >   vector<Geometry *> polygons;
> > >   vector<Geometry *> holes;
> > >   int i,j,k;
> > >   string resStr = "";
> > > 
> > >   boundaryPt poly1[5];
> > >   boundaryPt hole1[5];
> > >   boundary tmpBoundary;
> > >   boundaryList tmpBoundaryList;
> > >   polygonList ptPolygons;
> > > 
> > >   poly1[0].x = 339252;
> > >   poly1[0].y = 1019302;
> > >   poly1[1].x = 339252;
> > >   poly1[1].y = 1029077;
> > >   poly1[2].x = 348752;
> > >   poly1[2].y = 1029077;
> > >   poly1[3].x = 346102;
> > >   poly1[3].y = 1019653;
> > >   poly1[4].x = 339252;
> > >   poly1[4].y = 1019302;
> > > 
> > >   hole1[0].x = 340000;
> > >   hole1[0].y = 1020000;
> > >   hole1[1].x = 340000;
> > >   hole1[1].y = 1021000;
> > >   hole1[2].x = 341000;
> > >   hole1[2].y = 1021000;
> > >   hole1[3].x = 341000;
> > >   hole1[3].y = 1020000;
> > >   hole1[4].x = 340000;
> > >   hole1[4].y = 1020000;
> > > 
> > >   for (i=0; i<5; i++) {
> > >     tmpBoundary.push_back(poly1[i]);
> > >   }
> > >   tmpBoundaryList.push_back(tmpBoundary);
> > >   tmpBoundary.clear();
> > > 
> > >   for (i=0; i<5; i++) {
> > >     tmpBoundary.push_back(hole1[i]);
> > >   }
> > >   tmpBoundaryList.push_back(tmpBoundary);
> > >   tmpBoundary.clear();
> > > 
> > >   ptPolygons.push_back(tmpBoundaryList);
> > >   for(int i=0; i< static_cast<int>(tmpBoundaryList.size()); i++) {
> > >     tmpBoundaryList[i].clear();
> > >   }
> > >   tmpBoundaryList.clear();
> > > 
> > > 
> > >   try{
> > >     GeometryFactory *gf=new GeometryFactory(new PrecisionModel(),0);
> > > 
> > >     // for each outer ring create a CoordinateList and add all the points to it
> > >     for(i=0; i< static_cast<int>(ptPolygons.size()); i++) {
> > >       CoordinateList *cl1=CoordinateListFactory::internalFactory->createCoordinateList();
> > >       boundaryList bl = ptPolygons[i];
> > >       boundary b = bl[0];
> > >       cout << "Polygon " << i << endl;
> > >       for(k=0; k< static_cast<int>(b.size()); k++) {
> > >         cout << "Added point " << b[k].x << ',' << b[k].y << endl;
> > >         cl1->add(*(new Coordinate(b[k].x,b[k].y)));
> > >       }
> > > 
> > >       // for each hole create a coordinate list like above, create a polygon and add to a Geometry vector
> > >       for(j=1; j< static_cast<int>(bl.size()); j++) {
> > >         cout << "Hole " << j << endl;
> > >         boundary b = bl[j];
> > >         CoordinateList *cl2=CoordinateListFactory::internalFactory->createCoordinateList();
> > >         for(k=0; k< static_cast<int>(b.size()); k++) {
> > >           cout << "Added point " << b[k].x << ',' << b[k].y << endl;
> > >           cl2->add(*(new Coordinate(b[k].x,b[k].y)));
> > >         }
> > >         cout << "Creating polygon for hole " << j << ": " << cl2->toString() << endl;
> > >         Geometry *geom1=gf->createPolygon(gf->createLinearRing(cl2),NULL);
> > >         cout << "Geometry " << j << " : " << geom1->toString() << endl;
> > >         holes.push_back(geom1);
> > >         delete cl2;
> > >       }
> > >       // create a polygon from the 1st CoordinateList and the holes
> > >       cout << "Creating polygon " << i << ": " << cl1->toString() << endl;
> > >       Geometry *geom2=gf->createPolygon(gf->createLinearRing(cl1),&holes);                                             
> > >       cout << "test " << endl;
> > >       cout << "Geometry " << i << " : " << geom2->toString() << endl;
> > > 
> > >       // save this polygon in another vector
> > >       cout << "Adding polygon " << i << endl;
> > >       polygons.push_back(geom2);
> > >       delete cl1;
> > >     }
> > >     cout << "Creating multi-polygon" << endl;
> > >     // when all done create a MultiPolygon object
> > >     Geometry *geom3=gf->createMultiPolygon(&polygons);
> > >     cout << "Creating string" << endl;
> > >     resStr = geom3->toString();
> > > 
> > >     //clean up
> > >     delete geom3;
> > >     delete gf;
> > >   }
> > >   catch (GEOSException *ge) {
> > >     cout << "ERROR: " << ge->toString() << endl;
> > >   }
> > >   cout << resStr << endl;
> > > }
> > > 
> > > -----Original Message-----
> > > From: strk [mailto:strk at keybit.net]
> > > Sent: 28 June 2004 11:28
> > > To: GEOS Development List
> > > Subject: Re: [geos-devel] Problems creating polygons with holes
> > > 
> > > 
> > > Can you produce a neter code exploiting the bug ?
> > > I mean a .cpp file I can compile and run and debug :)
> > > 
> > > -strk;
> > > 
> > > On Mon, Jun 28, 2004 at 11:05:22AM +0100, Darren Carman wrote:
> > > > defs:
> > > > typedef struct boundaryPt {
> > > >   double x;
> > > >   double y;
> > > > };
> > > > typedef vector<boundaryPt>    boundary;
> > > > typedef vector<boundary>      boundaryList;
> > > > typedef vector<boundaryList>  polygonList;
> > > > 
> > > > class members used:
> > > >   polygonList       ptPolygons;
> > > >   string tmpStr;
> > > >   string errMsg;
> > > > 
> > > > class function:
> > > > string c_polygon_gml::WKTString(void) {
> > > >   TRACE_FUNC("c_polygon_gml::WKTString");
> > > >   vector<Geometry *> polygons;
> > > >   vector<Geometry *> holes;
> > > >   int i,j,k;
> > > >   string resStr = "";
> > > > 
> > > >   try{
> > > >     GeometryFactory *gf=new GeometryFactory(new PrecisionModel(),0);
> > > > 
> > > >     // for each outer ring create a CoordinateList and add all the points to it
> > > >     for(i=0; i< static_cast<int>(ptPolygons.size()); i++) {
> > > >       CoordinateList *cl1=CoordinateListFactory::internalFactory->createCoordinateList();
> > > >       boundaryList bl = ptPolygons[i];
> > > >       boundary b = bl[0];
> > > >       TRACE_OUTPUT("Polygon " << i);
> > > >       for(k=0; k< static_cast<int>(b.size()); k++) {
> > > >         TRACE_OUTPUT("Added point " << b[k].x << ',' << b[k].y);
> > > >         cl1->add(*(new Coordinate(b[k].x,b[k].y)));
> > > >       }
> > > > 
> > > >       // for each hole create a coordinate list like above, create a polygon and add to a Geometry vector
> > > >       for(j=1; j< static_cast<int>(bl.size()); j++) {
> > > >         TRACE_OUTPUT("Hole " << j);
> > > >         boundary b = bl[j];
> > > >         CoordinateList *cl2=CoordinateListFactory::internalFactory->createCoordinateList();
> > > >         for(k=0; k< static_cast<int>(b.size()); k++) {
> > > >           TRACE_OUTPUT("Added point " << b[k].x << ',' << b[k].y);
> > > >           cl2->add(*(new Coordinate(b[k].x,b[k].y)));
> > > >         }
> > > >         TRACE_OUTPUT("Creating polygon for hole " << j << ": " << cl2->toString());
> > > >         Geometry *geom1=gf->createPolygon(gf->createLinearRing(cl2),NULL);
> > > >         TRACE_OUTPUT("Geometry " << j << " : " << geom1->toString());
> > > >         holes.push_back(geom1);
> > > >         delete cl2;
> > > >       }
> > > >       // create a polygon from the 1st CoordinateList and the holes
> > > >       TRACE_OUTPUT("Creating polygon " << i << ": " << cl1->toString());
> > > >       Geometry *geom2=gf->createPolygon(gf->createLinearRing(cl1),&holes);
> > > >       TRACE_OUTPUT("test ");
> > > >       TRACE_OUTPUT("Geometry " << i << " : " << geom2->toString());
> > > > 
> > > >       // save this polygon in another vector
> > > >       TRACE_OUTPUT("Adding polygon " << i);
> > > >       polygons.push_back(geom2);
> > > >       delete cl1;
> > > >     }
> > > >     TRACE_OUTPUT("Creating multi-polygon");
> > > >     // when all done create a MultiPolygon object
> > > >     Geometry *geom3=gf->createMultiPolygon(&polygons);
> > > >     TRACE_OUTPUT("Creating string");
> > > >     resStr = geom3->toString();
> > > > 
> > > >     //clean up
> > > >     delete geom3;
> > > >     delete gf;
> > > >   }
> > > >   catch (GEOSException *ge) {
> > > >     errMsg = ge->toString();
> > > >   }
> > > >   return resStr;
> > > > }
> > > > 
> > > > XML read in by class and stored in ptPolygons:
> > > > <?xml version="1.0" encoding="UTF-8"?>
> > > > <AreaDefinition>
> > > > <GDSRequest>
> > > > <query>
> > > > <AreaQuery outputContent="public">
> > > > <queryArea>
> > > > <Polygon srsName="osgb:BNG">
> > > > <outerBoundaryIs>
> > > > <LinearRing>
> > > > <coordinates>
> > > > 339252,1019302
> > > > 339252,1029077
> > > > 348752,1029077
> > > > 346102,1019653
> > > > 339252,1019302
> > > > </coordinates>
> > > > </LinearRing>
> > > > </outerBoundaryIs>
> > > > <innerBoundaryIs>
> > > > <LinearRing>
> > > > <coordinates>
> > > > 340000,1020000
> > > > 340000,1021000
> > > > 341000,1021000
> > > > 341000,1020000
> > > > 340000,1020000
> > > > </coordinates>
> > > > </LinearRing>
> > > > </innerBoundaryIs>
> > > > </Polygon>
> > > > </queryArea>
> > > > </AreaQuery>
> > > > </query>
> > > > </GDSRequest>
> > > > </AreaDefinition>
> > > > 
> > > > Output:
> > > > TR: TRACE ENTRY:c_polygon_gml::WKTString line 168 file c_polygon_gml.cpp
> > > > TR:   Polygon 0
> > > > TR:   Added point 339252,1.0193e+06
> > > > TR:   Added point 339252,1.02908e+06
> > > > TR:   Added point 348752,1.02908e+06
> > > > TR:   Added point 346102,1.01965e+06
> > > > TR:   Added point 339252,1.0193e+06
> > > > TR:   Hole 1
> > > > TR:   Added point 340000,1.02e+06
> > > > TR:   Added point 340000,1.021e+06
> > > > TR:   Added point 341000,1.021e+06
> > > > TR:   Added point 341000,1.02e+06
> > > > TR:   Added point 340000,1.02e+06
> > > > TR:   Creating polygon for hole 1: (340000,1.02e+06,1.7e-308) (340000,1.021e+06,1.7e-308) (341000,1.021e+06,1.7e-308) (341000,1.02e+06,1.7e-308) (340000,1.02e+06,1.7e-308)
> > > > TR:   Geometry 1 : POLYGON ((340000.0000000000000000 1020000.0000000000000000, 340000.0000000000000000 1021000.0000000000000000, 341000.0000000000000000 1021000.0000000000000000, 341000.0000000000000000 1020000.0000000000000000, 340000.0000000000000000 1020000.0000000000000000))
> > > > TR:   Creating polygon 0: (339252,1.0193e+06,1.7e-308) (339252,1.02908e+06,1.7e-308) (348752,1.02908e+06,1.7e-308) (346102,1.01965e+06,1.7e-308) (339252,1.0193e+06,1.7e-308)
> > > > TR:   test
> > > > Segmentation fault
> > > > 
> > > > Output when the hole is removed:
> > > > TR: TRACE ENTRY:c_polygon_gml::WKTString line 168 file c_polygon_gml.cpp
> > > > TR:   Polygon 0
> > > > TR:   Added point 339252,1.0193e+06
> > > > TR:   Added point 339252,1.02908e+06
> > > > TR:   Added point 348752,1.02908e+06
> > > > TR:   Added point 346102,1.01965e+06
> > > > TR:   Added point 339252,1.0193e+06
> > > > TR:   Creating polygon 0: (339252,1.0193e+06,1.7e-308) (339252,1.02908e+06,1.7e-308) (348752,1.02908e+06,1.7e-308) (346102,1.01965e+06,1.7e-308) (339252,1.0193e+06,1.7e-308)
> > > > TR:   test
> > > > TR:   Geometry 0 : POLYGON ((339252.0000000000000000 1019302.0000000000000000, 339252.0000000000000000 1029077.0000000000000000, 348752.0000000000000000 1029077.0000000000000000, 346102.0000000000000000 1019653.0000000000000000, 339252.0000000000000000 1019302.0000000000000000))
> > > > TR:   Adding polygon 0
> > > > TR:   Creating multi-polygon
> > > > TR:   Creating string
> > > > TR: TRACE EXIT:c_polygon_gml::WKTString
> > > > ERROR:
> > > > WTF format
> > > > MULTIPOLYGON (((339252.0000000000000000 1019302.0000000000000000, 339252.0000000000000000 1029077.0000000000000000, 348752.0000000000000000 1029077.0000000000000000, 346102.0000000000000000 1019653.0000000000000000, 339252.0000000000000000 1019302.0000000000000000)))
> > > > 
> > > > 
> > > > -----Original Message-----
> > > > From: strk [mailto:strk at keybit.net]
> > > > Sent: 28 June 2004 11:01
> > > > To: GEOS Development List
> > > > Subject: Re: [geos-devel] Problems creating polygons with holes
> > > > 
> > > > 
> > > > Can you send the test code ?
> > > > There are currently no known bug ...
> > > > --strk;
> > > > 
> > > > On Mon, Jun 28, 2004 at 10:15:45AM +0100, Darren Carman wrote:
> > > > > Hi all,
> > > > > 
> > > > > I am creating polygons with holes and want to use geos to allow me to output them in WKT.
> > > > > 
> > > > > A polygon without a hole works fine, but as soon as I add a hole I get a segmentation fault after calling the createPolygon function (GeometryFactory) and trying to run toString on that polygon.
> > > > > 
> > > > > I am using version 1.0.
> > > > > 
> > > > > I have tried getting the latest version from CVS but this causes a segmentation fault to occur while adding the 5th polygon coordinate to the CoordinateList (there are only 4 coordinates in the test polygon, the 5th being the first one repeated - I tried making it 6 to see if it was a problem with closing the polygon but I still got a segmentation fault while adding the 5th).
> > > > > 
> > > > > Is there a fix for this, or has anyone come accross it?
> > > > > 
> > > > > Thanks in advance,
> > > > > Darren
> > > > > 
> > > > > 
> > > > > "The information in this e-mail and any attachment is confidential and may be privileged. If you have received this e-mail in error, please delete it immediately and destroy any copies on your system. You should not retain, copy or use this e-mail for any purpose, nor disclose all or any part of its content to any other person.
> > > > > Opinions expressed in this e-mail may not be endorsed by the company and unless explicitly indicated, this e-mail shall not form part of any binding agreement".
> > > > > 
> > > > > _______________________________________________
> > > > > geos-devel mailing list
> > > > > geos-devel at geos.refractions.net
> > > > > http://geos.refractions.net/mailman/listinfo/geos-devel
> > > > _______________________________________________
> > > > geos-devel mailing list
> > > > geos-devel at geos.refractions.net
> > > > http://geos.refractions.net/mailman/listinfo/geos-devel
> > > > _______________________________________________
> > > > geos-devel mailing list
> > > > geos-devel at geos.refractions.net
> > > > http://geos.refractions.net/mailman/listinfo/geos-devel
> > > _______________________________________________
> > > geos-devel mailing list
> > > geos-devel at geos.refractions.net
> > > http://geos.refractions.net/mailman/listinfo/geos-devel
> > > _______________________________________________
> > > geos-devel mailing list
> > > geos-devel at geos.refractions.net
> > > http://geos.refractions.net/mailman/listinfo/geos-devel
> > _______________________________________________
> > geos-devel mailing list
> > geos-devel at geos.refractions.net
> > http://geos.refractions.net/mailman/listinfo/geos-devel



More information about the geos-devel mailing list