Cesium3DTileset tests #3367

lilleyse · 2015-12-21T16:30:56Z

Added tests for Cesium3DTileset and Tileset3DTileContentProvider
Changed the replacement refinement approach to work closer with the selectTiles function. I'm still not 100% happy with the approach. I was experimenting with other approaches but it wasn't any better.
Fixed issue where tile with content box is not culled. It was checking the CullingVolume mask instead of Intersect.
Changed up the addToProcessingQueue and removeFromProcessingQueue functions which weren't operating correctly in certain edge cases, like failed requests. This area in general will change again with the request scheduler.
Implemented Cesium3DTileset.destroy. In order to handle cases where the tileset is destroyed while content requests are still in progress, when the request completes I throw an error to reject the ready promise. However I'm open to different ideas.
For the test tileset files, they all use copies of the same b3dm files which is a bit wasteful. Maybe we could support creating a Cesium3DTileset with a url to an explicit tiles.json file, right now you can only give it a folder.

pjcozzi · 2015-12-21T18:32:05Z

Fixed issue where tile with content box is not culled. It was checking the CullingVolume mask instead of Intersect.

Great, this fixes part of #3241:

Does culling with content.box still work? For example, the root tile for Canary Wharf is always rendered when just the tile's box is visible (even it the content box is not).

pjcozzi · 2015-12-21T18:33:12Z

Maybe we could support creating a Cesium3DTileset with a url to an explicit tiles.json file, right now you can only give it a folder.

Go for it. Perhaps assume the filename is tiles.json if one isn't provided.

pjcozzi · 2015-12-21T18:38:06Z

Source/Scene/Batched3DModel3DTileContentProvider.js

