search

erich666 / GraphicsGems

Code for the "Graphics Gems" book series
Other
1.39k stars 265 forks source link

Criteria to decide whether to try to re-parameterize in FitCurves is bad (scale-dependent) #23

Closed jimmyland closed 6 years ago

jimmyland commented 7 years ago

e.g. if units are such that user's error threshold is less than 1, the function will never try to re-parameterize. Based on the usage example (where error is 4.0), I think the intent would be preserved if you just set iterationError = error * 4.0;

bad code is here: https://github.com/erich666/GraphicsGems/blob/4d1d0a0e4f27f72e2930d369fa7132a253468bcf/gems/FitCurves.c#L113 (and iterationError is used at line 146)

erich666 commented 7 years ago

Thanks. I've attempted to contact the author, Phil Schneider, for his opinion.

jimmyland commented 7 years ago

Alternatively I could do a PR that exposes the error scale factor as a parameter? (and that line would then become iterationError = error * iterationErrorScaleFactor;?)

erich666 commented 7 years ago

The good news is that Philip Schneider recently responded, so I'm hoping he'll look at the code soon and figure out what's the most sensible.

erich666 commented 6 years ago

Pinged Philip.

erich666 commented 6 years ago

Philip replied: "The suggested change is probably OK." So I've checked it in, and pointed the interested user at this issue for more details.