[geos-devel] Problems creating polygons with holes

Martin Davis mbdavis at VividSolutions.com
Mon Jun 28 16:50:09 EDT 2004


Strk:

IMO the current paradigm (that of constructors taking ownership of their
components) is the correct one.  The reason is to avoid unecessary
copying of argument data (which could be large).  It should be left up
to the caller to decide when and where they need to copy arguments.

Yes, this is a potential source of errors for clients, if they don't
understand how to properly use the library.  But the alternative forces
clients to tolerate extra overhead even when they don't need it, which
is not friendly behaviour from a library.

If this is really onerous, two versions of each constructor could be
provided.  Or possibly even better, a pattern & support class to make it
easy to copy arguments.

Martin Davis, Senior Technical Architect
Vivid Solutions Inc.      www.vividsolutions.com
Suite #1A-2328 Government Street Victoria, B.C. V8T 5G5
Phone: (250) 385 6040 - Local 308 Fax: (250) 385 6046


> -----Original Message-----
> From: strk [mailto:strk at keybit.net] 
> Sent: June 28, 2004 12:59 PM
> To: Darren Carman
> Cc: GEOS Development List
> Subject: Re: [geos-devel] Problems creating polygons with holes
> 
> 
> 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