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

Remove old metadata implementation files #658

Merged
merged 5 commits into from
Jun 16, 2023

Conversation

j9liu
Copy link
Contributor

@j9liu j9liu commented May 30, 2023

This PR does the following:

  • removes the files related to the old metadata implementation for EXT_feature_metadata.
  • renames the files related to the new metadata implementation (removing the "MeshFeatures" and "StructuralMetadata" prefix)
  • takes the new metadata implementation out of the MeshFeatures and StructuralMetadata namespace.

This is based on #656, so merge that first.

@j9liu j9liu changed the base branch from main to property-textures May 30, 2023 17:13
@j9liu j9liu marked this pull request as draft May 30, 2023 17:45
@j9liu
Copy link
Contributor Author

j9liu commented May 30, 2023

Hold on merging this -- I'll work on integrating the new implementation into Unreal, using the old files as reference. When I confirm that it works in Unreal I'll un-draft this.

Base automatically changed from property-textures to upgrade-feature-metadata June 6, 2023 06:21
@j9liu j9liu force-pushed the replace-feature-metadata-files branch from 17e1c85 to a50093c Compare June 6, 2023 15:28
@@ -34,14 +32,14 @@ template <typename ElementType> class MetadataArrayView {
int64_t size() const noexcept { return static_cast<int64_t>(_values.size()); }

private:
const gsl::span<const ElementType> _values;
gsl::span<const ElementType> _values;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The const here was incorrect; it prevented me from using the array view in a std::variant in Cesium for Unreal.

@j9liu j9liu marked this pull request as ready for review June 6, 2023 15:37
@j9liu
Copy link
Contributor Author

j9liu commented Jun 6, 2023

@kring updated this PR, and happy to have it be merged now. There are some miscellaneous edits to BatchTableToGltfStructuralMetadata but this mainly concerns replacing the old metadata implementation files.

I tried to update the changelog in a comprehensive but readable way, but there's a lot of changes. So feel free to workshop it.

@j9liu
Copy link
Contributor Author

j9liu commented Jun 16, 2023

I'm going to self-merge this since this is mostly file renames (and I don't want to keep adding to @kring's backlog).

@j9liu j9liu merged commit 18a1ad9 into upgrade-feature-metadata Jun 16, 2023
@j9liu j9liu deleted the replace-feature-metadata-files branch June 16, 2023 17:50
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.

1 participant