Composite tiles #3158

lilleyse · 2015-11-02T16:19:57Z

Client-side implementation of composite tiles. We should wait on merging this until the server-side code is ready, as this introduces breaking changes to the tile formats.

Added byteLength to all tile formats, added batchSize to b3dm tiles.
Point clouds now define their bounding sphere from the tile's oriented bounding box.
No longer using contentHeader.
Moved a lot of the ContentProvider loading code from request to init. Now request only loads the url and passes the data along to init. Composite tiles will call init when creating inner tiles.

pjcozzi · 2015-11-03T19:19:58Z

This fails JSHint, https://travis-ci.org/AnalyticalGraphicsInc/cesium/builds/88820692

pjcozzi · 2015-11-03T19:33:41Z

added batchSize to b3dm tiles.

Did we remove this from tiles.json? Can we also remove type from tiles.json in this PR?

pjcozzi · 2015-11-03T19:37:30Z

Apps/Sandcastle/gallery/Cities.html

@@ -33,11 +33,12 @@
 var scene = viewer.scene;
 var city = scene.primitives.add(new Cesium.Cesium3DTileset({
    //url : 'http://localhost:8002/tilesets/Cambridge',
-    url : 'http://localhost:8002/tilesets/London_Canary_Wharf',
+    //url : 'http://localhost:8002/tilesets/London_Canary_Wharf',


Avoid submitting these temporary changes to Cities.html in the future.

pjcozzi · 2015-11-03T20:00:11Z

Source/Scene/Points3DTileContentProvider.js

+    Points3DTileContentProvider.prototype.init = function(arrayBuffer, byteOffset) {
+        byteOffset = defaultValue(byteOffset, 0);
+
+        var magic = getMagic(arrayBuffer, byteOffset);


Can you check if all the other tile formats, including the new composite, and verifying the magic number like this?

pjcozzi · 2015-11-03T20:12:40Z

Moved a lot of the ContentProvider loading code from request to init. Now request only loads the url and passes the data along to init. Composite tiles will call init when creating inner tiles.

This is an OK approach. If we stay with it, rename init to initialize to avoid the abbreviation.

However, can we do better? Can we decouple the request and the promise management (which is the same-ish) from parsing the tile's arraybuffer (which is different per tile type). For example, each tile's constructor would take an arraybuffer and offset, and would only be constructed once the request resolved.

pjcozzi · 2015-11-03T20:18:05Z

Source/Scene/Composite3DTileContentProvider.js

+        this._url = url;
+        this._tileset = tileset;
+        this._tile = tile;
+        this._contentProviders = [];


How do users of the Cesium API access this? Can you update Cities.html to support mouse-over color change and so on (and also update it for i3dm, which I think will lead us to think more deeply about how to expose this in a uniform way).

I'm still not sure about this. I could just add a getter, but then it's tile.content.contents. Not that great. Maybe tile.content could sometimes return an array? It's still odd.

The color changing works automatically though without any modifications.

The color changing works automatically though without any modifications.

Hmm. Maybe we don't need to expose this for now since users are interacting with the inner tiles in the same way they interact with standalone tiles? We could add a TODO and revisit later if needed.

Just curious - did you test composites nested in composites? That is test data we should have.

I just tested this and it works.

lilleyse · 2015-11-04T21:45:20Z

Did we remove this from tiles.json? Can we also remove type from tiles.json in this PR?

Both are removed from tiles.json.

Can we decouple the request and the promise management (which is the same-ish) from parsing the tile's arraybuffer

The one obstacle I see for that is ContentProvider usually needs to load models, and can only send the ready promise when the model is loaded.

pjcozzi · 2015-11-04T22:41:28Z

Can we decouple the request and the promise management (which is the same-ish) from parsing the tile's arraybuffer

The one obstacle I see for that is ContentProvider usually needs to load models, and can only send the ready promise when the model is loaded.

Ah, and the model resources, e.g., textures, may be separate files so the promise may resolve later.

I'm OK with sticking with your current approach. As we add more tile formats, we'll see if it makes sense to change.

pjcozzi · 2015-11-04T22:42:18Z

Let me know when this is ready.

Do you plan to support a tile.content.url that points to another tiles.json file in this PR or a new one?

lilleyse · 2015-11-04T22:58:15Z

I'll do it in the same PR. I'll have that ready later.

The rest of the changes are ready for review. I also standardized the getMagic code and made some changes to getStringFromTypedArray.

pjcozzi · 2015-11-05T13:39:08Z

Source/Scene/Batched3DModel3DTileContentProvider.js

-                // API async, and would double the number of numbers in some cases.
-                batchTableResources.batchTable = JSON.parse(batchTableString);
-            }
+        // TODO : rename to batchLength?


Let's rename to batchTableLength.

What if the tile has no batch table? The terminology could be confusing.

Then it would be zero. This is consistent with 3D Tiles naming: CesiumGS/3d-tiles#26

What do you suggest?

But we still need the batch size even if it doesn't have a batch table. My comment was to rename batchSize to batchLength.

Yes, you are right. Let's go with batchLength and batchTableLength.

pjcozzi · 2015-11-07T19:31:38Z

@lilleyse do you need to make any other updates to this PR before I do a final review?

lilleyse · 2015-11-08T03:51:23Z

I made the last couple of renaming changes. This is ready for another look.

pjcozzi · 2015-11-08T20:09:38Z

Source/Scene/Cesium3DTileset.js

                        t.cesium3DTile.children.push(childTile);

                        stack.push({
                            header : childHeader,
                            cesium3DTile : childTile
                        });
+
+                        // Check if the tile's content url is a tiles.json file.
+                        if (defined(childHeader.content)) {


Interesting approach. I planned to do this by implementing a 3DTileContentProvider with a Cesium3DTileset primitive, which would be a bit cleaner than this. However, this approach will help with cache management, rendering budget, etc. since it isn't spread across multiple Cesium3DTileset primitives. Let's see how it goes.

pjcozzi · 2015-11-08T20:11:21Z

Thanks again @lilleyse!

Composite tiles

lilleyse added 3 commits November 2, 2015 10:50

Added composite tile support

9ff3826

Merge remote-tracking branch 'origin/3d-tiles' into composite-tiles

12917e6

Small fixes

44feffd

pjcozzi mentioned this pull request Nov 3, 2015

Instanced 3D Model and Composite Tile Formats CesiumGS/3d-tiles#23

Closed

5 tasks

pjcozzi reviewed Nov 3, 2015
View reviewed changes

This was referenced Nov 3, 2015

Remove content.type? CesiumGS/3d-tiles#5

Closed

Remove content.batchSize? CesiumGS/3d-tiles#4

Closed

pjcozzi reviewed Nov 3, 2015
View reviewed changes

Added BoundingSphere.fromOrientedBoundingBox function

ea75a19

Pull request fixes

cc15e6c

pjcozzi reviewed Nov 5, 2015
View reviewed changes

lilleyse force-pushed the composite-tiles branch 2 times, most recently from 04febe8 to ae6f11b Compare November 6, 2015 16:26

Inner tiles.json

dc7f439

lilleyse force-pushed the composite-tiles branch from ae6f11b to dc7f439 Compare November 6, 2015 16:42

Handle corner cases

3f0059e

pjcozzi mentioned this pull request Nov 7, 2015

Instanced 3D Model updates CesiumGS/3d-tiles#33

Closed

3 tasks

Renamed variables to match 3d-tiles spec

b3b81dc

lilleyse force-pushed the composite-tiles branch from cda3538 to b3b81dc Compare November 8, 2015 03:51

pjcozzi mentioned this pull request Nov 8, 2015

content type: "3dtiles" CesiumGS/3d-tiles#18

Closed

pjcozzi reviewed Nov 8, 2015
View reviewed changes

pjcozzi added a commit that referenced this pull request Nov 8, 2015

Merge pull request #3158 from lilleyse/composite-tiles

b1b58ee

Composite tiles

pjcozzi merged commit b1b58ee into CesiumGS:3d-tiles Nov 8, 2015

pjcozzi deleted the composite-tiles branch November 8, 2015 20:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Composite tiles #3158

Composite tiles #3158

lilleyse commented Nov 2, 2015

pjcozzi commented Nov 3, 2015

pjcozzi commented Nov 3, 2015

pjcozzi Nov 3, 2015

pjcozzi Nov 3, 2015

pjcozzi commented Nov 3, 2015

pjcozzi Nov 3, 2015

lilleyse Nov 4, 2015

pjcozzi Nov 4, 2015

pjcozzi Nov 4, 2015

lilleyse Nov 5, 2015

lilleyse commented Nov 4, 2015

pjcozzi commented Nov 4, 2015

pjcozzi commented Nov 4, 2015

lilleyse commented Nov 4, 2015

pjcozzi Nov 5, 2015

lilleyse Nov 5, 2015

pjcozzi Nov 5, 2015

lilleyse Nov 5, 2015

pjcozzi Nov 7, 2015

pjcozzi commented Nov 7, 2015

lilleyse commented Nov 8, 2015

pjcozzi Nov 8, 2015

pjcozzi commented Nov 8, 2015

Composite tiles #3158

Composite tiles #3158

Conversation

lilleyse commented Nov 2, 2015

pjcozzi commented Nov 3, 2015

pjcozzi commented Nov 3, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Nov 3, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilleyse commented Nov 4, 2015

pjcozzi commented Nov 4, 2015

pjcozzi commented Nov 4, 2015

lilleyse commented Nov 4, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Nov 7, 2015

lilleyse commented Nov 8, 2015

Choose a reason for hiding this comment

pjcozzi commented Nov 8, 2015