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

Replace glTF model implementation #10644

Merged
merged 114 commits into from
Aug 5, 2022
Merged

Replace glTF model implementation #10644

merged 114 commits into from
Aug 5, 2022

Conversation

ptrgags
Copy link
Contributor

@ptrgags ptrgags commented Aug 5, 2022

Fixes #10346
Fixes #10416
Fixes #10379
Fixes #9303
Fixes #7062
Fixes #6299
Fixes #2387

This PR replaces CesiumJS's glTF model implementation. the old Model was removed, and ModelExperimental was renamed to Model as a drop-in replacement. The new architecture is a lot more decoupled, and it made it easier to implement features such as 3D Tiles Next metadata and CustomShader

This PR merges the staging branch replace-model into main. It includes changes from the following PRs:

To Do:

j9liu and others added 30 commits July 21, 2022 11:35
Throw `RuntimeError` for glTF 1.0 and unsupported extensions in `ModelExperimental`
Rename `ModelExperimental` files that don't conflict with `Model`
Ensure spec parity between `Model` and `ModelExperimental`
@ptrgags ptrgags requested a review from lilleyse August 5, 2022 19:13
@cesium-concierge
Copy link

Thanks for the pull request @ptrgags!

  • ✔️ Signed CLA found.
  • ❔ Unit tests were not updated.
    • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

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

@lilleyse
Copy link
Contributor

lilleyse commented Aug 5, 2022

Does this implicitly close #10416 as well?

@ptrgags
Copy link
Contributor Author

ptrgags commented Aug 5, 2022

@lilleyse yes, it closes that issue as well as #10379. Possibly others too

@j9liu
Copy link
Contributor

j9liu commented Aug 5, 2022

We should close the roadmap issue once this is merged: #10346
This could also close:

@ptrgags
Copy link
Contributor Author

ptrgags commented Aug 5, 2022

@j9liu I updated the description so this PR closes those issues.

@j9liu
Copy link
Contributor

j9liu commented Aug 5, 2022

@ptrgags @lilleyse merging now!

@j9liu j9liu merged commit 8c92d86 into main Aug 5, 2022
@j9liu j9liu deleted the replace-model branch August 5, 2022 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants