Harden Lua matrix and ParticleEffectAction against non-orthonormal inputs#7490
Conversation
…puts Two related fixes flagged by an audit of vm_vector_2_matrix_norm callers: ParticleEffectAction: locals.direction defaulted to vmd_zero_vector, which would trip vm_vector_2_matrix_norm's unit-length assertion (and divide by zero in release) when an action program emitted a particle effect without first running a Set Direction action. Default to vmd_z_vector so the unset-direction path always feeds a unit vector. matrix_h / Lua orientation type: the element indexer (m[1..9] = x) wrote arbitrary floats straight into mtx.a1d[idx] and the resulting non-orthonormal matrix could flow into obj->orient via the Orientation virt-vars on object/modelinstance/subsystem, breaking downstream engine code that assumes the basis vectors are unit length. getInterpolated had the same problem by design (pure linear interp, no normalization). Adds a fourth MatrixState, NeedsOrthonormalize, which the indexer setter and getInterpolated use to flag their output. ValidateMatrix and ValidateAngles now call vm_orthogonalize_matrix when they observe that state, so any consumer that reads through GetMatrix/GetAngles gets a valid orientation without each call site needing its own guard. getInterpolated is also marked ADE_FUNC_DEPRECATED at FSO 26.0 in favor of rotationalInterpolate; mods targeting older versions keep the legacy linear blend (now safely orthonormalized on use).
| } | ||
|
|
||
| ADE_FUNC(getInterpolated, | ||
| ADE_FUNC_DEPRECATED(getInterpolated, |
There was a problem hiding this comment.
Do we really need to deprecate the function fully now that we have the orthonormalization step?
It's allow to keep more scripts around without needing to modify them...
There was a problem hiding this comment.
The main dilemma here is that we don't know what scripters intended. One way or another, they're using the wrong tool for the job:
- if they want to interpolate rotations, they need to use
rotationalInterpolateinstead of this - if they want to do general purpose matrix interpolation, they would be doing it on an orientation matrix, which will break things
By definition and assumption, all matrixes in the Lua API are orientation matrixes. General-purpose matrixes aren't supported. So, deprecating this function makes that clear.
Now, one area for future development is to add support for general-purpose matrixes. This could be done by reusing matrix_h, adding a general-purpose vs. orientation flag, and making a new Lua type. Then we could un-deprecate getInterpolated and modify the rest of the API to use the appropriate type. But that would require a bit of effort, and would be beyond the scope of this PR.
Two related fixes flagged by an audit of vm_vector_2_matrix_norm callers:
ParticleEffectAction: locals.direction defaulted to vmd_zero_vector, which would trip vm_vector_2_matrix_norm's unit-length assertion (and divide by zero in release) when an action program emitted a particle effect without first running a Set Direction action. Default to vmd_z_vector so the unset-direction path always feeds a unit vector.
matrix_h / Lua orientation type: the element indexer (m[1..9] = x) wrote arbitrary floats straight into mtx.a1d[idx] and the resulting non-orthonormal matrix could flow into obj->orient via the Orientation virt-vars on object/modelinstance/subsystem, breaking downstream engine code that assumes the basis vectors are unit length. getInterpolated had the same problem by design (pure linear interp, no normalization).
Adds a fourth MatrixState, NeedsOrthonormalize, which the indexer setter and getInterpolated use to flag their output. ValidateMatrix and ValidateAngles now call vm_orthogonalize_matrix when they observe that state, so any consumer that reads through GetMatrix/GetAngles gets a valid orientation without each call site needing its own guard.
getInterpolated is also marked ADE_FUNC_DEPRECATED at FSO 26.0 in favor of rotationalInterpolate; mods targeting older versions keep the legacy linear blend (now safely orthonormalized on use).