Skip to content

Conversation

@jkenney2
Copy link

@jkenney2 jkenney2 commented Oct 29, 2025

Reasons for pull request: currently, CustomCurve does not accommodate curves that require and data other than control points to set up. In addition, it fails (at least sometimes) to find the correct closestPoint when attempting to follow the same path a second time.

Changes to the CustomCurve class, as follows:

  1. Add constructors that accept arbitrary data (as an object) that may be required to generate the custom curve (in addition to the control points).
  2. Add a generateCurve method (do-nothing by default) that can be overridden to generate the curve. Call this method from the initialize() method, before the call to approximateLength(), since approximateLength() requires that the curve be set up.
  3. Add logic and override the default getClosestPoint method so that, each time the initialize() method is called, the initialTValueGuess will get recalculated on the next call to getClosestPoint. This was needed because for some curves, leaving the initialTValueGuess near 1.0 and re-following the curve from the beginning would result in getClosestPoint return a local minimum near the end of the curve rather than the beginning (leading to bizarre behavior).

This modification is backwards compatible. Example of usage of the new functionality: https://gist.github.com/jkenney2/c96153fb42c4d07fad880e309f0112df

@BeepBot99
Copy link
Member

Could you please edit your initial message adding the reason for this and what advantages it provides?

@junkjunk123
Copy link
Member

(2) Is actually currently a bug. This was the intended behavior for the initialization() method--- it should be called before approximateLength(). Thank you for catching this. I don't think adding a generateCurve() method is necessary after patching this bug, since the user can simply override the existing initialization() method.

@jkenney2 jkenney2 changed the title Modified CustomCurve Modified CustomCurve to allow curves that require data in addition to control points Oct 29, 2025
@jkenney2 jkenney2 changed the title Modified CustomCurve to allow curves that require data in addition to control points Modified CustomCurve to allow curves that require data in addition to control points and to fix problem with getClosestPoint. Oct 29, 2025
@jkenney2
Copy link
Author

(2) Is actually currently a bug. This was the intended behavior for the initialization() method--- it should be called before approximateLength(). Thank you for catching this. I don't think adding a generateCurve() method is necessary after patching this bug, since the user can simply override the existing initialization() method.

Added a commit that removes the generateCurve() method and moves the call to initialization() to above the call to approximateLength().

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