Skip to content

Commit

Permalink
Merge pull request #10115 from CesiumGS/model-experimental-bounding-s…
Browse files Browse the repository at this point in the history
…phere

Fix inaccurate computation of `ModelExperimental`'s bounding sphere
  • Loading branch information
ptrgags authored Feb 18, 2022
2 parents 2289162 + 335af04 commit ff34e0c
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 5 deletions.
6 changes: 3 additions & 3 deletions Source/Scene/ModelExperimental/ModelExperimentalSceneGraph.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ ModelExperimentalSceneGraph.prototype.buildDrawCommands = function (
primitiveRenderResources.boundingSphere
);

boundingSpheres.push(primitiveRenderResources.boundingSphere);
boundingSpheres.push(runtimePrimitive.boundingSphere);

const drawCommands = buildDrawCommands(
primitiveRenderResources,
Expand All @@ -344,8 +344,8 @@ ModelExperimentalSceneGraph.prototype.buildDrawCommands = function (
this._boundingSphere = BoundingSphere.fromBoundingSpheres(boundingSpheres);
BoundingSphere.transform(
this._boundingSphere,
this._model.modelMatrix,
this._model._boundingSphere
model.modelMatrix,
model._boundingSphere
);
};

Expand Down
34 changes: 32 additions & 2 deletions Specs/Scene/ModelExperimental/ModelExperimentalSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ describe(
});
});

it("renders model with style", function () {
// see https://github.com/CesiumGS/cesium/pull/10115
xit("renders model with style", function () {
return loadAndZoomToModelExperimental(
{ gltf: buildingsMetadata },
scene
Expand Down Expand Up @@ -459,6 +460,32 @@ describe(
});
});

it("initializes with model matrix", function () {
const translation = new Cartesian3(10, 0, 0);
const transform = Matrix4.fromTranslation(translation);

return loadAndZoomToModelExperimental(
{
gltf: boxTexturedGlbUrl,
upAxis: Axis.Z,
forwardAxis: Axis.X,
modelMatrix: transform,
},
scene
).then(function (model) {
const sceneGraph = model.sceneGraph;
scene.renderForSpecs();
expect(Matrix4.equals(sceneGraph.computedModelMatrix, transform)).toBe(
true
);
verifyRender(model, false);
expect(model.boundingSphere.center).toEqual(translation);

expect(sceneGraph.computedModelMatrix).not.toBe(transform);
expect(model.modelMatrix).not.toBe(transform);
});
});

it("changing model matrix works", function () {
const updateModelMatrix = spyOn(
ModelExperimentalSceneGraph.prototype,
Expand All @@ -468,6 +495,7 @@ describe(
{ gltf: boxTexturedGlbUrl, upAxis: Axis.Z, forwardAxis: Axis.X },
scene
).then(function (model) {
verifyRender(model, true);
const sceneGraph = model.sceneGraph;

const transform = Matrix4.fromTranslation(new Cartesian3(10, 0, 0));
Expand All @@ -483,6 +511,7 @@ describe(
expect(Matrix4.equals(sceneGraph.computedModelMatrix, transform)).toBe(
true
);
verifyRender(model, false);
});
});

Expand All @@ -493,7 +522,7 @@ describe(
scene
).then(function (model) {
const transform = Matrix4.fromTranslation(translation);
expect(model.boundingSphere.center).toEqual(new Cartesian3());
expect(model.boundingSphere.center).toEqual(Cartesian3.ZERO);

Matrix4.multiplyTransformation(
model.modelMatrix,
Expand All @@ -503,6 +532,7 @@ describe(
scene.renderForSpecs();

expect(model.boundingSphere.center).toEqual(translation);
verifyRender(model, false);
});
});

Expand Down

0 comments on commit ff34e0c

Please sign in to comment.