Re: [mkgmap-dev] [Patch] Reduce bearingTo() calculations
Hi okay, safety first. Maybe Steve can say more about how important the precision It ultimately fits into a byte, so it is only accurate to about 1.5 degrees anyhow. I only saw a difference for the value 90 degrees when I tried WanMil's patch. But both are wrong in that they are only 179 degree apart instead of 180. Why not just reverse the integer value instead of suffering more rounding with floats? However I am not convinced that the original code is correct in what it is doing. If you have a road like this: B .---. C |\ | \ | \ | \ |* D | * A where * are routing nodes, and . are plain nodes. I thought that there were two directions, the initialHeading and the Bearing. The initialHeading was AB and the Bearing AD. (For the forward arc ABCD, for the reverse arc that would be DC and DA) However the code has initialHeading and finalHeading of AB and CD if I am reading it correctly. I could be wrong of course, since I only looked at the routing network very early on, and I'm not sure where this memory came from, since I never implemented the complex cases. ..Steve ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [Patch] Reduce bearingTo() calculations
Hi Steve, If I got you right, you think that the patch is ok but the maps produced with the trunk version might contain completely wrong values. I have no idea how one can find out what's right or wrong. Probably a comparison between garmin map data and our results? Gerd Steve Ratcliffe wrote Hi okay, safety first. Maybe Steve can say more about how important the precision It ultimately fits into a byte, so it is only accurate to about 1.5 degrees anyhow. I only saw a difference for the value 90 degrees when I tried WanMil's patch. But both are wrong in that they are only 179 degree apart instead of 180. Why not just reverse the integer value instead of suffering more rounding with floats? However I am not convinced that the original code is correct in what it is doing. If you have a road like this: B .---. C |\ | \ | \ | \ |* D | * A where * are routing nodes, and . are plain nodes. I thought that there were two directions, the initialHeading and the Bearing. The initialHeading was AB and the Bearing AD. (For the forward arc ABCD, for the reverse arc that would be DC and DA) However the code has initialHeading and finalHeading of AB and CD if I am reading it correctly. I could be wrong of course, since I only looked at the routing network very early on, and I'm not sure where this memory came from, since I never implemented the complex cases. ..Steve ___ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev -- View this message in context: http://gis.19327.n5.nabble.com/Patch-Reduce-bearingTo-calculations-tp5533880p5537549.html Sent from the Mkgmap Development mailing list archive at Nabble.com. ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [Patch] Reduce bearingTo() calculations
Hi Gerd If I got you right, you think that the patch is ok but the maps produced with the trunk version might contain completely wrong values. Yes. Well I am not sure, but it is worth investigating I think. The common case where the road only has two nodes is the same either way, and if the road is roughly straight there will be little difference either. I have no idea how one can find out what's right or wrong. Probably a Ideally we would have some visual way of displaying the routing graph. Perhaps we can write a .osm file to display it. ..Steve ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [Patch] Reduce bearingTo() calculations
Hi Steve, sorry, I have no idea what to do. Gerd Date: Mon, 5 Mar 2012 15:12:53 + From: st...@parabola.me.uk To: mkgmap-dev@lists.mkgmap.org.uk Subject: Re: [mkgmap-dev] [Patch] Reduce bearingTo() calculations Hi Gerd If I got you right, you think that the patch is ok but the maps produced with the trunk version might contain completely wrong values. Yes. Well I am not sure, but it is worth investigating I think. The common case where the road only has two nodes is the same either way, and if the road is roughly straight there will be little difference either. I have no idea how one can find out what's right or wrong. Probably a Ideally we would have some visual way of displaying the routing graph. Perhaps we can write a .osm file to display it. ..Steve ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [Patch] Reduce bearingTo() calculations
Hi Gerd, I have checked your patch by comparing old bearing to results with new results. I got lots of differences as you can see by trying attached patch. Can you please carefully check why there is a difference and which version is 'better'? WanMil Hi, in class RoadNetwork we calculate both p1.bearingTo(p2) and the reverse p2.bearingTo(p1). I think the reverse value should be calculated from the initial value. See attached patch. Gerd http://gis.19327.n5.nabble.com/file/n5533880/bearingTo.patch bearingTo.patch -- View this message in context: http://gis.19327.n5.nabble.com/Patch-Reduce-bearingTo-calculations-tp5533880p5533880.html Sent from the Mkgmap Development mailing list archive at Nabble.com. ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev Index: src/uk/me/parabola/mkgmap/general/RoadNetwork.java === --- src/uk/me/parabola/mkgmap/general/RoadNetwork.java (revision 2234) +++ src/uk/me/parabola/mkgmap/general/RoadNetwork.java (working copy) @@ -150,9 +150,21 @@ } } } -int forwardBearing = (int)lastCoord.bearingTo(bearingPoint); -int inverseForwardBearing = (int)bearingPoint.bearingTo(lastCoord); - +int orgFwBear = (int)lastCoord.bearingTo(bearingPoint); +int orgInvBear = (int)bearingPoint.bearingTo(lastCoord); + +double fwBrng = lastCoord.bearingTo(bearingPoint); +// reverse value +double invFwBrng = (fwBrng = 0) ? fwBrng + 180.0 : fwBrng -180; +int forwardBearing = (int)fwBrng ; +int inverseForwardBearing = (int)invFwBrng ; + +if (orgFwBear != forwardBearing || orgInvBear != inverseForwardBearing) { + log.error(Bearing differs); + log.error(Org: +orgFwBear+ +orgInvBear); + log.error(New: +forwardBearing+ +inverseForwardBearing); +} + bearingPoint = coordList.get(index - 1); if(co.equals(bearingPoint)) { // bearing point is too close to this node to be @@ -164,8 +176,11 @@ } } } -int reverseBearing = (int)co.bearingTo(bearingPoint); -int inverseReverseBearing = (int)bearingPoint.bearingTo(co); +double revBearing = co.bearingTo(bearingPoint); +double invRevBearing = (revBearing = 0) ? revBearing+180 : revBearing - 180; + +int reverseBearing = (int) revBearing; +int inverseReverseBearing = (int)invRevBearing; // Create forward arc from node1 to node2 RouteArc arc = new RouteArc(road.getRoadDef(), ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [Patch] Reduce bearingTo() calculations
Hi WanMil, the calculation in bearingTo() involves a lot of rounding errors because of the trig. functions. I verified that no value differed more than 0.5, which is good enough for us since we only use the integer value. See also http://www.movable-type.co.uk/scripts/latlong.html It says: For final bearing, simply take the initial bearing from the end point to the start point and reverse it (using θ = (θ+180) % 360). That's what I tried to implement. Gerd WanMil wrote Hi Gerd, I have checked your patch by comparing old bearing to results with new results. I got lots of differences as you can see by trying attached patch. Can you please carefully check why there is a difference and which version is 'better'? WanMil Hi, in class RoadNetwork we calculate both p1.bearingTo(p2) and the reverse p2.bearingTo(p1). I think the reverse value should be calculated from the initial value. See attached patch. Gerd http://gis.19327.n5.nabble.com/file/n5533880/bearingTo.patch bearingTo.patch -- View this message in context: http://gis.19327.n5.nabble.com/Patch-Reduce-bearingTo-calculations-tp5533880p5533880.html Sent from the Mkgmap Development mailing list archive at Nabble.com. ___ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev ___ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev -- View this message in context: http://gis.19327.n5.nabble.com/Patch-Reduce-bearingTo-calculations-tp5533880p5535815.html Sent from the Mkgmap Development mailing list archive at Nabble.com. ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [Patch] Reduce bearingTo() calculations
I don't know what's happening with the results of the bearing calculations. So as long as the results differ I don't want to commit that without a solid explanation that this is ok. WanMil Hi WanMil, the calculation in bearingTo() involves a lot of rounding errors because of the trig. functions. I verified that no value differed more than 0.5, which is good enough for us since we only use the integer value. See also http://www.movable-type.co.uk/scripts/latlong.html It says: For final bearing, simply take the initial bearing from the end point to the start point and reverse it (using θ = (θ+180) % 360). That's what I tried to implement. Gerd WanMil wrote Hi Gerd, I have checked your patch by comparing old bearing to results with new results. I got lots of differences as you can see by trying attached patch. Can you please carefully check why there is a difference and which version is 'better'? WanMil Hi, in class RoadNetwork we calculate both p1.bearingTo(p2) and the reverse p2.bearingTo(p1). I think the reverse value should be calculated from the initial value. See attached patch. Gerd http://gis.19327.n5.nabble.com/file/n5533880/bearingTo.patch bearingTo.patch -- View this message in context: http://gis.19327.n5.nabble.com/Patch-Reduce-bearingTo-calculations-tp5533880p5533880.html Sent from the Mkgmap Development mailing list archive at Nabble.com. ___ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev ___ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev -- View this message in context: http://gis.19327.n5.nabble.com/Patch-Reduce-bearingTo-calculations-tp5533880p5535815.html Sent from the Mkgmap Development mailing list archive at Nabble.com. ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Re: [mkgmap-dev] [Patch] Reduce bearingTo() calculations
Hi WanMil, okay, safety first. Maybe Steve can say more about how important the precision is. It is rounded again later and then writen to the *.img file. I assume that it is not a problem when the value differs a little bit. Gerd WanMil wrote I don't know what's happening with the results of the bearing calculations. So as long as the results differ I don't want to commit that without a solid explanation that this is ok. WanMil Hi WanMil, the calculation in bearingTo() involves a lot of rounding errors because of the trig. functions. I verified that no value differed more than 0.5, which is good enough for us since we only use the integer value. See also http://www.movable-type.co.uk/scripts/latlong.html It says: For final bearing, simply take the initial bearing from the end point to the start point and reverse it (using θ = (θ+180) % 360). That's what I tried to implement. Gerd WanMil wrote Hi Gerd, I have checked your patch by comparing old bearing to results with new results. I got lots of differences as you can see by trying attached patch. Can you please carefully check why there is a difference and which version is 'better'? WanMil Hi, in class RoadNetwork we calculate both p1.bearingTo(p2) and the reverse p2.bearingTo(p1). I think the reverse value should be calculated from the initial value. See attached patch. Gerd http://gis.19327.n5.nabble.com/file/n5533880/bearingTo.patch bearingTo.patch -- View this message in context: http://gis.19327.n5.nabble.com/Patch-Reduce-bearingTo-calculations-tp5533880p5533880.html Sent from the Mkgmap Development mailing list archive at Nabble.com. ___ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev ___ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev -- View this message in context: http://gis.19327.n5.nabble.com/Patch-Reduce-bearingTo-calculations-tp5533880p5535815.html Sent from the Mkgmap Development mailing list archive at Nabble.com. ___ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev ___ mkgmap-dev mailing list mkgmap-dev@.org http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev -- View this message in context: http://gis.19327.n5.nabble.com/Patch-Reduce-bearingTo-calculations-tp5533880p5535847.html Sent from the Mkgmap Development mailing list archive at Nabble.com. ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
[mkgmap-dev] [Patch] Reduce bearingTo() calculations
Hi, in class RoadNetwork we calculate both p1.bearingTo(p2) and the reverse p2.bearingTo(p1). I think the reverse value should be calculated from the initial value. See attached patch. Gerd http://gis.19327.n5.nabble.com/file/n5533880/bearingTo.patch bearingTo.patch -- View this message in context: http://gis.19327.n5.nabble.com/Patch-Reduce-bearingTo-calculations-tp5533880p5533880.html Sent from the Mkgmap Development mailing list archive at Nabble.com. ___ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev