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 camera slerp() BUG #6509

Merged
merged 3 commits into from Oct 30, 2023
Merged

fix camera slerp() BUG #6509

merged 3 commits into from Oct 30, 2023

Conversation

inaridarkfox4231
Copy link
Contributor

@inaridarkfox4231 inaridarkfox4231 commented Oct 29, 2023

fix column() to row().

Resolves #6508

Changes:

A bug occurred in camera's slerp() due to a change in the specifications of column() and row().
So I'll fix that.

CAMERA_SLERP_BUG

before

  // BUG IS HERE.

  var front0 = rotMat0.column(2);
  var front1 = rotMat1.column(2);
  var up0 = rotMat0.column(1);
  var up1 = rotMat1.column(1);

after

  // The specifications of column() and row() have been reversed, so column() must be rewritten as row().
  var front0 = rotMat0.row(2);
  var front1 = rotMat1.row(2);
  var up0 = rotMat0.row(1);
  var up1 = rotMat1.row(1); // prepare new vectors.

PR Checklist

fix column() to row()
@inaridarkfox4231
Copy link
Contributor Author

Since the unit test did not detect this bug, we will add one unit test to prevent this from happening in the future.

Add more unit tests to prevent bugs caused by changes to slerp().
@inaridarkfox4231
Copy link
Contributor Author

That's all.

Remove unnecessary line breaks
@inaridarkfox4231
Copy link
Contributor Author

I thought that for some reason there was a mismatch regarding line breaks, but it seems I was mistaken.

Copy link
Contributor

@davepagurek davepagurek left a comment

Choose a reason for hiding this comment

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

Thanks for the fix + the test case for preventing future regressions!

@davepagurek davepagurek merged commit e40c8e0 into processing:main Oct 30, 2023
2 checks passed
@inaridarkfox4231 inaridarkfox4231 deleted the fix_slerpBUG branch October 30, 2023 17:29
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.

p5.Camera.slerp() behaves strangely
2 participants