Hi Larry, It is not easy to understand the changes you did, but I tried to review your code and have some questions : In shellsOverlap, there is the following condition :
CGAlgorithms.isPointInRing(testPt, coordList) || (pointInList(testPt, coordList))) I'm not sure about shapefile, but for SFS, I think that a hole cannot touch the exterior ring but two exterior ring can touch each other. So I should not consider pointInList(testPt, coordList) as a case of a shell inside another shell Otherwise, there are several assumptions I don't understand in the read method : if ((shells.size()>1) && (holes.size()== 0)) do you assume that when there is a shell inside another one, that means that every hole is decribed as a shell (holes.size()==0) ? shells.remove(realShell) holes.addAll(shells) do you assume that if one shell contains another shell, every shell except that one is a hole ? I think that there is only one shapefile type for polygon and multipolygon, so that if every ring are described as shells, some may be real shells and others may be holes I hope I did not misunderstand what the code is supposed to do. Thanks, Michaël Larry Becker a écrit : > Thanks for the quick answer, Martin. I've made a mod to > PolygonHandler in SkyJUMP and committed my change. You can browse it at: > > http://skyjump.cvs.sourceforge.net/skyjump/skyjump/org/geotools/shapefile/PolygonHandler.java?view=markup > > My mod begins in read() at "// quick optimization:". I used some of > the geotools 2.5 code to help with the update. In essence, I do a > check for ((shells.size()>1) && (holes.size()== 0)) before trying > anything new to minimize the impact of the change. Then I call the > shellsOverlap() method which returns the index of the first shell that > contains another. I then make the big assumption that all the rest > are holes. I think I can get away with this because the > assignHolesToShells() method converts any unassigned holes back to shells. > > I would appreciate any comments. > > regards, > Larry Becker > > > On Thu, Apr 3, 2008 at 12:44 PM, Martin Davis <[EMAIL PROTECTED] > <mailto:[EMAIL PROTECTED]>> wrote: > > Ugh. > > Larry, I think your approach is probably what's necessary. I would > suggest NOT trying to do a JTS contains() to implement the "wholly > inside" test - it would be very slow (although the new > PreparedGeometry > work would make it a lot faster - but it would still be slow). > > You might consider just check say 3 points of the ring to see if they > are wholly inside - if so, assume it is a hole. That should be a lot > faster. > > Larry Becker wrote: > > I've found a shapefile that has what JTS thinks is a topology > error of > > "overlapping shells". In ESRI ArcMap it displays correctly as a > shell > > polygon with a hole, but in JUMP, it displays as overlapping > > polygons. It fails the QA "Basic Topology" test. I have verified > > that the "hole" polygon is not CCW (counter clockwise) and this is > > being interpreted as a shell by the org.geotools.PolygonHandler. > > > > It looks like another case where ESRI isn't following their own > > specifications. Any suggestions? I don't like to "fix" customer's > > data when it works fine in their ESRI system. > > > > I'm considering modifying the PolygonHandler code to test all of the > > polygons in a multipolygon shape to determine if they are completely > > inside, and then reversing the point order to force CCW. This might > > make shapefiles read slightly slower. > > > > regards, > > > > Larry Becker > > > > -- > > http://amusingprogrammer.blogspot.com/ > > > ------------------------------------------------------------------------ > > > > > ------------------------------------------------------------------------- > > Check out the new SourceForge.net Marketplace. > > It's the best place to buy or sell services for > > just about anything Open Source. > > > > http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace > > > ------------------------------------------------------------------------ > > > > _______________________________________________ > > Jump-pilot-devel mailing list > > Jump-pilot-devel@lists.sourceforge.net > <mailto:Jump-pilot-devel@lists.sourceforge.net> > > https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel > > > > -- > Martin Davis > Senior Technical Architect > Refractions Research, Inc. > (250) 383-3022 > > > ------------------------------------------------------------------------- > Check out the new SourceForge.net Marketplace. > It's the best place to buy or sell services for > just about anything Open Source. > > http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace > _______________________________________________ > Jump-pilot-devel mailing list > Jump-pilot-devel@lists.sourceforge.net > <mailto:Jump-pilot-devel@lists.sourceforge.net> > https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel > > > > > -- > http://amusingprogrammer.blogspot.com/ > >------------------------------------------------------------------------ > >------------------------------------------------------------------------- >Check out the new SourceForge.net Marketplace. >It's the best place to buy or sell services for >just about anything Open Source. >http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace > >------------------------------------------------------------------------ > >_______________________________________________ >Jump-pilot-devel mailing list >Jump-pilot-devel@lists.sourceforge.net >https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel > > ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace _______________________________________________ Jump-pilot-devel mailing list Jump-pilot-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
