From 85ced9bac3b337951680590552676868bf35f21d Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Wed, 19 Oct 2016 16:49:40 -0400 Subject: [PATCH 1/5] Fix updateBatchTableBoundingSpheres updateBatchTableBoundingSpheres was using the BoundingSphere center without applying the modelMatrix to the sphere first. Fixes #4431 and #4428 --- Source/Scene/Primitive.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Source/Scene/Primitive.js b/Source/Scene/Primitive.js index 4aa6975e3c51..93a1ee6821c1 100644 --- a/Source/Scene/Primitive.js +++ b/Source/Scene/Primitive.js @@ -1193,6 +1193,8 @@ define([ for (var i = 0; i < length; ++i) { var boundingSphere = boundingSpheres[i]; + var modelMatrix = defaultValue(primitive.modelMatrix, Matrix4.IDENTITY); + boundingSphere = BoundingSphere.transform(boundingSphere, modelMatrix, boundingSphere); var center = boundingSphere.center; var radius = boundingSphere.radius; From 06e9c36303f4156db44216a6e72ddb6386b817c3 Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Fri, 21 Oct 2016 11:02:08 -0400 Subject: [PATCH 2/5] Add unit test. --- Specs/Scene/PrimitiveSpec.js | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/Specs/Scene/PrimitiveSpec.js b/Specs/Scene/PrimitiveSpec.js index 2731adb01b92..c9ace8fe9115 100644 --- a/Specs/Scene/PrimitiveSpec.js +++ b/Specs/Scene/PrimitiveSpec.js @@ -17,6 +17,7 @@ defineSuite([ 'Core/Math', 'Core/Matrix4', 'Core/PolygonGeometry', + 'Core/CylinderGeometry', 'Core/PrimitiveType', 'Core/Rectangle', 'Core/RectangleGeometry', @@ -49,6 +50,7 @@ defineSuite([ CesiumMath, Matrix4, PolygonGeometry, + CylinderGeometry, PrimitiveType, Rectangle, RectangleGeometry, @@ -761,6 +763,42 @@ defineSuite([ expect(scene.renderForSpecs()).toEqual([0, 0, 0, 255]); }); + it('primitive with display condition properly transforms boundingSphere', function() { + var near = 10000.0; + var far = 1000000.0; + var translation = new Cartesian3(10, 20, 30); + + var cylinder = new GeometryInstance({ + id : 'cylinder', + vertexFormat : PerInstanceColorAppearance.VERTEX_FORMAT, + geometry : new CylinderGeometry({ + length : 10, + topRadius : 10, + bottomRadius : 10 + }), + attributes : { + color : new ColorGeometryInstanceAttribute(1.0, 1.0, 0.0, 1.0), + show : new ShowGeometryInstanceAttribute(true), + distanceDisplayCondition : new DistanceDisplayConditionGeometryInstanceAttribute(near, far) + } + }); + + primitive = new Primitive({ + geometryInstances : cylinder, + appearance : new PerInstanceColorAppearance(), + modelMatrix : Matrix4.fromTranslation(translation, new Matrix4()), + asynchronous : false + }); + + scene.primitives.add(primitive); + scene.frameState.scene3DOnly = true; + scene.renderForSpecs(); + + var boundingSphere = primitive.getGeometryInstanceAttributes('cylinder').boundingSphere; + var center = boundingSphere.center; + expect(center).toEqual(translation); + }); + it('getGeometryInstanceAttributes returns same object each time', function() { primitive = new Primitive({ geometryInstances : rectangleInstance1, From 15bf1d9ffb1cca6cd0c256833929b9d5b6ed1774 Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Fri, 21 Oct 2016 11:14:51 -0400 Subject: [PATCH 3/5] Always apply boundingSphere transform We were previously only applying if a display condition existed. --- Source/Scene/Primitive.js | 27 +++++++++++++++------------ Specs/Scene/PrimitiveSpec.js | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/Source/Scene/Primitive.js b/Source/Scene/Primitive.js index 93a1ee6821c1..b4beb54ebce7 100644 --- a/Source/Scene/Primitive.js +++ b/Source/Scene/Primitive.js @@ -1173,7 +1173,8 @@ define([ var scratchBoundingSphereCenter2D = new Cartesian3(); function updateBatchTableBoundingSpheres(primitive, frameState) { - if (!defined(primitive._batchTableAttributeIndices.distanceDisplayCondition) || primitive._batchTableBoundingSpheresUpdated) { + var hasDistanceDisplayCondition = defined(primitive._batchTableAttributeIndices.distanceDisplayCondition); + if (!hasDistanceDisplayCondition && primitive._batchTableBoundingSpheresUpdated) { return; } @@ -1195,20 +1196,22 @@ define([ var boundingSphere = boundingSpheres[i]; var modelMatrix = defaultValue(primitive.modelMatrix, Matrix4.IDENTITY); boundingSphere = BoundingSphere.transform(boundingSphere, modelMatrix, boundingSphere); - var center = boundingSphere.center; - var radius = boundingSphere.radius; - var encodedCenter = EncodedCartesian3.fromCartesian(center, scratchBoundingSphereCenterEncoded); - batchTable.setBatchedAttribute(i, center3DHighIndex, encodedCenter.high); - batchTable.setBatchedAttribute(i, center3DLowIndex, encodedCenter.low); + if (hasDistanceDisplayCondition) { + var center = boundingSphere.center; + var radius = boundingSphere.radius; - var cartographic = ellipsoid.cartesianToCartographic(center, scratchBoundingSphereCartographic); - var center2D = projection.project(cartographic, scratchBoundingSphereCenter2D); - encodedCenter = EncodedCartesian3.fromCartesian(center2D, scratchBoundingSphereCenterEncoded); - batchTable.setBatchedAttribute(i, center2DHighIndex, encodedCenter.high); - batchTable.setBatchedAttribute(i, center2DLowIndex, encodedCenter.low); + var encodedCenter = EncodedCartesian3.fromCartesian(center, scratchBoundingSphereCenterEncoded); + batchTable.setBatchedAttribute(i, center3DHighIndex, encodedCenter.high); + batchTable.setBatchedAttribute(i, center3DLowIndex, encodedCenter.low); - batchTable.setBatchedAttribute(i, radiusIndex, radius); + var cartographic = ellipsoid.cartesianToCartographic(center, scratchBoundingSphereCartographic); + var center2D = projection.project(cartographic, scratchBoundingSphereCenter2D); + encodedCenter = EncodedCartesian3.fromCartesian(center2D, scratchBoundingSphereCenterEncoded); + batchTable.setBatchedAttribute(i, center2DHighIndex, encodedCenter.high); + batchTable.setBatchedAttribute(i, center2DLowIndex, encodedCenter.low); + batchTable.setBatchedAttribute(i, radiusIndex, radius); + } } primitive._batchTableBoundingSpheresUpdated = true; diff --git a/Specs/Scene/PrimitiveSpec.js b/Specs/Scene/PrimitiveSpec.js index c9ace8fe9115..3f39548c8cd1 100644 --- a/Specs/Scene/PrimitiveSpec.js +++ b/Specs/Scene/PrimitiveSpec.js @@ -799,6 +799,39 @@ defineSuite([ expect(center).toEqual(translation); }); + it('primitive without display condition properly transforms boundingSphere', function() { + var translation = new Cartesian3(10, 20, 30); + + var cylinder = new GeometryInstance({ + id : 'cylinder', + vertexFormat : PerInstanceColorAppearance.VERTEX_FORMAT, + geometry : new CylinderGeometry({ + length : 10, + topRadius : 10, + bottomRadius : 10 + }), + attributes : { + color : new ColorGeometryInstanceAttribute(1.0, 1.0, 0.0, 1.0), + show : new ShowGeometryInstanceAttribute(true) + } + }); + + primitive = new Primitive({ + geometryInstances : cylinder, + appearance : new PerInstanceColorAppearance(), + modelMatrix : Matrix4.fromTranslation(translation, new Matrix4()), + asynchronous : false + }); + + scene.primitives.add(primitive); + scene.frameState.scene3DOnly = true; + scene.renderForSpecs(); + + var boundingSphere = primitive.getGeometryInstanceAttributes('cylinder').boundingSphere; + var center = boundingSphere.center; + expect(center).toEqual(translation); + }); + it('getGeometryInstanceAttributes returns same object each time', function() { primitive = new Primitive({ geometryInstances : rectangleInstance1, From 7276883473c0c01df8a65952233ccc15a29d3740 Mon Sep 17 00:00:00 2001 From: Matthew Amato Date: Fri, 21 Oct 2016 15:10:13 -0400 Subject: [PATCH 4/5] Update CHANGES Also clean up model modelMatrix check. --- CHANGES.md | 1 + Source/Scene/Primitive.js | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 6abe67e12f0c..037afe08333c 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,6 +9,7 @@ Change Log * * Fixed a crash that would occur when using dynamic `distanceDisplayCondition` properties. [#4403](https://github.com/AnalyticalGraphicsInc/cesium/pull/4403) * Fixed a bug affected models with multiple meshes without indices. [#4237](https://github.com/AnalyticalGraphicsInc/cesium/issues/4237) +* Fixed `BoundingSphere` computation for `Primitive` instances with a modelMatrix. [#4431](https://github.com/AnalyticalGraphicsInc/cesium/issues/4431) and [#4428](https://github.com/AnalyticalGraphicsInc/cesium/issues/4428) * Fixed a glTF transparency bug where `blendFuncSeparate` parameters were loaded in the wrong order. [#4435](https://github.com/AnalyticalGraphicsInc/cesium/pull/4435) * Fixed a bug where creating a custom geometry with attributes and indices that have values that are not a typed array would cause a crash. [#4419](https://github.com/AnalyticalGraphicsInc/cesium/pull/4419) * Fixed a bug with rotated, textured rectangles. [#4430](https://github.com/AnalyticalGraphicsInc/cesium/pull/4430) diff --git a/Source/Scene/Primitive.js b/Source/Scene/Primitive.js index b4beb54ebce7..88a9d436fa76 100644 --- a/Source/Scene/Primitive.js +++ b/Source/Scene/Primitive.js @@ -1194,8 +1194,10 @@ define([ for (var i = 0; i < length; ++i) { var boundingSphere = boundingSpheres[i]; - var modelMatrix = defaultValue(primitive.modelMatrix, Matrix4.IDENTITY); - boundingSphere = BoundingSphere.transform(boundingSphere, modelMatrix, boundingSphere); + var modelMatrix = primitive.modelMatrix; + if (defined(modelMatrix)) { + boundingSphere = BoundingSphere.transform(boundingSphere, modelMatrix, boundingSphere); + } if (hasDistanceDisplayCondition) { var center = boundingSphere.center; From f488e1567ca1011aabf361ec1340023b42d14ad3 Mon Sep 17 00:00:00 2001 From: Patrick Cozzi Date: Fri, 21 Oct 2016 15:28:53 -0400 Subject: [PATCH 5/5] Fix CHANGES.md --- CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 037afe08333c..c92d47375020 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -9,7 +9,7 @@ Change Log * * Fixed a crash that would occur when using dynamic `distanceDisplayCondition` properties. [#4403](https://github.com/AnalyticalGraphicsInc/cesium/pull/4403) * Fixed a bug affected models with multiple meshes without indices. [#4237](https://github.com/AnalyticalGraphicsInc/cesium/issues/4237) -* Fixed `BoundingSphere` computation for `Primitive` instances with a modelMatrix. [#4431](https://github.com/AnalyticalGraphicsInc/cesium/issues/4431) and [#4428](https://github.com/AnalyticalGraphicsInc/cesium/issues/4428) +* Fixed `BoundingSphere` computation for `Primitive` instances with a modelMatrix. [#4428](https://github.com/AnalyticalGraphicsInc/cesium/issues/4428) * Fixed a glTF transparency bug where `blendFuncSeparate` parameters were loaded in the wrong order. [#4435](https://github.com/AnalyticalGraphicsInc/cesium/pull/4435) * Fixed a bug where creating a custom geometry with attributes and indices that have values that are not a typed array would cause a crash. [#4419](https://github.com/AnalyticalGraphicsInc/cesium/pull/4419) * Fixed a bug with rotated, textured rectangles. [#4430](https://github.com/AnalyticalGraphicsInc/cesium/pull/4430)