Bounding sphere for ellipsoid with dynamic radii broken in 1.27

[hpinkos]

From the forum: https://groups.google.com/forum/?hl=en#!topic/cesium-dev/wm3VEHXk1wM

This example works in 1.26 but not in 1.27 and later. It also works fine if radii has a static value.
Likely culprit: #4461

var czml = [{
    "id":"document",
    "version":"1.0",
    "clock":{
      "interval":"2015-11-03T00:00:00Z/2015-11-04T00:00:00Z",
      "currentTime":"2015-11-03T00:00:00Z"
    }
  },
  {
    "id":"SAT",
    "name":"SAT",    
    "position":{
      "cartesian":[
        41135569.83604452,10529445.08775081,-2592340.399843352
      ]
    },
    "ellipsoid":{
      "radii":{
        "epoch":"2015-11-03T00:00:00Z",
        "cartesian":[
          0,952.1529108575653,160.27592173538292,23.918865122711754,
          86400,901.9691398345186,159.36635646200654,26.238895837307286
          ]
      },
      "fill":true
    }    
  }];

var viewer = new Cesium.Viewer('cesiumContainer');
var dataSource = Cesium.CzmlDataSource.load(czml);
viewer.dataSources.add(dataSource);
dataSource.then(function(ds) {
    viewer.trackedEntity = ds.entities.getById('SAT');
});

[emackey]

Not just the bounding sphere is broken: The position of the entity itself, as viewed by viewer.selectedEntity, viewer.trackedEntity, and viewer.zoomTo, is a distant point in space, far from the Earth. See sample code in #4893.

[emackey]

Git bisect says:

85ced9b is the first bad commit

[emackey]

/cc #4461 @mramato

[mramato]

Thanks @emackey that change you found with bisect fixes a related issue brought on by the moving of all primitive data into the batch table. What's most likely happening (I haven't actually looked at the code yet) is that EllipsoidGeometryUpdater is applying a transformation to the ellipsoid location that in no longer needs to do and the modelmatrix is essentially getting applied twice. This was probably a subtle breaking change in the API when @bagnell did the batch table work for primitive and we simply didn't notice until now.

I'll try to open a PR for this this weekend unless someone else beats me to it.

[mramato]

This bug affects all dynamic geometry, not just ellipsoid. dynamicGeoemtryGetBoundingSphere.js seems to be applying an additional BoundingSphere.transform that is no longer necessary. Replacing the two occurrences of line:

BoundingSphere.transform(attributes.boundingSphere, modelMatrix, result);

with

BoundingSphere.clone(attributes.boundingSphere, result);

Appears to fix the issue. I'm not sure if the old behavior of us having the transform the bounding sphere ourselves was intentional, or a bug we were unknowingly working around. So we might want to keep an eye out for other Primitive.boundingSphere regressions.

[emackey]

I think this bug affects "all dynamic geometry" that uses a custom modelMatrix, of which EllipsoidGeometryUpdater may be the only one. Even the similar BoxGeometryUpdater with its time-dynamic dimensions Cartesian, that seems so similar in concept to ellipsoid.radii, does not use a custom-modified modelMatrix (possibly recomputing the vertices instead?), and when tested, box.dimensions does not exhibit this bug. Neither does Ellipse apparently. I think Ellipsoid is the only one.

That said, I didn't test custom 3D models yet. It looks like ModelVisualizer contains a suspicious line that may need testing independently of dynamicGeoemtryGetBoundingSphere.