Clipping in TileSystem creates problems close to international date line

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Use MapView.getBoundingBox() around the international date line, for example 
to crop items before drawing them. Make sure the visible screen extends over 
the date line.
2. Validate the results (logging, etc)
3. The results are widely wrong because of clipping

What is the expected output? What do you see instead?
The bounding box to work properly.

What version of the product are you using? On what operating system?
3.0.8

Please provide any additional information below.
PixelXYToLatLong (and other methods in TileSystem) clips input points, but 
instead it should be wrapping them (i.e. 181 -> -179).

You could argue that input into these methods should be wrapped/limited 
already, but doing it here is actually a quite convenient place to do so and 
makes this class a bit more versatile.

New wrap method:
    /**
     * Returns a value that lies within <code>minValue</code> and <code>maxValue</code> by
     * subtracting/adding <code>interval</code>.
     * 
     * @param n
     *            the input number
     * @param minValue
     *            the minimum value
     * @param maxValue
     *            the maximum value
     * @param interval
     *            the interval length
     * @return a value that lies within <code>minValue</code> and <code>maxValue</code> by
     *         subtracting/adding <code>interval</code>
     */
    private static double wrap(double n, final double minValue, final double maxValue,
            final double interval) {
        if (minValue > maxValue)
            throw new IllegalArgumentException("minValue must be smaller than maxValue: "
                    + minValue + ">" + maxValue);
        if (interval > maxValue - minValue + 1)
            throw new IllegalArgumentException(
                    "interval must be equal or smaller than maxValue-minValue: " + "min: "
                            + minValue + " max:" + maxValue + " int:" + interval);
        while (n < minValue) {
            n += interval;
        }
        while (n > maxValue) {
            n -= interval;
        }
        return n;
    }

Enhanced existing methods:
    public static double GroundResolution(double latitude, final int levelOfDetail) {
        latitude = Clip(wrap(latitude, -90, 90, 180), MinLatitude, MaxLatitude);
        return Math.cos(latitude * Math.PI / 180) * 2 * Math.PI * EarthRadius
                / MapSize(levelOfDetail);
    }

    public static Point LatLongToPixelXY(double latitude, double longitude,
            final int levelOfDetail, final Point reuse) {
        final Point out = (reuse == null ? new Point() : reuse);

        latitude = Clip(wrap(latitude, -90, 90, 180), MinLatitude, MaxLatitude);
        longitude = wrap(longitude, MinLongitude, MaxLongitude, 360);

        final double x = (longitude + 180) / 360;
        final double sinLatitude = Math.sin(latitude * Math.PI / 180);
        final double y = 0.5 - Math.log((1 + sinLatitude) / (1 - sinLatitude)) / (4 * Math.PI);

        final int mapSize = MapSize(levelOfDetail);
        out.x = (int) Clip(x * mapSize + 0.5, 0, mapSize - 1);
        out.y = (int) Clip(y * mapSize + 0.5, 0, mapSize - 1);
        return out;
    }

BTW, the clippings in the end should not be required at all if you look closely 
at the vlaues.

    public static GeoPoint PixelXYToLatLong(final int pixelX, final int pixelY,
            final int levelOfDetail, final GeoPoint reuse) {
        final GeoPoint out = (reuse == null ? new GeoPoint(0, 0) : reuse);

        final double mapSize = MapSize(levelOfDetail);
        final double x = (wrap(pixelX, 0, mapSize - 1, mapSize) / mapSize) - 0.5;
        final double y = 0.5 - (wrap(pixelY, 0, mapSize - 1, mapSize) / mapSize);

        final double latitude = 90 - 360 * Math.atan(Math.exp(-y * 2 * Math.PI)) / Math.PI;
        final double longitude = 360 * x;

        out.setLatitudeE6((int) (latitude * 1E6));
        out.setLongitudeE6((int) (longitude * 1E6));
        return out;
    }

Another question would be why it restricts the latitude to +/-85.0511287. I 
don't think that's required but +/-90 should work.

Original issue reported on code.google.com by osei...@gmail.com on 20 May 2012 at 10:53

[GoogleCodeExporter]

Regarding your last question, read a bit on Mercator projection: "the Mercator 
projection distorts the size and shape of large objects, as the scale increases 
from the Equator to the poles, where it becomes infinite".

Original comment by neilboyd on 21 May 2012 at 8:07

[GoogleCodeExporter]

Do you have an easy way of demonstrating this issue?
And to make my life easier applying the suggested change, please could you make 
a patch :-)

Original comment by neilboyd on 24 May 2012 at 7:56

• Changed state: Accepted

[GoogleCodeExporter]

Will retry. Wasn't that straightforward.
How do I make a patch?

Original comment by osei...@gmail.com on 24 May 2012 at 8:00

[GoogleCodeExporter]

I use TortoiseSVN to make patches, but I'm sure there's other ways if you use 
other SVN tools.
Or you could just attached the changed file(s).

Original comment by neilboyd on 24 May 2012 at 8:03

[GoogleCodeExporter]

Ok (cheers to the Eclipse local history), to reproduce add the following 
logging statement to the getBoundingBox() method in MapView:

        logger.debug("getBoundingBox " + "centre: " + getScrollX() + ", screen <->: ("
                + screenRect.left + "," + screenRect.right + ")" + ", geo <->: ("
                + swGeoPoint.getLongitudeE6() + "," + neGeoPoint.getLongitudeE6() + ")");

Scrolling over 180 deg lon will get you something like the attached log file.

Original comment by osei...@gmail.com on 24 May 2012 at 8:19

Attachments:

• log.txt

[GoogleCodeExporter]

And here's the patch

Original comment by osei...@gmail.com on 24 May 2012 at 8:26

Attachments:

• TileSystem.java.patch

[GoogleCodeExporter]

Thanks. I'll have a look at it soon.

Original comment by neilboyd on 24 May 2012 at 9:00

• Changed state: Started

[GoogleCodeExporter]

TileSystem is a straight translation from the class referred to in it's header, 
so I don't want to change that.
I guess that means you should change the calling classes instead.

Original comment by neilboyd on 31 May 2012 at 8:16

[GoogleCodeExporter]

Well, as a matter of fact, the effected methods do not return correct results - 
for some input values (which do get passed in from osmdroid). But they fail to 
declare that and neither do any validation or throwing 
IllegalArgumentExceptions.
They do get called from a number of other places within the osmdroid, but 
potentially also from anybody referring to them not knowing they are broken.
So fixing the referring methods only helps if we did prevent calling the broken 
methods externally as well - or at least telling people they are not working 
correctly.

Suggestions?

A wrapper class around it that does longitude wrapping before passing input 
values through? I'm not that convinced.

Original comment by osei...@gmail.com on 1 Jun 2012 at 7:49

[GoogleCodeExporter]

I wonder what would happen if you pointed out the limitation of the original 
code. Perhaps they will be able to explain its intended use. There's a 
possibility to comment on it.
http://msdn.microsoft.com/en-us/library/bb259689.aspx

Original comment by neilboyd on 1 Jun 2012 at 8:23

[GoogleCodeExporter]

Ok, I've done two more patches, the first wraps TileSystem into a class that 
does all the wrapping and acts as a proxy to TileSystem and the second one does 
the wrapping separately externally.

Original comment by osei...@gmail.com on 2 Jun 2012 at 7:11

Attachments:

• WrappingTileSystem.patch
• ExternalWrapping.patch

[GoogleCodeExporter]

I prefer the first one, except I'd keep the same name so that only the import 
needs to change in the external classes. I've attached a patch modified like 
this.

Original comment by neilboyd on 3 Jun 2012 at 8:19

Attachments:

• wrap_TileSystem.patch

[GoogleCodeExporter]

Hi, thank you for this awesome library.
I encountered this issue while developing an app that draws a weather overlay 
on the map. Basically, the returned bounding box is incorrect.
Are you planning to have the fix released in the next version? That would be 
much appreciated.

Original comment by henri.la...@gmail.com on 7 Oct 2013 at 7:07

[GoogleCodeExporter]

Before I look into this again, can you tell me if the last patch fixes the 
problem? Then I can probably just apply that.

Original comment by neilboyd on 7 Oct 2013 at 8:20

[GoogleCodeExporter]

I have tested the patch.
It is better as the bounding box is now correct.
But, as far as I can tell, there is still an issue.
- if the longitude of the map center is west(negative ) then all is fine. The x 
coordinates of the points close to the International Date Line (IDL) are all 
negative.
- if the longitude of the map center is east (positive) then the points with a 
positive longitude are not drawn. Their x coordinate are positive for the 
eastern points and negative for the western ones.

Here is a simple project that illustrates this.
I try to draw a circle for 3 points:
- the first one has a longitude of 179E (+179),
- the second one is on the International Date Line (IDL) with a 180E longitude 
(+180)
- the third one has a 179W longitude (-179).

Main activity
=============
package com.hl.osmdroid_test_idl;

import android.os.Bundle;
import android.app.Activity;
import android.content.Context;
import android.view.Menu;

import org.osmdroid.api.IMapController;
import org.osmdroid.tileprovider.tilesource.TileSourceFactory;
import org.osmdroid.util.GeoPoint;
import org.osmdroid.views.MapView;
import org.osmdroid.views.overlay.Overlay;

import java.util.List;

public class MainActivity extends Activity {
    private Context mContext;
    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        mContext = getApplicationContext();        
        setContentView(R.layout.activity_main);

        // Configure Log4J
        try{
            ConfigureLog4J.configure();
        } catch (Exception e) {
            // do nothing
        }

        // Set MapView
        MapView mapView = (MapView) findViewById(R.id.mapview);
        mapView.setMultiTouchControls(true);
        mapView.setClickable(false);
        mapView.setBuiltInZoomControls(true);
        ((MapView)findViewById(R.id.mapview)).setTileSource(TileSourceFactory.MAPNIK); 
        IMapController mapController = mapView.getController();
        mapController.setZoom(6);

        // Center map on IDL
        GeoPoint centre = new GeoPoint(10*1000000, 180*1000000);
        mapController.setCenter(centre);

        // create map  overlay
        List<Overlay> mapOverlays = mapView.getOverlays();

        //Draw around the IDL
        IDLMapOverlay idlMapOverlay = new IDLMapOverlay(mContext);
         mapOverlays.add(idlMapOverlay);

    }

    @Override
    public boolean onCreateOptionsMenu(Menu menu) {
        // Inflate the menu; this adds items to the action bar if it is present.
        getMenuInflater().inflate(R.menu.main, menu);
        return true;
    }

}

IDLMapOverlay Overlay
=====================
package com.hl.osmdroid_test_idl;

import android.content.Context;
import android.graphics.Canvas;
import android.graphics.Color;
import android.graphics.Paint;
import android.graphics.Point;
import android.graphics.RectF;
import android.graphics.Paint.Style;
import android.util.Log;

import org.osmdroid.util.GeoPoint;
import org.osmdroid.views.MapView;
import org.osmdroid.views.overlay.Overlay;

public class IDLMapOverlay extends Overlay{
    private Paint p;

    public IDLMapOverlay(Context ctx) {
        super(ctx);
    }

    @Override
    protected void draw(Canvas c, MapView osmv, boolean shadow) {

        int lat1 = (int)(10*1000000);
        int lng1 = (int)(179*1000000);

        int lat2 = (int)(10*1000000);
        int lng2 = (int)(180*1000000);

        int lat3 = (int)(10*1000000);
        int lng3 = (int)(-179*1000000);

        GeoPoint point1 = new GeoPoint(lat1, lng1);
        GeoPoint point2 = new GeoPoint(lat2, lng2);
        GeoPoint point3 = new GeoPoint(lat3, lng3);
        GeoPoint mapCenter = (GeoPoint) osmv.getMapCenter();

        Point pt1 = osmv.getProjection().toPixels(point1, null);
        Point pt2 = osmv.getProjection().toPixels(point2, null);
        Point pt3 = osmv.getProjection().toPixels(point3, null);
        Point mc = osmv.getProjection().toPixels(mapCenter, null);

        Log.v("IDLMapOverlay","======================================");
        Log.v("IDLMapOverlay","mapCenter long = " + mapCenter.getLongitudeE6()/10000 +
                " mc.x " + mc.x);
        Log.v("IDLMapOverlay","point1 long = " + point1.getLongitudeE6()/10000 +
                " pt1.x " + pt1.x);
        Log.v("IDLMapOverlay","point2 long = " + point2.getLongitudeE6()/10000 +
            " pt2.x " + pt2.x);
        Log.v("IDLMapOverlay","point3 long = " + point3.getLongitudeE6()/10000 +
        " pt3.x " + pt3.x);

        //Initialize Paint
        p = new Paint();
        p.setStrokeWidth(4);
        p.setAntiAlias(true);
        p.setStyle(Style.STROKE);
        p.setColor(Color.RED);

        // Draw circles at points
        float r = (float) (10);
        RectF moval = new RectF(pt1.x - r, pt1.y - r, pt1.x + r,pt1.y  + r);
        c.drawOval(moval, p);
        moval = new RectF(pt2.x - r, pt2.y - r, pt2.x + r,pt2.y  + r);
        c.drawOval(moval, p);
        moval = new RectF(pt3.x - r, pt3.y - r, pt3.x + r, pt3.y  + r);
        c.drawOval(moval, p);
    }
 }

Here is the Log created while we pan the map around the IDL
10-08 22:05:38.624: V/IDLMapOverlay(4741): 
======================================
10-08 22:05:38.624: V/IDLMapOverlay(4741): mapCenter long = -17995 mc.x -8190
10-08 22:05:38.624: V/IDLMapOverlay(4741): point1 long = 17900 pt1.x -8238
10-08 22:05:38.624: V/IDLMapOverlay(4741): point2 long = 18000 pt2.x -8193
10-08 22:05:38.624: V/IDLMapOverlay(4741): point3 long = -17900 pt3.x -8146
10-08 22:05:38.664: V/IDLMapOverlay(4741): 
======================================
10-08 22:05:38.664: V/IDLMapOverlay(4741): mapCenter long = 17995 mc.x 8190
10-08 22:05:38.664: V/IDLMapOverlay(4741): point1 long = 17900 pt1.x 8146
10-08 22:05:38.664: V/IDLMapOverlay(4741): point2 long = 18000 pt2.x 8191
10-08 22:05:38.664: V/IDLMapOverlay(4741): point3 long = -17900 pt3.x -8146

I attach the project and 2 screen shots

Original comment by henri.la...@gmail.com on 8 Oct 2013 at 8:17

Attachments:

• device-2013-10-08-221023.png
• OSM_IDL_Test.zip

[GoogleCodeExporter]

I attached twice the same screenshot, ..; and I can't attach a new one ( 
attachment storage quota exceeded). Anyway, only 2 points are beeing drawn.

Original comment by henri.la...@gmail.com on 8 Oct 2013 at 8:21

[GoogleCodeExporter]

I see there's a patch attached to the end of issue 345 that seems to be 
related. Does that fix it?

Original comment by neilboyd on 9 Oct 2013 at 1:42

[GoogleCodeExporter]

Yes! After adding on top of the June 3rd, 2012 patch the  one attached at the 
end of issue 345, the issue seems to be fixed.

Original comment by henri.la...@gmail.com on 9 Oct 2013 at 2:16

[GoogleCodeExporter]

This issue was closed by revision r1364.

Original comment by neilboyd on 10 Oct 2013 at 7:11

• Changed state: Fixed

[GoogleCodeExporter]

Hi Neil,

Probably the new
import org.osmdroid.util.TileSystem;
has to be put at all classes that use directly the 
microsoft.mappoint.TileSystem ?
Like the MyLocationNewOverlay etc.

Regards.

Original comment by devemu...@gmail.com on 11 Oct 2013 at 7:53

[GoogleCodeExporter]

I thought about that, but I wanted to limit the scope of the change to the 
problem that was discussed. More investigation would be required for the other 
fixes. There's quite a lot of code that still uses the original one.

Original comment by neilboyd on 11 Oct 2013 at 10:52

[GoogleCodeExporter]

I see your point.
But doesn't the new TileSystem has to be put at least at MyLocationNewOverlay 
as it is already at MyLocationOverlay?
It seems to be used at the same places at both classes.

Regards.

Original comment by devemu...@gmail.com on 11 Oct 2013 at 11:32

[GoogleCodeExporter]

Ok, I've done that in revision 1366.

Original comment by neilboyd on 13 Oct 2013 at 6:28