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

Enable terrain by default in CesiumViewer #6198

Merged
merged 3 commits into from
Feb 13, 2018
Merged

Conversation

hpinkos
Copy link
Contributor

@hpinkos hpinkos commented Feb 8, 2018

Fixes #6192

@mramato is there anything else I had to do to enable clamping for the data sources? We were already passing in clampToGround for GeoJsonDataSource and for CzmlDataSource it's something you enable in the CZML, right?

@cesium-concierge
Copy link

Signed CLA is on file.

@hpinkos, thanks for the pull request! Maintainers, we have a signed CLA from @hpinkos, so you can review this at any time.

⚠️ I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to CHANGES.md and comment on this pull request so we know it was updated. For more info, see the Pull Request Guidelines.


I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 9, 2018

I think we should update CHANGES.md for this.

@hpinkos
Copy link
Contributor Author

hpinkos commented Feb 9, 2018

@pjcozzi done

@@ -58,6 +60,12 @@ define([
scene3DOnly : endUserOptions.scene3DOnly,
requestRenderMode : true
});

viewer.terrainProvider = new CesiumTerrainProvider({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes the BaseLayerPicker to still think Ellipsoid is selected. To fix that, you need to tell it to use terrain as configured by the picker. Because there's a chance we might not have the baseLayer picker (because of the imagery provider options), we need to handle both cases. Basically this code becomes:

    var baseLayerPicker = viewer.baseLayerPicker;
    if (defined(baseLayerPicker)) {
        var viewModel = baseLayerPicker.viewModel;
        viewModel.selectedTerrain = viewModel.terrainProviderViewModels[1];
    } else {
        viewer.terrainProvider = new CesiumTerrainProvider({
            url: 'https://assets.agi.com/stk-terrain/v1/tilesets/world/tiles',
            requestWaterMask: true,
            requestVertexNormals: true
        });
    }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside, we can probably get rid of the imagery provider option, but for this PR I wouldn't worry about it.

@hpinkos
Copy link
Contributor Author

hpinkos commented Feb 13, 2018

Thanks @mramato, fixed

@mramato mramato merged commit 14bb593 into master Feb 13, 2018
@mramato mramato deleted the cesium-viewer-terrain branch February 13, 2018 16:59
@mramato
Copy link
Contributor

mramato commented Feb 13, 2018

Thanks @hpinkos!

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

Successfully merging this pull request may close these issues.

4 participants