Skip to content

Convert MAXAR_content_geojson schema to glTF schema#1396

Merged
j9liu merged 15 commits into
mainfrom
maxar-geojson-schema
Jun 29, 2026
Merged

Convert MAXAR_content_geojson schema to glTF schema#1396
j9liu merged 15 commits into
mainfrom
maxar-geojson-schema

Conversation

@timoore

@timoore timoore commented Jun 16, 2026 •
edited by j9liu
Loading

Copy link
Copy Markdown
Contributor

Description

This is not very useful on its own, but is a stepping stone on the way to reading and transcoding the metadata included with GeoJson features in these tilesets. Also, I think the new getJson function, which fetches and parses a JSON file, could be generally useful; however, it introduces an inner call to thenInWorkerThread and I wonder if that is generally a good approach.

Issue number or link

#1355

Author checklist

  • I have submitted a Contributor License Agreement (only needed once).
  • I have done a full self-review of my code.
  • I have updated CHANGES.md with a short summary of my change (for user-facing changes).
  • I have added or updated unit tests to ensure consistent code coverage as necessary.
  • I have updated the documentation as necessary.

Remaining Tasks

Testing plan

Reviewer checklist

Thank you for taking the time to review this PR. By approving a PR you are taking as much responsibility for these changes as the author.

As you review, please go through the checklist below:

  • Review and run all parts of the test plan on this branch and verify it matches expectations.
    • If the issue is a bug please make sure you can reproduce the bug in the main branch and then checkout this branch to make sure it actually solved the issue.
  • Review the code and make sure you do not have any remaining questions or concerns. You should understand the code change and the chosen approach. If you are not confident or have doubts about the code, please do not hesitate to ask questions.
  • Review the unit tests and make sure there are no missing tests or edge cases.
  • Review documentation changes and updates to CHANGES.md to make sure they accurately cover the work in this PR.
  • Verify that the Contributor License Agreement has been submitted, if needed.

@timoore
Change handling of Schema URL
4a6b3b0 
Keep the schema URL as a local variable, not a member of
TilesetJsonLoader. Also, clean up redundant code.
j9liu
j9liu requested changes Jun 16, 2026
View reviewed changes

@j9liu j9liu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@timoore I took an early look; the main feedback I have is to take advantage of our auto-generation tools to create classes/readers from the schemas, which will make the code more robust and scalable. Otherwise, the overall approach seems fine!

Comment thread Cesium3DTilesSelection/src/TilesetJsonLoader.cpp Outdated
Comment thread Cesium3DTilesSelection/src/TilesetJsonLoader.cpp Outdated
Comment thread CesiumVectorData/src/GltfConverter.cpp Outdated
@j9liu j9liu added this to the July 2026 Release milestone Jun 16, 2026
timoore added 6 commits June 16, 2026 23:06
@timoore
Fix clang-tidy and documentation complaints
18cea0f
@timoore
Add ExtensionMetadataEntityMaxarContentGeoJson generated code
a665990 
This extension parses the metadata extension part of the
MAXAR_content_geojson extension which includes the URL of the GeoJson
schema.
@timoore
Renamed ExtensionMaxarContentGeoJson to ExtensionTilesetMaxarContentG…
64e8c79 
…eoJson
@timoore
clang-tidy fun
de6a54b
@timoore
Fix clang-tidy complaints about rapidjson::SizeValue
751bece 
It appears that the good order for rapidjson include files is:
 #include <rapidjson/document.h>
 #include <rapidjson/rapidjson.h>
@timoore
Class and readers for MAXAR_content_geojson schema
e706bbd
@timoore timoore mentioned this pull request Jun 24, 2026
Merged
j9liu
j9liu reviewed Jun 24, 2026
View reviewed changes

@j9liu j9liu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is looking a lot better @timoore ! The generated classes are a huge improvement.

I have a few small comments left, but shouldn't be anything major.

Comment thread CesiumVectorData/include/CesiumVectorData/GltfConverter.h Outdated
Comment thread Cesium3DTilesSelection/src/TilesetJsonLoader.cpp Outdated
Comment thread CesiumVectorData/src/GltfConverter.cpp Outdated
Comment thread CesiumVectorData/include/CesiumVectorData/GltfConverter.h Outdated
j9liu
j9liu reviewed Jun 24, 2026
View reviewed changes
Comment thread CesiumVectorData/src/GltfConverter.cpp
@timoore timoore marked this pull request as ready for review June 26, 2026 04:19
@timoore timoore force-pushed the maxar-geojson-schema branch from f1894bc to 6f3f436 Compare June 27, 2026 00:32
timoore added 2 commits June 27, 2026 22:14
@timoore
Unit test for schema conversion
4f5fc9d 
Also includes a change to schema conversion from #1402: use 64 bit
types.
@timoore
Add sample schema file
ab3c002
@timoore

timoore commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

@j9liu , ready for another look.

j9liu
j9liu approved these changes Jun 29, 2026
View reviewed changes

@j9liu j9liu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks Tim!

@j9liu j9liu merged commit a1c1fed into main Jun 29, 2026
28 checks passed
@j9liu j9liu deleted the maxar-geojson-schema branch June 29, 2026 14:13
@j9liu j9liu mentioned this pull request Jun 29, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@j9liu j9liu j9liu approved these changes

Assignees

@timoore timoore

Labels

None yet

Projects

None yet

Milestone

July 2026 Release

Development

Successfully merging this pull request may close these issues.

2 participants

@timoore @j9liu