Buildings overzoom

[devemux86]

Implement overzoom for BuildingLayer and S3DBLayer, i.e. render them only in zoom 17 and keep those extrusions in larger zooms.

The building layers are connected via TileLoaderThemeHook on vector tile layer's extraction zoom levels. So if extend them beyond 17, then extrusions render again for each extraction level >17 (which better be avoided).

Note: S3DBTileLayer plays directly with its own vector tiles, so it's fine and already working ok.

[Gustl22]

Mission accepted ;D
Are there any tile sources, which have more precision (/ less simplification) over zoom 17? So then the buildings should be rendered again and remove old one (?). Or keep it simple and begin with only render at zoom 17?

[devemux86]

• Mapsforge offline maps work with zoom intervals (last one without simplification) and have microdegrees accuracy. So chosen max extraction zoom 17 is a sane default also for rendering performance.
• Mapzen + Nextzen vector tiles have last extraction zoom 16.
• OpenMapTiles have last extraction zoom 14.

So indeed keep it simple and begin on how to render only at zoom 17 and keep it for larger zooms.

[devemux86]

Also @Gustl22 if building layers need core changes and different implementation to support overzoom, we better proceed with new classes for testing, leaving the original ones unmodified.

[Gustl22]

Ok :)

For clarification: current implementation doesn't seem to need overzoom as TileSource.MAX_ZOOM is always 17. So if you zoom over 17 in viewport, it uses already only zoom 17 tiles and doesn't process them multiple times on higher viewport zooms. Only if set TileSource.MAX_ZOOM higher than 17, it'll process them again. EDIT: you already said in #393 🙈 pardon.

My implementation then would work as follows. If you are already at a higher zoom e.g. 18, the level 17 zooms wouldn't be called anymore in BuildingLayer. No chance to render them. So you have to check each element, if it wasn't already processed, independently of zoom level. For that I would store them as hash in a integer list (wouldn't need much space, I think). Hash describes specific element over all zooms. Problem is that if render cache is deleted, list wouldn't be updated. So there, I'll insert some interface to clean up the list. EDIT: this won't work, as renderer could not differ buildings from different layers...

If its not what you're looking for, plz let me know.

[devemux86]

Online vector tiles normally don't have extraction zoom levels >17, so current implementation works.

Mapsforge maps can have max extraction zoom level >17, e.g.: MapFileTileSource(2, 18)

• If keep default constructor new BuildingLayer(mMap, l) then buildings disappear at zoom 18
• If change constructor new BuildingLayer(mMap, l, 17, 18, false) then buildings are recreated also at zoom 18

That's because building layers are connected via TileLoaderThemeHook on vector tile layer's max extraction zoom level, so if change it - they either disappear or get extracted / recreated again.

Implementation should be important for efficiency / performance.. 🙂

As you describe, if we zoom at 18 and move map far, then at new areas we won't have zoom 17 building data.
And gets more complicated, even if have the data, the animation effect of flat / 3D buildings still happens.

[Gustl22]

And gets more complicated, even if have the data, the animation effect of flat / 3D buildings still happens.

Yes, that would be avoided with above mentioned implementation, too (appears only at mapsforge maps).

There's a more native approach, too, but would need more changes in MapDatabase and core :/

[devemux86]

All map sources, either online vector tiles or Mapsforge maps, eventually should have max zoom that of Viewport so that render themes can use it in their rules.
Currently default max zoom is 17 (optional) due to buildings and so render themes cannot have zoom-min >17 in rules without that change (see #216).

So it's not a Mapsforge issue but a generic VTM one and should be handled globally.

[devemux86]

(appears only at mapsforge maps)

It appears in all vector maps if change their max zoom. Same behavior is everywhere.

[Gustl22]

It appears in all vector maps if change their max zoom. Same behavior is everywhere.

Ok sorry, we meant different issues. Mapsforge maps tiles aren't clipped for some reason at zooms which are larger than one block, so they are rendered multiple times, which causes double animation. Same with tile borders (at all sources).
But effect shouldn't be visible at regular overzoom, as renderer only displays the buildings at one specific z-level. EDIT: solved via #514.

Back to initial topic:

• My first approach failed: Handle it building by building doesn't work, cause renderer can't fetch each individual from different tile zooms. Can only handle one whole tile. Apart from that, this isn't a very adaptive implementation.

• My second approach had problems when starting with overzoomed levels. My plan was to remember the tiles, which are already rendered and avoid processing if all subtiles or the parent tile were rendered. But if one subtile is missing when zooming out, all others would be rendered again anyway.

• Now I introduced a ZoomLimiter, which has a zoomLimit and always, when higher zoom levels are requested, it will switch to tile of zoom limit. So it is rendered one time at this level (in BuildingLayer 17), and it won't be rendered again on each individual sub tile. If not wanted, can simply set zoomLimit to zoomMax.
  To get running smooth (especially on initial higher zoom levels), additional tiles (for zoom limit) had to be requested in TileManager.

I didn't add a PR yet. Maybe you can review the branch, before start PR. There I also started to implement zoom limit for LabelLayer, but you can ignore that.

Like said in FIXME:
BuildingLayer.MAX_ZOOM should be equal with TileSource.MAX_ZOOM or better TileSource.mZoomMax while initializing. EDIT: solved via #514.

All map sources, either online vector tiles or Mapsforge maps, eventually should have max zoom that of Viewport so that render themes can use it in their rules.

Yes, that would be good. This should be possible soon.

[devemux86]

My plan was to remember the tiles, which are already rendered and avoid processing if all subtiles or the parent tile were rendered. But if one subtile is missing when zooming out, all others would be rendered again anyway.

Using sub-tiles data seems like underzoom?
Though usually not recommended. (explained in https://github.com/mapbox/vector-tiles/issues/21)

BuildingLayer.MAX_ZOOM should be equal with TileSource.MAX_ZOOM or better TileSource.mZoomMax while initializing.

BuildingLayer.MAX_ZOOM is its overzoom, essentially want rendering to happen only there.
And like all building properties better remain self handled (as mentioned also in #514).
On the contrary tile sources are the ones which depend (now) on that, to avoid buildings recreation.

[Gustl22]

Using sub-tiles data seems like underzoom?
Though usually not recommended.

I discarded that approach anyway, it's too unsafe. I used the third option. Did you test it already? Or should I start PR with relevant changes?

[devemux86]

Thanks, will have a look. 🙂

[devemux86]

I checked the branch, looks promising! When have time will review more the code changes.

Regarding the result on screen:
Now at each new zoom level the buildings still flicker for ms, like disappear and created instantly (without the animation).
So it's not the same nice user experience as when zoom in already "cached" buildings, where the transition is totally smooth to the eyes.

BTW instead of any "zoom limit" as class / method, better use "overzoom", as it's that exactly what we want to introduce.

Also let's focus on one feature at a time, building layer first, label layer later, to easier check any code changes.

[Gustl22]

Now at each new zoom level the buildings still flicker for ms, like disappear and created instantly (without the animation).

Don't know if I can solve that. But I try to.

instead of any "zoom limit" as class / method, better use "overzoom", as it's that exactly what we want to introduce.

I think there's a little difference, "overzoom" clips its parent tile, as "zoom limit" uses exactly the parent to avoid unwanted artifacts. It could be used in not overzoomed sources, too, to avoid artifacts at tileborders.
So we could have used the whole tile in "overzoom" implementation, if it's the same.
And "zoom limit" isn't the same value as "overzoom" value. It can be changed in each individual layer. But for me it isn't relevant how it is named ;D

Also let's focus on one feature at a time, building layer first, label layer later, to easier check any code changes.

Yes, it was only for demonstration purpose.

[Gustl22]

Now at each new zoom level the buildings still flicker for ms, like disappear and created instantly (without the animation).

I solved this issue: the tile renderer always uses the newest tiles, but they aren't always at state READY, so now it uses its ancestors instead (which are loaded already, cause of zooming in).
The nice thing is, this mechanism can be used for other things, too, e.g. for base layer, when zooming in fast, the already rendered tiles are used until new one are READY and no gap appears. But has to be adapted to its layer, of course...

Do you want to have a second look, or shall I post a PR (only with affected parts)?

[devemux86]

I tested the branch, the results seem working very nice! 👍
You can post a PR to check better the changes.

[devemux86]

Implemented via PR #528.