Fix #11940 : add VerticalExaggeration option to models
Description
added allowVerticalExaggeration configuration to models: This is currently defaulted to false.
Models should not exaggerate with the terrain unless specified to do so.
Issue number and link
Turn off vertical exaggeration for some Models in the Scene Fixes #11940
Testing plan
in a local Local Sandcastle instance. Set allowVerticalExaggeration = true and run Cesium
- tested with reatlime + changes and static + changes. Set allowVerticalExaggeration = false and run Cesium
- tested with reatlime + changes and static + changes.
Run the following code.
const viewer = new Cesium.Viewer("cesiumContainer", {
timeline: false,
animation: false,
terrainProvider: await Cesium.createWorldBathymetryAsync({
requestVertexNormals: true,
}),
});
viewer.baseLayerPicker.viewModel.selectedImagery =
viewer.baseLayerPicker.viewModel.imageryProviderViewModels[11];
const scene = viewer.scene;
scene.verticalExaggeration = 1.1;
//-----------------------------------------------------------------------------------------
function createModel(url, height) {
viewer.entities.removeAll();
const positionexaggerate = Cesium.Cartesian3.fromDegrees(
-123.0746000,
44.0505000,
height
);
const position = Cesium.Cartesian3.fromDegrees(
-123.0744619,
44.0503706,
height
);
const positionlook = Cesium.Cartesian3.fromDegrees(
-123.0744619,
44.0503706,
10000
);
const heading = Cesium.Math.toRadians(135);
const pitch = 0;
const roll = 0;
const hpr = new Cesium.HeadingPitchRoll(heading, pitch, roll);
const orientation = Cesium.Transforms.headingPitchRollQuaternion(
position,
hpr
);
const item = {
uri: url,
minimumPixelSize: 128,
maximumScale: 20000,
//allowVerticalExaggeration: true,
};
const entity = viewer.entities.add({
name: "Exaggerated (false)",
position: position,
orientation: orientation,
model: item,
});
const itemexaggerate = {
uri: url,
minimumPixelSize: 128,
maximumScale: 20000,
allowVerticalExaggeration: true,
};
const entityexaggerate = viewer.entities.add({
name: "Exaggerated (true)",
position: positionexaggerate,
orientation: orientation,
model: itemexaggerate,
});
//viewer.trackedEntity = entityexaggerate;
viewer.scene.camera.flyTo({
destination: positionlook
});
viewer.trackedEntity = entity;
}
const options = [
{
text: "Milk Truck",
onselect: function () {
createModel(
"../../SampleData/models/CesiumMilkTruck/CesiumMilkTruck.glb",
0
);
},
},
{
text: "Ground Vehicle",
onselect: function () {
createModel(
"../../SampleData/models/GroundVehicle/GroundVehicle.glb",
0
);
},
},
{
text: "Aircraft",
onselect: function () {
createModel(
"../../SampleData/models/CesiumAir/Cesium_Air.glb",
5000.0
);
},
},
{
text: "Drone",
onselect: function () {
createModel(
"../../SampleData/models/CesiumDrone/CesiumDrone.glb",
150.0
);
},
},
{
text: "Hot Air Balloon",
onselect: function () {
createModel(
"../../SampleData/models/CesiumBalloon/CesiumBalloon.glb",
1000.0
);
},
},
{
text: "Skinned Character",
onselect: function () {
createModel(
"../../SampleData/models/CesiumMan/Cesium_Man.glb",
0
);
},
},
{
text: "Unlit Box",
onselect: function () {
createModel(
"../../SampleData/models/BoxUnlit/BoxUnlit.gltf",
10.0
);
},
},
{
text: "Draco Compressed Model",
onselect: function () {
createModel(
"../../SampleData/models/DracoCompressed/CesiumMilkTruck.gltf",
0
);
},
},
{
text: "KTX2 Compressed Balloon",
onselect: function () {
if (!Cesium.FeatureDetection.supportsBasis(viewer.scene)) {
window.alert(
"This browser does not support Basis Universal compressed textures"
);
}
createModel(
"../../SampleData/models/CesiumBalloonKTX2/CesiumBalloonKTX2.glb",
1000.0
);
},
},
{
text: "Instanced Box",
onselect: function () {
createModel(
"../../SampleData/models/BoxInstanced/BoxInstanced.gltf",
15
);
},
},
];
Sandcastle.addToolbarMenu(options);
Author checklist
- [x] I have submitted a Contributor License Agreement
- [x] I have added my name to
CONTRIBUTORS.md - [x] I have updated
CHANGES.mdwith a short summary of my change - [ ] I have added or updated unit tests to ensure consistent code coverage
- [ ] I have updated the inline documentation, and included code examples where relevant
- [x] I have performed a self-review of my code
Thank you for the pull request, @timeichfeld-msa! Welcome to the Cesium community!
In order for us to review your PR, please complete the following steps:
- [ ] Send in a Contributor License Agreement (CLA)
- [ ] Add yourself to the contributors file
Review Pull Request Guidelines to make sure your PR gets accepted quickly.
Hi @timeichfeld-msa, thanks for the PR! I can confirm we now have a CLA on file for you.
We will review this PR shortly!
@jjhembd Can you please add your thoughts?
Hi @timeichfeld-msa, many thanks for this contribution! We have had several users asking for it.
I just have one comment about the default: I think it would be better for allowVerticalExaggeration to default to true. Two reasons for this:
- Exaggeration of buildings, vehicles, and UI elements are important visual clues that the terrain is not realistic. Many geoscientists have objected to showing terrain with vertical exaggeration, without obvious indications that it is exaggerated. See this post from a NASA scientist and this paper from a geologist. If a user really does need to show exaggerated terrain with un-exaggerated Models, then they can set
allowVerticalExaggeration = false(and use a scale bar or other tool to communicate the exaggeration). - The exaggeration in
Modelis how we exaggerate 3D Tilesets, including tilesets that are used as a substitute for terrain. See for example the Google Photorealistic 3D Tiles. These should be exaggerated by default.
Hi Jeshurun,
Understood, makes sense. I can make those changes. I am on vacation until next week, I can submit the default update when I return :)
Please let me know about the protobufjs/dist/minimal/protobuf.js error and guidance if I need to take a look.
Thanks much, -Timothy
@timeichfeld-msa The protobufjs error is being addressed in https://github.com/CesiumGS/cesium/pull/12144.
The only action needed on your end will be to merge in the main branch when you return. Thanks!
@jjhembd, @ggetz
Hello, the PR with the above changes (default to true) + updates for Changes/Contributors is ready to go.
Please let me know if there is anything else needed.
Thasnk much! -Timothy
@jjhembd
Hi Jeshurun, I think I need a little help.
I am getting errors running coverage tests:
I cant see where the error originates from the cmd output.
Chrome 128.0.0.0 (Windows 10) ERROR An error was thrown in afterAll Unhandled promise rejection: undefined thrown Unhandled promise rejection: undefined thrown ...
Chrome won't open in the automation.
it is in the cleared list of programs, and I set chrome://flags/#allow-insecure-localhost to enabled.
I made the suggestions above, and eslint appears to have passed.
We can catch up next week.
Thanks much, -Timothy
-=-=- More info. Running all tests in the browser: not sure how to get these in the correct order. Please advise.
Scene/Model/ModelRuntimePrimitive > configures pipeline stages for vertical exaggeration
Expected 'LightingPipelineStage' to equal 'VerticalExaggerationPipelineStage'.
at <Jasmine>
at verifyExpectedStages (http://localhost:8080/Build/Specs/SpecList.js:259530:30)
at UserContext.
Hi @timeichfeld-msa, thanks for the update! I pushed a small change to your branch which changes the naming slightly to align with our existing properties. I also fixed and added some additional specs.
I noticed https://github.com/CesiumGS/cesium/issues/11936 and https://github.com/CesiumGS/cesium/issues/11974 are still indeed issues, but those is out of the scope of this PR.
@jjhembd Can you do a final review and merge if happy (after the 1.121 release is complete)?
Thanks @timeichfeld-msa!