Skip to content

fix(gum): store into the current slot before advancing in sceGumPushMatrix - #206

Open
ItsNoHax wants to merge 1 commit into
overdrivenpotato:masterfrom
ItsNoHax:fix-gum-push-matrix-stack-slot
Open

fix(gum): store into the current slot before advancing in sceGumPushMatrix#206
ItsNoHax wants to merge 1 commit into
overdrivenpotato:masterfrom
ItsNoHax:fix-gum-push-matrix-stack-slot

Conversation

@ItsNoHax

@ItsNoHax ItsNoHax commented Aug 13, 2026

Copy link
Copy Markdown

Fixes #205.

The bug

sceGumPushMatrix advances CURRENT_MATRIX and then stores the VFPU matrix, so it saves into the
slot above the stack pointer. sceGumPopMatrix decrements and then loads, so it reads the slot
below the pointer it was left at. Push writes P+1, pop reads P; the saved copy is never read
back.

The fix

Store into the slot CURRENT_MATRIX already points at, then advance — the ordering PSPSDK's
pspgum.c uses. One statement moved.

This is also the only ordering that survives a matrix sync. CURRENT_MATRIX names the slot that
mirrors the live matrix: both sceGumMatrixMode and sceGumUpdateMatrix write the VFPU matrix to
*CURRENT_MATRIX. Saving into the slot above the pointer and having pop read that slot back is
self-consistent in isolation, but sceGumDrawArray calls sceGumUpdateMatrix, so the first draw
after a push would overwrite exactly what the push saved. Storing below the pointer keeps the saved
copy out of the way of the sync.

Why this went unnoticed

The sync is also what hides it. Any draw performed before the first push writes the correct matrix
into the slot pop later reads, after which push/pop pairs appear to work. So the failure depends on
draw order rather than on the matrix code, and only shows when something pushes before it has drawn
anything — at which point pop loads a slot nothing ever wrote. MATRIX_STACK is zero-initialised,
so the restored matrix is all zeros and everything drawn afterwards collapses to a point.

The report in #205 has the real-world version: a car drawn as a body plus four wheels, where six
assets ordered their body meshes first and worked, and the seventh ordered its wheels first and lost
its entire body.

Tests

Four cases in ci/tests, all pure matrix arithmetic read back with sceGumStoreMatrix — no
rendering, so they need no GU:

  • gum_push_pop_restores_translation — the core property
  • gum_push_pop_nested_restores_translation — two deep, catching an off-by-one either way
  • gum_push_pop_survives_matrix_write — a write to *CURRENT_MATRIX between push and pop, which is
    what a draw does; this is the case that rules out the alternative fix
  • gum_push_leaves_current_matrix_alone — a push saves the matrix without disturbing it

sceGumMatrixMode provokes the write in the third rather than sceGumUpdateMatrix itself, because
sceGumUpdateMatrix ends by handing every stack to sceGuSetMatrix and this suite never brings the
GU up. It stores through the same four instructions.

Run with PPSSPPHeadless ... --timeout=40 -r . as ci/concourse/run-tests.sh does.

Before (master, a8be764):

[FAIL]: (gum_push_pop_restores_translation) (0.0, 0.0, 0.0) != (1.0, 2.0, 3.0)
[FAIL]: (gum_push_pop_nested_restores_translation) (0.0, 0.0, 0.0) != (1.0, 2.0, 3.0)
[FAIL]: (gum_push_pop_survives_matrix_write) (0.0, 0.0, 0.0) != (1.0, 2.0, 3.0)
[PASS]: (gum_push_leaves_current_matrix_alone) (1.0, 2.0, 3.0) == (1.0, 2.0, 3.0)
FINAL_FAILURE

After:

[PASS]: (gum_push_pop_restores_translation) (1.0, 2.0, 3.0) == (1.0, 2.0, 3.0)
[PASS]: (gum_push_pop_nested_restores_translation) (1.0, 2.0, 3.0) == (1.0, 2.0, 3.0)
[PASS]: (gum_push_pop_survives_matrix_write) (1.0, 2.0, 3.0) == (1.0, 2.0, 3.0)
[PASS]: (gum_push_leaves_current_matrix_alone) (1.0, 2.0, 3.0) == (1.0, 2.0, 3.0)
FINAL_SUCCESS

Verification

Verified end to end: a game that had been working around this by rebuilding its model matrix per
mesh was reverted to plain sceGumPushMatrix/sceGumPopMatrix and built twice against identical
game source and assets — once against master, once against this branch. On master the affected car
renders as a few pixels; on this branch it renders correctly.

The rest of the suite is unaffected, and cargo fmt is clean for both psp and ci/tests.

One incidental note found while testing this: opening with sceGumMatrixMode on a cold VFPU context
is a break instruction rather than a panic, since only sceGumLoadIdentity and sceGumLoadMatrix
create the context and everything else calls get_context_unchecked. That is #189 and is not
touched here.

…atrix

`sceGumPushMatrix` advanced `CURRENT_MATRIX` and then stored the VFPU matrix,
so it saved into the slot above the stack pointer. `sceGumPopMatrix` decrements
and then loads, so it reads the slot below the pointer it was left at. The two
never refer to the same slot, and what pop returns is not what push saved.

`CURRENT_MATRIX` names the slot that mirrors the live matrix -- `sceGumMatrixMode`
and `sceGumUpdateMatrix` both write the VFPU matrix to `*CURRENT_MATRIX` -- so
storing before the increment is also the only ordering that works. Saving into
the slot above and having pop read that slot back is self-consistent in
isolation, but `sceGumUpdateMatrix` writes the live matrix to exactly that slot,
and `sceGumDrawArray` calls it, so the first draw after a push would overwrite
what the push saved. This matches PSPSDK's pspgum.c, which stores and then
increments.

The bug hid behind that same sync. `MATRIX_STACK` is zero initialised, so a pop
with nothing beneath it loads an all-zero matrix and collapses everything drawn
afterwards to a point; but any draw performed before the first push syncs the
correct matrix into the slot pop reads, after which push/pop pairs appear to
work. Code that drew before pushing was fine. Code whose first drawing act was
a push -- for example a model whose wheel meshes are ordered before its body
meshes -- lost every mesh after the first pop.

Adds four regression tests to ci/tests covering push/pop, nesting, a matrix
write between push and pop, and that a push leaves the working matrix alone.
Against master the first three fail with (0.0, 0.0, 0.0) against the expected
translation and the suite ends FINAL_FAILURE; with this change all four pass and
the suite ends FINAL_SUCCESS.
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.

sceGumPushMatrix and sceGumPopMatrix use different stack slots, so a pop does not restore what was pushed

1 participant