[core] Placement order matches viewport-y sort #14486
Conversation
| placements.emplace(symbolInstance.crossTileID, JointPlacement(false, false, false)); | ||
| continue; | ||
| } | ||
| const bool zOrderByViewportY = bucket.layout.get<style::SymbolZOrder>() == style::SymbolZOrderType::ViewportY; |
There was a problem hiding this comment.
!= style::SymbolZOrderType::Source
There was a problem hiding this comment.
That will affect the default behavior for styles that did not specify symbol-z-order 🤔
There was a problem hiding this comment.
I think we want to avoid sorting for all layers by default since this will introduce more label flickering. I think your implemented approach is good.
One alternative would be the same logic used to decide whether to sort the symbols when rendering: https://github.com/mapbox/mapbox-gl-js/blob/29a27eba5ff79720fa184ec4dc8b9dd942f759f9/src/data/bucket/symbol_bucket.js#L318-L319 - the symbols are only sorting if they are allowed to overlap. This same kind of logic could be used here but I'm not sure if that is necessary or better.
I think I like your approach better because it is simpler.
There was a problem hiding this comment.
IMO checking just SymbolZOrder is fine for placement: it just tells which labels to be placed first, so it is functional even if both icon and text overlapping not allowed (unlike the situation in bucket).
pozdnyakov
force-pushed
the
mikhail_sorted_y_placement
branch
from
3f60469
to
2967239
Compare
Symbols are placed accordingly to their viewport Y order, if the style `symbol-z-order` is set to `viewport-y`. This improves rendering of symbol layers, where icons are allowed to overlap but not text.
pozdnyakov
force-pushed
the
mikhail_sorted_y_placement
branch
from
2967239
to
79f8759
Compare
pozdnyakov
added
⚠️ DO NOT MERGE
Symbols are placed accordingly to their viewport Y order,
if the style
symbol-z-orderis set toviewport-y.This improves rendering of symbol layers, where icons are allowed
to overlap but not text.