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

Jump to bottom

Tentative fix for issue 416: TileMapServiceImageryProvider extent #1010

Closed
wants to merge 1 commit into from
Closed

Tentative fix for issue 416: TileMapServiceImageryProvider extent #1010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 76 additions & 0 deletions Apps/Sandcastle/gallery/Test Imagery Layers.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<!DOCTYPE html>
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding a new Sandcastle app, you should add unit tests for this. Sandcastle apps are meant to demonstrate things to end-users, not to test Cesium for correct functionality.

Copy link
Author

Choose a reason for hiding this comment

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

Indeed it is my first bug fix, I find using Sandcastle for testing the most simple way. Could you point me to a unit test sample ?

Copy link
Member

Choose a reason for hiding this comment

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

Tests are found in the Specs directory:
https://github.com/AnalyticalGraphicsInc/cesium/tree/master/Specs

The tests for ImageryLayer are found here:
https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Specs/Scene/ImageryLayerSpec.js

Copy link
Author

Choose a reason for hiding this comment

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

I read the 2 unit test you wrote for fixing the imagery stretching issue ( #970 ). If I understand correctly the imagery of a base layer is always stretched to cover the whole ellipsoid, while for a non-base layer this is not the case and the real width and height are preserved.
I noticed you created a specific imagery source (Data/TMS/SmallArea). Indeed there is only the xml file used for defining the imagery parameters. Referring to the base layer case, it is not clear to me why the west terrain tile is made up by 4 imageries and the other tile by 2 imageries only. It should be dependent on the TilingScheme of the TileMapServiceImageryProvider, shouldn't it ? In general given a simple imagery source like the one you defined, how can I foresee how many imagery pieces are included inside a specific terrain tile (not necessary a zero level tile) ?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's a little confusing. Basically, the SmallArea/tilemapresource.xml file used in the test specifies that the first level is level 11, it uses the Web Mercator tiling scheme, and that the overall extent of the imagery is as specified. If you do the math on that (you can get WebMercatorTilingScheme to do it for you) that means there are 4 tiles at the first level, level 11.

Meanwhile, our terrain, which is the default EllipsoidTerrainProvider, has two tiles at level 0, one covering the western hemisphere and one the eastern. And our test is computing which imagery tiles overlap these two tiles.

All four imagery tiles are in the western hemisphere, so they all overlap the western hemisphere terrain tile. That's why there are four for tiles[0]. None of them overlap the eastern hemisphere terrain tile, though, so why is tiles[1].imagery.length expected to be 2? That's because the imagery layer is a base layer, so the two tiles eastern-most imagery tiles are stretched all the way to the eastern hemisphere of the globe.

I'm not completely sure if I've answered your question, so let me know if that helps.

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your explanation. I think to have understood. Moreover, if I got it correctly, both terrain and imagery tiles at each level become 4 times the tiles of the previous level, don't they ?
I would need some hint on how to design the unit test for my fix. Should I create a virtual imagery source (as your SmallArea) and define the extent attribute passed to the TileMapServiceImageryProvider in a way that I know what are the textureCoordinateExtent of each TileImagery that is included in a terrain tile ? For instance I could set the extent attribute to a value that for tiles[0] the first imagery has a bounding box [0.2, 0.5, 0.5, 0.8] the second imagery has a bbox [0.2, 0.2, 0.5, 0.5] and so on; no intersection with tiles[1] and then other precomputed bbox for children terrain tiles of tiles[0]. Would it be an effective way to write the unit test ? How much error tollerance should I use in comparing computed values with expected ones ? That is, instead of a bbox [0.2, 0.5, 0.5, 08] I could get [0.2001, 0.2, 0.4999, 0.5] .

Copy link
Member

Choose a reason for hiding this comment

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

Sorry for the late response on this!

Yes, in general, each tile has four child tiles. All four children don't necessarily exist, though.

Your proposed test sounds reasonable to me. I think if you compute what the texture coordinate extent should be, you can expect the answer Cesium comes up with to be very close to that. Maybe within 1e-14 or so, given that the numbers are all less than 1.0.

<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="Create imagery layers from multiple sources.">
<meta name="cesium-sandcastle-labels" content="Beginner, Tutorial">
<title>Cesium Demo</title>
<script type="text/javascript" src="../Sandcastle-header.js"></script>
<script type="text/javascript" src="../../../ThirdParty/requirejs-2.1.6/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" data-sandcastle-title="Cesium + require.js">
<style>
@import url(../templates/bucket.css);
</style>
<div id="cesiumContainer" class="fullSize"></div>
<div id="loadingOverlay"><h1>Loading...</h1></div>
<script id="cesium_sandcastle_script">
require(['Cesium'], function(Cesium) {
"use strict";

var widget = new Cesium.CesiumWidget('cesiumContainer');

var layers = widget.centralBody.getImageryLayers();
//layers.removeAll();
/*
layers.addImageryProvider(new Cesium.TileMapServiceImageryProvider({
url : 'http://cesium.agi.com/blackmarble',
maximumLevel : 8,
extent : new Cesium.Extent(
Cesium.Math.toRadians(-75.0),
Cesium.Math.toRadians(28.0),
Cesium.Math.toRadians(-67.0),
Cesium.Math.toRadians(29.75)),
credit : 'Black Marble imagery courtesy NASA Earth Observatory'
}));
*/

layers.addImageryProvider(new Cesium.TileMapServiceImageryProvider({
url : '../images/cesium_maptiler/Cesium_Logo_Color',
maximumLevel : 4,
extent : new Cesium.Extent(
Cesium.Math.toRadians(-75.0),
Cesium.Math.toRadians(28.0),
Cesium.Math.toRadians(-67.0),
Cesium.Math.toRadians(29.75)),
credit : 'Black Marble imagery courtesy NASA Earth Observatory'
}));


layers.addImageryProvider(new Cesium.SingleTileImageryProvider({
url : '../images/Cesium_Logo_overlay.png',
extent : new Cesium.Extent(
Cesium.Math.toRadians(-75.0),
Cesium.Math.toRadians(28.0),
Cesium.Math.toRadians(-67.0),
Cesium.Math.toRadians(29.75))
}));


layers.addImageryProvider(new Cesium.TileCoordinatesImageryProvider());



Sandcastle.finishedLoading();
});
</script>
</body>
</html>
6 changes: 5 additions & 1 deletion Source/Scene/ImageryLayer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ define([

var terrainExtent = tile.extent;
var imageryExtent = imageryTilingScheme.tileXYToExtent(northwestTileCoordinates.x, northwestTileCoordinates.y, imageryLevel);
imageryExtent = imageryExtent.intersectWith(extent);

var minU;
var maxU = 0.0;
Expand All @@ -497,6 +498,7 @@ define([
minU = maxU;

imageryExtent = imageryTilingScheme.tileXYToExtent(i, northwestTileCoordinates.y, imageryLevel);
imageryExtent = imageryExtent.intersectWith(extent);
maxU = Math.min(1.0, (imageryExtent.east - terrainExtent.west) / (terrainExtent.east - terrainExtent.west));

// If this is the eastern-most imagery tile mapped to this terrain tile,
Expand All @@ -513,6 +515,8 @@ define([
maxV = minV;

imageryExtent = imageryTilingScheme.tileXYToExtent(i, j, imageryLevel);
var imageryTileExtent = imageryExtent.clone();
Copy link
Member

Choose a reason for hiding this comment

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

The clone is not necessary here because the intersectWith below returns a new instance, it does not modify the existing one.

Copy link
Author

Choose a reason for hiding this comment

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

On Mon, 05 Aug 2013 23:56:29 +0200, Kevin Ring [email protected]
wrote:

@@ -513,6 +515,8 @@ define([
maxV = minV;

             imageryExtent = imageryTilingScheme.tileXYToExtent(i,

j, imageryLevel);

  •            var imageryTileExtent = imageryExtent.clone();

The clone is not necessary here because the intersectWith below
returns a new instance, it does not modify the existing one.

I was not aware of that, I will fix it.

-- Marco

Using Opera's revolutionary e-mail client: http://www.opera.com/mail/

imageryExtent = imageryExtent.intersectWith(extent);
minV = Math.max(0.0, (imageryExtent.south - terrainExtent.south) / (terrainExtent.north - terrainExtent.south));

// If this is the southern-most imagery tile mapped to this terrain tile,
Expand All @@ -524,7 +528,7 @@ define([
}

var texCoordsExtent = new Cartesian4(minU, minV, maxU, maxV);
var imagery = this.getImageryFromCache(i, j, imageryLevel, imageryExtent);
var imagery = this.getImageryFromCache(i, j, imageryLevel, imageryTileExtent);
tile.imagery.splice(insertionPoint, 0, new TileImagery(imagery, texCoordsExtent));
++insertionPoint;
}
Expand Down