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

SimplifyModifier: Add check if face are not undefined. #24169

Conversation

Suprhimp
Copy link
Contributor

@Suprhimp Suprhimp commented Jun 1, 2022

add check if face are not undefined

check with u.faces[i]

Description

add condition for undefined for loop

Now I’m making code for web 3d world viewer. I want to make lod system for my world and for that, I have to use simplify modifier for this.

Anyway, when I use simplify modifier to simplify gltf model it shows error with this

SimplifyModifier.js:293 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'hasVertex')
    at collapse (SimplifyModifier.js:293:1)
    at SimplifyModifier.modify (SimplifyModifier.js:99:1)

I spend lot of time to search about it, but I can’t find the hint

So I console log it to find out what happen to simplify modifier and I think this might be reason.

I console log this loop

  for (let i = u.faces.length - 1; i >= 0; i--) {
    console.log(u, i);
    if (u.faces[i].hasVertex(v)) {
      removeFace(u.faces[i], faces);
    }
  }

I found that when removeFace function run, it sometimes remove two faces at one loop. As you can see in picture above, first length of u.faces.length = 8, and value i = 7, at second loop u.faces.length = 6, and value i = 6 this cause error.

Is it okay with just add if (!u.faces[i]) continue; in for loop?
it seems running but I think it work not propery.

This contribution is funded by forum

add check if face are not undefinded

check with u.faces[i]
@Mugen87 Mugen87 changed the title add check if face are not undefined SimplifyModifier: Add check if face are not undefined. Jun 1, 2022
@mrdoob mrdoob added this to the r142 milestone Jun 2, 2022
@mrdoob mrdoob merged commit 8a77473 into mrdoob:dev Jun 2, 2022
@mrdoob
Copy link
Owner

mrdoob commented Jun 2, 2022

Thanks!

abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
add check if face are not undefinded

check with u.faces[i]
snagy pushed a commit to snagy/three.js-1 that referenced this pull request Sep 21, 2022
add check if face are not undefinded

check with u.faces[i]
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