@@ -122,6 +122,9 @@ define([
        this.state = Cesium3DTileContentState.LOADING;

        loadArrayBuffer(this._url).then(function(arrayBuffer) {
+            if (that.isDestroyed()) {
+                throw new Error('tileset is destroyed');


We only throw DeveloperError or RuntimeError, not Error; see https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Documentation/Contributors/CodingGuide/README.md#throwing-exceptions

In this case, there was an outstanding request after the tileset was destroyed, right? That is not an exceptional case. Can we instead just silently destroy the tile?

Yeah it can be silent. I did it this way so that the request scheduler and stats would get updated correctly, but since that all is changing anyways I think it's fine not to throw the errors.

pjcozzi · 2015-12-21T18:53:21Z

Do we have any test cases for both additive and replacement refinement in the same tileset?

lilleyse · 2015-12-21T18:55:20Z

TileReplacement2 has a mix, but I do think I should add a more general case test.

pjcozzi · 2015-12-21T19:04:07Z

Specs/Scene/Cesium3DTilesetSpec.js

+        return Cesium3DTilesTester.loadTileset(scene, tilesetUrl).then(function(tileset) {
+            tileset.debugShowStatistics = true;
+            scene.renderForSpecs();
+            // TODO : not sure how to test this


Use a spy for console log. Don't too tightly couple this test to the implementation though, just a coarse-grained test is plenty.

pjcozzi · 2015-12-21T19:11:47Z

The debug* options, such as debugShowBox need tests. As usual, coarse-grained checks are fine.

pjcozzi · 2015-12-21T19:17:34Z

Can we add an explicit test for:

// Use parent's geometric error with child's box to see if we already meet the SSE

https://github.com/AnalyticalGraphicsInc/cesium/blob/tileset-tests/Source/Scene/Cesium3DTileset.js#L461

This is an important optimization for additive refinement.

Basically, from one view we should not need to request the farther away children, then move the view closer and the children are requested.

pjcozzi · 2015-12-21T19:19:58Z

Can we add a test to make sure tiles are traversed (requested/rendered) in the a front-to-back order?

I think there is an issue here. From #3241:

Is the load order correct? For example, the buildings on the right should load sooner: http://cesiumjs.org/WashingtonDC/

We can also punt on this until the request scheduler if that makes more sense.

pjcozzi · 2015-12-21T19:29:25Z

Let's test the case when the content url exists but it has an unknown extension, e.g.,

            "content": {
              "url": "2/0/0.unknown"
            },

pjcozzi · 2015-12-21T19:38:55Z

Do we need a test case for a traditional quadtree where all the interior tiles are empty and only the leafs have content? Or is TilesetEmptyRoot enough?

pjcozzi · 2015-12-21T19:42:14Z

Would some of these tests be better if we mocked out (or just spied on) Cesium3DTile and friends?

It would make the tests more precise and depend on fewer moving parts.

pjcozzi · 2015-12-21T19:48:14Z

Would some of these tests be better if we mocked out (or just spied on) Cesium3DTile and friends?

It would make the tests more precise and depend on fewer moving parts.

Also, this is up to you if you think it is cleaner.

pjcozzi · 2015-12-21T20:02:59Z

Did coverage bump to 94%? :)

lilleyse · 2015-12-21T20:06:13Z

Not yet...

pjcozzi · 2015-12-21T20:12:59Z

That's all my comments. This looks really good.

lilleyse · 2015-12-22T15:08:57Z

Let's test the case when the content url exists but it has an unknown extension, e.g.,

This will be easier to test in Cesium3DTileSpec

lilleyse · 2015-12-22T15:32:24Z

This is ready for another look.

pjcozzi · 2015-12-22T17:26:52Z

Coverage for Cesium3DTile is not great, but it sounds like you plan to handle that in a separate PR.

Cesium3DTileset tests

lilleyse added 7 commits December 18, 2015 08:18

Added Cesium3DTileset tests

f824572

Tileset destroy specs

3e23c98

Try different replacement refinement approach

6e1fb14

Fix Tileset3DTile test

f88325c

Small fixes

6354ac0

Refining check

6b2ddb5

Added test files

4ffd013

pjcozzi reviewed Dec 21, 2015
View reviewed changes

lilleyse mentioned this pull request Dec 22, 2015

Rework external tiles.json #3328

Merged

lilleyse added 2 commits December 22, 2015 10:15

Added more tests and fixes

87c7a15

Pass along properties for external tileset

38e29e1

pjcozzi mentioned this pull request Dec 22, 2015

3D Tiles Roadmap #3241

Open

pjcozzi added a commit that referenced this pull request Dec 22, 2015

Merge pull request #3367 from AnalyticalGraphicsInc/tileset-tests

857ffe5

Cesium3DTileset tests

pjcozzi merged commit 857ffe5 into 3d-tiles Dec 22, 2015

pjcozzi deleted the tileset-tests branch December 22, 2015 17:26

lilleyse mentioned this pull request Jan 22, 2016

optimization of replacement refinement selection/loading: only request visible tile children #3449

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cesium3DTileset tests #3367

Cesium3DTileset tests #3367

lilleyse commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi Dec 21, 2015

lilleyse Dec 21, 2015

pjcozzi commented Dec 21, 2015

lilleyse commented Dec 21, 2015

pjcozzi Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

lilleyse commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

lilleyse commented Dec 22, 2015

lilleyse commented Dec 22, 2015

pjcozzi commented Dec 22, 2015

Cesium3DTileset tests #3367

Cesium3DTileset tests #3367

Conversation

lilleyse commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi Dec 21, 2015

Choose a reason for hiding this comment

lilleyse Dec 21, 2015

Choose a reason for hiding this comment

pjcozzi commented Dec 21, 2015

lilleyse commented Dec 21, 2015

pjcozzi Dec 21, 2015

Choose a reason for hiding this comment

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

lilleyse commented Dec 21, 2015

pjcozzi commented Dec 21, 2015

lilleyse commented Dec 22, 2015

lilleyse commented Dec 22, 2015

pjcozzi commented Dec 22, 2015