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.

Sign up for GitHub

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

Jump to bottom

Fix GLTFDocument so it can export CSG Meshes correctly. #92368

Merged
merged 1 commit into from
May 29, 2024
Merged

Fix GLTFDocument so it can export CSG Meshes correctly. #92368

merged 1 commit into from
May 29, 2024

Conversation

fire
Copy link
Member

@fire fire commented May 25, 2024 •
edited
Loading

When exporting a CSGMesh the gltf fails to export in a few situations:

  • You place a material in the override material slot of the csg and it would be lost
  • In some cases you can have a root node and two child CSG nodes and the nodes would be overlapping after export instead of preserving the geometric locations of the vertices.

@fire fire requested a review from aaronfranke May 25, 2024 23:57
@fire fire marked this pull request as ready for review May 25, 2024 23:57
@fire fire requested a review from a team as a code owner May 25, 2024 23:57
@AThousandShips AThousandShips added this to the 4.3 milestone May 26, 2024
@fire fire force-pushed the vsk-csg-gltf-4.3 branch 4 times, most recently from a7093b7 to 214e74b Compare May 26, 2024 23:42
@fire fire force-pushed the vsk-csg-gltf-4.3 branch from 214e74b to 61510d3 Compare May 27, 2024 14:33
lyuma
lyuma approved these changes May 29, 2024
View reviewed changes
Copy link
Contributor

@lyuma lyuma left a comment

Choose a reason for hiding this comment

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

Seems good - i like the simplification of using MeshInstance3D::get_active_material() since that is what it is for.

modules/gltf/gltf_document.cpp
@@ -5348,7 +5353,7 @@ void GLTFDocument::_convert_csg_shape_to_gltf(CSGShape3D *p_current, GLTFNodeInd
GLTFMeshIndex mesh_i = p_state->meshes.size();
p_state->meshes.push_back(gltf_mesh);
p_gltf_node->mesh = mesh_i;
p_gltf_node->transform = csg->get_meshes()[0];
p_gltf_node->transform = csg->get_transform();
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird: i wonder why get_meshes()[0] is hardcoded to always return Transform3D::IDENTITY

@fire fire force-pushed the vsk-csg-gltf-4.3 branch from 595ca6f to 5a24aec Compare May 29, 2024 14:11
@akien-mga akien-mga merged commit 5f57256 into godotengine:master May 29, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@fire fire deleted the vsk-csg-gltf-4.3 branch May 29, 2024 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AThousandShips AThousandShips AThousandShips left review comments

@lyuma lyuma lyuma approved these changes

@aaronfranke aaronfranke aaronfranke approved these changes

Assignees
No one assigned
Labels
bug topic:import topic:3d
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.
5 participants
@fire @akien-mga @aaronfranke @lyuma @AThousandShips