Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support custom source #29

Closed
HarelM opened this issue Dec 11, 2020 · 13 comments
Closed

Support custom source #29

HarelM opened this issue Dec 11, 2020 · 13 comments
Labels
enhancement New feature or request

Comments

@HarelM
Copy link
Collaborator

HarelM commented Dec 11, 2020

Motivation

Sometimes getting the info to present a tile, whether it's raster or vector is not possible with the current implementation.
In those cases you need to be able to write some code that will return to MapLibre the tile in order to present it.
My example is the following:
I have a Cordova app that uses (or will use in the near future) this library.
I want to allow offline usage of the tiles.
This is currently not possible as far as I know.
I have created a fork of Mapbox-gl and this is in prodcution for almost a year now and is working great.
The only problem I have is that it wasn't merged to Mapbox.
I'm hoping it will find a good home here.
I'll send a pull request shortly.
The changes in the code are relatively small.
The idea came from a solution I did to openlayers (which is a great library but lacks performance when it comes to vector tiles).

Design Alternatives

Another solution to this besides what I will send in the PR is allow deriving from the VectorSource class, add registration somewhere and write a lot of code to accomplish this, which I don't think is the right approach, although might be more OOP.

Design

Very low code footprint for this design will allow it not to be a hassle to maintain and understand.

Mock-Up

No changes to UI/UX besides allowing custom sources

Concepts

Using a source that start with custom:// in the style will allow a developer to integrate a custom source function to facilitate for tile loading.
Tile loading can be from IndexDB for example or even from sqlite (mbtiles) file in a cordova/ionic apps (I have tested this to work).

Implementation

Add the ability to register a custom source function
When fetching a tile call that function.

The PR will shortly follow and this will be a lot easier to understand.
I still don't know how to address this documentation wise, and I'll be glad to get support on that.
I can open another issue in the docs repo for this before/after this is merged.

I know that this might not be a good time for this, I'm just opening this for when you'll have the time to resolve this.

@lseelenbinder
Copy link
Member

Thanks for this issue (and the PR!) @HarelM! I rather like the idea of completely custom tile sources.

I think we can look at this once we have the first release out and we have our feet under us regarding where we're headed.

@nyurik nyurik added the enhancement New feature or request label Dec 11, 2020
@AbelVM
Copy link

AbelVM commented Dec 16, 2020

This opens the door to tiles' data processing prior to rendering! It would be great to add it (soon 🙂 ) as it looks it's harmless to the rest of the code

In the meantime, I'm starting working on a custom layer using this branch 🤓

@HarelM
Copy link
Collaborator Author

HarelM commented Dec 16, 2020

@AbelVM I have changed the original code that I know to work with a more flexible one - i.e. register any protocol and not using hard coded custom://. Unfortunately I haven't had the time to test the new solution fully.
Let me know if this works for you or if you have any issues or need a working example.
The callback method in not straight forward and it took me some reverse engineering to understand what to write in it...

@AbelVM
Copy link

AbelVM commented Dec 16, 2020

Hi @HarelM ! A working example would be allways welcome, of course 🙂

I'm building a little PoC that once validated might turn into a end-to-end custom layer 🤓 , so I will report any weirdness with your fork of the fork

@HarelM
Copy link
Collaborator Author

HarelM commented Dec 16, 2020

I've added documentation with an example above the registration function in my fork. Let me know if you understand it.
You can also checkout the original code that I'm using here:
https://github.com/IsraelHikingMap/Site/blob/a66f4912878e12cd17623b0007c7eac8489320a8/IsraelHiking.Web/sources/application/services/database.service.ts#L99

@AbelVM
Copy link

AbelVM commented Dec 16, 2020

Hmmm, looks like the addCustomTilesFunction is called just once and no {z}/{x}/{y} substitution is made 🤔

I'm building a minimal example like:

maplibre.addCustomTilesFunction('custom', (params, callback) => {
 /*
  params, evaluated here is :
  
  {
    url: "custom://______/{z}/{x}/{y}.pbf",
    type: "json"
  }

*/
});
const map = new maplibre.Map({
  container: "map",
  style: `https://basemaps.cartocdn.com/gl/voyager-gl-style/style.json`,
  'center': [-3.703793, 40.416687],
  'zoom': 16,
  'minZoom': 12,
});
map.on('load', () => {
  map.addSource('test-source', {
    'type': 'vector',
    'url': 'custom://______/{z}/{x}/{y}.pbf',
    'minzoom': 10,
    'maxzoom': 19
  });
  map.addLayer(
    {
      'id': 'test-layer',
      'type': 'fill',
      'source': 'test-source',
      'source-layer': 'test',
      'paint': {
        'fill-color': '#ff0000',
        'fill-opacity': 0.4
      }
    });
});

Am I missing something?

@HarelM
Copy link
Collaborator Author

HarelM commented Dec 16, 2020

Looks good in theory. What do you mean by once? Doesn't moving the map calls this according to the needed tiles?
Also I think that if you don't call the callback method things break. Check the console...
If you have a stackblitz you can share or something similar let me know...

@AbelVM
Copy link

AbelVM commented Dec 16, 2020

There you are

https://stackblitz.com/edit/customsources

You can pan as much as you like but the custom tiles function is fired just once, not per tile 😞

@tuukka
Copy link

tuukka commented Dec 16, 2020

Supporting custom code as a tile data source would be truly awesome. Just another idea to consider for the API: could transformRequest be modified to allow returning a Promise of the data? This way, any request (not just tiles, not just a custom source) could potentially be resolved with custom code instead of a HTTP fetch. I haven't looked at the code to see what it would take though.

EDIT: After a quick look, the smallest change might be to allow registering custom URI schemes and call their implementation from makeRequest instead of fetch or XMLHttpRequest:

export const makeRequest = function(requestParameters: RequestParameters, callback: ResponseCallback<any>): Cancelable {

@HarelM
Copy link
Collaborator Author

HarelM commented Dec 17, 2020

Damn... I now remembered why I chose the original solution.
Config object it not available in the worker context so the callback never gets called.
Try the following code - before the latest change of protocol generalization:
IsraelHikingMap@6cffa38
Also I think that I needed to use tiles: ['...'] instead of url: '...' in the source definition to make this work...

@HarelM
Copy link
Collaborator Author

HarelM commented Dec 17, 2020

Here's a working example:
https://stackblitz.com/edit/customsources-fix?file=index.html
I have change the ajax.js code to be the following instead of the what's currently in the PR:

if (/^custom:/.test(requestParameters.url)) {
        if (isWorker() && self.worker && self.worker.actor) {
            return self.worker.actor.send('getResource', requestParameters, callback);
        }
        if (!isWorker()) {
            let key = Object.keys(config.LOAD_TILES_FUNCTION_MAP || {}).find(k => requestParameters.url.startsWith(k));
            return config.LOAD_TILES_FUNCTION_MAP[key](requestParameters, callback);
        }
    }

@AbelVM
Copy link

AbelVM commented Dec 17, 2020

Cool! It would be great if this feature makes its way to the first non-Mapbox one, as it opens the door to many use cases I had in mind 🚀

@tuukka
Copy link

tuukka commented Dec 17, 2020

EDIT: After a quick look, the smallest change might be to allow registering custom URI schemes and call their implementation from makeRequest instead of fetch or XMLHttpRequest:

Nevermind, this is what you already did so it works for any request in addition to tile sources, e.g. with your code I was able to set style: "custom://style" and create the style definition asynchronously within the custom tiles function. 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants