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

Improve GLTF export logic for scene root nodes #79801

Merged
merged 1 commit into from Aug 2, 2023

Conversation

aaronfranke
Copy link
Member

@aaronfranke aaronfranke commented Jul 22, 2023

  • Fix GLTFDocument::_convert_scene_node writing data into the JSON array for the scene root node. At this stage of the export process, it is not appropriate to touch the JSON yet, in fact the data that was being written here was being ignored and overwritten. Instead, it now writes the root node to p_state->root_nodes.
  • Improve GLTFDocument::_serialize_scenes to no longer be hard-coded for a single root node. Now it just uses whatever data was given to it in p_state->root_nodes.
  • Improve GLTFDocument::_serialize_scenes to have the failure condition at the top with a message, instead of having it check if (p_state->nodes.size()) { and return invalid data if false.
  • Move the code to add a buffer above export_preflight and convert_scene_node so that the buffer is available to be used in those steps.
  • Fix an inconsistency of r_state vs p_state in the headers vs the cpp (I used r_state, let me know if p_state is preferred instead and I can change it to be consistently p_state).

This PR should not change behavior except that p_state->root_nodes and the buffer are now populated during export, and there will not be data prematurely written to JSON and overwritten later.

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

Looks ok

@akien-mga akien-mga merged commit 6b38024 into godotengine:master Aug 2, 2023
13 checks passed
@akien-mga
Copy link
Member

Thanks!

@aaronfranke aaronfranke deleted the gltf-scene-export-logic branch August 2, 2023 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants