WebMercator Tiling for CesiumTerrainProvider #8563

jtfell · 2020-01-22T22:46:03Z

I've integrated the WebMercatorTilingScheme as an option for the CesiumTerrainProvider. I'm not 100% happy with the API yet, and realise that the QM Spec explicitly states it is

a simple multi-resolution quadtree pyramid of heightmaps according to the Tile Map Service (TMS) layout and global-geodetic profile

I can confirm that this very small change works correctly for a terrain layer that I generated, so I can't see any reason it shouldn't be an option somehow. I am opening an issue on that repo to discuss adding this option to the spec, although the layer.json is a Cesium-specific manifest file.

I'm happy to discuss any pathways for enabling Cesium to consume terrain in alternative tiling schemes if this approach is not viable. Cheers!

…heme based on key in layer.json

cesium-concierge · 2020-01-22T22:46:07Z

Thank you so much for the pull request @jtfell! I noticed this is your first pull request and I wanted to say welcome to the Cesium community!

The Pull Request Guidelines is a handy reference for making sure your PR gets accepted quickly, so make sure to skim that.

❌ Missing CONTRIBUTORS.md entry.
- Please add yourself to the contributors file!
❌ Missing CLA.
- Please send in a Contributor License Agreement (CLA) and comment back here to let us know to check this!
❔ CHANGES.md was not updated.
- If this change updates the public API in any way, please add a bullet point to CHANGES.md.
❔ Unit tests were not updated.
- Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

Cesium Viewer works.
Works in 2D/CV.
Works (or fails gracefully) in IE11.

jtfell · 2020-01-22T22:47:31Z

Issue on quantized-mesh spec: CesiumGS/quantized-mesh#10

jtfell · 2020-01-22T22:50:08Z

Signed CLA.

OmarShehata · 2020-01-26T15:10:38Z

Thanks for the contribution @jtfell ! I can confirm we have your CLA.

hpinkos · 2020-01-29T15:03:08Z

@kring can you review this?

kring

Thanks for the PR @jtfell! This looks good to me, I'd just like to see a couple of minor changes for consistency. Would you mind merging in master while you're at it as well?

kring · 2020-01-30T00:34:37Z

Source/Core/CesiumTerrainProvider.js

@@ -198,6 +192,31 @@ import TileProviderError from './TileProviderError.js';
            var maxZoom = data.maxzoom;
            overallMaxZoom = Math.max(overallMaxZoom, maxZoom);
            // Keeps track of which of the availablity containing tiles have been loaded
