camptocamp / inkmap

A library for generating high-quality, printable maps on the browser.
Other
86 stars 17 forks source link

BingMaps on inkmap with OpenLayers v9.1.0 #65

Closed neo-garaix closed 6 months ago

neo-garaix commented 6 months ago

Hello !

My goal

I needed to print maps for an application with BingMaps layers so I looked up to add it on inkmap.

General Issues

However, I saw that we couldn't use BingMaps objects due to the line : document.createElement('script'); wich can be found in ol/net.js in the jsonp() function called by BingMaps.js in order to get the proper tile url depending of wich layer you want to use (Road, Aerial...).

We can't use DOM in workers, so I decided tu upgrade from OpenLayers 7.2.1 to OpenLayers 9.1.0. Indeed, in OpenLayers 9.1.0, instead of using jsonp() from net.js, BingMaps.js uses a fetch() that works in workers. Fine !

But on the other hand, the WMS layers in inkmap couldn't work in workers because in layers.js from inkmap we use prepareFrame(...) from ImageLayer.js where we call this.loadImage(image) wich finally comes to the function load() in ol/Image.js. Here there's a problem with some lines :

if (image 
    instanceof HTMLImageElement ||
    instanceof ImageBitmap ||
    instanceof HTMLCanvasElement ||
    instanceof HTMLVideoElement
) {
    this.image_ = image;
    }

Elements like "HTML..." generates some troubles with workers because we can't use them together. These lines weren't here in the older version 7.2.1 in ol/Image.js so it could work properly.

My temporary solution

For my use, I decided to desactivate the worker each time inkmap deploy itself, so i can stay on the main thread and benefits of the new OpenLayers 9.1.0 version and fits with the DOM calls.

Another BingMaps issue (Independent from the previous one)

While I was adding BingMaps to inkmap, I used the same way you generate tiles with your createTiledLayer(...) function. But, in order to generate tiles, I needed to keep the URL we use to call tiles to the API. But, in OpenLayers, the BingMaps object's creation routine works this way :

  1. Ask your API key an the type of Layer you want (Road, Aerial...)
  2. Generate the URL to get the precise one about the precise type of tiles
  3. Get the good URL for the good type of tiles
  4. Do his treatment with his function without storing the precise URL

Other types of layer have their URL stored in the source value that can be reuse to generate tiles in their function in layers.js.

My temporary solution

So, in order to do the same, I needed to store the URL by myself. I had to imitate the 3 first steps of a BingMaps object's creation routine.

To call the API, I used a fetch() request but it's an asynchronous behaviour so I needed to wait the end of it and the response. An await prefix was the best solutions so I passed my function with async and modified createLayer(...) from layer.js like this :

export async function createLayer(jobId, layerSpec, rootFrameState) {
  switch (layerSpec.type) {
    case 'XYZ':
      return createLayerXYZ(jobId, layerSpec, rootFrameState);
    case ...
    case 'BingMaps':
      return await createLayerBingMaps(jobId, layerSpec, rootFrameState);
  }

So, I also modified the way layerStates$ was defined in job.js and replaced it with this :

const layerStates$ = spec.layers.length
  ? combineLatest(
      await Promise.all(
        spec.layers.map((layer) => {
          return createLayer(job.id, layer, frameState);
        })
      )
    )
  : of([]);

My questions

Have you ever tried to implement the BingMaps layer in inkmap ? Did you already face to some issues like this working with OpenLayers and workers ? Have you some ideas on how i could do in order to fix the compatibility with workers ? Don't hesitate to correct me on things you noticed or just to share your ideas !

Here you can find my 4 commits about it if you want to check other things.

jahow commented 6 months ago

Hello, thank you for your interest in this library! Having a "bingmaps" type of layer would make sense seeing as the API is not the usual XYZ tile pattern, and also because of automatic attributions.

About updating OL, I'm not surprised that it might end up being a bit of work. Inkmap relies on some undocumented functions inside OL and this might need reworking. As for missing HTMLxx classes in the worker, this can be worked around by using polyfills like this one: https://github.com/camptocamp/inkmap/blob/main/src/worker/polyfills.js

And yes, it looks like offering BingMaps layers would require an update of OL. There is no funding that I know of for doing it but if you want to try it, please please be welcome to! I can definitely offer guidance if needed.

Thanks!

neo-garaix commented 6 months ago

Hey !

Thanks for your quick answer ! I followed your adviced and managed to obtained something working. I added these lines at the end of polyfills.js :

...

class FakeHTMLVideoElement {
  constructor() {}
}

self.HTMLImageElement = Image;
self.HTMLCanvasElement = OffscreenCanvas;
self.HTMLVideoElement = FakeHTMLVideoElement;

Now it's working and not stoping itself facing HTMLxx classes. I can now use the worker properly, thank you !