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

Cesium Inspector Undefined min/max in 1.55 #7672

Closed
OmarShehata opened this issue Mar 22, 2019 · 2 comments · Fixed by #7904
Closed

Cesium Inspector Undefined min/max in 1.55 #7672

OmarShehata opened this issue Mar 22, 2019 · 2 comments · Fixed by #7904

Comments

@OmarShehata
Copy link
Contributor

Go to:

https://cesiumjs.org/releases/1.55/Apps/Sandcastle/index.html?src=Cesium%20Inspector.html

Click "Terrain > pick tile", notice the min/max is undefined.

Go to:

https://cesiumjs.org/releases/1.54/Apps/Sandcastle/index.html?src=Cesium%20Inspector.html

Repeat the above, notice the min/max works. Is this a bug in terrain, or does the inspector just need to be updated to account for @kring 's changes?

@dennisadams
Copy link
Contributor

The inspector just needs to be updated. As part of the terrain overhaul (#7061) the minimumHeight and maximumHeight properties have moved from the QuadtreeTile's data to data.tileBoundingRegion.
To fix it simply add tileBoundingRegion in the missing places:

https://github.com/AnalyticalGraphicsInc/cesium/blob/10588c0e707334a0166078db7927caeaa098e6d5/Source/Widgets/CesiumInspector/CesiumInspectorViewModel.js#L887

Another place where it needs to be added is in Globe.prototype.getHeight:

https://github.com/AnalyticalGraphicsInc/cesium/blob/8cb03492558b6a19330ceef0eba801cdcf8c450d/Source/Scene/Globe.js#L678

These are the places I found that haven't been updated, there might be some more.

@OmarShehata
Copy link
Contributor Author

@dennisadams thanks for looking into this! If you have a chance to, it would be awesome to open a PR with this! And then even if there are other places the reviewer might recommend where to look or double check that this is all.

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

Successfully merging a pull request may close this issue.

3 participants