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

Thu Apr 23 23:38:20 BST 2009

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));
> +               }
> +       }
> +
>  }
>
>