Skip to content

Conversation

mrxz
Copy link
Contributor

@mrxz mrxz commented Nov 22, 2024

Description

Some of the equals functions in the unit tests (e.g. matrixEquals4) incorrectly compared signed delta values to the tolerance threshold, instead of absolute values. As a result differences where b > a would never "exceed" the tolerance and always pass.

Additionally the bufferAttributeEquals failed to actually compare the contents of the attributes. After resolving this issue, it became clear that the computeVertexNormals (indexed) test was broken. I tried to find out what the original intention of the test was but it seems it was introduced in a broken state in #12354. As such, I've taken the liberty to tweak it so that:

  • There are only 6 vertices instead of 7 (ensuring non-indexed computeVertexNormals can actually compute all values)
  • The index buffer uses the same two triangles as non-indexed, but in a counter-clockwise order, so that normals get flipped.

This contribution is funded by Fern Solutions

@Mugen87 Mugen87 added this to the r171 milestone Nov 22, 2024
@Mugen87 Mugen87 merged commit fab2d40 into mrdoob:dev Nov 23, 2024
11 checks passed
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