Skip to content

Harden Lua matrix and ParticleEffectAction against non-orthonormal inputs#7490

Merged
BMagnu merged 1 commit into
scp-fs2open:masterfrom
Goober5000:fix/matrix_normalization
Jun 3, 2026
Merged

Harden Lua matrix and ParticleEffectAction against non-orthonormal inputs#7490
BMagnu merged 1 commit into
scp-fs2open:masterfrom
Goober5000:fix/matrix_normalization

Conversation

@Goober5000

Copy link
Copy Markdown
Contributor

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).

…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).
@Goober5000 Goober5000 added the fix A fix for bugs, not-a-bugs, and/or regressions. label Jun 1, 2026
@Goober5000 Goober5000 added this to the Release 26.0 milestone Jun 1, 2026
}

ADE_FUNC(getInterpolated,
ADE_FUNC_DEPRECATED(getInterpolated,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@Goober5000 Goober5000 Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 rotationalInterpolate instead 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.

@BMagnu BMagnu merged commit 7145b3c into scp-fs2open:master Jun 3, 2026
20 checks passed
@Goober5000 Goober5000 deleted the fix/matrix_normalization branch June 3, 2026 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix A fix for bugs, not-a-bugs, and/or regressions.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants