Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Being able to render tile by tile #1085

Closed
monsieurtanuki opened this issue Sep 8, 2018 · 26 comments
Closed

Being able to render tile by tile #1085

monsieurtanuki opened this issue Sep 8, 2018 · 26 comments
Milestone

Comments

@monsieurtanuki
Copy link
Contributor

Regarding DatabaseRenderer:

  • the purpose is to render tiles
  • in the context of a visible map
  • being called by an instance of MapWorkerPool

But that doesn't match a slightly different way of seeing things: a simple "give me that tile".
Because DatabaseRenderer:

  • interacts with MapWorkerPool so that it forces a display refresh (MapWorkerPool.this.layer.requestRedraw();), that we don't want here
  • includes some optimizations, like not writing twice the same label (cf. Set<MapElementContainer> undrawableElements), that provokes side-effects (cf. Bug with MapForge in OpenStreetMapViewer osmdroid/osmdroid#1145)
  • in this last case, potentially because of a bug - as elements are never removed from tileDependencies except in MapWorkerPool, that we don't want to use because of the display refresh

My suggestion is to create a dedicated class that extends StandardRenderer.
The purpose is to have the same level of quality as DatabaseRenderer, but in a simpler context where we will avoid side-effects.
The name will be something like DirectRenderer.

@devemux86
Copy link
Collaborator

Mapsforge is not meant to be a raster tile generator, there are specific tools for that, like the ones used by all online raster tile providers.

Also Mapsforge offers more advanced real-time rendering of labels + symbols in separate layer, which cannot be written in the raster tiles of the map.

BTW Mapsforge has not any rendering problems via its engine. So cannot know how any external implementations use its low level API correctly or not.

However if someone wants to provide pull request which:

  • Doesn't break library's current working behavior
  • Can work as extension to help such workflows

We can certainly review it.

@devemux86
Copy link
Collaborator

There is the SaveTiles sample demonstrating similar workflow.

@monsieurtanuki
Copy link
Contributor Author

@devemux86 Oh of course I didn't mean to imply that rasters are better than vectors or that the only point of Mapsforge is to build rasters. Different technology, different usages.

Thanks for your SaveTiles sample. It's more or less what we used to do, and we may encounter the side-effects I described: if we repeatedly ask for the same neighboring tiles, sometimes the labels are not consistent anymore.

Btw I don't seem able to push, I get an error 403. Any idea why? (I'm not a git expert)

@devemux86
Copy link
Collaborator

Can check this forum discussion for more details.

Btw I don't seem able to push, I get an error 403. Any idea why?

Push where?

@monsieurtanuki
Copy link
Contributor Author

Push my commit to mapsforge, in order to create a pull request.

@devemux86
Copy link
Collaborator

Only administrators can push commits in Mapsforge repositories. 🙂
Perhaps you can check GitHub documentation about pull requests.

@monsieurtanuki
Copy link
Contributor Author

Looks like I was doing the right thing, but not from the right place.
I've just create the PR #1086.

@devemux86 devemux86 added this to the 0.11.0 milestone Sep 9, 2018
@devemux86
Copy link
Collaborator

Closed via #1086.

@devemux86
Copy link
Collaborator

You can use it via snapshot builds published in Sonatype.

@monsieurtanuki
Copy link
Contributor Author

@devemux86 Thank you, it works perfectly with the SNAPSHOT version!

Obviously I cannot keep master-SNAPSHOT as a version number for our app (osmdroid) in production.
How does it work from now: when do you plan to create a new version with a real version number?
On our side there's no big hurry on the subject, but I'd like to know when I'm able to merge my fix on osmdroid.

@devemux86
Copy link
Collaborator

Very nice to hear that, please make extensive tests to verify it! 🙂

I can publish a 0.10.1 release, but since we just had 0.10.0, prefer to wait a bit to see if something critical comes up.

@monsieurtanuki
Copy link
Contributor Author

@devemux86 That's fine by me. Just tell me when you're ready, and on our side we keep testing it.

@monsieurtanuki
Copy link
Contributor Author

Actually it's not 100% OK.
I found a little bug, that I think is also on the "standard" DatabaseRenderer.

It's again about labels, in method processLabels.
When we display labels, there are interactions with the neighboring tiles.
Roughly said, it's something like: "hi neighbor, do you have a label that overlaps on my area, if so give it to me".
I'm afraid that the neighbor knows its labels only after itself running processLabels.
That means that the results depend on the order we use to process the tiles. Left to right, top to bottom works most of the time. But I found a road from the bottom tile, which label is not rendered on the neighboring top tile the first time, because allegedly the labels have not been processed yet for the bottom tile.

First rendering:
tile-b 0-1

Second rendering, with the edge of road label "H6044" on the bottom part:
tile-b 1-1

The tile is (zoom = 14; x = 9269; y = 5275)

I'm working on it.

@devemux86
Copy link
Collaborator

I found a little bug, that I think is also on the "standard" DatabaseRenderer.

Have you seen a rendering problem using native Mapsforge library?
The aforementioned forum topic explains in detail how the cached labels work inside Mapsforge.
Outside the library, could be a totally different procedure.

@ludwigb
Copy link
Collaborator

ludwigb commented Sep 10, 2018 via email

@monsieurtanuki
Copy link
Contributor Author

@ludwigb Thank you for your comment.

That's what appears in the code but was not part of my initial assumptions: there's no such thing as a unique tile for zoom,x,y in mapsforge.

What you display depends on what you displayed before.
First, display whatever label that overlaps from neighboring tiles and that has been displayed.
Second, display all the labels of the current tile, ordered by priority, and that don't collide with the neighbors' labels.

I guess it can be marginally flawed, e.g. when you already displayed in neighboring tiles unimportant labels that prevent you from displaying an important label on the current tile (because of collision).

Regardless, I guess the whole process is OK if you display maps.

But I display tiles, and here's the additional issue I need to address: taking into account overlapping labels from neighboring tiles that have not been processed yet.
I think that means a third step in my case:

  • adding all the labels from neighboring tiles that haven't been processed yet
  • if those labels overlap with the current tile

Or something like that.

@applantation
Copy link
Collaborator

applantation commented Sep 12, 2018 via email

@monsieurtanuki
Copy link
Contributor Author

@ludwigb Here is how my problem is different:

  • I render tile (z,x,y) on the bottom of the screen - I save it as a 256x256 bitmap and put it in my memory cache
  • I pan downwards and have to display tile (z,x,y+1), that happens to have an overlapping label with the previous tile - I save it as a 256x256 bitmap, including the part of the overlapping label that fits into this tile, and put it in my memory cache
  • I now have a broken label in my memory cache, partly displayed in tile (z,x,y+1) but not at all displayed in tile (z,x,y) - and that's not good because I display what is in the memory cache - a broken label
  • I assume that in mapsforge it's not that bad - I assume that first you display tile (z,x,y) without the label, but as soon as you display tile (z,x,y+1) you display fully the label in both tiles. I'm not sure about that, but obviously you don't have this problem in the standard use of mapsforge.
  • Of course if for some reasons my tiles are not stored in my memory cache anymore, then they will be recomputed and with the tileDependencies mechanism used by mapsforge, the label will appear in both tiles
  • A solution for me could be to say - if when rendering tile (z,x,y+1) I notice that there's a label that should fit into tile (z,x,y) but was not previously taken into account, then I must "deprecate" the (z,x,y) bitmap and remove it from the memory cache. It will then be recomputed and include the label
  • Another solution would be to assume that labels don't span over more than 2 neighboring tiles (or that we can live with marginal beyond 2 tiles) and that checking the 8 neighboring tiles' labels is enough

@devemux86
Copy link
Collaborator

I assume that in mapsforge it's not that bad - I assume that first you display tile (z,x,y) without the label, but as soon as you display tile (z,x,y+1) you display fully the label in both tiles.

No, that doesn't work that way in Mapsforge, the aforementioned forum discussion describes the mechanism.

@ludwigb
Copy link
Collaborator

ludwigb commented Sep 12, 2018 via email

@monsieurtanuki
Copy link
Contributor Author

@ludwigb I'm more optimistic than you for 2 reasons:

  • sure there might be some recursion, but the tiles will be deprecated only if needed (e.g. overlapping label not being taken into account before)
  • the limit is not the world, but my memory cache - I won't deprecate tiles I don't currently display

I'll let you know how things evolve.

@monsieurtanuki
Copy link
Contributor Author

Could be somehow solved by osmdroid/osmdroid#1156.

@devemux86
Copy link
Collaborator

Vector maps on mobile devices certainly should have variable tile size (like Mapsforge and VTM provide), but the extra tile space does not remove the need for special handling of labels at tile borders.

@monsieurtanuki
Copy link
Contributor Author

@devemux86 I agree with you, but in order to implement what I originally had in mind I would have needed to introduce some kind of callbacks into DatabaseRenderer, and I want to have zero impact on it.
Perhaps I will have to add those callbacks anyway (on a copy of DatabaseRenderer?), perhaps I won't have to do it because of the way I manage the cache, so far I don't know.

@devemux86
Copy link
Collaborator

New DirectRenderer is meant for changes and external use, being in same package with similar access.

DatabaseRenderer in not used externally, remains for library purposes and should avoid changes.

monsieurtanuki added a commit to monsieurtanuki/mapsforge that referenced this issue Sep 24, 2018
monsieurtanuki added a commit to monsieurtanuki/mapsforge that referenced this issue Sep 24, 2018
@monsieurtanuki
Copy link
Contributor Author

Just PR'ed #1089
The solution I implemented is a refresh of all neighboring tiles whenever there are new overlapping labels.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants