Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

globe.getHeight randomly returns 'undefined' #3411

Open
xtassin opened this issue Jan 12, 2016 · 14 comments
Open

globe.getHeight randomly returns 'undefined' #3411

xtassin opened this issue Jan 12, 2016 · 14 comments

Comments

@xtassin
Copy link
Contributor

xtassin commented Jan 12, 2016

Following up on a discussion in the forum https://groups.google.com/forum/#!msg/cesium-dev/NvnW8_cNEQM/L6FCmkVABgAJ

I made this post with a test case to run in Sandcastle:
https://groups.google.com/d/msg/cesium-dev/08DYCm0z5sM/yWgltqPkDQAJ

  • getHeight will randomly return 'undefined'
  • the occurrence get significantly lower when getHeight is passed a Cartographic with a roughly accurate altitude (as stated in the forum post)
  • 'undefined' values get very frequent if the Cartographic's altitude is set to 0 or 100000
  • the occurrence get higher when the camera is getting closer to the terrain
@chris-cooper
Copy link
Contributor

I've been having similar issues. Looks like it's at tile boundaries based on the gap in this test:
image

This was from the code run with the small window size pictured:

var viewer = new Cesium.Viewer('cesiumContainer');
var camera = viewer.camera;
var scene = viewer.scene;
var globe = scene.globe;
var WGS84 = Cesium.Ellipsoid.WGS84;

var cesiumTerrainProviderMeshes = new Cesium.CesiumTerrainProvider({
    url: '//assets.agi.com/stk-terrain/world',
    requestWaterMask: true,
    requestVertexNormals: true
});
viewer.terrainProvider = cesiumTerrainProviderMeshes;

var west = 148.25;
var south = -36.46;
var east = 148.27;
var north = -36.44;

var rectangle = Cesium.Rectangle.fromDegrees(west, south, east, north);
viewer.camera.setView({
    destination: rectangle
});

// Show the rectangle.  Not required; just for show.
viewer.entities.add({
    rectangle : {
        coordinates : rectangle,
        fill : false,
        outline : true,
        outlineColor : Cesium.Color.WHITE
    }
});


function runTest() {
    console.log("running test!");
    var n = 100;
    for (var y = 0; y <= n; ++y) {
        for (var x = 0; x <= n; ++x) {
            var longitude = west + x * (east - west) / n;
            var latitude = south + y * (north - south) / n;
            var cartographic = Cesium.Cartographic.fromDegrees(longitude, latitude);
            var height = globe.getHeight(cartographic);

            if(!Cesium.defined(height)){
                continue;
            }

            cartographic.height = height;

            viewer.entities.add({
                name: name,
                position: WGS84.cartographicToCartesian(cartographic),
                billboard: {
                    verticalOrigin: Cesium.VerticalOrigin.BOTTOM,
                    scale: 0.5,
                    image: '../images/facility.gif'
                }
            });

        }
    }
}

setTimeout(runTest, 5000);

@duvifn
Copy link
Contributor

duvifn commented Nov 7, 2016

the occurrence get higher when the camera is getting closer to the terrain

I had the same problem and it was solved with #4598
The problem is that the ray in globe.getHeight has a wrong direction so it doesn't hit any triangle in this case.

@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 26, 2016

@xtassin @chris-cooper this should be fixed in master (and the Cesium 1.29 release next week) thanks to #4622 by @duvifn. Let us know if you still have issues.

@pjcozzi pjcozzi closed this as completed Dec 26, 2016
@duvifn
Copy link
Contributor

duvifn commented Dec 27, 2016

Hi @pjcozzi,
Thanks a lot for merging this!

I'd like to mention that this PR didn't address @chris-cooper 's issue.
The problem of the edge of the tiles still exists, and I believe it depends on the terrain provider, data type (mesh or heighmap), upsampled or not, and zoom level.
While this PR solves 99% of the undefined issue in @xtassin's example, there are still 1% of undefined results that were not solved because of the edge issue (With the fixed code, I got 70 undefined results in about 150,000 frames in @xtassin's example, and all of them were less than Epsilon7 from the edge). This, of course, depends on the use case.

So I think that there is a need to open a different issue for @chris-cooper 's problem. There could be use cases where this can be significant.

@pjcozzi
Copy link
Contributor

pjcozzi commented Dec 27, 2016

OK, reopening.

@pjcozzi pjcozzi reopened this Dec 27, 2016
@duvifn
Copy link
Contributor

duvifn commented Jan 9, 2017

I looked a bit more into the edge issue.

  1. @chris-cooper's issue was almost completely solved with Fix billboard on terrain and Globe.getHeight #4622. It was a noticeable problem when geocentric normal was used.
    But now it's almost not noticeable because the actual gap between tiles is not so big (it's possible to see it by eyes but it's hard).

Here is a new code to reproduce this issue (@chris-cooper's code no longer reproduces this after the update). Copy and paste it into sandcastle and wait until tiles are fully loaded.
The tile should be in the exact level so make sure that the drawn rectangle is aligned with the correct tile,
like in the following print screen:
edge_problem

Then click on Get Height button. undefined should be logged to the console.

var viewer = new Cesium.Viewer('cesiumContainer');
var camera = viewer.camera;
var scene = viewer.scene;
var globe = scene.globe;
var ellipsoid = Cesium.Ellipsoid.WGS84;

var cesiumTerrainProviderMeshes = new Cesium.CesiumTerrainProvider({
    url: '//assets.agi.com/stk-terrain/world',
    requestWaterMask: true,
    requestVertexNormals: true
});
viewer.terrainProvider = cesiumTerrainProviderMeshes;

var west = 0.120513366;
var south = 0.802271952;
var east = 0.120537334;
var north = 0.802295921;
var cartographic = new Cesium.Cartographic(0.120535405,0.802295918,0.0);
var rectangle = new Cesium.Rectangle(west, south, east, north);
viewer.camera.setView({
    destination: rectangle
});

// show tile rectangle. 
viewer.entities.add({
    rectangle : {
        coordinates : rectangle,
        material : Cesium.Color.RED.withAlpha(0.5)
    }
});
Sandcastle.addDefaultToolbarButton('Get Height', function() {
    console.log("height: " + globe.getHeight(cartographic));
});
function addPoint() {
    viewer.scene.globe.depthTestAgainstTerrain = false;
    viewer.entities.add({
        name: name,
        position: ellipsoid.cartographicToCartesian(cartographic),
        point: {
            pixelSize: 10.0,
            heightReference: Cesium.HeightReference.CLAMP_TO_GROUND
        }
    });
    
}
viewer.extend(Cesium.viewerCesiumInspectorMixin);

//add point after terrain is loadded, workaround for some billboard on terrain problems
setTimeout(addPoint, 20000);
  1. I've noticed that all undefined results I still got in @xtassin example were always very close to the northern or the eastern edges only, a fact that led me to suspect that this is a rounding down issue.
    I indeed found there is a rounding down issue in AttributeCompression.
    When I changed
var x = (textureCoordinates.x * 4095.0) | 0;
var y = (textureCoordinates.y * 4095.0) | 0;

To

var x = Math.round(textureCoordinates.x * 4095.0);
var y = Math.round(textureCoordinates.y * 4095.0);

(which seems to me more correct anyway), The problem almost completely disappeared:

a. With original tiles (not upsampled ones) the problem still exists, but it much smaller, and I believe it depends on the original data. With STK terrain this is very rare (I got 8 undefined results in about 200,000 frames in @xtassin's example) and the gap is very small.
b. With upsampled tiles I didn't get any undefined in about 200,000 frames in @xtassin's example, and I think that's because the upsampling process moves vertices that are below some threshold to the edges (here).

@pjcozzi, if this seem to you, I would make a PR for this rounding issue.

There will still be an undfined result when levelZeroTiles are not yet loaded, of course.

I think that doing some threshold checking (even 1/4000 of the tile width/height) when creating the mesh, as in the upsampling process, will further improve this case...

@duvifn
Copy link
Contributor

duvifn commented Jan 11, 2017

I indeed found there is a rounding down issue in AttributeCompression.
When I changed
var x = (textureCoordinates.x * 4095.0) | 0;
var y = (textureCoordinates.y * 4095.0) | 0;
To:
var x = Math.round(textureCoordinates.x * 4095.0);
var y = Math.round(textureCoordinates.y * 4095.0);

I can reproduce #3710 before this change but not after. So I think this is at least part of the reason to #3710

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 20, 2017

@bagnell what do you think of @duvifn's suggested change, #3411 (comment)?

@lilleyse
Copy link
Contributor

lilleyse commented Mar 15, 2017

I can't reproduce this on 1.31, but #5021 was merged and contained a fix for terrain cracking.

@bagnell can you still check that the suggested change here is good to add?

@bagnell
Copy link
Contributor

bagnell commented Mar 16, 2017

I can't reproduce this either. The change suggested in #3411 (comment) is a good change. @duvifn, can you open a PR?

@duvifn
Copy link
Contributor

duvifn commented Mar 26, 2017

@pjcozzi @lilleyse @bagnell thanks!
I fixed this but encountered some problems with specs of HeightmapTesselator (it's possible that it is the class itslef).
It seems that when it uses AttributeCompression sometimes it passes NaN into it.
With current code these NaNs are converted to 0 due to the strangeness of javascript.
With the fixed code the output is NaN in these cases.
I'll try to look at this on tuesday.

I can't reproduce this either

FYI, I checked this issues again on my machine, with 1.31 (before your fix), and I can still reproduce both the undefined in the tile's boundaries and the terrain cracking.

@xtassin
Copy link
Contributor Author

xtassin commented May 15, 2017

After moving to 1.32 I noticed that the way I calculate terrain normal (sampling elevation for three points at 1m distance) sometimes returns wrong values around tiles boundaries. (my humble physics engine did not like that)

I reverted back to 1.31 which still returns the "undefined" value that, by now, I managed to work around.

1.32 never returns "undefined" but will, every now and then, return a value that if off by a couple of meters.

It was easy to discard "undefined values" and use the best neighbor sampled elevation. Having results slightly off is much more difficult to filter out.

@spencerparkin
Copy link
Contributor

Normals can be turned on for lighting the terrain in the terrain provider. Can I get access to that information just as I can get access to the height being used in the rendered terrain? I don't want to have to calculate the normals myself if I don't have to.

@Cootcha
Copy link

Cootcha commented Sep 30, 2019

getHeight still return undefined, so its still broken :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants