-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Why does BaseLayerPickerViewModel set depthTestAgainstTerrain ? #6991
Comments
It has always been like this (since the feature was added over 4 years ago #1607 In general, people don't expect data to show through the terrain and not having it turn on automatically leads to a lot of "why is my data wavy/floaty" questions on the forum. But ultimately it's a moot point because when we enable terrain by default, depthTestAgainstTerrain will also default to true. At that point, I would be okay with removing it from the BaseLayerPicker. I wouldn't do anything before that time though. I'm okay with leaving this open until that change happens just so we don't forget about it. |
I was just confused by this today. It makes sense why adding terrain changes the depthTestAgainstTerrain default to true, but removing baseLayerPicker sets it back to false even when there is terrain in the scene. In my scene I added buildings and terrain and disabled the baseLayerPicker and the buildings were showing through the terrain, but that doesn't seem like the expected default behavior?
|
Reported on the forum: https://community.cesium.com/t/seeing-osm-buildings-through-terrain/23153/4 |
https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Source/Widgets/BaseLayerPicker/BaseLayerPickerViewModel.js#L247
This recently led to some confusion in behavior in #6990
I don't think BaseLayerPicker should be setting this option. This seems like a decision the developer should make, not the widget.
We also set
depthTestAgainstTerrain
inCesiumInpector
. I think I did this initially because one of the features needed it, but we should see if we can remove it from there now too.Thoughts? @mramato @emackey
The text was updated successfully, but these errors were encountered: