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

Add support for OGC:3DTILES protocol #6950

Merged
merged 6 commits into from
Oct 9, 2023
Merged

Add support for OGC:3DTILES protocol #6950

merged 6 commits into from
Oct 9, 2023

Conversation

landryb
Copy link
Contributor

@landryb landryb commented Mar 28, 2023

Cf georchestra/geonetwork#226 & georchestra/geonetwork#227

  • only tested with an external viewer, mapstore2 (with support for direct 3dtiles loading since #8055 allow to load 3dtiles tileset via viewer parameters geosolutions-it/MapStore2#9021), and in the context of georchestra (eg not 'vanilla geonetwork')
  • i have no idea if the default gn 3d viewer has support for 3dtiles datasets, and wont really be able to work on that -> help/finishing touches welcome !
  • OGC:3DTILES as a protocol type should be a valid type since its an official OGC standard
  • the dataset entry point is a regular http/https url, pointing at a json file
  • with what i've done, im not 100% sure by default OGC:3DTILES is added to search->linkTypes->layers in the ui config..

@landryb
Copy link
Contributor Author

landryb commented Apr 12, 2023

@fgravin @fxprunayre feedback welcome.. thanks !

@landryb
Copy link
Contributor Author

landryb commented Apr 26, 2023

@fgravin @fxprunayre that was merged downstream in georchestra/geonetwork#227, can you have a look at that PR ? thanks !

@landryb
Copy link
Contributor Author

landryb commented Oct 3, 2023

just rebased my branch on top of main without conflicts, can this get reviewed and merged ?

i'm going to do a similar feature/PR for OGC:COG support since support for COG is coming to mapstore.

fgravin
fgravin approved these changes Oct 3, 2023
View reviewed changes
Copy link
Member

@fgravin fgravin left a comment

Choose a reason for hiding this comment

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

Hi @landryb, I had a look about this PR already.

The issue that I have regarding the GeoNetwork community is that the 3D tiles format seems not to be handled by GeoNetwork itself. GeoNetwork map viewer has a 3D view, which might not be well maintained. I guess if no external viewer is set up, then the 3D tiles resource can't be used as other resources, am I correct ?

The PR is not intrusive though so it could be merged IMO.

Anyone else would have an objection ?

web-ui/src/main/resources/catalog/components/metadataactions/RelatedResourcesService.js
@@ -126,6 +126,8 @@

var addWMSToMap =
gnViewerSettings.resultviewFns && gnViewerSettings.resultviewFns.addMdLayerToMap;
var add3dTilesToMap =
Copy link
Member

Choose a reason for hiding this comment

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

Does it work in GN map viewer ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i have no idea since i never really used/tried the internal GN viewer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and to be fully honest, since i dont know the internals of the GN viewer at all, i added 3dtiles everywhere i saw wms/wmts until opening 3dtiles in external viewer worked, so maybe its not needed for my usecase.

@landryb landryb mentioned this pull request Oct 3, 2023
Merged
@jahow
Copy link
Contributor

jahow commented Oct 3, 2023

I think this would need an alert saying "This format is not supported yet" for people using the default map viewer and clicking on such a link. @landryb is it ok if someone else amends your PR to handle that?

@landryb
Copy link
Contributor Author

landryb commented Oct 4, 2023

@landryb is it ok if someone else amends your PR to handle that?

oh definitely, dont hesitate to improve :) (and same thing for #7387)

@jahow jahow merged commit f7ff865 into geonetwork:main Oct 9, 2023
@josegar74 josegar74 added this to the 4.4.1 milestone Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jahow jahow jahow approved these changes

@fgravin fgravin fgravin approved these changes

@fxprunayre fxprunayre Awaiting requested review from fxprunayre

Assignees
No one assigned
Labels
changelog
Projects
None yet
Milestone
4.4.1
Development

Successfully merging this pull request may close these issues.
4 participants
@landryb @jahow @fgravin @josegar74