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

Adding support for WMS-T Time parameter to WebMapServiceImageryProvider #6348

Merged
merged 18 commits into from
Oct 4, 2018
Merged

Adding support for WMS-T Time parameter to WebMapServiceImageryProvider #6348

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
9bd6f5c
WebMapServiceImageryProvider now supports clock and times
tamarmot Mar 17, 2018
9773587
code cleanup and testing for adding time to web map service imagery p…
tamarmot Mar 19, 2018
eed558c
merged with upstream
tamarmot Mar 19, 2018
cc7d7c5
corrected tests
tamarmot Mar 19, 2018
e3e47ba
clean up the handling of data from intervals
tamarmot Mar 19, 2018
7b9c87f
merged changes.md
tamarmot Mar 19, 2018
78381d8
merged CHANGES.md
tamarmot Mar 20, 2018
24b88cb
refactored WebMapServiceImageryProvider to be agnostic about parameters
tamarmot Mar 20, 2018
19b0d31
code cleanup
tamarmot Mar 23, 2018
b8648f1
Merge remote-tracking branch 'upstream/master'
tamarmot Mar 23, 2018
d7e2863
merged with upstream
tamarmot Mar 30, 2018
3b5b340
code cleanup for WMS-T
tamarmot Mar 30, 2018
08b64b6
merged changes
tamarmot Apr 3, 2018
42ba843
Merge remote-tracking branch 'agi/master'
Aug 30, 2018
9501620
Updated CHANGES, added in time dynamic pickFeature and general cleanup.
Aug 30, 2018
2e0327c
Merged agi master
Oct 4, 2018
d4ca1aa
Fixed CHANGES typo
Oct 4, 2018
34a99a1
Fixed CHANGES typo
Oct 4, 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
2 changes: 2 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Change Log
* Added a `ClippingPlane` object to be used with `ClippingPlaneCollection`.
* Added 3D Tiles use-case to Terrain Clipping Planes Sandcastle
* Updated `WebMapServiceImageryProvider` so it can take an srs or crs string to pass to the resource query parameters based on the WMS version. [#6223](https://github.com/AnalyticalGraphicsInc/cesium/issues/6223)
* Updated `WebMapServiceImageryProvider` so it can optionally take a clock and times and pass time as a parameter with the requests for image tiles.
* Sharing Sandcastle examples now works by storing the full example directly in the URL instead of creating GitHub gists, because anonymous gist creation was removed by GitHub. Loading existing gists will still work. [#6342](https://github.com/AnalyticalGraphicsInc/cesium/pull/6342)
* Added additional query parameter options to the CesiumViewer demo application:
* sourceType specifies the type of data source if the URL doesn't have a known file extension.
Expand All @@ -34,6 +35,7 @@ Change Log
* Fixed default value of `alphaCutoff` in glTF models. [#6346](https://github.com/AnalyticalGraphicsInc/cesium/pull/6346)
* Fixed rendering vector tiles when using `invertClassification`. [#6349](https://github.com/AnalyticalGraphicsInc/cesium/pull/6349)
* Fixed animation for glTF models with missing animation targets. [#6351](https://github.com/AnalyticalGraphicsInc/cesium/pull/6351)
* Fixed WMS-T (time) support in WebMapServiceImageryProvider [#2581](https://github.com/AnalyticalGraphicsInc/cesium/issues/2581)

### 1.43 - 2018-03-01

Expand Down
117 changes: 115 additions & 2 deletions Source/Scene/WebMapServiceImageryProvider.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ define([
'../Core/WebMercatorTilingScheme',
'../ThirdParty/Uri',
'./GetFeatureInfoFormat',
'./TimeDynamicImagery',
'./UrlTemplateImageryProvider'
], function(
combine,
Expand All @@ -29,7 +30,8 @@ define([
WebMercatorTilingScheme,
Uri,
GetFeatureInfoFormat,
UrlTemplateImageryProvider) {
TimeDynamicImagery,
UrlTemplateImageryProvider ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix white space

'use strict';

/**
Expand Down Expand Up @@ -69,6 +71,9 @@ define([
* @param {String|String[]} [options.subdomains='abc'] The subdomains to use for the <code>{s}</code> placeholder in the URL template.
* If this parameter is a single string, each character in the string is a subdomain. If it is
* an array, each element in the array is a subdomain.
* @param {Clock} [options.clock] A Clock instance that is used when determining the value for the time dimension. Required when options.times is specified.
* @param {TimeIntervalCollection} [options.times] TimeIntervalCollection with its data property being an object containing time dynamic dimension and their values.
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove extra blank line.

*
* @see ArcGisMapServerImageryProvider
* @see BingMapsImageryProvider
Expand Down Expand Up @@ -103,6 +108,10 @@ define([
}
//>>includeEnd('debug');

if (defined(options.times) && !defined(options.clock)) {
throw new DeveloperError('options.times was specified, so options.clock is required.');
}

if (defined(options.proxy)) {
deprecationWarning('WebMapServiceImageryProvider.proxy', 'The options.proxy parameter has been deprecated. Specify options.url as a Resource instance and set the proxy property there.');
Copy link
Contributor

Choose a reason for hiding this comment

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

This deprecation was removed in the master for our release next week. deprecationWarning isn't included in this file anymore.

}
Expand All @@ -113,6 +122,29 @@ define([

var pickFeatureResource = resource.clone();

var that = this;
this._reload = undefined;
if (defined(options.times)) {
this._timeDynamicImagery = new TimeDynamicImagery({
clock : options.clock,
times : options.times,
requestImageFunction : function(x, y, level, request, interval) {
return requestImage(that, x, y, level, request, interval);
},
reloadFunction : function() {
if (defined(that._reload)) {
that._reload();
}
}
});
}

if (defined(options.clock)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the time parameter a special case? Can't the dynamic properties remain generic like WMTS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The user can pass any parameters they want, but in this case we are building a time parameter out of the time intervals / clock information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made changes, but I still get the current time from the clock and create a time parameter if it does not exist in the parameters from the interval.

Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't be hard coding that time parameter. When I hooked it up into WMTS, there were many different ways server used this parameter. For instance some of them expected TIME as the parameter. Others had a parameter epoch, and time was the offset from this time or some expected time to be a range in the format start/end. In these cases, this code wouldn't work. We don't do this in WMTS and I don't think we should do it here. This needs to be generic and not specific to your use case.

if (defined(options.parameters)) {
options.parameters['time'] = options.clock.currentTime.toString();
}
}

resource.setQueryParameters(WebMapServiceImageryProvider.DefaultParameters, true);
pickFeatureResource.setQueryParameters(WebMapServiceImageryProvider.GetFeatureInfoDefaultParameters, true);

Expand Down Expand Up @@ -174,6 +206,33 @@ define([
});
}

function requestImage(imageryProvider, col, row, level, request, interval) {
var dynamicIntervalData = defined(interval) ? interval.data : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Following the Cesium style guidelines we save off any properties accessed more than once in the same function. Here for instance we would do something like the following because imageryProvider._tileProvider is used a few times.

var tileProvider = imageryProvider._tileProvider;
if (defined(dynamicIntervalData)) {
    tileProvider._resource.setQueryParameters(dynamicIntervalData);
}
return tileProvider.requestImage(col, row, level, request);

Also modifying tileProvider._resource shouldn't be an issue, since we create a copy of the resource before any async calls are made.

var resource = imageryProvider._tileProvider._resource; // We actually want to set the time parameter within the tile provider.
var parameters = {};
var keys = [];
if (defined(dynamicIntervalData)) {
if (dynamicIntervalData instanceof Object){
try {
Object.keys(dynamicIntervalData).forEach(function (key, index) {
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 just copying dynamicIntervalData. We can just pass that directly into setQueryParameters.

parameters[key] = dynamicIntervalData[key];
keys.push(key.toLowerCase());
});
} catch (err){
// Warn the user, this may not be a problem
console.warn('Data from interval has problems.', err);
Copy link
Contributor

Choose a reason for hiding this comment

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

In Cesium we don't output errors to the console if its a error by the developer. We occasionally use it if there is an error from a remote server. You can search for DeveloperError to see how we use it and also remove it from the minified build. Although I anticipate that this try/catch will go away.

}
}
}
if (!('time' in keys)){
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 removed like above.

// Get the time from the clock if a value was not provided in the data from the interval.
parameters['time'] = imageryProvider.clock.currentTime;
}
resource.setQueryParameters(parameters);

return imageryProvider._tileProvider.requestImage(col, row, level, request);
}

defineProperties(WebMapServiceImageryProvider.prototype, {
/**
* Gets the URL of the WMS server.
Expand Down Expand Up @@ -388,6 +447,35 @@ define([
set : function(enablePickFeatures) {
this._tileProvider.enablePickFeatures = enablePickFeatures;
}
},

/**
* Gets or sets a clock that is used to get keep the time used for time dynamic parameters.
* @memberof WebMapServiceImageryProvider.prototype
* @type {Clock}
*/
clock : {
get : function() {
return this._timeDynamicImagery.clock;
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is already merged, but there's a bug here. If times isn't passed in the constructor, this._timeDynamicImagery is undefined so this (and all other such references in the getters/setters below) will throw. Once a WMSImageryProvider is initialized as "non-dynamic" it's impossible to make it dynamic, so any references to dynamic properties should return undefined.

},
set : function(value) {
this._timeDynamicImagery.clock = value;
}
},
/**
* Gets or sets a time interval collection that is used to get time dynamic parameters. The data of each
* TimeInterval is an object containing the keys and values of the properties that are used during
* tile requests.
* @memberof WebMapServiceImageryProvider.prototype
* @type {TimeIntervalCollection}
*/
times : {
get : function() {
return this._timeDynamicImagery.times;
},
set : function(value) {
this._timeDynamicImagery.times = value;
}
}
});

Expand Down Expand Up @@ -421,7 +509,32 @@ define([
* @exception {DeveloperError} <code>requestImage</code> must not be called before the imagery provider is ready.
*/
WebMapServiceImageryProvider.prototype.requestImage = function(x, y, level, request) {
return this._tileProvider.requestImage(x, y, level, request);
var result;
var timeDynamicImagery = this._timeDynamicImagery;
var currentInterval;

if (!defined(timeDynamicImagery)){
return this._tileProvider.requestImage(x, y, level, request);
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 requestImage(this, x, y, level, request), just like below but without the interval.

}

// Try and load from cache
if (defined(timeDynamicImagery)) {
currentInterval = timeDynamicImagery.currentInterval;
result = timeDynamicImagery.getFromCache(x, y, level, request);
}

// Couldn't load from cache
if (!defined(result)) {
result = requestImage(this, x, y, level, request, currentInterval);
}

// If we are approaching an interval, preload this tile in the next interval
if (defined(result) && defined(timeDynamicImagery)) {
timeDynamicImagery.checkApproachingInterval(x, y, level, request);
}

return result;

};

/**
Expand Down
Loading