cesium icon indicating copy to clipboard operation
cesium copied to clipboard

Drawing and picking issues on terrain with negative WGS84 elevation

Open chris-cooper opened this issue 6 years ago • 16 comments

Hi Cesium team!

There seems to be various issues with rendering and picking at negative WGS84 elevations. Appologies for the poor capture but hopefully you can still see the issues with the lat/lon display disappearing and the line also disappearing at certain angles. Note that I think this issue predates the work done on log depth buffers.

Sandcastle Repro

picking

chris-cooper avatar Sep 28 '18 06:09 chris-cooper

Hmm the behaviour has changed with Cesium 1.50. There can be large discrepencies between results from scene.pickPosition() and globe.pick().

Sandcastle

picking2

chris-cooper avatar Oct 02 '18 00:10 chris-cooper

For the second gif I'm also seeing discrepancies in 1.49. I wonder though if the problem is the pickPosition is hitting the label object rather than the terrain. I tweaked the pixel offset so the label is above the cursor and the results seemed much better.

Demo

lilleyse avatar Oct 02 '18 19:10 lilleyse

Ah, my mistake @lilleyse, thanks for looking. After some further investigation it seems some of the issues we're seeing in our app are specific to having depthTestAgainstTerrain disabled and might be DepthPlane related. You might notice geometry disappearing and difficulty navigating (e.g. zooming in) in this example:

Sandcastle

chris-cooper avatar Oct 02 '18 21:10 chris-cooper

Here's an example which more clearly shows the rendering issues. The red polygon in particular doesn't appear until viewed from afar.

Sandcastle

more-with-the-negative-terrain

chris-cooper avatar Oct 03 '18 01:10 chris-cooper

It looks like some of the rendering issues are related to going below the limits specified in ApproximateTerrainHeights. Unfortunately some of our users like to dig big holes in the ground. Offsetting the ApproximateTerrainHeights down allows me to render more predictably.

chris-cooper avatar Oct 03 '18 05:10 chris-cooper

Initial testing shows these workarounds address picking and rendering: https://github.com/PropellerAero/cesium/commit/e25b16313b48f064ba8404d71dda7223eded8c0f

chris-cooper avatar Oct 03 '18 06:10 chris-cooper

@ggetz We're continuing to try to contribute modifications in our Cesium fork back into CesiumGS/cesium. I'm re-investigating this issue at the moment. The following change in particular:

https://github.com/PropellerAero/cesium/commit/96d96606c71c02edb6283eab3c1e5212484ee0f2

To contribute this type of change it would be good if we could control the max excavation depth. e.g. normally there is no offset applied, but we (or other users who run into this issue) can set it to -250. I'm not sure how to do that since ApproximateTerrainHeights.initialize() is called both from the main thread and also from a web worker. Can you suggest a way to pass a variable in?

chris-cooper avatar Oct 19 '23 04:10 chris-cooper

Thanks @chris-cooper!

Long term, relying on ApproximateTerrainHeights is likely something we'll want to move away from.

I'm not sure I see where ApproximateTerrainHeights.initialize() is called in a web worker. Do you mind telling me where you see it?

ggetz avatar Oct 19 '23 16:10 ggetz

It sounds like a good idea to move away from ApproximateTerrainHeights, but also any assumptions around ellipsoid height greater than zero. I think both have caused us some hard to track down rendering and picking issues.

For this current issue it's createGroundPolylineGeometry but there may be other indirect calls to it too from other workers.

chris-cooper avatar Oct 19 '23 22:10 chris-cooper

Got it, thanks @chris-cooper.

Besides the other issues ApproximateTerrainHeights is causing, it's not architected well for setting an offset value like you suggest. As it stands now, you'll need to pass the offset value as a parameter to the workers. Then you could set that value on ApproximateTerrainHeights in the worker code, or pass it to initialize.

Perhaps a custom ellipsoid, or a new property on Ellipsoid may be best, as it is usually saved as a property on ground geometries, which are usually what is already passed into the workers.

If you do have any further bandwidth to work on https://github.com/CesiumGS/cesium/issues/8480, please let us know!

ggetz avatar Oct 20 '23 17:10 ggetz

I tried adding an attribute to Ellipsoid and Polyline and the render looks good, bounding box needs some fixing.

const WGS84 = new Cesium.Ellipsoid(6378137.0, 6378137.0, 6356752.3142451793); WGS84.underGroudOffset = 250;

Little catch WGS84 etc are frozen objects, so this fails const WGS84 = Cesium.Ellipsoid.WGS84; WGS84.underGroudOffset = 250;

Any preference on how to handle it? I would just add getWGS84withOffset()

image

katSchmid avatar Nov 16 '23 04:11 katSchmid

We're also considering another workaround which is to monkey patch approximateTerrainHeights.json as a npm postinstall script. This means after the cesium dependency is installed, the heights in the approximateTerrainHeights.json files are modified. That modified file then gets shipped with our frontend.

This is relying on the json file being loaded asynchronously. It would break if the cesium build changed to e.g. inline the json files within the web workers. Are there any plans to do that @ggetz, or do you see approximateTerrainHeights.json as always being an async load?

chris-cooper avatar Nov 17 '23 03:11 chris-cooper

@chris-cooper We've tossed around the possibility of inlining JSON files, and approximateTerrainHeights.json would be included. However there is no definite timeline for that at the moment.

ggetz avatar Nov 17 '23 21:11 ggetz

I just hard coded the ellipsoid custom values to test above and if it has the values its fine, just passing ellipsoid is kind of tricky. Unless this all got fixed passing an ellipsoid with an entity isnt as straightforward as i thought. Would you know the current status?https://github.com/CesiumGS/cesium/issues/3543

katSchmid avatar Dec 01 '23 05:12 katSchmid

@katSchmid We don't have immediate plans to work on https://github.com/CesiumGS/cesium/issues/3543. But we'd be happy to discuss details and/or accept a Pull Request for this feature.

ggetz avatar Dec 01 '23 16:12 ggetz

Since picking is used for camera controls, we've noticed this causing issues when the camera goes below the ellipsoid, such as what is described @mramato's comment here.

ggetz avatar Feb 29 '24 21:02 ggetz

This should be fixed as of https://github.com/CesiumGS/cesium/pull/11983. There's still a slight discrepancy due to the differing picking approaches, but they are much closer in value.

image

ggetz avatar May 14 '24 20:05 ggetz