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.

Sign up for GitHub

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

Jump to bottom

Fix #9189 3d height from terrain not working with cesium terrain provider #9191

Merged
merged 1 commit into from
May 24, 2023
Merged

Fix #9189 3d height from terrain not working with cesium terrain provider #9191

merged 1 commit into from
May 24, 2023

Conversation

allyoucanmap
Copy link
Contributor

Description

This PR fixes a problem due to the missing .catch method in the default terrain provider. Cesium.sampleTerrainMostDetailed is using a promise implemented with the when.js library that does not exposes the catch method. There was a similar issue with the leader line style that is using the same method

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix

Issue

What is the current behavior?

#9189

What is the new behavior?

It is possible to use the measure height from terrain and leader line style also with cesium terrain provider

Breaking change

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • No

Other useful information

@allyoucanmap allyoucanmap added this to the 2023.01.02 milestone May 24, 2023
@allyoucanmap allyoucanmap self-assigned this May 24, 2023
@allyoucanmap allyoucanmap linked an issue May 24, 2023 that may be closed by this pull request
3d height from terrain not working with cesium terrain provider #9189
Closed
@tdipisa tdipisa requested a review from MV88 May 24, 2023 08:59
@tdipisa tdipisa added BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch 3D All issues related to the 3D rendering in CesiumJs labels May 24, 2023
MV88
MV88 approved these changes May 24, 2023
View reviewed changes
Copy link
Contributor

@MV88 MV88 left a comment

Choose a reason for hiding this comment

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

LGTM

@MV88 MV88 merged commit 70636ec into geosolutions-it:master May 24, 2023
@MV88
Copy link
Contributor

MV88 commented May 24, 2023

@ElenaGallo please test it in DEV

@allyoucanmap please create backport

@landryb
Copy link
Collaborator

landryb commented May 24, 2023

thanks @allyoucanmap for the super quick fix, i've tested it working fine on master with my cesium terrain.

image

@tdipisa tdipisa mentioned this pull request May 24, 2023
Closed
@ElenaGallo
Copy link
Contributor

Test passed on DEV, @allyoucanmap please backport to 2023.01.xx

allyoucanmap added a commit to allyoucanmap/MapStore2 that referenced this pull request May 24, 2023
@allyoucanmap
Fix geosolutions-it#9189 3d height from terrain not working with cesi…
75b8b15 
…um terrain provider (geosolutions-it#9191)
@allyoucanmap allyoucanmap removed the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label May 24, 2023
tdipisa pushed a commit that referenced this pull request May 25, 2023
@allyoucanmap
[Backport 2023.01.xx] Fix #9189 3d height from terrain not working wi…
8b2dbc5 
…th cesium terrain provider (#9191) (#9192)

* Fix #9189 3d height from terrain not working with cesium terrain provider (#9191)

* fix failing test

* remove path test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MV88 MV88 MV88 approved these changes

Assignees

@allyoucanmap allyoucanmap

Labels
3D All issues related to the 3D rendering in CesiumJs bug
Projects
None yet
Milestone
2023.01.02
Development

Successfully merging this pull request may close these issues.

3d height from terrain not working with cesium terrain provider
5 participants
@allyoucanmap @MV88 @landryb @ElenaGallo @tdipisa