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

Expose barycentric coord and triangle index #103

Merged
merged 2 commits into from
Feb 18, 2024

Conversation

ottah
Copy link
Contributor

@ottah ottah commented Nov 11, 2023

This change exposes the intersection's barycentric coord and the triangle index so they can be used to look up vertex attributes and texcoords.

@aevyrie aevyrie merged commit e850210 into aevyrie:main Feb 18, 2024
4 of 5 checks passed
@brookman
Copy link

Maybe I misunderstand something, but it seems that triangle_index contains only the first vertex index of the triangle. How can I get the other two vertex indices?

@aevyrie
Copy link
Owner

aevyrie commented Mar 3, 2024

This should give you the triange index. In indexed geometry, you would look up the triangle with this index, and get the three vertices. In a non-indexed geometry the next three vertices should give you the triangle.

@brookman
Copy link

brookman commented Jul 8, 2024

Hi @aevyrie

Sorry for bothering you again with this issue, but after looking at the code again I'm pretty sure that it does not contain the triangle index but a vertex index.
Looking at this line:

let triangle_index = Some(index[0].into_usize());

Current implementation

for index in indices.chunks(3) {
    let triangle_index = Some(index[0].into_usize());

How it could be implemented instead:

for (i, index) in indices.chunks(3).enumerate() {
    let triangle_index = Some(i);

@aevyrie
Copy link
Owner

aevyrie commented Jul 8, 2024

Ah, I see the issue. Please open a PR or a new issue, or this is going to get lost.

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.

3 participants