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.

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

Turn off throttle option when requesting for an availability tile from the parent terrain layer #9099

Merged
merged 4 commits into from
Aug 19, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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 CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

- Fixed several artifcats on mobile devices caused by using insufficient precision. [#9064](https://github.com/CesiumGS/cesium/pull/9064)
- Fixed handling of `data:` scheme for the Cesium ion logo URL. [#9085](https://github.com/CesiumGS/cesium/pull/9085)
- Fixed an issue where a request for availability tile of the reference layer is delayed when throttle option is on. [##9099](https://github.com/CesiumGS/cesium/pull/9099)

### 1.72 - 2020-08-03

Expand Down
2 changes: 1 addition & 1 deletion Source/Core/ArcGISTiledElevationTerrainProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ function requestAvailability(that, level, x, y) {
}

var request = new Request({
throttle: true,
throttle: false,
throttleByServer: true,
type: RequestType.TERRAIN,
});
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/CesiumTerrainProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -1278,7 +1278,7 @@ function checkLayer(provider, x, y, level, layer, topLayer) {
// For cutout terrain, if this isn't the top layer the availability tiles
// may never get loaded, so request it here.
var request = new Request({
throttle: true,
throttle: false,
throttleByServer: true,
type: RequestType.TERRAIN,
});
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/GoogleEarthEnterpriseTerrainProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ GoogleEarthEnterpriseTerrainProvider.prototype.getTileDataAvailable = function (
if (metadata.isValid(quadKey)) {
// We will need this tile, so request metadata and return false for now
var request = new Request({
throttle: true,
throttle: false,
throttleByServer: true,
type: RequestType.TERRAIN,
});
Expand Down
2 changes: 1 addition & 1 deletion Source/Scene/TimeDynamicImagery.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ function addToCache(that, tile, interval) {

var keyElements = getKeyElements(key);
var request = new Request({
throttle: true,
throttle: false,
throttleByServer: true,
type: RequestType.IMAGERY,
priorityFunction: tile.priorityFunction,
Expand Down
11 changes: 5 additions & 6 deletions Specs/Core/ArcGISTiledElevationTerrainProviderSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -474,12 +474,11 @@ describe("Core/ArcGISTiledElevationTerrainProvider", function () {
var promise;
var i;
for (i = 0; i < RequestScheduler.maximumRequestsPerServer; ++i) {
promise = terrainProvider.requestTileGeometry(
0,
0,
0,
createRequest()
);
var request = new Request({
throttle: true,
throttleByServer: true,
});
promise = terrainProvider.requestTileGeometry(0, 0, 0, request);
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it a problem that we're no longer calling createRequest here? So the request has no URL, for one thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it seems like createRequest() returns an empty request object with only throttleByServer option on. I kind of create the new Request for only the test case. Should I modify the createRequest() with throttle to be true for all of them?

Copy link
Member

@kring kring Aug 19, 2020

Choose a reason for hiding this comment

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

Oh ok, I hadn't realized createRequest also created a request without a URL. That makes sense, no need to change anything.

}
RequestScheduler.update();
expect(promise).toBeDefined();
Expand Down
4 changes: 2 additions & 2 deletions Specs/Scene/TimeDynamicImagerySpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ describe("Scene/TimeDynamicImagery", function () {
options.reloadFunction.calls.reset(); // Constructor calls reload

var request = new Request({
throttle: true,
throttle: false,
throttleByServer: true,
type: RequestType.IMAGERY,
});
Expand Down Expand Up @@ -173,7 +173,7 @@ describe("Scene/TimeDynamicImagery", function () {
options.reloadFunction.calls.reset(); // Constructor calls reload

var request = new Request({
throttle: true,
throttle: false,
throttleByServer: true,
type: RequestType.IMAGERY,
});
Expand Down