Skip to content

Add WMTS and WMS layer components for 3D #423

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

Merged
merged 5 commits into from
Jul 12, 2023

Conversation

vladyslav-tk
Copy link
Contributor

@vladyslav-tk vladyslav-tk commented Jul 5, 2023

@vladyslav-tk vladyslav-tk requested a review from pakb July 5, 2023 16:11
@vladyslav-tk vladyslav-tk changed the title Add WMTS layer component for 3D Add WMTS and WMS layer component for 3D Jul 6, 2023
@vladyslav-tk vladyslav-tk changed the title Add WMTS and WMS layer component for 3D Add WMTS and WMS layer components for 3D Jul 6, 2023
v-if="layerConfig.type === LayerTypes.WMTS && !layerConfig.isExternal"
:wmts-layer-config="layerConfig"
:preview-year="previewYear"
:projection="WEBMERCATOR"
Copy link
Contributor

Choose a reason for hiding this comment

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

don't know if it would make sense to directly use the same logic as the one for OL, meaning :

:projection="layerConfig.isExternal ? WEBMERCATOR : LV95"

Some of our WMTS layers do not behave totally correctly when requested as EPSG:3857 or EPSG:4326

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cesium makes wrong requests when using LV95 for WMTS layer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe we can investigate and try to use WebMapServiceImageryProvider instead of UrlTemplateImageryProvider if LV95 required for WMTS

Copy link
Contributor

Choose a reason for hiding this comment

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

let's keep mercator there then, it's something we should investigate if we want to have LV95 projection for 3D in the future (I doubt it's a good idea anyway, for 3D at least)

@vladyslav-tk vladyslav-tk force-pushed the feat-BGDIINF_SB-2979_add_3d_wmts_component branch from 0903082 to 6fed90a Compare July 10, 2023 15:14
@vladyslav-tk vladyslav-tk force-pushed the feat-BGDIINF_SB-2979_add_3d_wmts_component branch from 6fed90a to 8f1e147 Compare July 11, 2023 14:13
@vladyslav-tk
Copy link
Contributor Author

You do not state that you require a createImagery(...) method, and its input/outputs
You are watching url in the mixin, but it is never stated that this is required by the mixin

I added this to the doc

I don't know if this would align nicely with future GeoJSON and KML implementation, maybe you could require that the component using the mixin provides a Cesium.ImageryProvider() as this.provider and you could react to changes to this provider directly. Don't know if it would be better in regards to future GeoJSON and KML implementation?!

We can't use this mixin for GeoJSON and KML because it will be not imagery layers and zIndex, opacity, and URL will be handled differently. Maybe I will create one more mixin, more general

Copy link
Contributor

@pakb pakb left a comment

Choose a reason for hiding this comment

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

looking good! small issue with opacity to fix, and some unnecessary WMTS check in the utils function and we should be good to go!

@vladyslav-tk vladyslav-tk force-pushed the feat-BGDIINF_SB-2979_add_3d_wmts_component branch from b7a2867 to c1f3b05 Compare July 12, 2023 09:43
@pakb pakb merged commit 6ade17f into develop Jul 12, 2023
@pakb pakb deleted the feat-BGDIINF_SB-2979_add_3d_wmts_component branch July 12, 2023 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants