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

BUG: findSpan may be stuck in an infinite loop in this case. #16

Open
HsiaoYeekwan opened this issue May 31, 2022 · 3 comments
Open

BUG: findSpan may be stuck in an infinite loop in this case. #16

HsiaoYeekwan opened this issue May 31, 2022 · 3 comments

Comments

@HsiaoYeekwan
Copy link

Hi ~ Tinynurbs is a nice library ~
But I encountered a problem when I was using the following code.
findSpan will be stuck in an infinite loop.

vector<float> knots = {0, 1, 1.3, 2.1, 3.6, 4.0};
int x = tinynurbs::findSpan(2, knots, 2.1f);
@henrij22
Copy link

henrij22 commented Feb 25, 2023

Hi,
I ran into the same bug. Apperently it gets stuck on the last knot.
I fixed it by uncommenting some lines in the upper part in the findSpan function.

// For u that is equal to last knot value
if (util::close(u, knots[n + 1])) {
    return n;
}

This at least cured my problems

@alektron
Copy link
Contributor

I stumbled upon the same issue and I'm relatively sure I know what's up.
Uncommenting the code that @henrij22 mentioned can indeed work. However the issue lies in the lines of code directly below.

// For values of u that lies outside the domain
if (u > (knots[n + 1] - std::numeric_limits<T>::epsilon()))
{
    return n;
}
if (u < (knots[degree] + std::numeric_limits<T>::epsilon()))
{
    return degree;
}

std::numeric_limits::epsilon() is defined to be FLT_EPSILON which is defined as
difference between 1.0 and the next representable value for float.

That means if knots[n + 1] is indeed 1.0f, this works. By the nature of floating point representation however, the delta between a value and closest possible next value increases for bigger values. In my case where knots[n +1] was 3.0f, this delta is 2.384185791015625​e-7 which is bigger than FLT_EPSILON (1.192092896e-07F).
Therefore 3.0f - FLT_EPSILON is equal to 3.0f so the check fails and we enter the loop below which gets stuck in an infinite loop.

The question now is why this specific approach was chosen instead of just using <= and >= instead of < and > respectively.
As far as I can tell, this would resolve the issue.

@mantunezr
Copy link

Hi,
I've submitted a PR to fix the issue with the findSpan method entering an infinite loop. The fix replaces the custom binary search with std::lower_bound, which improves robustness and prevents the loop. Please review the PR here.

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

No branches or pull requests

4 participants