Curve: Check for finiteness before performing calculations in sample_baked() functions

[jweisberg]

Prevented crash in Curve2D::_find_interval() and Curve3D::_find_interval() by checking for finiteness before performing any mathematical operations. Resolves #98372. This is my first time contributing, please let me know if anything needs to be changed.

[AThousandShips]

This looks good to me

One thing is that it might be better to put this change in the methods that call this

Partially because it makes the error message more helpful (hard to figure out what you're doing wrong if it says _find_interval if you called sample_baked)

It might also be good to check this earlier, before baking the curve if it isn't baked

But this should be good as it is, but might be good to improve

[jweisberg]

Thanks for the feedback! Yeah I initially thought putting the check in the _find_interval functions made more sense because multiple functions call them, so it's less repetitive to put the checks there, but I can see how that would make it harder for the user to figure out where things started going wrong. I'll make those changes.

[AThousandShips]

Looks good to me, needs to fix some formatting errors to run all the checks to make sure the tests don't need to be adjusted

But would be good to get a second pair of eyes on this just to confirm

[Calinou]

Looks good to me.

I suggest adding unit tests to cover this use case in tests/test_curve.h, tests/test_curve_2d.h and tests/test_curve3d.h, so we can make sure this doesn't regress in the future. See Unit testing in the documentation. You'll need to use ERR_PRINT_OFF; and ERR_PRINT_ON; to silence errors before and after the calls that would return NaN otherwise.

[KoBeWi]

IMO the comment is unnecessary, the code is self-explanatory.
At least there is no reason to repeat it in every method.

[KoBeWi]

Looks fine.
I noticed all existing error checks happen after bake(). Doesn't seem important though.

[Repiteo]

Please squash your commits into one, see the interactive rebase for details

[akien-mga]

Please squash your commits into one, see the interactive rebase for details

Did so myself to ease the process, so this should be ready to merge.

[Repiteo]

Thanks! Congratulations on your first contribution! 🎉

[jweisberg]

Please squash your commits into one, see the interactive rebase for details

Did so myself to ease the process, so this should be ready to merge.

Thanks for the assist!