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

Mend gaps in meshes caused by trigonometric funcs. #100020

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

Lielay9
Copy link
Contributor

@Lielay9 Lielay9 commented Dec 4, 2024

Fixes: #93865
Supersedes: #94199

Simple, ugly solution: Ensure last iteration uses the same values as the first. Here's a cheat-sheet to help reviewing:

sin(τ) = 0
cos(τ) = 1
sin(π) = 0
cos(π) = -1
sin(π/2) = 1
cos(π/2) = 0
sin(0) = 0
cos(0) = 1

@Lielay9 Lielay9 requested review from a team as code owners December 4, 2024 18:55
Comment on lines 530 to 551
v += 1.0;
w = sin(0.5 * Math_PI * v);
y = radius * cos(0.5 * Math_PI * v);
if (j == (rings + 1)) {
w = 0.0;
y = -radius;
} else {
w = cos(0.5 * Math_PI * v);
y = -sin(0.5 * Math_PI * v) * radius;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Avoid unnecessary addition and subtraction.

sin((x+1)π/2) == cos(xπ/2)
cos((x+1)π/2) == -sin(xπ/2)

Comment on lines -2162 to +2211
ADD_TANGENT(-Math::cos(angi), 0.0, Math::sin(angi), 1.0);
ADD_TANGENT(normali.y, 0.0, -normali.x, 1.0);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cosine and sine already calculated above: reuse.

@fire fire requested a review from a team December 5, 2024 04:27
@AThousandShips AThousandShips added this to the 4.4 milestone Dec 5, 2024
@Lielay9 Lielay9 force-pushed the mend-round-primitives branch from 9ce9a18 to 9b83465 Compare December 5, 2024 16:34
@Lielay9 Lielay9 requested a review from a team as a code owner December 5, 2024 16:34
@Lielay9 Lielay9 changed the title Mend gaps in primitive-meshes Mend gaps in meshes caused be imprecise trigonometric funcs. Dec 5, 2024
@Lielay9
Copy link
Contributor Author

Lielay9 commented Dec 5, 2024

Hi, I also work on CSG, would it be ok to also test CSGMesh3D too?

Originally posted by @fire in #94199 (comment)

Tested CSG-meshes! Since only the sphere had gaps, I stuffed it to this pr~

@fire
Copy link
Member

fire commented Dec 5, 2024

I suspect the capsule also has the same gap.

@Lielay9
Copy link
Contributor Author

Lielay9 commented Dec 5, 2024

I suspect the capsule also has the same gap.

I don't think we have CSGCapsules...

@fire
Copy link
Member

fire commented Dec 5, 2024

Any of these meshes can be put into CSGMesh3D https://docs.godotengine.org/en/stable/classes/class_primitivemesh.html

From earlier testing RibbonTrailMesh, SphereMesh and PlaneMesh mathematically cannot work.

@Lielay9
Copy link
Contributor Author

Lielay9 commented Dec 5, 2024

Ah! I see! All the primitive meshes that had gaps are already included CapsuleMesh, CylinderMesh, SphereMesh, TorusMesh and TubeTrailMesh ^^/ This means there shouldn't be gaps in CSGMesh3D either! Or at least those that are caused by the trig functions... but I haven't found other causes and my naive test passes for all the shapes.

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.

I have tested all primitive mesh shapes except those that aren't mathematically usable with CSG.

The manifold definition requires the meshes to be watertight.

Note that the pull request is insufficient; will also need the Godot Engine master as of eb51030 or newer

image

csg-mend-gaps-game-project.zip

Edited:

Did not do a style review.

@fire

This comment was marked as outdated.

@Lielay9 Lielay9 force-pushed the mend-round-primitives branch from 9b83465 to b4c6f9b Compare December 6, 2024 15:12
@Lielay9 Lielay9 changed the title Mend gaps in meshes caused be imprecise trigonometric funcs. Mend gaps in meshes caused by trigonometric funcs. Dec 6, 2024
@fire fire mentioned this pull request Dec 8, 2024
16 tasks
@Repiteo Repiteo merged commit 101b78f into godotengine:master Dec 9, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Dec 9, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Bad
Development

Successfully merging this pull request may close these issues.

Round primitive meshes contain gaps.
5 participants