[mkgmap-dev] Commit: r996: Fix rounding in two places.

Thu Apr 30 20:50:13 BST 2009

Hi Ben,

yes, my patch was only the one half and your solution is more correct. 
But also not 100%. While looking at your patch I see another small error:

Not the value of y0 should be compared to zero, but the result of  (y0 + 
t[0] * dy)

The best solution would be in my eyes a new constructor for a Coord 
object which accepts floats or doubles and does the rounding internal.

Regards,
Johann

Ben Konrath schrieb:
> Hi Steve,
>
> Just wondering if you've had a chance to look at this patch?
>
> Thanks, Ben
>
> On Mon, Apr 6, 2009 at 12:57 AM, Ben Konrath <ben at bagu.org> wrote:
>   
>> Hi Steve,
>>
>> I ran across a case that didn't get covered by Johann Gail patch. Please
>> find a patch against trunk attached to fix this issue along with a unit
>> test.
>>
>> As an aside, I noticed that ends[1] is calculated using y0 and x0 values,
>> not y1 and x1. I haven't read the algorithm on web page mentioned in the
>> comments, but I'm just wondering if that's correct?
>>
>> Thanks, Ben
>>
>> svn commit wrote:
>>     
>>> Version 996 was commited by steve on 2009-04-05 19:43:28 +0100 (Sun, 05
>>> Apr 2009)
>>> Fix rounding in two places.
>>>
>>> Before the patch clipping was a little inaccurate. Some points 2m outside
>>> the bounding box gets included in the img. With this patch it works
>>> correctly now.
>>>
>>> I've tested this since some days and up to now found no new failures.
>>> Whatever the reason for this small delta was, there should be no bad
>>> effects, if one adds a somewhat bigger delta.
>>>
>>> - Johann Gail
>>> _______________________________________________
>>> mkgmap-dev mailing list
>>> mkgmap-dev at lists.mkgmap.org.uk
>>> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
>>>       
>> diff --git a/src/uk/me/parabola/mkgmap/general/LineClipper.java
>> b/src/uk/me/parabola/mkgmap/general/LineClipper.java
>> index 55aa51d..76e730d 100644
>> --- a/src/uk/me/parabola/mkgmap/general/LineClipper.java
>> +++ b/src/uk/me/parabola/mkgmap/general/LineClipper.java
>> @@ -136,10 +136,10 @@ public class LineClipper {
>>
>>                double d = 0.5;
>>                if (t[0] > 0)
>> -                       ends[0] = new Coord((int) (y0 + t[0] * dy + d),
>> (int) (x0 + t[0] * dx + d));
>> +                       ends[0] = new Coord((int) (y0 + t[0] * dy + (y0 > 0
>> ? d : -d)), (int) (x0 + t[0] * dx + (x0 > 0 ? d : -d)));
>>
>>                if (t[1] < 1)
>> -                       ends[1] = new Coord((int)(y0 + t[1] * dy + d), (int)
>> (x0 + t[1] * dx + d));
>> +                       ends[1] = new Coord((int)(y0 + t[1] * dy + (y0 > 0 ?
>> d : -d)), (int) (x0 + t[1] * dx + (x0 > 0 ? d : -d)));
>>                return ends;
>>        }
>>
>> diff --git a/test/uk/me/parabola/mkgmap/general/LineClipperTest.java
>> b/test/uk/me/parabola/mkgmap/general/LineClipperTest.java
>> index 80fdbb9..85ab7a9 100644
>> --- a/test/uk/me/parabola/mkgmap/general/LineClipperTest.java
>> +++ b/test/uk/me/parabola/mkgmap/general/LineClipperTest.java
>> @@ -51,6 +51,11 @@ public class LineClipperTest {
>>                                new Coord(132, 230)
>>                };
>>                assertArrayEquals("example result", result,
>> listList.get(0).toArray());
>> +
>> +               // All points should be inside Area
>> +               for (List<Coord> list : listList) {
>> +                       assertTrue("all coords should be inside area",
>> a.allInside(list));
>> +               }
>>        }
>>
>>        /**
>> @@ -76,6 +81,10 @@ public class LineClipperTest {
>>                // No empty lists
>>                for (List<Coord> lco : list)
>>                        assertTrue("empty list", !lco.isEmpty());
>> +
>> +               // All points should be inside Area
>> +               for (List<Coord> lco : list)
>> +                       assertTrue("all coords should be inside area",
>> a.allInside(lco));
>>
>>                // Check values
>>                Coord[] firstExpectedLine = {
>> @@ -106,4 +115,25 @@ public class LineClipperTest {
>>                List<List<Coord>> list = LineClipper.clip(a, l);
>>                assertNull("all lines inside", list);
>>        }
>> +
>> +       /**
>> +        * This test comes from real osm data that caused a problem.
>> +        */
>> +       @Test
>> +       public void testAreaInSecondQuadrant() {
>> +               Area a = new Area(435809, -3916474, 436165, -3915960);
>> +               Coord[] co = {
>> +                               new Coord(436009, -3915955),
>> +                               new Coord(435956, -3916034),
>> +               };
>> +
>> +               List<List<Coord>> listList = LineClipper.clip(a,
>> Arrays.asList(co));
>> +               assertTrue("list should be empty", !listList.isEmpty());
>> +
>> +               // All points should be inside Area
>> +               for (List<Coord> list : listList) {
>> +                       assertTrue("all coords should be inside area",
>> a.allInside(list));
>> +               }
>> +       }
>> +
>>  }
>>
>>
>>     
> _______________________________________________
> mkgmap-dev mailing list
> mkgmap-dev at lists.mkgmap.org.uk
> http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
> .
>
>   