+
+            if (!data.tilingScheme || data.tilingScheme === 'GlobalGeodetic') {


Rather than add a new property, we should use the projection property already found in the layer.json. WebMercatorTilingScheme should be specified with "EPSG:3857".

Ok that sounds good, should it default to the existing behaviour in all other cases?

I think the general logic you have here is good. If it's EPSG:3857, do web mercator. If it's undefined or EPSG:4326, do geographic. If it's something else, complain.

kring · 2020-01-30T00:36:11Z

Source/Core/CesiumTerrainProvider.js

-        var yTiles = provider._tilingScheme.getNumberOfYTilesAtLevel(level);
-
-        var tmsY = (yTiles - y - 1);
+        if (provider._tilingScheme instanceof GeographicTilingScheme) {


Quantized mesh tile URL Y coordinates should always be numbered from the South and increasing toward the North, TMS-style. It's confusing for this to change based on the projection.

While we're at it, we can make this configurable separately right? Maybe when the scheme key is set to tms it counts from the south, and counts from the north otherwise. Or to avoid breaking existing uses, a new slippy value for scheme triggers counting from the north.

That's ok with me if you have a use case and as long as it doesn't add too much complexity to the URL generation (it shouldn't be too bad).

kring · 2020-01-30T00:39:00Z

Also, I do think this is a useful addition to the spec, so I'd be happy to merge a PR there, too. Thanks!

jtfell · 2020-01-30T01:51:31Z

Thanks for the feedback @kring! I will create a PR for the spec after we have decided on what to do with the projection and scheme options here.

…n-tiling-scheme

…l/cesium into web-mercator-terrain-tiling-scheme

kring · 2020-01-30T11:15:46Z

@jtfell your changes look good, but there seems to be a problem when using Cesium World Terrain and zooming in slightly:

It's not immediately obvious to me how your changes caused this, but I can't reproduce it in master so I think it must be a new problem in this PR. Any ideas?

kring · 2020-01-30T11:21:50Z

Oh I think the problem is that createQuantizedMeshTerrainData was previously given a proper Cesium Y-coordinate numbered from the North, and now for "scheme": "tms" terrain like Cesium World Terrain, it's being given a TMS Y-coordinate numbered from the South.

Don't ask me why CesiumJS and Cesium World Terrain use different tile numbering, even though it's totally my fault that they do. 😆

jtfell · 2020-01-30T23:43:48Z

The behaviour for "scheme": "tms" should be unchanged though? Maybe I'm misunderstanding the problem. Do you know how to fix it?

kring · 2020-01-31T00:30:29Z

Source/Core/CesiumTerrainProvider.js

+        // The TileMapService scheme counts from the bottom left
+        if (!provider._scheme || provider._scheme === 'tms') {
+          var yTiles = provider._tilingScheme.getNumberOfYTilesAtLevel(level);
+          y = (yTiles - y - 1);


This is the cause of the bug. Previously the y coordinate as left unchanged in the TMS case, so createQuantizedMeshTerrainData was given Cesium coordinates (0=North) rather than TMS coordinates (0=South). After your change, createQuantizedMeshTerrainData is given TMS coordinates for TMS terrain, causing things to break.

Ah I see, that makes sense. I've pushed a fix to the branch.

…es (not TMS coordinates)

…l/cesium into web-mercator-terrain-tiling-scheme

jtfell · 2020-02-17T22:37:08Z

Have you had a chance to look at this again @kring?

…ain-tiling-scheme In-repo version of jtfell's #8563

kring · 2020-02-18T05:09:56Z

Thanks for following up, @jtfell. And thanks for the pull request!

jtfell · 2020-04-09T00:12:42Z

Hey @kring not sure if you're the right person but it would be great to get this change formalised in the quantized-mesh spec. I've drafted up a PR here

Add option to switch CesiumTerrainProvider to a WebMercator tiling sc…

5c7a277

…heme based on key in layer.json

jtfell mentioned this pull request Jan 22, 2020

Alternative Tiling Schemes CesiumGS/quantized-mesh#10

Open

Update CONTRIBUTORS.md

9fddd3f

Require layerjson before testing geometric error

11f36af

kring requested changes Jan 30, 2020

View reviewed changes

jtfell and others added 4 commits January 30, 2020 11:52

Merge remote-tracking branch 'jtfell/master' into web-mercator-terrai…

6890bd4

…n-tiling-scheme

Merge branch 'master' into web-mercator-terrain-tiling-scheme

4ca5c08

use 'scheme' and 'projection' options to modify the tiling scheme

a829cfd

Merge branch 'web-mercator-terrain-tiling-scheme' of github.com:jtfel…

f2988ec

…l/cesium into web-mercator-terrain-tiling-scheme

jtfell requested a review from kring January 30, 2020 04:57

Update CHANGES.md.

e83db92

kring reviewed Jan 31, 2020

View reviewed changes

jtfell and others added 3 commits January 31, 2020 12:40

Ensure createQuantizedMeshTerrainData is called with cesium coordinat…

14f0e83

…es (not TMS coordinates)

Merge branch 'web-mercator-terrain-tiling-scheme' of github.com:jtfel…

d1bea6d

…l/cesium into web-mercator-terrain-tiling-scheme

Merge branch 'master' into web-mercator-terrain-tiling-scheme

7279ca8

jtfell requested a review from kring February 4, 2020 06:42

kring mentioned this pull request Feb 18, 2020

In-repo version of jtfell's #8563 #8616

Merged

kring added a commit that referenced this pull request Feb 18, 2020

Merge pull request #8616 from AnalyticalGraphicsInc/web-mercator-terr…

ef95027

…ain-tiling-scheme In-repo version of jtfell's #8563

kring merged commit 7279ca8 into CesiumGS:master Feb 18, 2020

jtfell mentioned this pull request Mar 30, 2020

Formalise the ability to use alternative SRS and tiling scheme CesiumGS/quantized-mesh#11

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WebMercator Tiling for CesiumTerrainProvider #8563

WebMercator Tiling for CesiumTerrainProvider #8563

jtfell commented Jan 22, 2020

cesium-concierge commented Jan 22, 2020

jtfell commented Jan 22, 2020

jtfell commented Jan 22, 2020

OmarShehata commented Jan 26, 2020

hpinkos commented Jan 29, 2020

kring left a comment

kring Jan 30, 2020

jtfell Jan 30, 2020

kring Jan 30, 2020

kring Jan 30, 2020

jtfell Jan 30, 2020

kring Jan 30, 2020

kring commented Jan 30, 2020

jtfell commented Jan 30, 2020

kring commented Jan 30, 2020

kring commented Jan 30, 2020

jtfell commented Jan 30, 2020

kring Jan 31, 2020

jtfell Jan 31, 2020

jtfell commented Feb 17, 2020

kring commented Feb 18, 2020

jtfell commented Apr 9, 2020

WebMercator Tiling for CesiumTerrainProvider #8563

WebMercator Tiling for CesiumTerrainProvider #8563

Conversation

jtfell commented Jan 22, 2020

cesium-concierge commented Jan 22, 2020

jtfell commented Jan 22, 2020

jtfell commented Jan 22, 2020

OmarShehata commented Jan 26, 2020

hpinkos commented Jan 29, 2020

kring left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kring commented Jan 30, 2020

jtfell commented Jan 30, 2020

kring commented Jan 30, 2020

kring commented Jan 30, 2020

jtfell commented Jan 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtfell commented Feb 17, 2020

kring commented Feb 18, 2020

jtfell commented Apr 9, 2020