Hi Steve, thanks for looking into this. I also tried to find a sample case where this code is needed, without success, but that is probably caused by the filters. Still, I also think we can remove the code, the test will only find cases where the offset doesn't fit into e.g. 24 bits for resolution 24, but the limit is much smaller - I think 32768, so 15 bits - and it is tested again later in MapObject: protected void setDeltaLong(int deltaLong) { assert deltaLong >= -0x8000 && deltaLong <= 0x7fff : "deltaLong = " + deltaLong; this.deltaLong = deltaLong; } Gerd ________________________________ Von: mkgmap-dev <mkgmap-dev-bounces@lists.mkgmap.org.uk> im Auftrag von Steve Ratcliffe <steve@parabola.me.uk> Gesendet: Mittwoch, 20. Juli 2016 23:30:24 An: Development list for mkgmap Betreff: Re: [mkgmap-dev] Bit operatons in mkgmap Hi Gerd
I'd like to change this code in LinePreparer (and a copy in LinePreparerFilter)
// XXX: relies on the fact that java integers are 32 bit signed final int offset = 8+shift; to
final int offset = (Integer.SIZE - 24) + shift;
As you see I assume the value 8 is the result of 32 - 24. and I guess that 24 is the maximum resolution. Maybe we have a proper constant for that ?
Yes I think that it is supposed to be the difference between the 24 bit size of the Garmin word and the 32 bit java int. But... does the following code that uses that value actually work? int dx = (lon - lastLong) << offset >> offset; int dy = (lat - lastLat) << offset >> offset; I assume that the intent is to sign extend the value, but the way the value is calculated they should be already properly signed values. If the dx, dy values are too big, they will be corrupted by the shift operations. However the following assertions: assert (dx == 0 && lon != lastLong) == false: ("delta lon too large: " + (lon - lastLong)); assert (dy == 0 && lat != lastLat) == false: ("delta lat too large: " + (lat - lastLat)); claim to be a test for the delta being too large, but this cannot work either. But that is just looking at the code... I added a test to see if there was ever a difference between the plain (lon - lastLong) value and the double shifted value. None found on compiling a UK map. So I think that 'offset' can be removed entirely unless you can see a case where it is needed? ..Steve _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev