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

Drawing and picking issues on terrain with negative WGS84 elevation #7092

Closed
chris-cooper opened this issue Sep 28, 2018 · 17 comments
Closed

Comments

@chris-cooper
Copy link
Contributor

chris-cooper commented Sep 28, 2018

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
Copy link
Contributor Author

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

Sandcastle

picking2

@lilleyse
Copy link
Contributor

lilleyse commented Oct 2, 2018

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

@chris-cooper
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

chris-cooper commented Oct 3, 2018

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
Copy link
Contributor Author

Initial testing shows these workarounds address picking and rendering: PropellerAero@e25b163

@chris-cooper
Copy link
Contributor Author

@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:

PropellerAero@96d9660

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?

@ggetz
Copy link
Contributor

ggetz commented Oct 19, 2023

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?

@chris-cooper
Copy link
Contributor Author

chris-cooper commented Oct 19, 2023

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.

@ggetz
Copy link
Contributor

ggetz commented Oct 20, 2023

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 #8480, please let us know!

@katSchmid
Copy link

katSchmid commented Nov 16, 2023

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

@chris-cooper
Copy link
Contributor Author

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?

@ggetz
Copy link
Contributor

ggetz commented Nov 17, 2023

@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.

@katSchmid
Copy link

katSchmid commented Dec 1, 2023

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?#3543

@ggetz
Copy link
Contributor

ggetz commented Dec 1, 2023

@katSchmid We don't have immediate plans to work on #3543. But we'd be happy to discuss details and/or accept a Pull Request for this feature.

@ggetz
Copy link
Contributor

ggetz commented Feb 29, 2024

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
Copy link
Contributor

ggetz commented May 14, 2024

This should be fixed as of #11983. There's still a slight discrepancy due to the differing picking approaches, but they are much closer in value.

image

@ggetz ggetz closed this as completed May 14, 2024
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

5 participants