fix matrix stack error conditions and handling (fixes #39) (savestates are broken)

This commit is contained in:
zeromus 2017-02-20 23:22:54 -06:00
parent fb2cfc4e9c
commit 1cc22ae2aa
3 changed files with 108 additions and 104 deletions

View File

@ -1,6 +1,6 @@
/* /*
Copyright (C) 2006 yopyop Copyright (C) 2006 yopyop
Copyright (C) 2008-2016 DeSmuME team Copyright (C) 2008-2017 DeSmuME team
This file is free software: you can redistribute it and/or modify This file is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by it under the terms of the GNU General Public License as published by
@ -28,6 +28,13 @@
//if they do, then we need to copy them out in doFlush!!! //if they do, then we need to copy them out in doFlush!!!
//--------------- //---------------
//old tests of matrix stack:
//without the mymode==(texture) namco classics galaxian will try to use pos=1 and overrun the stack, corrupting emu
//according to gbatek, 31 works but sets the stack overflow flag. spider-man 2 tests this on the spiderman model (and elsewhere)
//see "Tony Hawk's American Sk8land" work from revision 07806eb8364d1eb2d21dd3fe5234c9abe0a8a894
//from MatrixStackSetStackPosition: "once upon a time, we tried clamping to the size. this utterly broke sims 2 apartment pets. changing to wrap around made it work perfectly"
//Water Horse Legend Of The Deep does seem to exercise the texture matrix stack, which does probably need to exist *but I'm not 100% sure)
#include "gfx3d.h" #include "gfx3d.h"
#include <assert.h> #include <assert.h>
@ -282,10 +289,11 @@ static FragmentColor *_gfx3d_colorRGBA6665 = NULL;
static u16 *_gfx3d_colorRGBA5551 = NULL; static u16 *_gfx3d_colorRGBA5551 = NULL;
// Matrix stack handling // Matrix stack handling
//TODO: decouple stack pointers from matrix stack type
CACHE_ALIGN MatrixStack mtxStack[4] = { CACHE_ALIGN MatrixStack mtxStack[4] = {
MatrixStack(1, 0), // Projection stack MatrixStack(1, 0), // Projection stack
MatrixStack(31, 1), // Coordinate stack MatrixStack(32, 1), // Coordinate stack
MatrixStack(31, 2), // Directional stack MatrixStack(32, 2), // Directional stack
MatrixStack(1, 3), // Texture stack MatrixStack(1, 3), // Texture stack
}; };
@ -577,7 +585,6 @@ void gfx3d_reset()
MatrixStackInit(&mtxStack[0]); MatrixStackInit(&mtxStack[0]);
MatrixStackInit(&mtxStack[1]); MatrixStackInit(&mtxStack[1]);
MatrixStackInit(&mtxStack[2]); MatrixStackInit(&mtxStack[2]);
MatrixStackInit(&mtxStack[3]);
clCmd = 0; clCmd = 0;
clInd = 0; clInd = 0;
@ -936,95 +943,128 @@ static void gfx3d_glMatrixMode(u32 v)
static void gfx3d_glPushMatrix() static void gfx3d_glPushMatrix()
{ {
//this command always works on both pos and vector when either pos or pos-vector are the current mtx mode //in case we're in Position-Vector mode, actually work on the Position matrix first (we'll update the vector matrix secondarily)
const MatrixMode mymode = ((mode == MATRIXMODE_POSITION) ? MATRIXMODE_POSITION_VECTOR : mode); const MatrixMode mymode = ((mode == MATRIXMODE_POSITION_VECTOR) ? MATRIXMODE_POSITION : mode);
MatrixStackPushMatrix(&mtxStack[mymode], mtxCurrent[mymode]); //1. apply offset specified by push (well, it's always +1) to internal counter
//2. mask that bit off to actually index the matrix for reading
//3. SE is set depending on resulting internal counter
if(mymode == MATRIXMODE_PROJECTION || mymode == MATRIXMODE_TEXTURE)
{
u32& index = mtxStack[mymode].position;
MatrixCopy(MatrixStackGetPos(&mtxStack[mymode], index&1), mtxCurrent[mymode]);
index += 1;
index &= 3;
if(index >= 2) MMU_new.gxstat.se = 1; //unknown if this applies to the texture matrix
}
else
{
u32& index = mtxStack[MATRIXMODE_POSITION].position;
MatrixCopy(MatrixStackGetPos(&mtxStack[MATRIXMODE_POSITION], index&31), mtxCurrent[MATRIXMODE_POSITION]);
if(mode == MATRIXMODE_POSITION_VECTOR)
MatrixCopy(MatrixStackGetPos(&mtxStack[MATRIXMODE_POSITION_VECTOR], index&31), mtxCurrent[MATRIXMODE_POSITION_VECTOR]);
index += 1;
index &= 63;
if(index >= 32) MMU_new.gxstat.se = 1;
}
GFX_DELAY(17); GFX_DELAY(17);
if (mymode == MATRIXMODE_POSITION_VECTOR)
MatrixStackPushMatrix(&mtxStack[1], mtxCurrent[1]);
} }
static void gfx3d_glPopMatrix(s32 i) static void gfx3d_glPopMatrix(s32 v)
{ {
// The stack has only one level (at address 0) in projection mode, //in case we're in Position-Vector mode, actually work on the Position matrix first (we'll update the vector matrix secondarily)
// in that mode, the parameter value is ignored, the offset is always +1 in that mode. const MatrixMode mymode = ((mode == MATRIXMODE_POSITION_VECTOR) ? MATRIXMODE_POSITION : mode);
if (mode == MATRIXMODE_PROJECTION) i = 1;
//this command always works on both pos and vector when either pos or pos-vector are the current mtx mode //1. apply offset specified by pop to internal counter
const MatrixMode mymode = ((mode == MATRIXMODE_POSITION) ? MATRIXMODE_POSITION_VECTOR : mode); //2. SE is set depending on resulting internal counter
//3. mask that bit off to actually index the matrix for reading
//i = (i<<26)>>26; if(mymode == MATRIXMODE_PROJECTION || mymode == MATRIXMODE_TEXTURE)
//previously, we sign extended here. that isnt really necessary since the stacks are apparently modularly addressed. so i am somewhat doubtful that this is a real concept. {
//example: //parameter ignored and treated as sensible, as a pop argument anyway
//suppose we had a -30 that would be %100010. v = 1;
//which is the same as adding 34. if our stack was at 17 then one way is 17-30(+32)=19 and the other way is 17+34(-32)=19
u32& index = mtxStack[mymode].position;
//please note that our ability to skip treating this as signed is dependent on the modular addressing later. if that ever changes, we need to change this back.
MatrixStackPopMatrix(mtxCurrent[mymode], &mtxStack[mymode], i); index -= v;
index &= 3;
if(index >= 2) MMU_new.gxstat.se = 1; //unknown if this applies to the texture matrix
MatrixCopy(mtxCurrent[mymode], MatrixStackGetPos(&mtxStack[mymode], index&1));
}
else
{
u32& index = mtxStack[MATRIXMODE_POSITION].position;
index -= v;
index &= 63;
if(index >= 32) MMU_new.gxstat.se = 1;
MatrixCopy(mtxCurrent[MATRIXMODE_POSITION], MatrixStackGetPos(&mtxStack[MATRIXMODE_POSITION], index&31));
if(mode == MATRIXMODE_POSITION_VECTOR)
MatrixCopy(mtxCurrent[MATRIXMODE_POSITION_VECTOR], MatrixStackGetPos(&mtxStack[MATRIXMODE_POSITION_VECTOR], index&31));
}
GFX_DELAY(36); GFX_DELAY(36);
if (mymode == MATRIXMODE_POSITION_VECTOR)
MatrixStackPopMatrix(mtxCurrent[1], &mtxStack[1], i);
} }
static void gfx3d_glStoreMatrix(u32 v) static void gfx3d_glStoreMatrix(u32 v)
{ {
//this command always works on both pos and vector when either pos or pos-vector are the current mtx mode //in case we're in Position-Vector mode, actually work on the Position matrix first (we'll update the vector matrix secondarily)
const MatrixMode mymode = ((mode == MATRIXMODE_POSITION) ? MATRIXMODE_POSITION_VECTOR : mode); const MatrixMode mymode = ((mode == MATRIXMODE_POSITION_VECTOR) ? MATRIXMODE_POSITION : mode);
//limit height of these stacks. if(mymode == MATRIXMODE_PROJECTION || mymode == MATRIXMODE_TEXTURE)
//without the mymode==3 namco classics galaxian will try to use pos=1 and overrun the stack, corrupting emu {
if (mymode == MATRIXMODE_PROJECTION || mymode == MATRIXMODE_TEXTURE) //parameter ignored and treated as sensible
v = 0; v = 0;
}
else
{
//out of bounds function fully properly, but set errors
if(v >= 31) MMU_new.gxstat.se = 1;
}
//the counter is [0,63] but only the bottom 5 bits index the stack
v &= 31; v &= 31;
//according to gbatek, 31 works but sets the stack overflow flag
//spider-man 2 tests this on the spiderman model (and elsewhere)
//i am somewhat skeptical of this, but we'll leave it this way for now.
//a test shouldnt be too hard
if (v == 31)
MMU_new.gxstat.se = 1;
MatrixStackLoadMatrix(&mtxStack[mymode], v, mtxCurrent[mymode]); MatrixStackLoadMatrix(&mtxStack[mymode], v, mtxCurrent[mymode]);
GFX_DELAY(17); //apply to Position-Vector matrix too, if appropriate
if (mode == MATRIXMODE_POSITION_VECTOR)
MatrixStackLoadMatrix(&mtxStack[MATRIXMODE_POSITION_VECTOR], v, mtxCurrent[MATRIXMODE_POSITION_VECTOR]);
if (mymode == MATRIXMODE_POSITION_VECTOR) GFX_DELAY(17);
MatrixStackLoadMatrix(&mtxStack[1], v, mtxCurrent[1]);
} }
static void gfx3d_glRestoreMatrix(u32 v) static void gfx3d_glRestoreMatrix(u32 v)
{ {
//this command always works on both pos and vector when either pos or pos-vector are the current mtx mode //in case we're in Position-Vector mode, actually work on the Position matrix first (we'll update the vector matrix secondarily)
const MatrixMode mymode = ((mode == MATRIXMODE_POSITION) ? MATRIXMODE_POSITION_VECTOR : mode); const MatrixMode mymode = ((mode == MATRIXMODE_POSITION_VECTOR) ? MATRIXMODE_POSITION : mode);
//limit height of these stacks if(mymode == MATRIXMODE_PROJECTION || mymode == MATRIXMODE_TEXTURE)
//without the mymode==3 namco classics galaxian will try to use pos=1 and overrun the stack, corrupting emu {
if (mymode == MATRIXMODE_PROJECTION || mymode == MATRIXMODE_TEXTURE) //parameter ignored and treated as sensible
v = 0; v = 0;
}
v &= 31; else
{
//according to gbatek, 31 works but sets the stack overflow flag //out of bounds errors function fully properly, but set errors
//spider-man 2 tests this on the spiderman model (and elsewhere) MMU_new.gxstat.se = v>=31;
//i am somewhat skeptical of this, but we'll leave it this way for now. }
//a test shouldnt be too hard
if (v == 31)
MMU_new.gxstat.se = 1;
MatrixCopy(mtxCurrent[mymode], MatrixStackGetPos(&mtxStack[mymode], v)); MatrixCopy(mtxCurrent[mymode], MatrixStackGetPos(&mtxStack[mymode], v));
GFX_DELAY(36); //apply to Position-Vector matrix too, if appropriate
if (mode == MATRIXMODE_POSITION_VECTOR)
MatrixCopy(mtxCurrent[MATRIXMODE_POSITION_VECTOR], MatrixStackGetPos(&mtxStack[MATRIXMODE_POSITION_VECTOR], v));
if (mymode == MATRIXMODE_POSITION_VECTOR) GFX_DELAY(36);
MatrixCopy(mtxCurrent[1], MatrixStackGetPos(&mtxStack[1], v));
} }
static void gfx3d_glLoadIdentity() static void gfx3d_glLoadIdentity()
@ -2544,7 +2584,7 @@ void gfx3d_savestate(EMUFILE* os)
for (size_t i = 0; i < polylist->count; i++) for (size_t i = 0; i < polylist->count; i++)
polylist->list[i].save(os); polylist->list[i].save(os);
for (size_t i = 0; i < 4; i++) for (size_t i = 0; i < ARRAY_SIZE(mtxStack); i++)
{ {
OSWRITE(mtxStack[i].position); OSWRITE(mtxStack[i].position);
for(size_t j = 0; j < mtxStack[i].size*16; j++) for(size_t j = 0; j < mtxStack[i].size*16; j++)
@ -2595,7 +2635,7 @@ bool gfx3d_loadstate(EMUFILE* is, int size)
if (version >= 2) if (version >= 2)
{ {
for (size_t i=0; i < 4; i++) for (size_t i=0; i < ARRAY_SIZE(mtxStack); i++)
{ {
OSREAD(mtxStack[i].position); OSREAD(mtxStack[i].position);
for(size_t j = 0; j < mtxStack[i].size*16; j++) for(size_t j = 0; j < mtxStack[i].size*16; j++)

View File

@ -1,6 +1,6 @@
/* /*
Copyright (C) 2006-2007 shash Copyright (C) 2006-2007 shash
Copyright (C) 2007-2012 DeSmuME team Copyright (C) 2007-2017 DeSmuME team
This file is free software: you can redistribute it and/or modify This file is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by it under the terms of the GNU General Public License as published by
@ -216,7 +216,7 @@ void MatrixStackSetMaxSize (MatrixStack *stack, int size)
{ {
int i; int i;
stack->size = (size + 1); stack->size = size;
if (stack->matrix != NULL) { if (stack->matrix != NULL) {
free (stack->matrix); free (stack->matrix);
@ -227,8 +227,6 @@ void MatrixStackSetMaxSize (MatrixStack *stack, int size)
{ {
MatrixInit (&stack->matrix[i*16]); MatrixInit (&stack->matrix[i*16]);
} }
stack->size--;
} }
@ -238,42 +236,10 @@ MatrixStack::MatrixStack(int size, int type)
this->type = type; this->type = type;
} }
static void MatrixStackSetStackPosition (MatrixStack *stack, int pos)
{
stack->position += pos;
if((stack->position < 0) || (stack->position > stack->size))
MMU_new.gxstat.se = 1;
//once upon a time, we tried clamping to the size.
//this utterly broke sims 2 apartment pets.
//changing to wrap around made it work perfectly
stack->position = ((u32)stack->position) & stack->size;
}
void MatrixStackPushMatrix (MatrixStack *stack, const s32 *ptr)
{
//printf("Push %i pos %i\n", stack->type, stack->position);
if ((stack->type == 0) || (stack->type == 3))
MatrixCopy (&stack->matrix[0], ptr);
else
MatrixCopy (&stack->matrix[stack->position*16], ptr);
MatrixStackSetStackPosition (stack, 1);
}
void MatrixStackPopMatrix (s32 *mtxCurr, MatrixStack *stack, int size)
{
//printf("Pop %i pos %i (change %d)\n", stack->type, stack->position, -size);
MatrixStackSetStackPosition(stack, -size);
if ((stack->type == 0) || (stack->type == 3))
MatrixCopy (mtxCurr, &stack->matrix[0]);
else
MatrixCopy (mtxCurr, &stack->matrix[stack->position*16]);
}
s32* MatrixStackGetPos(MatrixStack *stack, const size_t pos) s32* MatrixStackGetPos(MatrixStack *stack, const size_t pos)
{ {
assert(pos<31); assert(pos < stack->size);
return &stack->matrix[pos*16]; return &stack->matrix[pos*16];
} }
@ -284,7 +250,7 @@ s32* MatrixStackGet (MatrixStack *stack)
void MatrixStackLoadMatrix (MatrixStack *stack, const size_t pos, const s32 *ptr) void MatrixStackLoadMatrix (MatrixStack *stack, const size_t pos, const s32 *ptr)
{ {
assert(pos<31); assert(pos < stack->size);
MatrixCopy(&stack->matrix[pos*16], ptr); MatrixCopy(&stack->matrix[pos*16], ptr);
} }

View File

@ -1,6 +1,6 @@
/* /*
Copyright (C) 2006-2007 shash Copyright (C) 2006-2007 shash
Copyright (C) 2007-2012 DeSmuME team Copyright (C) 2007-2017 DeSmuME team
This file is free software: you can redistribute it and/or modify This file is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by it under the terms of the GNU General Public License as published by
@ -37,7 +37,7 @@ struct MatrixStack
{ {
MatrixStack(int size, int type); MatrixStack(int size, int type);
s32 *matrix; s32 *matrix;
s32 position; u32 position;
s32 size; s32 size;
u8 type; u8 type;
}; };
@ -58,8 +58,6 @@ void MatrixIdentity (s32 *matrix);
void MatrixStackInit (MatrixStack *stack); void MatrixStackInit (MatrixStack *stack);
void MatrixStackSetMaxSize (MatrixStack *stack, int size); void MatrixStackSetMaxSize (MatrixStack *stack, int size);
void MatrixStackPushMatrix (MatrixStack *stack, const s32 *ptr);
void MatrixStackPopMatrix (s32 *mtxCurr, MatrixStack *stack, int size);
s32* MatrixStackGetPos (MatrixStack *stack, const size_t pos); s32* MatrixStackGetPos (MatrixStack *stack, const size_t pos);
s32* MatrixStackGet (MatrixStack *stack); s32* MatrixStackGet (MatrixStack *stack);
void MatrixStackLoadMatrix (MatrixStack *stack, const size_t pos, const s32 *ptr); void MatrixStackLoadMatrix (MatrixStack *stack, const size_t pos, const s32 *ptr);