Skip to content

Add halfedge_tangents to MeshBoolImpl and MeshGL#6

Draft
JaminKoke wants to merge 2 commits intoBorgerLand:masterfrom
JaminKoke:halfedge_tangent
Draft

Add halfedge_tangents to MeshBoolImpl and MeshGL#6
JaminKoke wants to merge 2 commits intoBorgerLand:masterfrom
JaminKoke:halfedge_tangent

Conversation

@JaminKoke
Copy link
Copy Markdown
Contributor

Adding this for an upcoming PR that (I think) requires this.

Also add `MeshGL::halfedge_tangent` and `TransformTangents`
@luisfonsivevo
Copy link
Copy Markdown
Member

I'm not very familiar with this feature, but when I search halfedgeTangent across the C++ codebase I see 149 results, some of which are in files that haven't been ported yet (namely smoothing.cpp). Do you know if they're necessary for correct calculations? My fear is that this PR might be missing some important logic judging by the number of changed lines. I also think it could be beneficial for performance to feature-gate this behind #[cfg(feature = "halfedge_tangent")], in case someone may not need it.

@JaminKoke
Copy link
Copy Markdown
Contributor Author

I'm not very familiar with this feature, but when I search halfedgeTangent across the C++ codebase I see 149 results, some of which are in files that haven't been ported yet (namely smoothing.cpp). Do you know if they're necessary for correct calculations?

I'm not familiar with it either, and from what I can tell it does seem to be mostly related to smoothing and subdivision (I did not realize this and assumed it was necessary for other things).

My fear is that this PR might be missing some important logic judging by the number of changed lines.

Maybe, I did look over it again and saw that there were a handful of areas I missed porting.

I also think it could be beneficial for performance to feature-gate this behind #[cfg(feature = "halfedge_tangent")], in case someone may not need it.

Good idea, and thats something that I would make use of too since my game I'm working on doesn't require smoothing or subdivision!

Anyways, I guess I'll make this as a draft again until I've sorted all that out (I'm going to focus on other areas for now). Also, would you prefer that I port smoothing.cpp and subdivision.cpp in this PR too just so that way it's all here?

@JaminKoke JaminKoke marked this pull request as draft November 28, 2025 23:27
@luisfonsivevo
Copy link
Copy Markdown
Member

I haven't looked too closely at the C++ implementation so I can't say for sure, but if smoothing and subdivision are required for correct usage of this feature, then yes this PR is a good place for them.

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.

2 participants