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

Add technical definition for manifoldness #10419

Merged
merged 1 commit into from
Dec 23, 2024

Conversation

tetrapod00
Copy link
Contributor

Part of #10417. Also documents changes for godotengine/godot#94321.

@tetrapod00 tetrapod00 added enhancement area:manual Issues and PRs related to the Manual/Tutorials section of the documentation topic:3d labels Dec 17, 2024
@tetrapod00 tetrapod00 added this to the 4.4 milestone Dec 17, 2024
@tetrapod00 tetrapod00 requested a review from fire December 17, 2024 21:10
Comment on lines 99 to 102
- it must be closed,
- it must not self-intersect,
- it must not contain internal faces,
- every edge must connect to only two other faces.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to check that this layman's definition is still correct

Copy link
Member

@fire fire Dec 17, 2024

Choose a reason for hiding this comment

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

The triangle vertices must appear clockwise when viewed from outside the Godot Engine manifold mesh.

It can be split into two layperson explanations.

  1. The Godot Engine triangle mesh winding order should be clockwise.
  2. All the windings should be clockwise. There is no mixing between clockwise and counterclockwise.

Copy link
Member

Choose a reason for hiding this comment

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

@elalish Do you know if we need to explain something about normals? I didn't see that in your explanation.

Copy link

Choose a reason for hiding this comment

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

Technically the three of these except for self-intersection are more precisely defined by the manifold definition below. Self-intersection should be avoided, but Manifold is robust to it at least, and it's difficult to test in practice.

As for winding direction - the manifoldness definition below already restricts us to an oriented 2-manifold, so there can't be any mixing of CW and CCW. Like self-intersection, CCW can be hard to test reliably. And manifold will handle inverted meshes just fine - it's simply that CW triangles will lead to negative volume. I would trust a positive volume check over checking CCW. Likewise, normals are not very trustworthy.

Copy link

Choose a reason for hiding this comment

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

I guess I would say the triangles are CCW - this is what defines the sign of the volume and inside vs outside. There's nothing you can do to make them CW.

Copy link
Member

Choose a reason for hiding this comment

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

Clarification for others:

introduction_to_3d_coordinate_systems

Godot Engine has a different handedness than elalish/manifold. So, the winding order is swapped.

Copy link
Member

@fire fire Dec 18, 2024

Choose a reason for hiding this comment

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

REQUIRED

  • it must be closed,
  • every edge must connect to only two other faces.
  • volume must exist

NOT RECOMMENDED

  • negative volume
  • self-intersection
  • interior faces

Copy link

Choose a reason for hiding this comment

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

Hmm, Manifold is in the lower-right - Z-up and right-handed, so it looks like Godot has the same handedness? Or does Godot just use CW triangle winding as positive?

Copy link
Member

Choose a reason for hiding this comment

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

https://docs.godotengine.org/en/latest/classes/class_arraymesh.html#arraymesh

Note: Godot uses clockwise winding order for front faces of triangle primitive modes.

@fire
Copy link
Member

fire commented Dec 19, 2024

What do we need to do to get this completed?

@tetrapod00
Copy link
Contributor Author

tetrapod00 commented Dec 19, 2024

If it's all correct to you now, this is mergable, but I'd like to leave it up for a day or so before merging to give others a chance to review, since I did write it myself.

Copy link

@elalish elalish left a comment

Choose a reason for hiding this comment

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

LGTM

@tetrapod00
Copy link
Contributor Author

Pushed a minor wording correction.

@tetrapod00 tetrapod00 merged commit 5eebe5f into godotengine:master Dec 23, 2024
1 check passed
@tetrapod00
Copy link
Contributor Author

Merged. Thanks fire and especially elalish for taking the time to review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation enhancement topic:3d
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants