Added support of parentUrl property in layer.json #5864
Conversation
|
@tfili, thanks for the pull request! Maintainers, we have a signed CLA from @tfili, so you can review this at any time. I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to I am a bot who helps you make Cesium awesome! Thanks again. |
Source/Core/CesiumTerrainProvider.js
Outdated
| @@ -505,7 +587,22 @@ define([ | |||
| } | |||
| //>>includeEnd('debug'); | |||
|
|
|||
| var urlTemplates = this._tileUrlTemplates; | |||
| var layers = this._layers; | |||
| var layerToUse = layers[0]; | |||
Source/Core/CesiumTerrainProvider.js
Outdated
| var layers = this._layers; | ||
| var layerToUse = layers[0]; | ||
| var layerCount = layers.length; | ||
| for (var i=0;i<layerCount;++i) { |
Source/Core/CesiumTerrainProvider.js
Outdated
| return undefined; | ||
| } | ||
|
|
||
| var urlTemplates = layerToUse.tileUrlTemplates; |
There was a problem hiding this comment.
I haven't measured or anything, but there's the potential for a somewhat high performance cost here, especially on slow systems with slow internet connections. On these systems, Cesium tends to spend a lot of time traversing the quadtree to select the tiles it wishes it has, then calling requestTileGeometry on them, only to see it return undefined because too many tile requests are already in progress.
Before this pull request, Cesium only had to do some string manipulations to build up a URL, and then it would hand it off to loadArrayBuffer which would check for too many requests in flight and potentially return undefined.
But after this pull request, Cesium now has to do a lot of tile availability checking before handing off to loadArrayBuffer. I'd be surprised if this doesn't show up pretty clearly in a profiler.
So two suggestions, one easy and one harder:
- If the terrain provider has only one layer, don't bother checking availability at all. (easy)
- Check if we need to throttle (and do so) before checking tile availability.
There was a problem hiding this comment.
I don't know if 2 is possible. Even if we check the throttle status, we still need to know whether that layer contains the tile so we can return a Promise/undefined or if we should go to the next layer, which may be from a different domain and have a different throttle status.
There was a problem hiding this comment.
Yeah true you could only optimize some special cases. i.e. if all the layers are from the same host, and that host already has too many requests in flight, you can skip availability checking. Or another less useful special case: if all layers already have too many tile requests in flight, just return undefined without checking availability. Anyway, the important thing is to avoid the performance regression for the single layer case (as you've already done) and we can worry about optimizing the multi layer case if it ever really becomes an issue.
|
@kring This should be good now. |
|
@tfili is there a public tileset with a parentUrl that I can try out? In any case, I'm sure you've tried it, so I'm merging it on the basis of code inspection and tests. |
Nothing public yet. I'll let you know when we do. |
We can have a dataset that only has partial terrain, but points to a full global dataset for missing tiles. We just add a
parentUrlproperty to the layer.json and it will load that tileset and manage which one to use based onavailability.Also added a test to verify it handles extensions correctly.
To Do