Skip to content

Comments

fix camera slerp() BUG#6509

Merged
davepagurek merged 3 commits intoprocessing:mainfrom
inaridarkfox4231:fix_slerpBUG
Oct 30, 2023
Merged

fix camera slerp() BUG#6509
davepagurek merged 3 commits intoprocessing:mainfrom
inaridarkfox4231:fix_slerpBUG

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
@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