maplibre-gl-js
maplibre-gl-js copied to clipboard
Globe - symbols & symbol bugfixes
Symbol layer adaptation + symbol bugfixes
Issue & discussion of globe:
https://github.com/maplibre/maplibre/issues/190 https://github.com/maplibre/maplibre/discussions/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
andpainter
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 correcticon-translate
. - Fixed
text-translate
andicon-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).
- Fixed symbol collision debug view (
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
- [x] Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
- [x] Briefly describe the changes in this PR.
- [x] Link to related issues.
- [x] Include before/after visuals or gifs if this PR includes visual changes.
- [x] 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.
- [x] Add an entry to
CHANGELOG.md
under the## main
section.
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
).
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
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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.
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.
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.
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?
I believe so, as long as we keep the changes minimal I think it will be a good way forward.
Let's focus on the following PR first:
- #4071
Once merged to main and to globe branch and to globe-pr-symbol I'll review this PR. Thanks for your patience.
There's a need to resolve conflict I suppose. Let me know if there's anything I can do to help.
I've managed to remove useSpecialProjectionForSymbols
altogether in my dev branch, as discussed in this thread: https://github.com/maplibre/maplibre-gl-js/pull/4067#discussion_r1602805394
GREAT! Thanks for the update!