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 #9775 Visibility limits not working in 3D for detached layers #9777

Merged
merged 3 commits into from
Dec 6, 2023
Merged

Fix #9775 Visibility limits not working in 3D for detached layers #9777

merged 3 commits into from
Dec 6, 2023

Conversation

allyoucanmap
Copy link
Contributor

@allyoucanmap allyoucanmap commented Nov 30, 2023 •
edited
Loading

Description

This PR introduces following changes:

  • ensure correct visibility when a layer is created
  • review visibility workflow of 3D detached layers. The changes are centralizing the add/remove based on visibility inside the Layer.jsx component instead to have specific implementation per layer type
  • compute of the zoom level based on the distance between camera and target point
  • remove a side effect in the visibility limits form that was showing alerts every time the settings panel was opened

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?

#9775

What is the new behavior?

Breaking change

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

  • No

Other useful information

@allyoucanmap allyoucanmap added bug 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 C027-COMUNE_FI-2023-SUPPORT labels Nov 30, 2023
@allyoucanmap allyoucanmap added this to the 2023.02.02 milestone Nov 30, 2023
@allyoucanmap allyoucanmap self-assigned this Nov 30, 2023
@tdipisa tdipisa requested a review from MV88 November 30, 2023 14:29
@tdipisa tdipisa linked an issue Dec 1, 2023 that may be closed by this pull request
Visibility limits not working in 3D for detached layers #9775
Closed
1 task
MV88
MV88 requested changes Dec 4, 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.

overall looks ok to me, but I have some inline comments for you to checkout,

feel free to merge this if those are not necessary

web/client/components/map/cesium/Layer.jsx Outdated Show resolved Hide resolved
web/client/components/map/cesium/Layer.jsx Outdated Show resolved Hide resolved
web/client/components/map/cesium/Layer.jsx Outdated Show resolved Hide resolved
web/client/components/map/cesium/Map.jsx Show resolved Hide resolved
@MV88 MV88 enabled auto-merge (squash) December 6, 2023 11:19
@MV88 MV88 merged commit eca6b14 into geosolutions-it:master Dec 6, 2023
6 checks passed
@MV88
Copy link
Contributor

MV88 commented Dec 6, 2023

@ElenaGallo please test it in DEV

@ElenaGallo
Copy link
Contributor

test passed, @allyoucanmap please backport to 2023.02.xx. Thanks

allyoucanmap added a commit to allyoucanmap/MapStore2 that referenced this pull request Dec 13, 2023
@allyoucanmap
Fix geosolutions-it#9775 Visibility limits not working in 3D for deta…
577959a 
…ched layers (geosolutions-it#9777)

* Fix geosolutions-it#9775 Visibility limits not working in 3D for detached layers

* requested changes

* udpate zoom from height comment
@allyoucanmap allyoucanmap removed the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Dec 13, 2023
tdipisa pushed a commit that referenced this pull request Dec 13, 2023
@allyoucanmap
[Backport 2023.02.xx] Fix #9775 Visibility limits not working in 3D f…
bf050d7 
…or detached layers (#9777) (#9812)

* Fix #9775 Visibility limits not working in 3D for detached layers (#9777)

* Fix #9775 Visibility limits not working in 3D for detached layers

* requested changes

* udpate zoom from height comment

* fix failing tests
This was referenced Dec 14, 2023
Merged
Merged
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 C027-COMUNE_FI-2023-SUPPORT
Projects
None yet
Milestone
2023.02.02
Development

Successfully merging this pull request may close these issues.

Visibility limits not working in 3D for detached layers
3 participants
@allyoucanmap @MV88 @ElenaGallo