kamil/fix-ease-to-padding-transition-on-globe-projection

[kamil-sienkiewicz-asi]

Launch Checklist

Fixes #5133

• 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.
• Document any changes to public APIs.
• Post benchmark scores.
• Add an entry to CHANGELOG.md under the ## main section.

[kamil-sienkiewicz-asi]

@HarelM Hey, soo i've tried to debug this issue that i've created today and what I noticed was that padding wasn't really interpolated on every easeTo step as it is in mercator projection. At first glance seems like copying solution from mercator fixed the issue 🤔 . But I'm not familiar with codebase, so if you could let me know if maybe something else need adjustment along with that PR I would be grateful 🙏

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.82%. Comparing base (8dba921) to head (3060860).
Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5134   +/-   ##
=======================================
  Coverage   91.82%   91.82%           
=======================================
  Files         278      278           
  Lines       38346    38347    +1     
  Branches     6703     6706    +3     
=======================================
+ Hits        35210    35213    +3     
+ Misses       3002     3001    -1     
+ Partials      134      133    -1     

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

[HarelM]

I remember there was an issue with adding padding, but I'm not sure what it was...
I'll see if I can find similar issues related to it.

[HarelM]

I think it might be related to the following issue:

• flyTo() with padding is somewhat broken with Globe #4891

Padding is applied differently.
Can you check if the linked issue is solved/changed/the same?

[HarelM]

@NathanMOlson @kubapelc can you guys take a look at this?

[kubapelc]

Does this also work if the easeTo animation also changes the map center?

Code changes look good to me!

[kamil-sienkiewicz-asi]

Kapture.2024-11-28.at.16.17.47.webm

@HarelM I don't see a difference between pre.3 and my branch regarding this issue that you've linked.
To add to that, I think padding is applied correctly (see debug padding), but position to which map should center isn't. Assumption is that consecutive calls of flyTo takes into consideration previous padding(?) which would explain why flyTo to California ends up near Hawaii. And if I go even further by flying to both of those positions over and over again without moving map it's shifting to the left and bottom on every call and eventually breaks and lands somewhere in Asia.

[kamil-sienkiewicz-asi]

@kubapelc if I understood you correctly, yes. On every button click im swapping padding and center which seems to work correctly.

    let leftPadding = 100;
    let center = [-123.0237,37.9952];
    const triggerEaseTo = () =>{
        map.easeTo({center ,padding: {left:leftPadding}});
        leftPadding = leftPadding ? 0 : 100;
        center = leftPadding ? [-123.0237,37.9952] : [-124.7109,48.3818];
    };

Kapture.2024-11-28.at.16.35.13.webm

[kamil-sienkiewicz-asi]

@HarelM Hey, I think it's ready to review, not sure why codecov/project is shouting at me tho 🤔

nvm - it went through

[HarelM]

My main concern about this code which I think I recently had a look at is the following comment:

maplibre-gl-js/src/geo/projection/mercator_camera_helper.ts

Line 172 in e25a8d5

// When padding is being applied, Transform#centerPoint is changing continuously,

I'm not sure what to make of it and which scenarios we didn't tests as part of this PR...

[kamil-sienkiewicz-asi]

@HarelM ah I see why that was needed on mercator, unfortunately I don't have enough knowledge about codebase yet to say something valuable why this works on globe out of the box, I mainly relied on tests, but I could add some more tests if you can figure out some scenarios where it would break 👌

[HarelM]

If I'm looking at the mercator code I see some references to offset that can be used (although I'm not sure I understand what's the difference between offset and padding).
I'm also not very knowledgeable in this area of the code, but I would expect some complexities when setting both for globe, can you see if you can find scenarios where this might work differently between mercator and globe?

[kamil-sienkiewicz-asi]

Kapture.2024-11-29.at.16.54.33.webm

Mercator and globe with padding and offset are behaving the same. LngLat values that you see printed out in are unique map.getCenter values (unique to 8th decimal place) - so it doesn't behave weirdly on every consecutive call. I've also tested pitch, roll, elevation, offset, padding and bearing together, and seems like these combinations yield same map.getCenter() (to some decimal place, I assume that its a approximation error) in both projections

@HarelM

[HarelM]

Let's hope this won't create other issues, thanks!!