Separate Model and View Matrices.

[deveshidwivedi]

Resolves #5287

Changes:

Introduced a distinct modelMatrix. By separating these matrices (modelViewMatrix), it would become easier to manipulate each aspect of the transformation independently, allowing for more flexibility especially when working with shaders.

PR Checklist

• [ ] npm run lint passes
• [x] Inline documentation is included / updated
• [ ] Unit tests are included / updated

[davepagurek]

Looks like tests are failing because:

It seems like it's because of this line still setting uMVMatrix rather than the model and view matrices: https://github.com/processing/p5.js/blob/2ceca4ec656f4d137390c3554b3bd085d698ed60/src/webgl/p5.Camera.js#L1393-L1395

Might be worth searching in the repo for this.uMVMatrix or _renderer.uMVMatrix to make sure we aren't leaving any other lingering references to it

[deveshidwivedi]

@davepagurek I made a few changes, please let me know if they make sense.. I rechecked files to see where I missed initialization but couldn't really make any improvements.. I had a doubt, do we need to introduce or update any camera properties specifically to get things working?

[davepagurek]

Looks like tests are still failing because it still tries to set uMVMatrix here too: https://github.com/processing/p5.js/blob/f5baf367c2fb413ed2f1923912b185971c52e717/src/webgl/p5.Camera.js#L1394

Maybe try searching this file to make sure it doesn't reference uMVMatrix anywhere any more?

I had a doubt, do we need to introduce or update any camera properties specifically to get things working?

I think the camera matrices are OK as is! I think your code is making the right changes already, so it's just a matter of finding any bits that we missed where we need to apply the same pattern.

[deveshidwivedi]

There are some different errors now @davepagurek, how could these be resolved?

[deveshidwivedi]

Made these changes, please have a look..

[deveshidwivedi]

Hi @davepagurek! I tried updating a few other tests, please take a look

[deveshidwivedi]

Should we update the matrix here also?

[davepagurek]

Should we update the matrix here also?

Ah yes good catch, I think so!

[deveshidwivedi]

Hi @davepagurek! Sorry for the delay. I have made these changes and the tests do no fail anymore. Are any more updates required?

[davepagurek]

Sorry for the delay! I think it's looking good now. We're just finishing up the p5.js website redesign (the source of the delays in my reviews 😅) and will probably be doing another quick release just to push out more documentation updates. After that, we'll be back in a more normal release schedule, including a beta release to do further testing in case we've missed anything from the tests. So I'll merge this in after that little release!

[deveshidwivedi]

Sure @davepagurek! Looking forward!🎉

[Qianqianye]

Thank you all. @davepagurek, are we good to merge this?

[davepagurek]

good to go if we've done our last docs deploy!

[Qianqianye]

Just to clarify, do you mean the updating all the reference doc? @davepagurek

[davepagurek]

right, mostly we were just waiting to put this into a release where we would be able to have a standard beta testing period to catch any bugs. if we're going to be back into a regular release cadence again then this is good to merge!

[davepagurek]

@all-contributors please add @deveshidwivedi for code

[allcontributors[bot]]

@davepagurek

@deveshidwivedi already contributed before to code