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

Bug with MapForge in OpenStreetMapViewer #1145

Closed
pavelkv96 opened this issue Aug 28, 2018 · 32 comments

Comments

@pavelkv96
Copy link

commented Aug 28, 2018

Issue Type

Bug

Description and/or steps/code to reproduce the problem

I have file ".map".
I open example More Sample -> TileProviders -> Mapsforge tiles.

And I zooming by level 9, I seeing next... It's good.

screenshot_1535450704

When I set zoom in (10 level). I have next... It's good too.

screenshot_1535450713

But when I return to level 9, I have next...

screenshot_1535450719

Version of osmdroid the issue relates to:

6.0.2

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Aug 28, 2018

@pavelkv96 My questions:

  • does it happen with other tile providers than your .map files?
  • are you 100% sure you're on integer zoom level 9 when you go back to zoom 9 (hint: display the zoom level in the log using MapView.getZoomLevelDouble() in order to be sure). If it's zoom level 8.9999 that could be an explanation, for the missing title and the larger gray zone on the bottom of the screen
  • I don't know anything about MapForge .map files - are there expiration dates for their tiles?
@pavelkv96

This comment has been minimized.

Copy link
Author

commented Aug 29, 2018

@monsieurtanuki Sorry.
I did not notice gray zone on the bottom, when I did screenshot.
I would like to show an error with the display of the names of settlements.
When I do zoom in and I do next zoom out. I get a part of the city name for this level
My .map file (I put in zip)
test.zip

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Aug 30, 2018

@pavelkv96 Thanks for the .map file, I can reproduce the bug.
I'll keep you posted.

@monsieurtanuki monsieurtanuki self-assigned this Aug 30, 2018
@monsieurtanuki monsieurtanuki added the bug label Aug 30, 2018
@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Aug 31, 2018

@pavelkv96 The problem comes from MapsForge, possibly in combination with osmdroid.
When I ask MapsForge the tile for z=14/x=9270/y=5275 the first time I have the labels. The next time the labels disappear (on my smartphone the problem occurs systematically on the same city with zoom level 14).

14-9270-5275 1
14-9270-5275 2

@MKergall As the code dealing with MapsForge originally came from osmbonuspack, does this issue by any chance sound familiar to you?

@pavelkv96

This comment has been minimized.

Copy link
Author

commented Aug 31, 2018

@monsieurtanuki
I tested with MapsForge and they don't have this bug

@MKergall

This comment has been minimized.

Copy link
Collaborator

commented Aug 31, 2018

@monsieurtanuki No idea. I just picked an "adaptor" found on the Internet, and I packed it in OBP.
Code of this adaptor is quite small - basically, just requesting MapsForge lib to build the tiles with relevant params.
So, if it's not a MapsForge bug, it's going to be really tricky to understand and solve...

@pavelkv96 You tested with MapsForge. But did you test with exactly the same version of MapsForge than the version used inside osmdroid?
Current osmdroid is using org.mapsforge:mapsforge-map:0.8.0, which is quite old now.

@spyhunter99

This comment has been minimized.

Copy link
Collaborator

commented Aug 31, 2018

i can look into updating the adapter and probably cut a new version of osmdroid this weekend

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 1, 2018

@pavelkv96

I tested with MapsForge and they don't have this bug

That's not that simple anyway: you experience the bug at zoom level 10, I do at level 14.
More something like a side-effect. About memory?
Maybe you would experience this bug with MapsForge but on a different spot and a different zoom level.

Just another test: if I go directly to the "suspicious" zone

mMapView.getController().setZoom(14);
mMapView.setExpectedCenter(new GeoPoint(53.809, 23.709));

there's no label at all.
But, if I close the app, turn the Hardware Acceleration off (in "Settings"), and go back, then there's the label. Afterwards, zooming to 13 then to 14, only part of the label is displayed (the part on the right).

@pavelkv96 Could you try to run your app using that line:

Configuration.getInstance().setMapViewHardwareAccelerated(false));

@spyhunter99 Perhaps the infamous "polyline/polygon rendering with hardware acceleration on" bug again...

@pavelkv96

This comment has been minimized.

Copy link
Author

commented Sep 3, 2018

@monsieurtanuki
May be bug with save tile? Because Mapsforge don't save their.
Because when I don't check tile in cache, and I generate anew, all work good

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 3, 2018

@pavelkv96 What do you mean with "when I don't check tile in cache, and I generate a new"? How do you do that?

Besides that, unfortunately I don't think that your "save tile/don't save tile" hypothesis is right, because the first time the tile is displayed the label is correctly displayed.
For the record, the tile display works more or less that way:

  • in the display loop, we try to find each tile in the memory cache
  • for each tile that is not in the memory cache, we loop asynchronously through all providers until one of them knows this tile, and then puts it in the memory cache and runs a display refresh

The extra step about the Mapsforge provider is that if it knows a tile, it has to compute and render it on a bitmap tile. For other providers it's more straight forward: I already have a bitmap, I give you this bitmap and there's no computation.

You say Mapsforge library seems to work fine in your case and there's obviously a bug in the Mapsforge x osmdroid combination: just out of curiosity, wouldn't you be better off using Mapsforge only, or are there special features in osmdroid that you need for your app? That doesn't mean the bug won't be fixed ;)

@spyhunter99

This comment has been minimized.

Copy link
Collaborator

commented Sep 3, 2018

I tried updating but to the newer version of forge but ran into some issues that i could not resolve.

@MKergall

This comment has been minimized.

Copy link
Collaborator

commented Sep 5, 2018

I tested with MapsForge and they don't have this bug

Which version of MapsForge?

@MKergall

This comment has been minimized.

Copy link
Collaborator

commented Sep 5, 2018

Looking at Mapsforge repo, I just discovered its VTM fork. I think it would be great to integrate VTM as a rendered of Mapsforge maps in osmdroid.
With @devemux86 help?...

@devemux86

This comment has been minimized.

Copy link

commented Sep 6, 2018

Regarding VTM please see also my comment here.

@pavelkv96

This comment has been minimized.

Copy link
Author

commented Sep 6, 2018

@MKergall version 0.8.0

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 8, 2018

The problem comes from mapsforge. I've just created an issue (mapsforge/mapsforge#1085).

Let me explain the problem.

When a tile is rendered, mapsforge looks for all the neighboring tiles' potential overlapping labels. That's cool.

But at some point, as we use this process in a slightly different way as planned (no direct display, not really using a mapsforge cache), there's a side effect about the label display.

In the mapsforge context, sometimes it makes sense to display the labels only once, even if they're overlapping on several tiles: displaying it once will cover both tiles.

In our context, we definitely always want to display the labels because we don't care about the neighboring tiles' bitmap being already processed or in cache: all we need to know is if they have overlapping labels.

More specifically, what happened is that the very first time we want to display "Василевичи" on level 14, both tiles are not processed yet and one of them says: "btw, display the left part of it, I'll display the right part". So far so good.
But the next time, after going to zoom 13 and back to 14, that's were the bug is in mapsforge - as both tiles have been processed (though we at osmdroid didn't necessarily cache them), mapsforge decides that only one of the tiles should display the label.

@monsieurtanuki monsieurtanuki removed the bug label Sep 9, 2018
@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 9, 2018

The fix will be to use the new DirectRenderer class instead of DatabaseRenderer in MapsForgeTileSource.
Assuming that mapsforge/mapsforge#1086 is merged and that we manage to upgrade our version of mapsforge to the latest.

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 9, 2018

@pavelkv96 Could you try with #1152? Should be fixed.

@pavelkv96

This comment has been minimized.

Copy link
Author

commented Sep 10, 2018

@monsieurtanuki , @MKergall , @spyhunter99 , @devemux86
Thanks to everyone for their help.

@pavelkv96 pavelkv96 closed this Sep 10, 2018
@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 10, 2018

@pavelkv96 It's not over yet, there are still some bugs, cf. mapsforge/mapsforge#1085.

@secaw

This comment has been minimized.

Copy link
Contributor

commented Sep 10, 2018

@monsieurtanuki Cool, this looks promising! The sometimes broken mapsforge labels have been annoying for a long time...

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 12, 2018

@secaw Don't hesitate to provide me with samples of broken mapsforge labels. I need to test and test again.

@secaw

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2018

@monsieurtanuki Unfortunately I noticed, that broken labels are not exactly reproducible across devices. I guess, it's dependend on the map scale (screen density), where it actually happens. As far as I saw, it occurs at some point in any map file I tried. For me it has improved after moving to mapsforge 0.9.1 (less occurences), so I haven't paid attention that much since then.

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 19, 2018

Could be solved by #1156.

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 23, 2018

Interesting work in progress: when a new tile is displayed and when inside this new tile a label is to be displayed also on neighboring tiles, I trigger a refresh of those neighboring tiles.
I still need to fine tune that.

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 24, 2018

Just PR'ed mapsforge/mapsforge#1089
That's the mapsforge part of the fix. When this fix is properly packaged (depending on mapsforge's agenda), I'll PR the osmdroid part of the fix.

