Ticket #158 (closed defect: fixed)

Opened 6 years ago

Last modified 4 years ago

Some external rings dropped during buffer operation

Reported by: carl.anderson@… Owned by: hkaiser
Priority: major Milestone: 3.2.0
Component: Core Version: svn-trunk
Severity: Critical Keywords: imported,phpbugtracker
Cc: mloskot

Description (last modified by mloskot) (diff)

sample for XMLTester

<run>
<!--precisionModel scale="1.0" offsetx="0.0" offsety="0.0"/-->
<precisionModel type="FLOATING"/>
<case>
<desc>

Test One Outer ring is returned instead of two outer rings. One of the Polygons is not in the resultant set

</desc>
<a>
MULTIPOLYGON (((
            139 819, 
            485 185, 
            139 184, 
            139 819
        )), ((
            215 1191, 
            1620 1203, 
            1483 0, 
            215 1191
        )))
</a>
<test>
<desc>

Buffer One Outer ring is returned instead of two outer rings. One of the Polygons is not in the resultant set

</desc>

<op name="buffer" arg1="a" arg3="0.0">
MULTIPOLYGON (((
            527 725, 
            349 334, 
            128 193, 
            527 725
        )), ((
            215 1191, 
            1620 1203, 
            1483 0, 
            215 1191
        )))</op>
</test>
</case>
</run>

Attachments

u5small.xml Download (0.9 KB) - added by carl.anderson@… 6 years ago.
XMLTester file
SubgrahpDepthLocater.patch Download (2.5 KB) - added by carl.anderson@… 6 years ago.
patch with a incomplete hack to fix and lots of comments

Change History

Changed 6 years ago by carl.anderson@…

XMLTester file

Changed 6 years ago by carl.anderson@…

patch with a incomplete hack to fix and lots of comments

Changed 6 years ago by carl.anderson@…

apply the patch to make SubgraphDepthLocater more chatty

Notice that the LineSegments pushed back into the stabbedSegments list are NOT the same

as the values retrieved from the list

Need to fix this by adding a deep copy to the constructor for DepthSegment


bash-3.2$ XMLTester -v u5small.xml
 SubgraphDepthLocater::findStabbedSegments:  evaluate segment 0 (215 1191 1620 1203)  segment above or below stabbing line, skipping 
 SubgraphDepthLocater::findStabbedSegments:  evaluate segment 1 (1620 1203 1483 0)      depth: 1  right:1, left:0 swapped:1
     push back: (LINESEGMENT(1483 0,1620 1203) 1)
 SubgraphDepthLocater::findStabbedSegments:  evaluate segment 2 (1483 0 215 1191)      depth: 0  right:1, left:0 swapped:0
     push back: (LINESEGMENT(1483 0,215 1191) 0)
 SubgraphDepthLocater::DepthSegmentLessThen LINESEGMENT(1483 0,215 1191):0  LINESEGMENT(1483 0,215 1191):1 == 0
 SubgraphDepthLocater::getDepth()
   found these stabbed segments
    (0: LINESEGMENT(1483 0,215 1191) 0)
    (1: LINESEGMENT(1483 0,215 1191) 1)

SubgraphDepthLocater::getDepth(485 185): 0




Changed 6 years ago by carl.anderson@…

Issue was that DepthSegment was not createing a copy of the LineSegment and was only cachign the pointer passed.

Except that findStabbedSegments was repeatedly initializing DepthSegments using the same var pointer.

This patch causes a copy of the LineSegment to be made.
Other corrective techniques exist.


--- SNIP ---
--- source/operation/buffer/SubgraphDepthLocater.cpp.orig       2007-02-21 05:14:20.000000000 -0500
+++ source/operation/buffer/SubgraphDepthLocater.cpp    2007-07-23 21:36:16.000000000 -0400
@@ -55,7 +55,7 @@
 
 private:
 
-       geom::LineSegment& upwardSeg;
+       geom::LineSegment upwardSeg;
 
        /*
         * Compare two collinear segments for left-most ordering.
@@ -81,12 +81,14 @@
        int leftDepth;
 
        DepthSegment(geom::LineSegment &seg, int depth)
-               :
-               upwardSeg(seg),
-               leftDepth(depth)
+//             :
+//             upwardSeg(seg),
+//             leftDepth(depth)
        {
                // input seg is assumed to be normalized
                //upwardSeg.normalize();
+               leftDepth = depth;
+               upwardSeg = seg;
        }
 
        /**




Changed 6 years ago by mloskot

  • cc mloskot added
  • description modified (diff)

Changed 6 years ago by mloskot

  • milestone imported deleted

Changed 5 years ago by mloskot

  • priority changed from 1 to major
  • version changed from 3.0.0 to svn-trunk
  • component changed from Build scripts to Core
  • milestone set to 3.0.0

Changed 5 years ago by mloskot

  • owner strk@… deleted
  • status changed from assigned to new

Changed 4 years ago by anonymous

  • milestone 3.0.1 deleted

Milestone 3.0.1 deleted

Changed 4 years ago by pramsey

  • owner set to pramsey
  • milestone set to 3.1.0

Changed 4 years ago by pramsey

  • milestone changed from 3.1.0 to 3.2.0

I'm not competent to work this, kicking to 3.2.

Changed 4 years ago by pramsey

  • owner changed from pramsey to hkaiser

Changed 4 years ago by strk

  • status changed from new to closed
  • resolution set to fixed

Oh my god. I've spent *hours* to figure this out, and Carl already found it ! See bug #244.

Well, Carl, happy to have a double-blind confirmation the patch was correct then :) I assume this is fixed, w/out even checking.

Changed 4 years ago by warmerdam

It appears the relavent patch was r2503 in trunk, I see no sign that it has been backported.

Note: See TracTickets for help on using tickets.