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

fix computeFaceNormal: use corner whose angle is closest to right angle for cross product #131

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions source/xatlas/xatlas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ Copyright (c) 2012 Brandon Pelfrey
#include <assert.h>
#include <float.h> // FLT_MAX
#include <limits.h>
#define _USE_MATH_DEFINES
#include <math.h>
#define __STDC_LIMIT_MACROS
#include <stdint.h>
Expand Down Expand Up @@ -814,6 +815,12 @@ static float length(const Vector3 &v)
return sqrtf(lengthSquared(v));
}

static float angle(const Vector3 &a, const Vector3 &b)
{
const Vector3 c = cross(a, b);
return abs(atan2(length(c), dot(a, b)));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want you can try using a polynomial fit for this. it musnt be precise. look how cute it is when I try the montreal university fit (Sreeraman Rajan et al.)
image

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so sure, atan2 should give results in the range [-pi, pi], while atan gives results in the range [-pi/2, pi/2].
Apart from that, that smells like premature optimization imo ("no visible slowdown").

Copy link

@siliconvoodoo siliconvoodoo Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it is premature optimization. don't touch it, the test showed it's not necessary

}

static bool isNormalized(const Vector3 &v, float epsilon = kNormalEpsilon)
{
return equal(length(v), 1.0f, epsilon);
Expand Down Expand Up @@ -2707,9 +2714,27 @@ class Mesh
const Vector3 &p0 = m_positions[m_indices[face * 3 + 0]];
const Vector3 &p1 = m_positions[m_indices[face * 3 + 1]];
const Vector3 &p2 = m_positions[m_indices[face * 3 + 2]];
const Vector3 e0 = p2 - p0;
const Vector3 e1 = p1 - p0;
const Vector3 normalAreaScaled = cross(e0, e1);
const Vector3 e00 = p2 - p0;
const Vector3 e01 = p1 - p0;
const Vector3 e10 = p0 - p1;
const Vector3 e11 = p2 - p1;
const Vector3 e20 = p1 - p2;
const Vector3 e21 = p0 - p2;

// use the corner whose angle is the closest to a right angle,
// as that will give the most stable results for the cross product calculation
const float diff0 = abs(M_PI_2 - angle(e00, e01));
const float diff1 = abs(M_PI_2 - angle(e10, e11));
const float diff2 = abs(M_PI_2 - angle(e20, e21));
Vector3 normalAreaScaled;
if (diff0 <= diff1 && diff0 <= diff2) {
normalAreaScaled = cross(e00, e01);
} else if (diff1 <= diff0 && diff1 <= diff2) {
normalAreaScaled = cross(e10, e11);
} else {
normalAreaScaled = cross(e20, e21);
}

return normalizeSafe(normalAreaScaled, Vector3(0, 0, 1));
}

Expand Down