The result is smooth.
The idea is to display the tiles (as we used to do), but whenever a new neighboring tile is displayed with new overlapping labels, we "deprecate" the impacted tile. How do we deprecate it? With the EXPIRED status, which means we still have the wrong (but not that stupid) bitmap on the screen, and that bitmap will be replaced by the correct one when computed (with the new labels).

@secaw

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2018

@monsieurtanuki Sounds very promising indeed. Does it recompute neighboring tiles always, or only under certain circumstances. I'm still very much aware of cumpute resources regarding mapsforge / osmdroid ongoing vector to pixel conversion.
Since you're PRing to mapsforge also, I have big wish: Would it be possible to return a NULL-tile, when requesting a tile that is out of bounds of the specific map file, like we do across any osmdroid providers? This would be great, so we can actually use mapsforge files as layers too. (just slightly related: #1072)
I found a solution for myself some time ago by customizing the osmdroid mapsforge implementation that way that I'm saving the bounds of the maps file and then checking against every tile, by converting it to its bounds coordinates. That is pretty computation intense on top of the actual tile rendering and might be solved way better, if mapsforge would just return by itself, if the map includes a tile or not (NULL).

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Sep 24, 2018

@secaw It recomputes only the neighboring tiles that are already in the memory cache and that have a new neighbor with overlapping labels.

Regarding the NULL-tile, I've just added this "out-of-box tile returns NULL" feature in Mapsforge's DirectRenderer. Obviously that doesn't solve everything, but this issue goes beyond my understanding of Mapsforge (something like bounding box vs. data range). You'll see the results.

@secaw

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2018

@monsieurtanuki Fantastic, thank you very much. I'm looking forward to checking it all out...

@pavelkv96

This comment has been minimized.

Copy link
Author

commented Oct 5, 2018

@monsieurtanuki How to solve this problem?

@monsieurtanuki

This comment has been minimized.

Copy link
Collaborator

commented Oct 5, 2018

@pavelkv96 We just have to wait the next mapsforge release, then I'll merge my osmdroid code.
Or are you talking about another issue than #1145?

@pavelkv96

This comment has been minimized.

Copy link
Author

commented Oct 5, 2018

@monsieurtanuki Thank you. No, I'm talking about #1145

monsieurtanuki added a commit that referenced this issue Jun 7, 2019
spyhunter99 added a commit that referenced this issue Sep 2, 2019
monsieurtanuki added a commit that referenced this issue Sep 24, 2019
Impacted class:
* `MapsForgeTileSource`: fine-tuned method `getBoundsOsmdroid` about the latitude 90/-90 side effect
monsieurtanuki added a commit that referenced this issue Sep 24, 2019
# Conflicts:
#	osmdroid-mapsforge/src/main/java/org/osmdroid/mapsforge/MapsForgeTileSource.java
spyhunter99 added a commit that referenced this issue Sep 26, 2019
* bug/#1145 - using the new DirectRenderer class of mapsforge

That fixes the issue.

Not to be merged as is, because we still rely on a `SNAPSHOT` version of `mapsforge`.
As soon as `mapsforge` issues a new version, the `build.gradle` file must be edited and marginally also a comment in `MapsForgeTileSource` about the `mapsforge` version number.

Impacted files
* `osmdroid-mapsforge/build.gradle`: now using the latest version of `mapsforge`, that includes the new `DirectRenderer` class that I coded in `mapsforge`
* `MapsForgeTileSource`: used `DirectRenderer` instead of `DatabaseRenderer`

* Use of new MapsForge's DirectRenderer.TileRefresher

Impacted classes:
* `MapTileProviderBase`: created method `expireInMemoryCache`, in order to "expire" a MapsForge tile that needs to be refreshed due to new overlapping labels from new neighboring tiles
* `MapsForgeTileProvider`: added a call to a tile refresher callback whenever a tile needs to be refreshed
* `MapsForgeTileSource`: method `renderTile` now returns `null` when MapsForge doesn't know the tile - used to be a yellow bitmap instead; created method `addTileRefresher`

* Impact of new mapsforge version

* edited the build.gradle according to the new mapsforge version number
* marginally edited a comment in MapsForgeTileSource

* #1145 fixes crashing due to out of bounds coordindates

* bug/#1145 - fine-tuning latitude 90/-90 side effect

Impacted class:
* `MapsForgeTileSource`: fine-tuned method `getBoundsOsmdroid` about the latitude 90/-90 side effect
@spyhunter99 spyhunter99 added this to the v6.1.1 milestone Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.