moagrius / MapView

(Deprecated, prefer https://github.com/moagrius/TileView) Android widget roughly described as a hybrid between com.google.android.maps.MapView and iOS's CATiledLayer
http://moagrius.github.com/MapView/documentation
69 stars 35 forks source link

Intersection callculation issue #34

Closed frankowskid closed 11 years ago

frankowskid commented 11 years ago

I think that intersection is not calculated correctly. In zoomlevel.java you are scaling tile size(tileWidth * scale) but using basemap size to calculate tile row/column.

Updated version of method below:

public LinkedList<MapTile> getIntersections() {
int zoom = zoomManager.getZoom();
double scale = zoomManager.getRelativeScale();
double offsetWidth = tileWidth * scale;
double offsetHeight = tileHeight * scale;
LinkedList<MapTile> intersections = new LinkedList<MapTile>();
Rect boundedRect = new Rect( zoomManager.getViewport() );
boundedRect.top = Math.max( boundedRect.top, 0 );
boundedRect.left = Math.max( boundedRect.left, 0 );
boundedRect.right = Math.min( boundedRect.right, (int)(mapWidth*scale) );
boundedRect.bottom = Math.min( boundedRect.bottom, (int)(mapHeight*scale) );
int sr = (int) Math.floor( boundedRect.top / offsetHeight );
int er = (int) Math.ceil( boundedRect.bottom / offsetHeight );
int sc = (int) Math.floor( boundedRect.left / offsetWidth );
int ec = (int) Math.ceil( boundedRect.right / offsetWidth );
for ( int r = sr; r < er; r++ ) {
for ( int c = sc; c < ec; c++ ) {
MapTile m = new MapTile( zoom, r, c, tileWidth, tileHeight, pattern );
intersections.add( m );
}
}
return intersections;
}
moagrius commented 11 years ago

have you tested it?

frankowskid commented 11 years ago

yes, it works on my machine :)

In the snippet there was missing brackets - (int)(mapWidth*scale)

With orginal implementation on huge maps the tiles form the right and bottom - were not loaded at all. And after scaling map, on small zoom, request for tiles outside the map were sent, so I had to change "<=" to "<". Now i works fine.

moagrius commented 11 years ago

I see - you're not scaling the viewpot rect directly, you're just making sure the bounds are appropriate... Very cool. Although I'd think we'd still need <= for edges...

I just committed a big update earlier today, but I'll verify this patch and apply it when I can (assuming there's no less-than-obvious gotchas).

I'd be interested to see the very large maps you've mentioned (or at least the setup code), if you're not under NDA and are willing.

Thanks for the feedback - I'll post back next week or sooner after testing.

frankowskid commented 11 years ago

Basically setup look like this. Where map width/height is as big as 266842x90562. And for a tile size 512 it has up to 11 levels. for (int i = 0; i <= lvl; i++) { mapView.addZoomLevel(mapWidth >> i, mapHeight >> i, pattern, Consts.TILE_SIZE, Consts.TILE_SIZE); }

I will try to prepare some video.

moagrius commented 11 years ago

wow - that must be an insane number of tiles - how big is the .apk? or maybe you're fetching them remotely?

frankowskid commented 11 years ago

As I reported earlier - the integer is to small to store area :) Images(all levels) are up to 2GB. We are using image web server.

moagrius commented 11 years ago

very cool - did you just change MapTile.decode to grab the images over HTTP, or was there more required?

frankowskid commented 11 years ago

Yes, it is enough to change implementation of decode method. Now I'm wondering about multithreading. Any tips?

moagrius commented 11 years ago

that method (MapTile.decode) only ever runs in an AsyncTask, which is backed by it's own thread pool, so you should be OK. AFAICT there's no substantial benefit to using a ExecutorService or managing your own thread pool over AsyncTask (although I know some people disagree).

frankowskid commented 11 years ago

My idea was that i could download and decode few tiles at once to speed up loading process. As some tiles are already in cache I could load some from cache and in the same time download other from server. I.e. http://developer.android.com/reference/java/util/concurrent/BlockingQueue.html

moagrius commented 11 years ago

That could work - looks like AsyncTask went back to a single thread some time ago (Honeycomb). You'll probably need a complete rewrite of the TileManager, since that task's status (progress, suppression, cancellation, gc reference state, etc) is checked a lot to avoid manage what should and shouldn't be decoded and displayed (and managing memory generally). I'd be interested to hear how it works out, or even see it in action.

moagrius commented 11 years ago

I've just implemented the revised intersection math in my local build - not only does it "work", it prevents a great deal of unnecessary tile loads and noticeably increased performance - big thanks for this patch. I'm in the middle of a big update and probably won't commit for several days (or longer), but the next push will contain this update.