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

Update 3D Tiles Next extensions #443

Merged
merged 14 commits into from
Mar 29, 2022
Merged

Update 3D Tiles Next extensions #443

merged 14 commits into from
Mar 29, 2022

Conversation

IanLilleyT
Copy link
Contributor

@IanLilleyT IanLilleyT commented Feb 17, 2022

The whole PR is autogenerated code except for files in the tools folder. There I made a few changes:

  • glTF.json and 3dTiles.json now use updated schemas for 3D Tiles 1.1. I also rearranged the items by extension, by alphabetical order.
  • Support inner references in JSON schema, like definitions.schema.json#/definitions/numericValue. Currently this doesn't affect any of our schemas because the ones that do use the # end up having many possible types so it defaults to JsonValue regardless.
  • This is a draft PR because the 3D Tiles 1.1 spec is not 100% final. I'm posting this early to show how the generated files look in case there are mistakes. I didn't look super closely at all the generated code and haven't tried calling these generated classes from other code, but that will happen soon. cesium-native builds and the generated file names look correct to me.

@IanLilleyT IanLilleyT marked this pull request as draft February 17, 2022 01:27
@IanLilleyT IanLilleyT closed this Mar 15, 2022
@IanLilleyT
Copy link
Contributor Author

Closing for now because the schema directory structure changed and we need a way to do search paths to find schemas that are not in the same tree

@lilleyse lilleyse changed the title Prepare generators for 3D Tiles 1.1 Update 3D Tiles Next extensions Mar 16, 2022
@lilleyse lilleyse reopened this Mar 16, 2022
@lilleyse lilleyse marked this pull request as ready for review March 16, 2022 13:16
@lilleyse
Copy link
Contributor

Re-opened and ready for review.

Updates just the 3D Tiles and glTF extensions. 3D Tiles 1.1 support will be a follow up PR.

@kring
Copy link
Member

kring commented Mar 22, 2022

After merging #465, there are a lot of conflicts in this PR. That isn't surprising, and I intended to just fix it. But the problem is that this branch and that branch generate the glTF classes from different branches of the glTF repo, and apparently their common ancestor is over 5 months old and each have over 150 diverging commits! Scanning the commits, I'm pretty sure this isn't actually true, but instead is a symptom of rebasing. Please don't rebase! Have I mentioned that before? Rebasing publicly-visible branches is pure evil. This is one big reason why.

Anyway, I'm probably going to need some help merging these two glTF branches before I can merge this PR.

@lilleyse
Copy link
Contributor

The glTF branches should be sorted out now. I merged CesiumGS/glTF#47 and KhronosGroup/glTF#2088 into the 3d-tiles-next branch.

I'm definitely guilty of rebasing KhronosGroup/main into CesiumGS/3d-tiles-next a while back, but lately I've avoided it.

@kring
Copy link
Member

kring commented Mar 29, 2022

Thanks @IanLilleyT and @lilleyse!

@kring kring merged commit 9493b9b into main Mar 29, 2022
@kring kring deleted the 3dtiles-1.1 branch March 29, 2022 04:58
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