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 support for MultiPoint and Point in MAXAR_content_geojson #10762

Merged
merged 2 commits into from
Sep 7, 2022

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Sep 6, 2022

Adds support for MultiPoint and Point in GeoJsonLoader.

For background see the original PR where support for MAXAR_content_geojson was added: #10356

The diff makes the PR look more involved than it actually this. Basically there are two changes:

  • Update the parsing code to read MultiPoint and Point and save the point coordinates in the feature
  • Convert the points to a ModelComponents.Primitive

Local sandcastle

@lilleyse lilleyse requested a review from ptrgags September 6, 2022 15:46
@cesium-concierge
Copy link

Thanks for the pull request @lilleyse!

  • ✔️ Signed CLA found.
  • CHANGES.md was not updated.
    • If this change updates the public API in any way, please add a bullet point to CHANGES.md.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.

Copy link
Contributor

@ptrgags ptrgags left a comment

Choose a reason for hiding this comment

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

@lilleyse looks good, I just noticed a few places that could use some minor refactoring

Source/Scene/Model/GeoJsonLoader.js Outdated Show resolved Hide resolved
Source/Scene/Model/GeoJsonLoader.js Outdated Show resolved Hide resolved
Source/Scene/Model/GeoJsonLoader.js Show resolved Hide resolved
Source/Scene/Model/GeoJsonLoader.js Show resolved Hide resolved
Source/Scene/Model/GeoJsonLoader.js Show resolved Hide resolved
@lilleyse
Copy link
Contributor Author

lilleyse commented Sep 6, 2022

@ptrgags updated!

@ptrgags
Copy link
Contributor

ptrgags commented Sep 7, 2022

Looks good, thanks @lilleyse!

@ptrgags ptrgags merged commit ba2835b into main Sep 7, 2022
@ptrgags ptrgags deleted the geojson-points branch September 7, 2022 15:24
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.

3 participants