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

getTileDataAvailable() handling for terrain providers #2102

Merged
merged 16 commits into from
Sep 18, 2014
Merged

getTileDataAvailable() handling for terrain providers #2102

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e0d0341
first pass
Sep 1, 2014
7a380e9
add default function definition for supportsTileDataAvailable()
Sep 1, 2014
6a60a5b
Don't crash if supportsTileDataAvailable is not defined.
kring Sep 2, 2014
427b6ac
use single function getTileDataAvailable() and add to other terrain p…
Sep 2, 2014
576320a
move profile scene
Sep 2, 2014
7b3626a
add SanFrancisco profile scene
Sep 2, 2014
e65ee54
jshint pass
Sep 2, 2014
ffbfd66
Merge branch 'master' into tile-loading
Sep 2, 2014
66229ac
move profiling test to sandcastle
Sep 3, 2014
b1c95a0
update CHANGES.md
Sep 3, 2014
0c46ebb
add specs for getTileDataAvailable()
Sep 3, 2014
25d222c
test the getTileDataAvailable() false case as well
Sep 3, 2014
4517047
ammend documentation
Sep 4, 2014
a361e49
Merge remote-tracking branch 'origin/master' into tile-loading
kring Sep 17, 2014
c815459
Add a deprecationWarning when getTileDataAvailable is not implemented.
kring Sep 17, 2014
3fbdd62
Add note to Deprecated section.
kring Sep 18, 2014
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
12 changes: 12 additions & 0 deletions Source/Core/ArcGisImageServerTerrainProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,5 +244,17 @@ define([
return this._levelZeroMaximumGeometricError / (1 << level);
};

/**
* Determines whether data for a tile is available to be loaded.
*
* @param {Number} x The X coordinate of the tile for which to request geometry.
* @param {Number} y The Y coordinate of the tile for which to request geometry.
* @param {Number} level The level of the tile for which to request geometry.
* @returns {Number} Undefined if not supported, otherwise true or false.
*/
ArcGisImageServerTerrainProvider.prototype.getTileDataAvailable = function(x, y, level) {
return undefined;
};

return ArcGisImageServerTerrainProvider;
});
22 changes: 22 additions & 0 deletions Source/Core/CesiumTerrainProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -636,5 +636,27 @@ define([
return false;
}

/**
* Determines whether data for a tile is available to be loaded.
*
* @param {Number} x The X coordinate of the tile for which to request geometry.
* @param {Number} y The Y coordinate of the tile for which to request geometry.
* @param {Number} level The level of the tile for which to request geometry.
* @returns {Number} Undefined if not supported, otherwise true or false.
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be {Boolean}, right?

*/
CesiumTerrainProvider.prototype.getTileDataAvailable = function(x, y, level) {
var available = this._availableTiles;

if (!available || available.length === 0) {
return undefined;
} else {
if (level >= available.length) {
return false;
}
var levelAvailable = available[level];
return isTileInRange(levelAvailable, x, y);
}
};

return CesiumTerrainProvider;
});
12 changes: 12 additions & 0 deletions Source/Core/EllipsoidTerrainProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,5 +166,17 @@ define([
return this._levelZeroMaximumGeometricError / (1 << level);
};

/**
* Determines whether data for a tile is available to be loaded.
*
* @param {Number} x The X coordinate of the tile for which to request geometry.
* @param {Number} y The Y coordinate of the tile for which to request geometry.
* @param {Number} level The level of the tile for which to request geometry.
* @returns {Number} Undefined if not supported, otherwise true or false.
*/
EllipsoidTerrainProvider.prototype.getTileDataAvailable = function(x, y, level) {
return undefined;
};

return EllipsoidTerrainProvider;
});
11 changes: 11 additions & 0 deletions Source/Core/TerrainProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,5 +192,16 @@ define([
*/
TerrainProvider.prototype.getLevelMaximumGeometricError = DeveloperError.throwInstantiationError;

/**
* Determines whether data for a tile is available to be loaded.
* @function
*
* @param {Number} x The X coordinate of the tile for which to request geometry.
* @param {Number} y The Y coordinate of the tile for which to request geometry.
* @param {Number} level The level of the tile for which to request geometry.
* @returns {Number} Undefined if not supported by the terrain provider, otherwise true or false.
*/
TerrainProvider.prototype.getTileDataAvailable = DeveloperError.throwInstantiationError;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a breaking change for folks who implemented TerrainProvider, right?

We should provide a default implementation to not break them and perhaps warn if we want to eventually require this. See the Deprecation Guide.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see isDataAvailable below in GlobeSurfaceTile.js. Is that the right place? I would expect to treat this as an abstract base class instead so all users do not need to check for the function.

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently there are no "base classes" or inheritance in Cesium. TerrainProvider only exists as documentation of an implicit interface, so there are no breaking changes here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right. This is still a little sketchy. It's as if we want to document this function as optional so users know they need to check to see if the function exists or we need to start using inheritance of whatever form to avoid this.

Copy link
Member

Choose a reason for hiding this comment

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

It's as if we want to document this function as optional so users know they need to check to see if the function exists or we need to start using inheritance of whatever form to avoid this.

I believe inheritance would have a performance cost we'd rather not pay. I agree it should be documented as optional, though.


return TerrainProvider;
});
12 changes: 12 additions & 0 deletions Source/Core/VRTheWorldTerrainProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,5 +335,17 @@ define([
return !Rectangle.isEmpty(Rectangle.intersectWith(tileRectangle, rectangle, rectangleScratch));
}

/**
* Determines whether data for a tile is available to be loaded.
*
* @param {Number} x The X coordinate of the tile for which to request geometry.
* @param {Number} y The Y coordinate of the tile for which to request geometry.
* @param {Number} level The level of the tile for which to request geometry.
* @returns {Number} Undefined if not supported, otherwise true or false.
*/
VRTheWorldTerrainProvider.prototype.getTileDataAvailable = function(x, y, level) {
return undefined;
};

return VRTheWorldTerrainProvider;
});
12 changes: 10 additions & 2 deletions Source/Scene/GlobeSurfaceTile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ define([
surfaceTile.upsampledTerrain = new TileTerrain(upsampleTileDetails);
}

if (isDataAvailable(tile)) {
if (isDataAvailable(tile, terrainProvider)) {
surfaceTile.loadedTerrain = new TileTerrain();
}

Expand Down Expand Up @@ -618,7 +618,15 @@ define([
}
}

function isDataAvailable(tile) {
function isDataAvailable(tile, terrainProvider) {

if (defined(terrainProvider.getTileDataAvailable)) {
var tileDataAvailable = terrainProvider.getTileDataAvailable(tile.x, tile.y, tile.level);
if (defined(tileDataAvailable)) {
return tileDataAvailable;
}
}

var parent = tile.parent;
if (!defined(parent)) {
// Data is assumed to be available for root tiles.
Expand Down
57 changes: 57 additions & 0 deletions Specs/Profile/HalfDome.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<!DOCTYPE html>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why submit this and SanFrancisco.html? Why not profile with the Sandcastle app? I would prefer that we not include somewhat random files like these in the repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect this is to get the total seconds as part of the network profile?

I'd still rather not have these here; it is yet another place we need to remember to update. Perhaps just separate Sandcastle examples would be OK - long-term, we may have a label for examples not included in the final release but used for testing and profiling. Or perhaps there is a way another way to get a time interval from the network profile? Or just not submit anything for now.

<html lang="en">
<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=Edge,chrome=1">
<!-- Use Chrome Frame in IE -->
<meta name="viewport" content="width=device-width, height=device-height, initial-scale=1, maximum-scale=1, minimum-scale=1, user-scalable=no">
<meta name="description" content="Use Viewer to start building new applications or easily embed Cesium into existing applications.">
<meta name="cesium-sandcastle-labels" content="Beginner, Showcases">
<title>Cesium Demo</title>
<script type="text/javascript" src="../../Apps/Sandcastle/Sandcastle-header.js"></script>
<script type="text/javascript" src="../../ThirdParty/requirejs-2.1.9/require.js"></script>
<script type="text/javascript">
require.config({
baseUrl : '../../../Source',
waitSeconds : 60
});
</script>
</head>
<body class="sandcastle-loading" data-sandcastle-bucket="bucket-requirejs.html">
<style>
@import url(../../Apps/Sandcastle/templates/bucket.css);
</style>
<div id="cesiumContainer" class="fullSize"></div>
<div id="toolbar"></div>
<script id="cesium_sandcastle_script">
function startup(Cesium) {
"use strict";

var cesiumTerrainProviderMeshes = new Cesium.CesiumTerrainProvider({
url : '//cesiumjs.org/stk-terrain/tilesets/world/tiles'
});

var viewer = new Cesium.Viewer('cesiumContainer');
var scene = viewer.scene;
scene.terrainProvider = cesiumTerrainProviderMeshes;

var setCamera = function(){
var eye = new Cesium.Cartesian3(-2496304.1498512086, -4391818.97382059, 3884176.4503971986);
var target = Cesium.Cartesian3.add(eye, new Cesium.Cartesian3(0.9279518715011381, -0.29488412129953234, -0.22792252890604328), new Cesium.Cartesian3());
var up = new Cesium.Cartesian3(-0.11836693744723503, -0.8130611584421428, 0.5700182635106171);
scene.camera.lookAt(eye, target, up);
}

setCamera();

// Half Dome
setInterval(setCamera, 1000);
}
if (typeof Cesium !== "undefined") {
startup(Cesium);
} else if (typeof require === "function") {
require(["Cesium"], startup);
}
</script>
</body>
</html>
58 changes: 58 additions & 0 deletions Specs/Profile/SanFrancisco.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=Edge,chrome=1">
<!-- Use Chrome Frame in IE -->
<meta name="viewport" content="width=device-width, height=device-height, initial-scale=1, maximum-scale=1, minimum-scale=1, user-scalable=no">
<meta name="description" content="Use Viewer to start building new applications or easily embed Cesium into existing applications.">
<meta name="cesium-sandcastle-labels" content="Beginner, Showcases">
<title>Cesium Demo</title>
<script type="text/javascript" src="../../Apps/Sandcastle/Sandcastle-header.js"></script>
<script type="text/javascript" src="../../ThirdParty/requirejs-2.1.9/require.js"></script>
<script type="text/javascript">
require.config({
baseUrl : '../../../Source',
waitSeconds : 60
});
</script>
</head>
<body class="sandcastle-loading" data-sandcastle-bucket="bucket-requirejs.html">
<style>
@import url(../../Apps/Sandcastle/templates/bucket.css);
</style>
<div id="cesiumContainer" class="fullSize"></div>
<div id="toolbar"></div>
<script id="cesium_sandcastle_script">
function startup(Cesium) {
"use strict";

var cesiumTerrainProviderMeshes = new Cesium.CesiumTerrainProvider({
url : '//cesiumjs.org/stk-terrain/tilesets/world/tiles'
});

var viewer = new Cesium.Viewer('cesiumContainer');
var scene = viewer.scene;
scene.terrainProvider = cesiumTerrainProviderMeshes;

var setCamera = function(){
var eye = new Cesium.Cartesian3(-2703493, -4261486, 3887588);
var target = Cesium.Cartesian3.add(eye, new Cesium.Cartesian3(-0.3, 0.9, 0.4), new Cesium.Cartesian3());
var up = new Cesium.Cartesian3(-0.6502679490649945, -0.3129458646313862, 0.6922546353438556);
scene.camera.lookAt(eye, target, up);
}

setCamera();

setInterval(function(){
setCamera();
}, 1000);
}
if (typeof Cesium !== "undefined") {
startup(Cesium);
} else if (typeof require === "function") {
require(["Cesium"], startup);
}
</script>
</body>
</html>