Use b3dm tilesets for classification #6033

bagnell · 2017-12-06T00:36:20Z

This adds a classificationType option to Cesium3DTileset that will turn the glTF of a b3dm into a classification volume.

ClassificationModel has support for some of the same options that Model does (e.g. terrain clamping). Can this be removed or should we make this part of the public API?

Limitations:

only works for b3dm
all indices with the same batch id must be contiguous in memory
ignores all shaders and techniques
the only extension supported is WEB3D_quantized_attributes

Note that this is a PR into the vector-tiles branch.

…This makes draw command management easier and reduces the memory for draw commands by half.

…gment shaders.

… picking and rebatching.

cesium-concierge · 2017-12-06T00:36:21Z

@bagnell, thanks for the pull request! Maintainers, we have a signed CLA from @bagnell, so you can review this at any time.

⚠️ I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to CHANGES.md and comment on this pull request so we know it was updated. For more info, see the Pull Request Guidelines.

I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

pjcozzi · 2017-12-06T16:35:23Z

Limitations all sound OK to me. Later I could see users wanting to classify with i3dm - it is is kind of in between b3dm and geom.

pjcozzi · 2017-12-06T16:38:16Z

ClassificationModel has support for some of the same options that Model does (e.g. terrain clamping). Can this be removed or should we make this part of the public API?

I don't think ClassificationModel needs to be exposed through the public API; Cesium API users will use the ClassificationPrimitive if they are not using 3D Tiles...I think.

So it only needs to have the API needed to support Cesium3DTileset, which does not have terrain clamping I believe.

pjcozzi · 2017-12-06T16:39:54Z

Source/Scene/Cesium3DTileset.js

+        },
+
+        /**
+         * Determines whether terrain, 3D Tiles or both will be classified by this tileset.


Label as @readonly.

pjcozzi · 2017-12-06T16:40:28Z

Source/Scene/Cesium3DTileset.js

@@ -222,6 +222,8 @@ define([

        this._readyPromise = when.defer();

+        this._classificationType = options.classificationType;


Is this in the reference doc?

Also include the limitations in the reference doc. It might be easiest to add this once in the property getter and then reference it from the constructor function.

pjcozzi · 2017-12-06T16:40:46Z

Update CHANGES.md.

pjcozzi · 2017-12-06T16:41:00Z

Add a Sandcastle example.

pjcozzi · 2017-12-06T16:42:34Z

Source/Scene/Vector3DTilePrimitive.js

@@ -153,8 +153,18 @@ define([
         * @type {ClassificationType}
         * @default ClassificationType.CESIUM_3D_TILE
         */
-        this.classificationType = ClassificationType.CESIUM_3D_TILE;
-        this._classificationType = this.classificationType;
+        this.classificationType = defaultValue(options.classificationType, ClassificationType.CESIUM_3D_TILE);


Add to reference doc even though the class is private.

pjcozzi · 2017-12-06T16:43:55Z

Add unit tests.

pjcozzi · 2017-12-06T16:45:06Z

Source/Scene/Batched3DModel3DTileContent.js

+                pickObject : pickObject
+            });
+        } else {
+            content._model = new ClassificationModel({


Can you add a code comment about how this is a transcode - with limitations on the input glTF - so that we can use the geom rebatching rendering pipeline?

pjcozzi · 2017-12-06T16:58:39Z

Source/Scene/ClassificationModel.js

@@ -0,0 +1,2482 @@
+define([


Looks like it does more than it needs to given the limitations classification has. Are you sure there aren't other limitations we can add to make this even more simple, e.g., vertex formats, removing the cache (is it needed), etc.

Then perhaps factor out some common functions to share with Model?

bagnell · 2017-12-11T20:42:58Z

Do you get test failures like this:

I don't see any test failure locally and the Travis tests pass. Also, running the tests from the link you posted give strange failures like:

Core/Cartesian3 renders with external gltf

bagnell · 2017-12-11T20:46:24Z

The requirement for floating point positions and uint16 batch ids were removed as well.Different formats are required to support quantization.

bagnell · 2017-12-11T21:01:15Z

@pjcozzi I think this is ready. The only thing missing would be only supporting one primitive per mesh, but I could do that in a separate PR.

Add Geometry tile support

pjcozzi · 2017-12-12T02:22:45Z

The only thing missing would be only supporting one primitive per mesh, but I could do that in a separate PR.

Separate PR is OK, but let's do it right away to keep the code streamlined.

pjcozzi · 2017-12-12T02:24:15Z

Source/Scene/ClassificationModel.js

+        };
+    }
+
+    function computeBoundingSphere(model) {


Are you going to move some of these functions to a common file? Or do you want to do that in a separate PR?

…onstructor.

bagnell · 2017-12-13T21:26:49Z

@pjcozzi This is ready for another look. I added the restriction that only one primitive per mesh is supported, factored out some of the common code with Model, and added ClassificationModel tests.

pjcozzi · 2017-12-15T01:50:02Z

Source/Scene/ClassificationModel.js

+    var aMinScratch = new Cartesian3();
+    var aMaxScratch = new Cartesian3();
+
+    function computeBoundingSphere(model) {


Can this be a common function?

This function is different enough from model that I didn't create a common function. The only part I could pull out is finding the min and max.

pjcozzi · 2017-12-15T01:50:55Z

Source/Scene/ClassificationModel.js

+        }
+    }
+
+    function createCommand(model) {


Maybe rename this since it no longer creates a command...directly.

pjcozzi · 2017-12-15T02:01:04Z

Tests and Sandcastle example look good. Just those trivial questions.

bagnell · 2017-12-15T19:22:13Z

@pjcozzi This should be ready.

pjcozzi · 2017-12-15T22:16:23Z

Thanks!

bagnell added 14 commits November 30, 2017 19:14

Add option to make a b3dm tileset classify another tileset.

27972ff

Add a separate pass for classification on both terrain and 3D Tiles. …

b4f367a

…This makes draw command management easier and reduces the memory for draw commands by half.

Optimize b3dm classification fragment shader.

e7ec3be

Initial rework.

2b0f52f

Remove unneeded code.

f1ab457

Only create classification commands. Only create basic vertex and fra…

33ff56e

…gment shaders.

Only use position and batch id attributes.

7fe3e37

Remove more unneeded code.

b3993b5

Remove shader parsing.

cc7a228

Remove unused uniform semantics.

fad8860

Mostly working. Needs clean up. There are issues with model matrices,…

a2a1e2f

… picking and rebatching.

Fix model matrix.

0d2f1e1

Fix rebatching and remove derived commands.

6981a68

Clean up.

e1218d7

bagnell added 2 commits December 5, 2017 19:46

Fix test.

9173555

Revert Sandcastle example.

c1ad375

pjcozzi mentioned this pull request Dec 6, 2017

3D Tiles Roadmap #3241

Open

pjcozzi added the category - 3d tiles label Dec 6, 2017

pjcozzi reviewed Dec 6, 2017

View reviewed changes

Remove mesh support from vector tiles.

b448116

Updates from review.

772a9c3

bagnell and others added 3 commits December 11, 2017 16:04

Merge branch 'b3dm-class' into vector-split

9fbc848

Update CHANGES.md.

612c5f5

Merge pull request #6034 from AnalyticalGraphicsInc/vector-split

618a892

Add Geometry tile support

pjcozzi reviewed Dec 12, 2017

View reviewed changes

bagnell added 11 commits December 12, 2017 15:25

Remove some code expecting multiple primitives. Move glTF checks to c…

d566de4

…onstructor.

Only create one primitive.

045e458

Don't parse shaders since they are generated.

6e57010

Remove runtimeNode.

15380ef

Create only one uniform map.

92709e8

Factor out common code.

36b2b53

Factor out gltf semantic uniforms.

a4c4506

Remove unused requires.

5232f88

Pass debug settings to the vector primitive.

029abad

Add ClassificationModel tests.

1fa1976

Update restrictions in the doc.

8f389cb

Merge branch 'vector-tiles' into b3dm-class

5f4ed4c

pjcozzi reviewed Dec 15, 2017

View reviewed changes

Update function name.

2385d20

pjcozzi merged commit 7d27a23 into vector-tiles Dec 15, 2017

pjcozzi deleted the b3dm-class branch December 15, 2017 22:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use b3dm tilesets for classification #6033

Use b3dm tilesets for classification #6033

bagnell commented Dec 6, 2017 •

edited

cesium-concierge commented Dec 6, 2017

pjcozzi commented Dec 6, 2017

pjcozzi commented Dec 6, 2017

pjcozzi Dec 6, 2017

pjcozzi Dec 6, 2017

pjcozzi Dec 6, 2017

pjcozzi commented Dec 6, 2017

pjcozzi commented Dec 6, 2017

pjcozzi Dec 6, 2017

pjcozzi commented Dec 6, 2017

pjcozzi Dec 6, 2017

pjcozzi Dec 6, 2017

bagnell commented Dec 11, 2017

bagnell commented Dec 11, 2017

bagnell commented Dec 11, 2017

pjcozzi commented Dec 12, 2017

pjcozzi Dec 12, 2017

bagnell commented Dec 13, 2017

pjcozzi Dec 15, 2017

bagnell Dec 15, 2017

pjcozzi Dec 15, 2017

pjcozzi commented Dec 15, 2017

bagnell commented Dec 15, 2017

pjcozzi commented Dec 15, 2017

		@@ -222,6 +222,8 @@ define([

		this._readyPromise = when.defer();

		this._classificationType = options.classificationType;

Use b3dm tilesets for classification #6033

Use b3dm tilesets for classification #6033

Conversation

bagnell commented Dec 6, 2017 • edited

cesium-concierge commented Dec 6, 2017

pjcozzi commented Dec 6, 2017

pjcozzi commented Dec 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Dec 6, 2017

pjcozzi commented Dec 6, 2017

Choose a reason for hiding this comment

pjcozzi commented Dec 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bagnell commented Dec 11, 2017

bagnell commented Dec 11, 2017

bagnell commented Dec 11, 2017

pjcozzi commented Dec 12, 2017

Choose a reason for hiding this comment

bagnell commented Dec 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Dec 15, 2017

bagnell commented Dec 15, 2017

pjcozzi commented Dec 15, 2017

bagnell commented Dec 6, 2017 •

edited