CesiumGS / cesium

An open-source JavaScript library for world-class 3D globes and maps :earth_americas:
https://cesium.com/cesiumjs/
Apache License 2.0
12.98k stars 3.5k forks source link

Ellipsoid.geodeticSurfaceNormal returns wrong result if the cartesian is not on the ellpsoid surface #4682

Open duvifn opened 7 years ago

duvifn commented 7 years ago

I encountered this while working on #4622.

Sandcastle for this issue

The following code log false to console:

var latitude = 45.0;
var longitude = 90;

var cartographic = Cesium.Cartographic.fromDegrees(longitude,latitude);
var cartesian = Cesium.Ellipsoid.WGS84.cartographicToCartesian(cartographic,new Cesium.Cartesian3());
var surfaceNormal =  Cesium.Ellipsoid.WGS84.geodeticSurfaceNormal(cartesian,new Cesium.Cartesian3());

cartographic.height= 2000000;
var cartesian2 = Cesium.Ellipsoid.WGS84.cartographicToCartesian(cartographic,new Cesium.Cartesian3());
var surfaceNormal2 =  Cesium.Ellipsoid.WGS84.geodeticSurfaceNormal(cartesian2,new Cesium.Cartesian3());

//log 'false'
console.log(Cesium.Cartesian3.equalsEpsilon(surfaceNormal,surfaceNormal2, Cesium.Math.EPSILON8));

However, Ellipsoid.geodeticSurfaceNormalCartographic returns accurate result in this case:

var cartographic = Cesium.Cartographic.fromDegrees(longitude,latitude);
var surfaceNormalFromCartographic = Cesium.Ellipsoid.WGS84.geodeticSurfaceNormalCartographic(cartographic);

cartographic.height= 2000000;
var surfaceNormalFromCartographic2 = Cesium.Ellipsoid.WGS84.geodeticSurfaceNormalCartographic(cartographic);

//log 'true'
console.log(Cesium.Cartesian3.equalsEpsilon(surfaceNormalFromCartographic,surfaceNormalFromCartographic2, Cesium.Math.EPSILON8));
hpinkos commented 7 years ago

Thanks @duvifn

pjcozzi commented 7 years ago

For performance, this might just be a doc fix so that geodeticSurfaceNormal requires a height of zero. @duvifn do you have another suggestion?

hpinkos commented 7 years ago

@pjcozzi if that's the case, shouldn't we throw a DeveloperError if the height is greater than zero?

pjcozzi commented 7 years ago

I am suggesting that the performance for the conversion is not worth it.

duvifn commented 7 years ago

Thanks @pjcozzi and @hpinkos for reviewing this.

@duvifn do you have another suggestion?

Not really. Maybe additional parameter that explicitly indicates if the input value is on the surface.

Anyway, please consider reviewing of the code that currently uses this function (js and glsl). I saw some places where the input is not necessarily on the ellipsoid surface.

I believe that for most of the needs the bias is not significant, but there can be places in which this could lead to error