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

Allow promise to url for Cesium terrain, 3d tiles and TMS imagery #6204

Merged
merged 4 commits into from
Feb 13, 2018
Merged

Allow promise to url for Cesium terrain, 3d tiles and TMS imagery #6204

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fef5d25
allow promise to url for CesiumTerrainProvider, createTileMapServiceI…
Feb 9, 2018
3067a77
handle rejection
Feb 12, 2018
5c56ec5
cleanup
Feb 12, 2018
64d50a8
cleanup
Feb 13, 2018
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
29 changes: 16 additions & 13 deletions Source/Core/CesiumTerrainProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ define([
* @constructor
*
* @param {Object} options Object with the following properties:
* @param {Resource|String} options.url The URL of the Cesium terrain server.
* @param {Resource|String|Promise<Resource>|Promise<String>} options.url The URL of the Cesium terrain server.
* @param {Boolean} [options.requestVertexNormals=false] Flag that indicates if the client should request additional lighting information from the server, in the form of per vertex normals if available.
* @param {Boolean} [options.requestWaterMask=false] Flag that indicates if the client should request per tile water masks from the server, if available.
* @param {Ellipsoid} [options.ellipsoid] The ellipsoid. If not specified, the WGS84 ellipsoid is used.
Expand Down Expand Up @@ -112,11 +112,6 @@ define([
deprecationWarning('CesiumTerrainProvider.proxy', 'The options.proxy parameter has been deprecated. Specify options.url as a Resource instance and set the proxy property there.');
}

var resource = Resource.createIfNeeded(options.url, {
proxy: options.proxy
});
resource.appendForwardSlash();

this._tilingScheme = new GeographicTilingScheme({
numberOfLevelZeroTilesX : 2,
numberOfLevelZeroTilesY : 1,
Expand Down Expand Up @@ -159,17 +154,27 @@ define([
this._ready = false;
this._readyPromise = when.defer();

var lastResource = resource;
var metadataResource = lastResource.getDerivedResource({
url: 'layer.json'
});

var that = this;
var lastResource;
var metadataResource;
var metadataError;

var layers = this._layers = [];
var attribution = '';
var overallAvailability = [];
when(options.url)
Copy link
Contributor

Choose a reason for hiding this comment

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

If options.url rejects, this will fail silently, don't we need to move this entire block into requestMetadata itself? Also, request Metadata is using old (improper) promise usage, change the success and failure functions off of this promise when you move it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you want the behavior to be when options.url rejects? I don't think calling metadataFailure is the right option because it will still try to load the tileset even if fetching the metadata fails. And we literally can't to anything with the URL.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, I assume we want to just propagate the url rejection to the readyPromise so that it rejects. This way a user gets the exact error.

.then(function(url) {
var resource = Resource.createIfNeeded(url, {
proxy: options.proxy
});
resource.appendForwardSlash();
lastResource = resource;
metadataResource = lastResource.getDerivedResource({
url: 'layer.json'
});

requestMetadata();
});

function parseMetadataSuccess(data) {
var message;
Expand Down Expand Up @@ -346,8 +351,6 @@ define([
var metadata = metadataResource.fetchJson();
when(metadata, metadataSuccess, metadataFailure);
}

requestMetadata();
}

/**
Expand Down
53 changes: 30 additions & 23 deletions Source/Scene/Cesium3DTileset.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ define([
* @constructor
*
* @param {Object} options Object with the following properties:
* @param {Resource|String} options.url The url to a tileset.json file or to a directory containing a tileset.json file.
* @param {Resource|String|Promise<Resource>|Promise<String>} options.url The url to a tileset.json file or to a directory containing a tileset.json file.
* @param {Boolean} [options.show=true] Determines if the tileset will be shown.
* @param {Matrix4} [options.modelMatrix=Matrix4.IDENTITY] A 4x4 transformation matrix that transforms the tileset's root tile.
* @param {ShadowMode} [options.shadows=ShadowMode.ENABLED] Determines whether the tileset casts or receives shadows from each light source.
Expand Down Expand Up @@ -165,26 +165,9 @@ define([
Check.defined('options.url', options.url);
//>>includeEnd('debug');

var resource = Resource.createIfNeeded(options.url);

var tilesetResource = resource;
var basePath;

if (resource.extension === 'json') {
basePath = resource.getBaseUri(true);
} else if (resource.isDataUri) {
basePath = '';
} else {
resource.appendForwardSlash();
tilesetResource = resource.getDerivedResource({
url: 'tileset.json'
});
basePath = resource.url;
}

this._url = resource.url;
this._tilesetUrl = tilesetResource.url;
this._basePath = basePath;
this._url = undefined;
this._tilesetUrl = undefined;
this._basePath = undefined;
this._root = undefined;
this._asset = undefined; // Metadata for the entire tileset
this._properties = undefined; // Metadata for per-model/point/etc properties
Expand Down Expand Up @@ -699,9 +682,33 @@ define([
this._brokenUrlWorkaround = false;

var that = this;
var tilesetResource;
when(options.url)
.then(function(url) {
var basePath;
var resource = Resource.createIfNeeded(url);

tilesetResource = resource;

if (resource.extension === 'json') {
basePath = resource.getBaseUri(true);
} else if (resource.isDataUri) {
basePath = '';
} else {
resource.appendForwardSlash();
tilesetResource = resource.getDerivedResource({
url: 'tileset.json'
});
basePath = resource.url;
}

// We don't know the distance of the tileset until tileset.json is loaded, so use the default distance for now
Cesium3DTileset.loadJson(tilesetResource)
that._url = resource.url;
that._tilesetUrl = tilesetResource.url;
that._basePath = basePath;

// We don't know the distance of the tileset until tileset.json is loaded, so use the default distance for now
return Cesium3DTileset.loadJson(tilesetResource);
})
.then(function(tilesetJson) {
return detectBrokenUrlWorkaround(that, tilesetResource, tilesetJson);
})
Expand Down
27 changes: 16 additions & 11 deletions Source/Scene/createTileMapServiceImageryProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ define([
* @exports createTileMapServiceImageryProvider
*
* @param {Object} [options] Object with the following properties:
* @param {Resource|String} [options.url='.'] Path to image tiles on server.
* @param {Resource|String|Promise<Resource>|Promise<String>} [options.url='.'] Path to image tiles on server.
* @param {String} [options.fileExtension='png'] The file extension for images on the server.
* @param {Credit|String} [options.credit=''] A credit for the data source, which is displayed on the canvas.
* @param {Number} [options.minimumLevel=0] The minimum level-of-detail supported by the imagery provider. Take care when specifying
Expand Down Expand Up @@ -95,19 +95,25 @@ define([
deprecationWarning('createTileMapServiceImageryProvider.proxy', 'The options.proxy parameter has been deprecated. Specify options.url as a Resource instance and set the proxy property there.');
}

var resource = Resource.createIfNeeded(options.url, {
proxy : options.proxy
});
resource.appendForwardSlash();

var xmlResource = resource.getDerivedResource({
url: 'tilemapresource.xml'
});

var deferred = when.defer();
var imageryProvider = new UrlTemplateImageryProvider(deferred.promise);

var metadataError;
var resource;
var xmlResource;
when(options.url)
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar comment to CesiumTerrainProvider regarding failure and requestMetadata.

.then(function(url) {
resource = Resource.createIfNeeded(url, {
proxy : options.proxy
});
resource.appendForwardSlash();

xmlResource = resource.getDerivedResource({
url: 'tilemapresource.xml'
});

requestMetadata();
});

function metadataSuccess(xml) {
var tileFormatRegex = /tileformat/i;
Expand Down Expand Up @@ -282,7 +288,6 @@ define([
xmlResource.fetchXML().then(metadataSuccess).otherwise(metadataFailure);
}

requestMetadata();
return imageryProvider;
}

Expand Down
11 changes: 11 additions & 0 deletions Specs/Core/CesiumTerrainProviderSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,17 @@ defineSuite([
});
});

it('resolves readyPromise when url promise is used', function() {
var provider = new CesiumTerrainProvider({
url : when.resolve('made/up/url')
});

return provider.readyPromise.then(function (result) {
expect(result).toBe(true);
expect(provider.ready).toBe(true);
});
});

it('resolves readyPromise with Resource', function() {
var resource = new Resource({
url : 'made/up/url'
Expand Down
17 changes: 17 additions & 0 deletions Specs/Scene/Cesium3DTilesetSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,23 @@ defineSuite([
});
});

it('Constructor works with promise to resource', function() {
var resource = new Resource({
url: 'Data/Cesium3DTiles/Tilesets/TilesetOfTilesets'
});

// setup tileset with invalid url (overridden loadJson should replace invalid url with correct url)
var tileset = new Cesium3DTileset({
url : when.resolve(resource)
});

return tileset.readyPromise.then(function() {
expect(tileset.ready).toEqual(true);
}).otherwise(function(error) {
fail('should not fail');
});
});

it('Constructor works with directory resource', function() {
var resource = new Resource({
url: 'Data/Cesium3DTiles/Tilesets/TilesetOfTilesets'
Expand Down
11 changes: 11 additions & 0 deletions Specs/Scene/createTileMapServiceImageryProviderSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,17 @@ defineSuite([
});
});

it('resolves readyPromise when promise url is used', function() {
var provider = createTileMapServiceImageryProvider({
url : when.resolve('made/up/tms/server/')
});

return provider.readyPromise.then(function(result) {
expect(result).toBe(true);
expect(provider.ready).toBe(true);
});
});

it('resolves readyPromise with Resource', function() {
var resource = new Resource({
url: 'made/up/tms/server/'
Expand Down