Fix writable mat4/mat3 setters for p5.Matrix to restore Camera.slerp() in ortho mode
This PR fixes issue #7837.
In p5.js v2, Matrix.mat4 is exposed via a getter that returns an internal Float32Array.
Camera.slerp() (and potentially other code) assigns to uPMatrix.mat4 which throws:
TypeError: setting getter-only property "mat4"
This change adds a set mat4(values) on Matrix that:
- only applies to 4×4 matrices,
- expects an array-like of length 16, and
- copies values into the existing
Float32Arrayinstead of reassigning it. - Adds a symmetric
set mat3(values)for 3×3 matrices.
This fixes the intended writable behavior (used by Camera.slerp()), while preserving the internal buffer identity required by WebGL bindings and other code that holds references to the matrix storage.
🎉 Thanks for opening this pull request! For guidance on contributing, check out our contributor guidelines and other resources for contributors! 🤔 Please ensure that your PR links to an issue, which has been approved for work by a maintainer; otherwise, there might already be someone working on it, or still ongoing discussion about implementation. You are welcome to join the discussion in an Issue if you're not sure! 🌸 Once your PR is merged, be sure to add yourself to the list of contributors on the readme page !
Thank You!
Rebasing this PR onto the latest dev-2.0. I’ve re-tested the fix locally using the built lib/p5.js, and the ortho Camera.slerp() issue is resolved.
Hi @nakednous, sorry for the delay on this! I think this approach works, but it might be for the best to not allow these to be settable for safety? @holomorfo @limzykenneth do you have thoughts?
If we don't want to let those properties be settable, rather than cloning + updating the existing matrix here: https://github.com/processing/p5.js/blob/a6e0b4617c919b89f6463444c9af73c3ff114188/src/webgl/p5.Camera.js#L1593-L1594
...maybe we just set the state directly to a clone of the camera's matrix?
I double-checked this locally and can confirm both variants remove the ortho slerp() crash:
if (this._isActive()) {
this._renderer.states.setValue('uPMatrix', this._renderer.states.uPMatrix.clone());
}
and
if (this._isActive()) {
this._renderer.states.setValue('uPMatrix', this._renderer.states.uPMatrix.clone());
const dst = this._renderer.states.uPMatrix.mat4;
const src = this.projMatrix.mat4;
for (let i = 0; i < dst.length; i++) {
dst[i] = src[i];
}
}
In the minimal test sketch I used, the ortho projection parameters of cam0 and cam1 are identical, so simply cloning uPMatrix is sufficient to avoid the crash. However, the second version preserves the intended behavior of propagating interpolated ortho projection changes (when they differ) without assigning to mat4, keeping it getter-only.
Happy to go with whichever direction you prefer.
Thanks for taking a look! Have you tried if something like this works?
this._renderer.states.setValue('uPMatrix', this.projMatrix.clone());
This would be directly creating a clone of the intended matrix rather than cloning the renderer's matrix and updating it, since the result in both cases is a new matrix with the intended values.
Thanks! I implemented the suggested fix and opened a new PR. It works on my repro example; no crash anymore.