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

Add missing supported_crs to mosaicjson wmts endpoint #995

Closed

Conversation

AndrewAnnex
Copy link
Contributor

also made the bounds_crs discovered from the tms's geographic crs. This enables non-earth TMSs to work with GDAL/QGIS

currently I call '.srs' on the geographic crs to get the bounds crs but it may make sense to add a function to determine the more compact urn representation first, failing that allow the WKT representation to pass through.

I was able to sort of test this by manually editing the WMTS response until it worked with QGIS but let me think about the srs question above a bit more before merging. As far as I could tell, QGIS and GDAL had no issue parsing a WKT2 crs string for the supported_crs but I am not exactly sure if that is true also for the bounding box crs.

It will be difficult to test with non earth TMSs within this repository as the ones I am developing are not official with the naming authority so this may take a few tries. At least it shouldn't break any existing functionality or tests..

also made the bounds_crs discovered from the tms's geographic crs

currently I call '.srs' on the geographic crs to get the bounds crs but it may make sense to add a function to determine the more compact urn representation first, failing that allow the WKT
representation to pass through.
@@ -37,7 +37,7 @@
<ows:Title>{{ title }}</ows:Title>
<ows:Identifier>{{ layer_name }}</ows:Identifier>
<ows:Abstract>{{ title }}</ows:Abstract>
<ows:WGS84BoundingBox crs="urn:ogc:def:crs:OGC:2:84">
<ows:WGS84BoundingBox crs={{ bounds_crs }}>
Copy link
Member

Choose a reason for hiding this comment

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

would be weird to have WGS84BoundingBox not being in WGS84

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is but I think this is just an artifact of the the WMTS spec, GDAL (via gdalinfo/qgis) seemed happy to work with a mars urn here in the crs. Unless its possible to call it a BoundingBox sans geoid name? I'll have to dig more into the wmts spec to see if there is a different choice to make.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh looks like OGC 06-121r3 does mention an alternative BoundingBox that is more extendable (can be n dimensional for example). Now I can either update to only use BoundingBox or use WGS84BoundingBox if the bounds_crs is 4326. thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay went with the 2nd option, so now BoundingBox is only used if the geographic_crs is not WGS84/4326. I tried to add a test that is failing in a unexpected way, maybe something to do with the mock interface or how rasterio works but I'd expect the endpoints to always return a uri and not a urn

@AndrewAnnex
Copy link
Contributor Author

@vincentsarago, I noticed that when trying to make a WMTS from a single cog that the geographic_crs bounds were getting set to the whole globe, so grabbing the geographic_crs from the TMS I think makes sense to do here without needing to update rio_tiler, so I updated the pr to be more explicit about that.

I think however that the coord_crs may need to be added as a query parameter to /info and /info.geojson and similar functions where the no TMS is assumed, but the correct geographic_crs should be discoverable from the cog itself.

@vincentsarago
Copy link
Member

I think however that the coord_crs may need to be added as a query parameter to /info and /info.geojson and similar functions where the no TMS is assumed, but the correct geographic_crs should be discoverable from the cog itself.

I feel I should remove the geographic bounds and info about min/max zoom in /info and let that info only in the /tilejson.json endpoint

@AndrewAnnex
Copy link
Contributor Author

AndrewAnnex commented Oct 7, 2024

@vincentsarago I don't see that as correct, we have ARD COGs that are using non earth CRSs, and these endpoints are still valid and useful without the TMS. Removing them from rio-tiler is also premature unless I am misunderstanding something

@vincentsarago
Copy link
Member

@AndrewAnnex the issue is that having tms related information in /info makes things way more complicated that it should be.

same for the bounds, I'm not convinced that having bounds in the /info response is used and also makes sense without returning the CRS.

I'm going to push a PR in rio-tiler explaining the changes.

@vincentsarago
Copy link
Member

done in #1004

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.

2 participants