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.

Sign up for GitHub

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

WebMercator Tiling for CesiumTerrainProvider #8563

Merged
merged 11 commits into from
Feb 18, 2020
Merged

WebMercator Tiling for CesiumTerrainProvider #8563

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5c7a277
Add option to switch CesiumTerrainProvider to a WebMercator tiling sc…
jtfell Jan 22, 2020
9fddd3f
Update CONTRIBUTORS.md
jtfell Jan 22, 2020
11f36af
Require layerjson before testing geometric error
jtfell Jan 23, 2020
6890bd4
Merge remote-tracking branch 'jtfell/master' into web-mercator-terrai…
jtfell Jan 30, 2020
4ca5c08
Merge branch 'master' into web-mercator-terrain-tiling-scheme
jtfell Jan 30, 2020
a829cfd
use 'scheme' and 'projection' options to modify the tiling scheme
jtfell Jan 30, 2020
f2988ec
Merge branch 'web-mercator-terrain-tiling-scheme' of github.com:jtfel…
jtfell Jan 30, 2020
e83db92
Update CHANGES.md.
kring Jan 30, 2020
14f0e83
Ensure createQuantizedMeshTerrainData is called with cesium coordinat…
jtfell Jan 31, 2020
d1bea6d
Merge branch 'web-mercator-terrain-tiling-scheme' of github.com:jtfel…
jtfell Jan 31, 2020
7279ca8
Merge branch 'master' into web-mercator-terrain-tiling-scheme
jtfell Feb 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CONTRIBUTORS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,4 @@ See [CONTRIBUTING.md](CONTRIBUTING.md) for details on how to contribute to Cesiu
* [Tinco Andringa](https://github.com/tinco)
* [André Borud](https://github.com/andreborud)
* [Nathan Schulte](https://github.com/nmschulte)
* [Julian Fell](https://github.com/jtfell)
54 changes: 37 additions & 17 deletions Source/Core/CesiumTerrainProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import defineProperties from './defineProperties.js';
import DeveloperError from './DeveloperError.js';
import Event from './Event.js';
import GeographicTilingScheme from './GeographicTilingScheme.js';
import WebMercatorTilingScheme from './WebMercatorTilingScheme.js';
import getStringFromTypedArray from './getStringFromTypedArray.js';
import HeightmapTerrainData from './HeightmapTerrainData.js';
import IndexDatatype from './IndexDatatype.js';
Expand Down Expand Up @@ -72,18 +73,11 @@ import TileProviderError from './TileProviderError.js';
}
//>>includeEnd('debug');

this._tilingScheme = new GeographicTilingScheme({
numberOfLevelZeroTilesX : 2,
numberOfLevelZeroTilesY : 1,
ellipsoid : options.ellipsoid
});

this._heightmapWidth = 65;
this._levelZeroMaximumGeometricError = TerrainProvider.getEstimatedLevelZeroGeometricErrorForAHeightmap(this._tilingScheme.ellipsoid, this._heightmapWidth, this._tilingScheme.getNumberOfXTilesAtLevel(0));

this._heightmapStructure = undefined;
this._hasWaterMask = false;
this._hasVertexNormals = false;
this._ellipsoid = options.ellipsoid;

/**
* Boolean flag that indicates if the client should request vertex normals from the server.
Expand Down Expand Up @@ -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') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

that._tilingScheme = new GeographicTilingScheme({
numberOfLevelZeroTilesX : 2,
numberOfLevelZeroTilesY : 1,
ellipsoid : that._ellipsoid
});
} else if (data.tilingScheme === 'WebMercator') {
that._tilingScheme = new WebMercatorTilingScheme({
numberOfLevelZeroTilesX : 1,
numberOfLevelZeroTilesY : 1,
ellipsoid : that._ellipsoid
});
} else {
message = 'The layer.json tilingScheme "' + data.tilingScheme + '" is invalid or not supported.';
metadataError = TileProviderError.handleError(metadataError, that, that._errorEvent, message, undefined, undefined, undefined, requestLayerJson);
return;
}

that._levelZeroMaximumGeometricError = TerrainProvider.getEstimatedLevelZeroGeometricErrorForAHeightmap(
that._tilingScheme.ellipsoid,
that._heightmapWidth,
that._tilingScheme.getNumberOfXTilesAtLevel(0)
);

var availabilityTilesLoaded;

// The vertex normals defined in the 'octvertexnormals' extension is identical to the original
Expand Down Expand Up @@ -402,7 +421,7 @@ import TileProviderError from './TileProviderError.js';
};
}

function createHeightmapTerrainData(provider, buffer, level, x, y, tmsY) {
function createHeightmapTerrainData(provider, buffer, level, x, y) {
var heightBuffer = new Uint16Array(buffer, 0, provider._heightmapWidth * provider._heightmapWidth);
return new HeightmapTerrainData({
buffer : heightBuffer,
Expand All @@ -415,7 +434,7 @@ import TileProviderError from './TileProviderError.js';
});
}

function createQuantizedMeshTerrainData(provider, buffer, level, x, y, tmsY, layer) {
function createQuantizedMeshTerrainData(provider, buffer, level, x, y, layer) {
var littleEndianExtensionSize = layer.littleEndianExtensionSize;
var pos = 0;
var cartesian3Elements = 3;
Expand Down Expand Up @@ -633,9 +652,10 @@ import TileProviderError from './TileProviderError.js';
return undefined;
}

var yTiles = provider._tilingScheme.getNumberOfYTilesAtLevel(level);

var tmsY = (yTiles - y - 1);
if (provider._tilingScheme instanceof GeographicTilingScheme) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

var yTiles = provider._tilingScheme.getNumberOfYTilesAtLevel(level);
y = (yTiles - y - 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

var extensionList = [];
if (provider._requestVertexNormals && layerToUse.hasVertexNormals) {
Expand All @@ -650,7 +670,7 @@ import TileProviderError from './TileProviderError.js';

var headers;
var query;
var url = urlTemplates[(x + tmsY + level) % urlTemplates.length];
var url = urlTemplates[(x + y + level) % urlTemplates.length];
var resource = layerToUse.resource;
if (defined(resource._ionEndpoint) && !defined(resource._ionEndpoint.externalType)) {
// ion uses query paremeters to request extensions
Expand All @@ -669,7 +689,7 @@ import TileProviderError from './TileProviderError.js';
version: layerToUse.version,
z: level,
x: x,
y: tmsY
y: y
},
queryParameters: query,
headers: headers,
Expand All @@ -682,9 +702,9 @@ import TileProviderError from './TileProviderError.js';

return promise.then(function (buffer) {
if (defined(provider._heightmapStructure)) {
return createHeightmapTerrainData(provider, buffer, level, x, y, tmsY);
return createHeightmapTerrainData(provider, buffer, level, x, y);
}
return createQuantizedMeshTerrainData(provider, buffer, level, x, y, tmsY, layerToUse);
return createQuantizedMeshTerrainData(provider, buffer, level, x, y, layerToUse);
});
}

Expand Down
12 changes: 9 additions & 3 deletions Specs/Core/CesiumTerrainProviderSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,13 +206,19 @@ describe('Core/CesiumTerrainProvider', function() {
});

it('returns reasonable geometric error for various levels', function() {
returnQuantizedMeshTileJson();

var provider = new CesiumTerrainProvider({
url : 'made/up/url'
});

expect(provider.getLevelMaximumGeometricError(0)).toBeGreaterThan(0.0);
expect(provider.getLevelMaximumGeometricError(0)).toEqualEpsilon(provider.getLevelMaximumGeometricError(1) * 2.0, CesiumMath.EPSILON10);
expect(provider.getLevelMaximumGeometricError(1)).toEqualEpsilon(provider.getLevelMaximumGeometricError(2) * 2.0, CesiumMath.EPSILON10);
return pollToPromise(function() {
return provider.ready;
}).then(function() {
expect(provider.getLevelMaximumGeometricError(0)).toBeGreaterThan(0.0);
expect(provider.getLevelMaximumGeometricError(0)).toEqualEpsilon(provider.getLevelMaximumGeometricError(1) * 2.0, CesiumMath.EPSILON10);
expect(provider.getLevelMaximumGeometricError(1)).toEqualEpsilon(provider.getLevelMaximumGeometricError(2) * 2.0, CesiumMath.EPSILON10);
});
});

it('logo is undefined if credit is not provided', function() {
Expand Down