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

Barcelona label blinking #286

Open
wipfli opened this issue Aug 21, 2024 · 20 comments
Open

Barcelona label blinking #286

wipfli opened this issue Aug 21, 2024 · 20 comments

Comments

@wipfli
Copy link
Collaborator

wipfli commented Aug 21, 2024

Location
Include a link to a public URL with coordinates in the hash and zoom level.

Screenshots
z9.87:
image

z10.78:
image

Required information

  • Browser [e.g. chrome, safari]
  • Renderer version [e.g. MapLibre GL JS 3.3.0]
  • Tileset version and source
  • Style JSON or NPM version
@wipfli
Copy link
Collaborator Author

wipfli commented Aug 21, 2024

The style does not honor population_rank, and adding "symbol-sort-key": ["*", -1, ["get", "pmap:population_rank"]] to the places_locality layer's layout properties fixes the issue. However, better I think would be to sort the features in the tiles according to pmap:population_rank to avoid this issue.

@wipfli
Copy link
Collaborator Author

wipfli commented Aug 21, 2024

We are using setSortKey already here:

feat.setSortKey(getSortKey(minZoom, kindRank, populationRank, population, sf.getString("name")));

The planetiler docs state:

For symbols (text/icons) where clients try to avoid label collisions, features are placed in the order they appear in each layer, so features that appear earlier (lower sort key) will show up at lower zoom levels than feature that appear later (higher sort key) in a layer.

@wipfli
Copy link
Collaborator Author

wipfli commented Aug 21, 2024

l'Hospitalet de Llobregat:

id | 17593125925341
-- | --
name:ru | Оспиталет-де-Льобрегат
capital | 8
name:el | Οσπιταλέτ ντε Λιοβρεγάτ
name | l'Hospitalet de Llobregat
pmap:kind_detail | city
pmap:min_zoom | 8
pmap:kind | locality
pmap:pgf:name | l'Hospitalet de Llobregat
wikidata | Q15470
pmap:population_rank | 10
population | 274455

collides away the Barcelona label:

id | 17592338408581
-- | --
capital | 4
name:uk | Барселона
pmap:kind_detail | city
pmap:pgf:name:hi | 
name:el | Βαρκελώνη
name:it | Barcellona
pmap:min_zoom | 4
name:en | Barcelona
name:es | Barcelona
name:zh | 巴塞罗那
pmap:population_rank | 12
name:ar | برشلونة
name:ja | バルセロナ
name:pl | Barcelona
pmap:kind | locality
name:ro | Barcelona
name:tr | Barselona
population | 1660122
name:ru | Барселона
name:pt | Barcelona
name:de | Barcelona
name:hi | बार्सॆलोना
name:lt | Barselona
name | Barcelona
name:fr | Barcelone
name:zh-Hans | 巴塞罗那
name:zh-Hant | 巴塞隆納
pmap:pgf:name | Barcelona
wikidata | Q1492

@wipfli
Copy link
Collaborator Author

wipfli commented Aug 21, 2024

getSortKey(4, 2, 12, 1660122, "Barcelona")
return -2647735
getSortKey(8, 2, 10, 274455, "l'Hospitalet de Llobregat")
returns -1207431

which looks correct.

@wipfli
Copy link
Collaborator Author

wipfli commented Aug 21, 2024

com.onthegomap.planetiler.collection.FeatureGroup.SORT_KEY_MIN is -4194304 so there should not be an overflow

@bdon
Copy link
Member

bdon commented Aug 22, 2024

@wipfli what may be happening is although the sort order within a tile is correct, these two labels for l'Hospitalet de Llobregat and Barcelona actually come from different tiles:

CleanShot 2024-08-22 at 15 56 56@2x

@bdon
Copy link
Member

bdon commented Aug 22, 2024

A better screenshot: CleanShot 2024-08-22 at 16 00 45@2x

@wipfli
Copy link
Collaborator Author

wipfli commented Aug 22, 2024

Good observation. We can supply a sort key to MapLibre GL JS in that case.

@wipfli
Copy link
Collaborator Author

wipfli commented Aug 24, 2024

@wipfli
Copy link
Collaborator Author

wipfli commented Aug 24, 2024

@wipfli
Copy link
Collaborator Author

wipfli commented Aug 24, 2024

Any alternative to including a sort key in the tiles for MapLibre GL JS?

@nvkelso
Copy link
Collaborator

nvkelso commented Aug 24, 2024

What is the tile buffer set to for the places layer?

@wipfli
Copy link
Collaborator Author

wipfli commented Aug 24, 2024

The buffer size is 64 I think:

// We set the sort keys so the label grid can be sorted predictably (bonus: tile features also sorted)
// NOTE: The buffer needs to be consistent with the innteral grid pixel sizes
//feat.setPointLabelGridSizeAndLimit(13, 64, 4); // each cell in the 4x4 grid can have 4 items
feat.setPointLabelGridPixelSize(LOCALITY_GRID_SIZE_ZOOM_FUNCTION)
.setPointLabelGridLimit(LOCALITY_GRID_LIMIT_ZOOM_FUNCTION)
.setBufferPixels(64);
// and also whenever you set a label grid size limit, make sure you increase the buffer size so no
// label grid squares will be the consistent between adjacent tiles
feat.setBufferPixelOverrides(ZoomFunction.maxZoom(12, 64));

@msbarry have you experience previously that labels start to blink if the buffer size was set too low on a symbol layer?

@msbarry
Copy link
Contributor

msbarry commented Aug 24, 2024

