Globe - symbols & symbol bugfixes

[kubapelc]

Symbol layer adaptation + symbol bugfixes

Issue & discussion of globe:

maplibre/maplibre#190
maplibre/maplibre#161

Additions and changes since the previous PR:

• Adapted symbol layer type for globe.
• Fixed another source of tiny seams in the fill layer when globe is enabled (the draw_fill and painter changes are related to this).
• Symbol bugfixes:
  • Fixed symbol collision debug view (showCollisionBoxes) not showing the actual bounding boxes used for collision and click areas. The displayed boxes now match actual collision boxes exactly. Displayed collision boxes will react to map motion with some latency. (Just as the actual collision boxes do.)
  • Fixed symbol collisions using inaccurate and sometimes entirely wrong collision boxes when the map is pitched or rotated. (#210)
  • Fixed symbol collision boxes not being accurate for variable-anchor symbols.
  • Fixed icon collision boxes using text-translate property for translation instead of the correct icon-translate.
  • Fixed text-translate and icon-translate behaving weirdly and inconsistently with other -translate properties. (#3456)
  • Updated some old render tests to reflect new symbol & collision box behaviour, added several new render tests (in addition to render tests for symbols on a globe).

I've updated the public globe demo with MapLibre build from this PR:

https://kvaleya.gitlab.io/maplibre/globe/globedemo.html

Main vector globe branch with all changes

Launch Checklist

• Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
• Briefly describe the changes in this PR.
• Link to related issues.
• Include before/after visuals or gifs if this PR includes visual changes.
• Write tests for all new functionality.
  • Most changes and new functionality is covered by render tests.
• Document any changes to public APIs.
• Post benchmark scores.
• Add an entry to CHANGELOG.md under the ## main section.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 80.21390% with 37 lines in your changes are missing coverage. Please review.

Project coverage is 86.94%. Comparing base (d2d8f75) to head (66b7891).

Files	Patch %	Lines
src/geo/projection/globe.ts	67.85%	15 Missing and 3 partials ⚠️
src/symbol/symbol_layout.ts	29.41%	11 Missing and 1 partial ⚠️
src/geo/projection/mercator.ts	66.66%	4 Missing and 1 partial ⚠️
src/render/draw_collision_debug.ts	83.33%	0 Missing and 1 partial ⚠️
src/render/draw_symbol.ts	91.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##            globe    #4067       +/-   ##
===========================================
+ Coverage   70.12%   86.94%   +16.82%     
===========================================
  Files         249      250        +1     
  Lines       34756    34854       +98     
  Branches     1326     2122      +796     
===========================================
+ Hits        24371    30303     +5932     
+ Misses       9059     3548     -5511     
+ Partials     1326     1003      -323     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kubapelc]

Globe render tests for symbols are duplicated - each test has two variants, one with collision box debug disabled, and one with enabled. In base MapLibre, collision tests are separate, so I separated them here as well. But I'm wondering if I could keep just the variant with collisions for globe render tests, since it also inherently tests regular symbol rendering.

[kubapelc]

Also, in the second screenshot on the left side you can notice the pitch-alignment: map texts having their bounding box slightly off. I have no idea why that happens. I've decided to go ahead with the PR anyway, since it is a minor offset and only happens on pitch-alignment: map texts that are "planetary scale", which is probably an edge case in real maps.

[HarelM]

I'll review this in the coming days, thanks for this!
I thought I already asked, but I prefer to ask again, just to be on the safe side:
Is it possible to fix the collision boxes in the main branch and not only here?
It would be beneficial to people using the production version until this branch is merged.

[kubapelc]

Is it possible to fix the collision boxes in the main branch and not only here?

I know I've previously said that the changes are too deeply rooted into globe, but after taking another look at the actual diffs of the files, I think it might not be too much work to port it over to main. The easiest approach is to create a greatly reduced version of the Projection interface from globe and use that in symbols. Is that an ok approach?

[HarelM]

I believe so, as long as we keep the changes minimal I think it will be a good way forward.

[HarelM]

Let's focus on the following PR first:

• Symbols: fix translation, variable-anchors and collision boxes #4071

Once merged to main and to globe branch and to globe-pr-symbol I'll review this PR.
Thanks for your patience.

[HarelM]

There's a need to resolve conflict I suppose.
Let me know if there's anything I can do to help.

[kubapelc]

I've managed to remove useSpecialProjectionForSymbols altogether in my dev branch, as discussed in this thread: #4067 (comment)

[HarelM]

GREAT! Thanks for the update!