[postgis-users] shp2pgsql inner/outer ring detection bug FIX

[strk]

>From "ESRI Shapefile Technical Description" (An ESRI White Paper-July 1998):

   first-vertex must be equal to last-vertex in the following cases:
      - rings in Multipatch shapes
      - rings of PolygonZ shapes
      - rings of PolygonM shapes
      - rings of Polygon shapes

This is expressed in the "Notes" section for each shape type description:
> [..]
> The rings are closed (the first and last vertex of a ring MUST be the same).
> [..]

--strk;
~
strk wrote:
> I'm sorry but you got me wrong: the bug is NOT in postgis_fn.c, but
> in loader/shp2pgsql.c. There is a PIP function also there with the
> loop check running while i<n ... It is for the rest identical to
> the one you posted here (apart from vt declared inside the loop and
> itscomputatio casted to (float)).
> 
> I'm not familiar with shapelib code myself, I'll check the 
> last-vertex issue, however even if the last vertex != first vertex
> referencing V[n] will go past the allocated memory for the
> array... in this case shp2pgsql should check the value of i
> and using V[i], V[0] when i=n-1.
> 
> --strk;
> 
> dblasby wrote:
> > > Anyway for my case shapefile was not "dirty". The PIP function was ...
> > >
> > > Checking the same function in postgis_fn.c I've found the bug:
> > > the edge loop check has to be i<(n-1) instead of i<n
> > > Fixing this everything works fine.
> > 
> > I dont think this is correct - the V array has n points in it - V[0] ... V[n-1]
> > 
> > If you change "i<(n-1)" to "i<n" then (in the loop), "i" will eventually get to be
> > the value (n-1).
> > 
> > The V[i+1] reference will then be V[(n-1)+1] = V[n] which is past the end of the
> > array (bad news).
> > 
> > "i" is the edge number - there are (n-1) edges in a [PostGIS] polygon that has the
> > 1st point = last point.
> > 
> > Now, I've looked at the shapefile spec, and it doesnt seem to say that the first
> > point of a polygon must equal the last point.
> > 
> > So, I think the postgis code is correct (could you verify that for me?), but the
> > shapefile code maybe not checking the last edge (connecting the last point to the
> > first point).  I'm not familiar with the shapefile code....
> > 
> > dave
> > 
> > 
> > 
> > 
> > int PIP( POINT3D *P, POINT3D *V, int n )
> > {
> >     int    cn = 0;    // the crossing number counter
> >    int i;
> > 
> >  double vt;
> > 
> >     // loop through all edges of the polygon
> > 
> >      for (i=0; i< (n-1) ; i++)
> >  {    // edge from V[i] to V[i+1]
> > 
> >         if (((V[i].y <= P->y) && (V[i+1].y > P->y))    // an upward crossing
> >           || ((V[i].y > P->y) && (V[i+1].y <= P->y))) // a downward crossing
> >    {
> > 
> >            vt = (double)(P->y - V[i].y) / (V[i+1].y - V[i].y);
> >              if (P->x < V[i].x + vt * (V[i+1].x - V[i].x))    // P.x <intersect
> >                    ++cn;   // a valid crossing of y=P.y right of P.x
> >        }
> >     }
> >     return (cn&1);    // 0 if even (out), and 1 if odd (in)
> > 
> > }
> > 
> > 
> > _______________________________________________
> > postgis-users mailing list
> > postgis-users at postgis.refractions.net
> > http://postgis.refractions.net/mailman/listinfo/postgis-users
> 
> _______________________________________________
> postgis-users mailing list
> postgis-users at postgis.refractions.net
> http://postgis.refractions.net/mailman/listinfo/postgis-users