Opened 9 years ago

Closed 9 years ago

#364 closed defect (fixed)

Changes to BufferBuilder.cpp & operations\valid

Reported by: tomatsafe Owned by: strk
Priority: major Milestone:
Component: Core Version: master
Severity: Annoyance Keywords: isvalid, valid, buffer, bufferbuilder
Cc:

Description

Safe Software has made changes to GEOS code and would like to commit it back to trunk.

A summary of the changes is below, and a patchfile is attached. We'll wait a few days for any responses.

BufferBuilder?.cpp

  • Fixed a crash caused by BufferBuilder::bufferLineSingleSided(..)

IsValidOp?.cpp, ConnectedInteriorTester?.cpp & TestValid2.xml

  • Fixed handling of some WKT Polygon EMPTY cases that would previously crash. (As spoken about with Martin Davis, Paul Ramsey, and Kevin Wiebe)
  • Added those 2 WKT cases to an existing xml test.

Attachments (1)

GEOS_SafeWork.patch (4.9 KB) - added by tomatsafe 9 years ago.
final revision of patch

Download all attachments as: .zip

Change History (4)

comment:1 Changed 9 years ago by strk

Few points:

 1: Did Martin Davis agree that POLYGON(<shell>, EMPTY, EMPTY, EMPTY) is to be considered
    valid ? Will this be reflected in JTS code ?
 2: Please respect the indenting you find in the modified pfiles
 3: The copyright notice seems to take more space than required, the usual way
    is just to add a line near the others. See top of src/operation/buffer/BufferBuilder.cpp
    for an example of that

Changed 9 years ago by tomatsafe

Attachment: GEOS_SafeWork.patch added

final revision of patch

comment:2 Changed 9 years ago by tomatsafe

I've attached another revision where copyright headers now match the style of existing ones and tabbing is respected on a per file basis.

I'd like to commit these changes in tomorrow under Stephen Wong's account as per Paul Ramsey's suggestion.

RE: WKT/POLYGON's, I can't speak for a port to JTS, but Martin Davis was included in the discussion about such cases, and we reached a reasonable conclusion that OGC spec was unspecific in this case, and that this would be a good interpretation. Support for this is that an EMPTY LinearRing? (*note, different from LineString) should be closed because EMPTY's should be valid. Thus a Polygon, which consists of LinearRings?, should not be made invalid only because it has EMPTY inner-shells.

comment:3 Changed 9 years ago by tomatsafe

Resolution: fixed
Status: newclosed

No more responses after a few days, the changes have now been submitted in revision #3087. Closing this ticket.

Note: See TracTickets for help on using tickets.