I've always seen labels blinking on gl js, it tries to optimize labels on the current viewport but that leads to labels disappearing and reappearing when you zoom in or out 😕. 64px seems like plenty, I'm only using 4px on onthegomap. I'm not exactly sure how it handles label collisions preference across tiles based on ordering within each tile but it seems like specifying a global ordering should help, except that it might show a label when one tile loads but then need to hide it when adjacent one loads.

@bdon
Copy link
Member

bdon commented Sep 4, 2024

Seeing the same thing for Florence: https://maps.protomaps.com/#map=9.25/43.7929/11.0961&theme=light&lang=en

@bdon
Copy link
Member

bdon commented Sep 16, 2024

@nvkelso I think we should just add a sort_rank to all layers which will also aid Tilezen backwards-compatibility.

@msbarry
Copy link
Contributor

msbarry commented Sep 16, 2024

Here's the order of locality features in that tile (9/271/186). It matches what I'd expect based on this logic:

static int getSortKey(double minZoom, int kindRank, int populationRank, long population, String name) {
return SortKey
// (nvkelso 20230803) floats with significant single decimal precision
// but results in "Too many possible values"
// Order ASCENDING (smaller manually curated Natural Earth min_zoom win over larger values, across kinds)
.orderByInt((int) minZoom, 0, 15)
// Order ASCENDING (smaller values win, countries then locality then neighbourhood, breaks ties for same minZoom)
.thenByInt(kindRank, 0, 6)
// Order DESCENDING (larger values win, San Francisco rank 11 wins over Oakland rank 10)
.thenByInt(populationRank, 15, 0)
// Order DESCENDING (larger values win, Millbrea 40k wins over San Bruno 20k, both rank 7)
.thenByLog(population, 1000000000, 1, 100)
// Order ASCENDING (shorter strings are better than longer strings for map display and adds predictability)
.thenByInt(name == null ? 0 : name.length(), 0, 31)
.get();
}

city pmap:min_zoom pmap:kind_detail pmap:population_rank population
Firenze 5.6 city 12 382808
Pisa 6.1 city 10 97888
Prato 8 city 9 191150
Pistoia 8 city 8 90315
Lucca 8 city 8 89046
Scandicci 8 town 8 50609
Empoli 8 town 7 48109
Sesto Fiorentino 8 town 7 48958
Cascina 8 town 7 45257
Capannori 8 town 7 46252
Campi Bisenzio 8 town 7 46166
Quarrata 8 town 7 26190
Pontedera 8 town 7 29223
Poggibonsi 8 town 7 29196
San Miniato 8 town 7 27934
San Giuliano Terme 8 town 7 31399
Fucecchio 8 town 7 23618
Bagno a Ripoli 8 town 7 25611
Monsummano Terme 8 town 7 21338
Lastra a Signa 8 town 7 20156
Montecatini Terme 8 town 7 20409
Signa 8 town 6 19179
Pescia 8 town 6 19644
Agliana 8 town 6 17525
Calenzano 8 town 6 17489
Montemurlo 8 town 6 18456
Castelfiorentino 8 town 6 17504
Borgo San Lorenzo 8 town 6 18211
San Casciano in Val di Pesa 8 town 6 17062
Vinci 8 town 6 14604
Fiesole 8 town 6 13969
Certaldo 8 town 6 16121
Ponsacco 8 town 6 15611
Impruneta 8 town 6 14615
Altopascio 8 town 6 15481
Carmignano 8 town 6 14450
Collesalvetti 8 town 6 16827
Greve in Chianti 8 town 6 13862
Montelupo Fiorentino 8 town 6 14098
Santa Croce sull'Arno 8 town 6 14601
Vecchiano 8 town 6 12189
Montespertoli 8 town 6 13537
Santa Maria a Monte 8 town 6 13253
Serravalle Pistoiese 8 town 6 11659
Castelfranco di Sotto 8 town 6 13427
Montale 8 town 6 10737
Cerreto Guidi 8 town 6 10870
Poggio a Caiano 8 town 6 10007
Barberino di Mugello 8 town 6 10836
Montopoli in Val d'Arno 8 town 6 11148
Vaiano 8 town 5 9895
Ponte Buggianese 8 town 5 8804

@msbarry
Copy link
Contributor

msbarry commented Sep 16, 2024

I'm looking at 3 labels that blink when you zoom in: Montale, Montemurlo, and Agliana. My hunch is that a large (and growing as you zoom in) value for text-padding is causing this.

When you're fully zoomed in there is space for all 3:

image

Then you zoom out some more and it correctly hides the least important one (Montale)

image

But then you zoom out a little further and since Montemurlo is too close and collides with Agliana, it gets hidden and there is now room for Montale

image

It would help if there were a way to set map.showCollisionBoxes=true in that style viewer.

That's only one source of blinking though, it's very strange that florence gets hidden sometimes even though it appears first in the 2 tiles it's on the border of. My guess with that one is that you need the sort key for maplibre to know how to handle collisions across neighboring tiles. Or we could go into the maplibre code and try to make it smarter about interleaving features when handling collisions across neighboring tiles.

bdon added a commit that referenced this issue Sep 17, 2024
bdon added a commit that referenced this issue Sep 17, 2024
* add show debug boxes to viewer [#286]

* freeze tileset version [#282]
@bdon
Copy link
Member

bdon commented Sep 17, 2024

Added a "boxes" option on maps.protomaps.com that shows tile boundaries and label bbox boundaries.

@nvkelso
Copy link
Collaborator

nvkelso commented Sep 18, 2024

I propose pre-culling excess labels in tiles in #300. Once that's merged, the too large collision boxes can be trimmed down to more reasonable few pixels (needs a style PR), and the collision flicker should mostly be eliminated.

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

No branches or pull requests

4 participants