heremaps / harp.gl

3D web map rendering engine written in TypeScript using three.js
Apache License 2.0
1.29k stars 197 forks source link

Decoder URL not respected #1986

Closed harrycollin closed 3 years ago

harrycollin commented 3 years ago

My harp.gl app is not hosted at the root path - i.e https://mywebsite.com/. It is a few levels down with some dynamic parameters in-between. Because of this I have to manually set the decoder path to something like this:

const decoderUrl = window.location.origin + "/decoder.bundle.js"

The MapView and VectorTile's decoderUrl/concurrentDecoderScriptUrl parameters are set to use the snippet above. This works fine! But when adding a FeatureDatasource to the map it is searching for the decoder at the current route. It doesn't seem to respect the URL gave it.

A few additional notes:

Steps to reproduce the behavior:

  1. Create a MapView on a route other than root
  2. Specify the location of the decoder.bundle.js
  3. Try adding a FeatureDataSource

image Here you can see that something is searching for the decoder at the current route when it should use the snippet above.

harrycollin commented 3 years ago

I managed to fix the issue, although I don't believe it should be marked as a solution since it's quite unintuitive. I needed to set both workerTileUrl & concurrentDecoderScriptUrl when creating a new FeaturesDataSource.

new FeaturesDataSource( { gatherFeatureAttributes: true, name: this.name, styleSetName: this.id, workerTilerUrl: decoderUrl, concurrentDecoderScriptUrl: decoderUrl } Any thoughts on this would be great!

nzjony commented 3 years ago

Thanks for creating the issue @harrycollin and glad to see you still around. I take a look and see if we can improve things.

nzjony commented 3 years ago

I took a look, basically they are doing different things, i.e. the workerTilerUrl points to the web worker handling the tiling of the GeoJson (which the FeaturesDataSource uses) and the concurrentDecoderScriptUrl points to to the web worker handling the decoding of tiles.

Probably in your decoder you have something like:

VectorTileDecoderService.start();
GeoJsonTilerService.start();

Some options we could look to implement:

I will ask in the team if there is a consensus, or maybe we wait until there is more complaint from the community.

harrycollin commented 3 years ago

Hey @nzjony, also glad to see you're around 😂. That you for clearing this up! I do in fact have both services running in my decoder bundle. I think it would be a good idea if decoder detection could be expanded to look not only at the current route/path but also at the root of the app. Maybe this is asking too much, but it would be convenient!

nzjony commented 3 years ago

@harrycollin , this passes the script url to the Worker: https://github.com/heremaps/harp.gl/blob/5c22a7cc21e35a64ff6cc8bccbe3507339fb418a/%40here/harp-mapview/lib/ConcurrentWorkerSet.ts#L199

Where it eventually calls: const worker = new Worker(scriptUrl);

The ConcurrentWorkerSet is used by both the ConcurrentDecoderFacade and ConcurrentTilerFacade. Possibly we could introduce some way to configure the root app directory, and this is some sort of global string that can be accessed anywhere.

nzjony commented 3 years ago

Actually, I wonder if this would work: https://github.com/heremaps/harp.gl/blob/5c22a7cc21e35a64ff6cc8bccbe3507339fb418a/%40here/harp-utils/lib/UrlPlatformUtils.ts#L15

harrycollin commented 3 years ago

@nzjony Maybe this should be moved somewhere else since the title of the issue wasn't relevant. What do you think?

FraukeF commented 3 years ago

Hi @harrycollin

As @nzjony is out of office for a couple of weeks, I wanted to follow up, to see if using "getAppBaseUrl()" as @nzjony proposed solved your issue?

harrycollin commented 3 years ago

Hey @FraukeF, sorry for the late reply on this. It doesn't appear to work. The only thing that works for me is this: const decoderUrl = window.location.origin + "/decoder.bundle.js"

Perhaps I'm using it wrong?

FraukeF commented 3 years ago

Hi @harrycollin,

thanks for your reply, I created an internal task to look into the issue, we ´ll let you know once there is a solution in place.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.