add methods getTiles and setTiles to mimick maplibre-gl-js interface

[xabbu42]

This adds the methods setTiles and getTiles to VectorSource to mimick the maplibre-gl-js interface
introduced in mapbox/mapbox-gl-js#8048. This is useful to change parametrized data sources without having to load a complete new style.

Todo:

• To completely port the maplibre-gl-js interface, also setUrl should be added.
• Is the additional call to observer->onSourceLoaded correct/necessary?
• There is a race and the map sometimes still requests old tiles even after setTile is called.

Any input especially about the race is appreciated.

[xabbu42]

We use this patch in production for over a year now. I just finally got around to update to latest maplibre-native opengl-2 branch and it still works as expected. Moreover I can no longer reproduce the race and I doubt it was a problem with this patch anyway.

I no longer thinks we should block this on an implementation of setUrl. That is a separate issue and a bit more complex, so I'd rather just get this in for now.

From my reading of the code the call to observer->onSourceLoaded is not necessary for Core funktionality, as onSourceLoaded and onSourceChanged ist handled exactly the same as onSourceLoaded in https://github.com/maplibre/maplibre-native/blob/main/src/mbgl/style/style_impl.cpp#L314. I'm happy to leave it in or take it out whatever the semantics of an external Observer are supposed to be.

So this ready for reviewing from my point of view.

[wipfli]

Nice @xabbu42! Please remove the draft status in that case

[xabbu42]

I'll have to look in the failed checks tomorrow.

[codecov]

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (e92cae0) 85.72% compared to head (3d5a9e3) 63.15%.
Report is 1 commits behind head on main.

❗ Current head 3d5a9e3 differs from pull request most recent head 34775ee. Consider uploading reports for the commit 34775ee to get more accurate results

Files	Patch %	Lines
src/mbgl/style/sources/vector_source.cpp	0.00%	16 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #149       +/-   ##
===========================================
- Coverage   85.72%   63.15%   -22.57%     
===========================================
  Files         569      588       +19     
  Lines       28047    32008     +3961     
===========================================
- Hits        24043    20215     -3828     
- Misses       4004    11793     +7789     

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

[github-actions]

Size test result

Old size: 8887808 bytes
New size: 8888576 bytes
Difference: -0.01%

[github-actions]

Render test results at https://maplibre-native-test-artifacts.s3.eu-central-1.amazonaws.com/5178722700-linux-gcc8-release-style.html

[louwers]

Thanks for your contribution.

Could you add:

• A test
• A doxygen docstring (since these methods are part of the public API)

[xabbu42]

Sorry for the long silence. I'll be away for the next 3 weeks but I plan to get back to this after that.

[github-actions]

Bloaty Results 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0% +16.4Ki  +0.0% +5.14Ki    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-149-compared-to-main.txt

Compared to d387090 (legacy)

    FILE SIZE        VM SIZE    
 --------------  -------------- 
   +19% +21.6Mi  +400% +23.9Mi    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-149-compared-to-legacy.txt

[xabbu42]

The checks run through before the merge and it looks like the failures now have nothing to do with my changes. @louwers I hope I addressed your requests adequately (I'm not familiar with doxygen or GoogleTest). So this should be ready to merge now.

[github-actions]

Bloaty Results (iOS) 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +744  [ = ]       0    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-149-compared-to-main.txt

[louwers]

@xabbu42 Seems like it runs through cleanly now. Let me have another look.

[louwers]

Two more nits, the getter can be marked const. And you can remove the type from the Doxygen comments. Then it looks good to merge!

[xabbu42]

I fixed the docstrings. I'm not sure I understand about the missing const.

[louwers]

@xabbu42 Yup, that is what I meant.

https://en.cppreference.com/w/cpp/language/member_functions#Member_functions_with_cv-qualifiers

Thanks for your contribution!