-
Notifications
You must be signed in to change notification settings - Fork 439
Add the draping of imagery layers on 3d tiles functionality #11466
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
base: master
Are you sure you want to change the base?
Add the draping of imagery layers on 3d tiles functionality #11466
Conversation
…at/support-3d-tiles-imagery-lyr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are few issue to solve:
- We should move the _msUpdatePrimitivesImageryLayersTimeout function setup inside componentDidMount of the cesium/Map.jsx file so we are sure is created once. We should also set it to undefined on componentWillUnmount
- The overlay is not working properly when a imagery layer is moved after the 3D Tiles or removed by the TOC, steps:
- open this map
- move the test_dataset layer below the 3D Tiles layer (test again by remove it from the TOC)
- Expected the overlay is not applied
- Current the overlay is still applied
- It seems the overlay system is a bit slow to apply so I think it would be better to enable it with an options. My proposal is to include a new property called
enableImageryOverlayfor 3D tiles layer. This property should be undefined by default and accessible as checkbox in the display panel
…at/support-3d-tiles-imagery-lyr
47d6fcb to
831a874
Compare
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@subashtiwari1010 I pushed a code refactor because the way we implemented it before was not making it possible to remove imagery layers. The refactor moved all the logic to the 3D Tiles again similar to your initial approach but added logic in the generic Layer component to trigger an update count every time a new imagery layer is added removed or sorted in the layer tree. Additionally few fixes have been introduced inside the imagery visibility and opacity management to use as much as possible the native CesiumJS api.
Note: with this approach we are forced to recreate the 3D Tiles when the layer tree is reorder to ensure all the imagery layer are correctly placed
Please complete the PR with following:
- review the overall code and test it locally to ensure everything is working as expected
- review all the messages are requested inline
- fix failing test
| "eyeDomeLightingRadius": "Lighting radius" | ||
| } | ||
| }, | ||
| "enableImageryOverlay": "Enable imagery overlay" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should explain a bit more, e.g.
| "enableImageryOverlay": "Enable imagery overlay" | |
| "enableImageryOverlay": "Enable imagery layers overlay" |
I think we should add an inline question mark icon info with additional information explaining that all the imagery layers such as WMS, TMS, WMTS, ... positioned above the 3D Tiles will be rendered in order on top of it.
Please use the same component already used for other checkbox in settings, e.g. this is visible for WMS
Description
Introduced the possibility to drape imagery layers on top of 3D Tiles. The drape is only done of those imagery layers that are above the 3d tiles (upon which the drape is done) in the ToC.
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (check one with "x", remove the others)
Issue
What is the current behavior?
There is no possibility to drape the imagery layers on the 3d tiles.
Fixes #11257
What is the new behavior?
When the imagery layers are above the 3d tiles in the ToC, those layers can be draped on the 3d tiles.
Breaking change
Does this PR introduce a breaking change? (check one with "x", remove the other)
Other useful information