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

Wrong calculation with faceVertexCount? #135

Open
SamuelReithmeir opened this issue Aug 26, 2024 · 9 comments
Open

Wrong calculation with faceVertexCount? #135

SamuelReithmeir opened this issue Aug 26, 2024 · 9 comments

Comments

@SamuelReithmeir
Copy link

I just stumbled over a problem with FaceVertexCount, i get an invalid memory access.
A sample would be:
face0: 3 vertices
face1: 4 vertices
face2: 5 vertices
this results in a total IndexCount of 12

when adding that mesh declaration i get a invalid memory access (i use XAtlas in c# using swig).
After looking at the code i found this code:

polygon[i] = DecodeIndex(meshDecl.indexFormat, meshDecl.indexData, meshDecl.indexOffset, face * faceVertexCount + i);

According to my understanding face * faceVertexCount + i seems to be a logic error. To explain:
This seems to calculate the offset/beginning of the indices of this face and add i. But the problem is that it assumes that all faces before also have the same faceVertexCount, which destroys the entire purpose of having faceVertexCount be an array in the first place.

Please correct me if i misunderstand the logik here, but i will provide a PR, which will change the logic here to a variable which will sum up the previous faceVertexCounts.

@mifth
Copy link

mifth commented Oct 27, 2024

I've created a pull request which should fix the issue. #138

@SamuelReithmeir
Copy link
Author

Better fixed by #138

@SamuelReithmeir SamuelReithmeir closed this as not planned Won't fix, can't repro, duplicate, stale Oct 28, 2024
@mifth
Copy link

mifth commented Oct 29, 2024

Thanks a lot. I'll continue this thread. I've found another one issue. NGon is gnored if it's too polygonal. If NGon has at least one zero area triangle it will be ignored.

image

A problem is here. I'm checking it.
image

@mifth
Copy link

mifth commented Oct 29, 2024

This has worked out. We need to calculate an area of all triangles. And then make a decision if it should be ignored.
image

Now it works well even with a 10000 NGon.
image
image

I'll update my pull request.

mifth added a commit to mifth/xatlas_fixes that referenced this issue Oct 29, 2024
@mifth
Copy link

mifth commented Oct 30, 2024

Another one problem is found: XAtlas triangulates Quads/NGons and considers them as uniquee triangles. For example, 2 triangles of a quad can be in different charts.

image
image
image

This doesn't happen if an original mesh is triangulated:
image
image
image

@mifth
Copy link

mifth commented Oct 30, 2024

I think it can be fixed in the computeCharts() function. Looking at it.

@mifth
Copy link

mifth commented Nov 1, 2024

It was pretty hard to understand what's going on there but I almos fixed quads and ngons determination by charts. Hopefully, to finish today/tomorrow.

image

image

@mifth
Copy link

mifth commented Nov 4, 2024

Finally, I made've final changes/fixes for Quads/NGons.

My commit:
22f4d64
#138 (comment)

Pictures:
Some pictures of a final result after a generation:
image
image
image
image
image

@mifth
Copy link

mifth commented Nov 4, 2024

Some pictures to explain a new computeBoundaryIntersection variable which I've added with the latest commit.

image
image
image
image

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 a pull request may close this issue.

3 participants
@mifth @SamuelReithmeir and others