Viewport bug

[Ian]

On a few of the games with my renderer, the projection matrix (view angles are clearly wrong).
The bug never showed up in the previous renderer because not all the values were used.

In the APi there are two functions for setting the view angles

void SetFieldOfView ( float left , float right , float top , float bottom ) ;

and

void SetFieldOfView ( float horizontal_fov , float vertical_fov ) ;

The first one, I assume all 4 values are copied directly to the hardware. The 2nd one will calculate the values for say the left side, then negate them for the right and bottom values.

When debugging lost world, on the very first frame it it sets these values 3 times. First two look right, last one ends up corrupted somehow.

This image shows the 2nd one, and the final one. The final one is broken. The values have been copied and negated, but in the final one it's copied the wrong value and negated it. Off by one error .. i guesss.
Could this be a possible CPU core bug? It effects games with 1.x and 2.x stepping so is not a transfer bug that I can see.

[Bart]

Thanks for digging into this, Ian. I know that delving into the CPU side of the equation is especially painstaking and tedious.

This is a very interesting finding. Strictly speaking, nothing can be ruled out at this point, but I am inclined to trust the CPU emulator given that it has been stress tested pretty well over the years.

That said, it's worth looking into this at some point. CPU bugs are particularly nasty. As are timing/IRQ bugs, which is what I suspect is the problem here. Our understanding of IRQs (what triggers them and how frequently) is very sketchy. There is also a Real3D status bit, which seems to indicate when the device is busy, that games poll. This is hard to debug because it involves several different parts of the game code working together in an asynchronous fashion (IRQ handlers, rendering loop, routines that trigger DMA transfers, etc.).

In this case, I'd want to take a look at how that first transfer is triggered, and what interrupts are fired and which registers are read between the first transfer and the second. The SCSI IRQ on Step 1.x doesn't have a control bit mapped to it on the IRQ controller. Each time the PowerPC receives an external interrupt, the interrupt handler polls both the IRQ controller and a SCSI register to determine where the IRQ(s) came from. If another video-related IRQ is pending, it will handle those. But what if the programmers assume that certain IRQs are mutually exclusive?

Could also be a SCSI bug. But this would not explain similar problems in Step 2.x games. Mis-handling Real3D IRQs and status bits would.

[Ian]

My guess is CPU core bug
I assume all the calculations are done on the CPU. It simply uses 1 transfer to the GPU to send the entire viewport data. Not sure how IRQs would effect this, but my understanding in this area is pretty limited.

I can imagine that stepping through hundreds of thousands of lines of code is pretty painful. Thankfully it goes 'wrong' in the very first frame, after like the 5th transfer or something.

[Ian]

I just had a look at dirt devils in mame

You can see the video here
https://www.youtube.com/watch?v=jnvHFP7r7ME

Clearly has the same bug in attract mode

[Bart]

Nik built a pretty great debugger that can be compiled in and enabled by defining SUPERMODEL_DEBUGGER. Activate with Alt +B. It's command oriented and I think supports memory watches. Alternatively, you'd have to manually add printf's to CModel3::Write32 to trap the address (and/or value, to be even more specific) in question and work back from there. I can have a look later this week. Do any valid viewports use this negated value? Or do you think that this particular number does not belong anywhere. If the latter, a stupid-simple thing to do is have Write32() look for data==<hex value of that float> and then: printf("wrote bad float at PC=%08x\n", ppc_get_pc())).

Using Nik's debugger, or dumping RAM (see ~CModel3()) and running if through my PowerPC disassembler (which can be compiled as a standalone utility) will allow the code to be examined.

An off by 1 error could occur if the game is doing something weird like enqueueing a list of viewports and then building+transferring the actual Real3D-formatted data under control of IRQs generated by the DMA device.

[Ian]

Yeah, you are supposed to use all 8 numbers to calculate the view frustum. Before I think you guys were reading only 4 values, which is enough for normal viewports that are symmetric.

Mame has the viewport code documented here where you can see these numbers

https://github.com/mamedev/mame/blob/ma ... 3.cpp#L727

Not sure exactly what the very first frame is doing, my guess is not much as it doesn't draw any 3d data at all.

[Ian]

Decompiled from the lib files. When writing to the hardware it actually writes 8 values, for x/y values. But you can see in the shorter version, it just copies and negates the parameters. That eliminates any possible flag that might be passed to indicate an asymmetric view.

void SetFieldOfView ( float horizontal_fov , float vertical_fov ) ;

Code: Select all

void __thiscall PRO_Viewport::SetFieldOfView(PRO_Viewport *this, float a2, float a3)
{
  double v3; // st7@1
  double v4; // st7@1

  v3 = a2 * 0.5;
  *((float *)this + 34) = -0.0174532925199433 * v3;
  *((float *)this + 35) = v3 * 0.0174532925199433;
  v4 = a3 * 0.5;
  *((_DWORD *)this + 14) |= 0x80u;
  *((float *)this + 36) = -0.0174532925199433 * v4;
  *((float *)this + 37) = v4 * 0.0174532925199433;
  PRO_Pingpong_Data_Block::NotifyUpdate(this);
}

void SetFieldOfView ( float left , float right , float top , float bottom ) ;

Code: Select all

void __thiscall PRO_Viewport::SetFieldOfView(PRO_Viewport *this, float a2, float a3, float a4, float a5)
{
  *((float *)this + 34) = a2 * 0.017453292;
  *((float *)this + 35) = a3 * 0.017453292;
  *((_DWORD *)this + 14) |= 0x80u;
  *((float *)this + 36) = a4 * -0.017453292;
  *((float *)this + 37) = a5 * -0.017453292;
  PRO_Pingpong_Data_Block::NotifyUpdate(this);
}

[Bart]

Interesting. What about those extra 2 values that precede the frustum data? No sign of those anywhere?

[Ian]

Which 2 values?

This ? *((_DWORD *)this + 14) |= 0x80u;

I've no idea what that's doing

[Ian]

Well ..
I finally found in the API where the actual viewport values are calculated

Code: Select all

void __thiscall PRO_Viewport::ComputeDerivedParameters(PRO_Viewport *this)
{
  double v1; // st7@1
  double v2; // st7@4
  double v3; // ST54_8@7
  double v4; // ST4C_8@7
  double v5; // st7@7
  double v6; // st7@10
  double v7; // st7@15
  double v8; // st7@19
  double v9; // st7@25
  double v10; // st7@29
  double v11; // st7@33
  double v12; // st7@39
  double v13; // st7@42
  double v14; // st7@45
  PRO_Viewport *v15; // [sp+5Ch] [bp-64h]@1
  double v16; // [sp+60h] [bp-60h]@1
  float v17; // [sp+68h] [bp-58h]@1
  double v18; // [sp+6Ch] [bp-54h]@4
  float v19; // [sp+74h] [bp-4Ch]@1
  double v20; // [sp+78h] [bp-48h]@36
  double v21; // [sp+80h] [bp-40h]@28
  float v22; // [sp+88h] [bp-38h]@1
  double v23; // [sp+8Ch] [bp-34h]@18
  double v24; // [sp+94h] [bp-2Ch]@1
  float v25; // [sp+9Ch] [bp-24h]@1
  double v26; // [sp+A0h] [bp-20h]@1
  double v27; // [sp+A8h] [bp-18h]@1
  double v28; // [sp+B0h] [bp-10h]@39
  double v29; // [sp+B8h] [bp-8h]@7

  v15 = this;
  PRO_Viewport::updateLobeParameters(this);
  PRO_Viewport::updateLOSPosition(v15);
  v19 = *((float *)v15 + 34);
  v25 = *((float *)v15 + 35);
  v22 = *((float *)v15 + 36);
  v17 = *((float *)v15 + 37);
  PRO_Viewport::ClipBox(v15, &v22, &v17, &v19, &v25);
  v16 = tan(v19);
  v26 = tan(v25);
  v24 = tan(v22);
  v27 = tan(v17);
  v1 = v25 + v19;
  if ( _adjust_fdiv )
    _adj_fdiv_m64(LODWORD(_T6029), HIDWORD(_T6029));
  else
    v1 = v1 / 2.0;
  v18 = v1;
  v2 = v17 + v22;
  if ( _adjust_fdiv )
    _adj_fdiv_m64(LODWORD(_T6029), HIDWORD(_T6029));
  else
    v2 = v2 / 2.0;
  v29 = v2;
  v3 = cos(-v18);
  *((float *)v15 + 44) = cos(v29) * v3;
  v4 = sin(-v18);
  *((float *)v15 + 45) = cos(v29) * v4;
  *((float *)v15 + 46) = sin(v29);
  v5 = v26 - v16;
  if ( _adjust_fdiv )
    _adj_fdivr_m64(LODWORD(_T6054), HIDWORD(_T6054));
  else
    v5 = 1.0 / v5;
  *(float *)(*((_DWORD *)v15 + 7) + 32) = v5;
  v6 = v27 - v24;
  if ( _adjust_fdiv )
    _adj_fdivr_m64(LODWORD(_T6054), HIDWORD(_T6054));
  else
    v6 = 1.0 / v6;
  *(float *)(*((_DWORD *)v15 + 7) + 36) = v6;
  *(float *)(*((_DWORD *)v15 + 7) + 40) = -(*(float *)(*((_DWORD *)v15 + 7) + 36) * v24);
  *(float *)(*((_DWORD *)v15 + 7) + 44) = -(*(float *)(*((_DWORD *)v15 + 7) + 32) * v16);
  if ( v16 >= 0.0 || v26 <= 0.0 )
  {
    v8 = tan(v25 - v19);
    if ( _adjust_fdiv )
      _adj_fdiv_m32(*((_DWORD *)v15 + 48));
    else
      v8 = v8 / *((float *)v15 + 48);
    v23 = v8;
  }
  else
  {
    v7 = tan(v25);
    if ( _adjust_fdiv )
      _adj_fdiv_r(v15, v7);
    else
      v7 = v7 / (*(float *)(*((_DWORD *)v15 + 7) + 32) * v26 * *((float *)v15 + 48));
    v23 = v7;
  }
  if ( v24 >= 0.0 || v27 <= 0.0 )
  {
    v10 = tan(v17 - v22);
    if ( _adjust_fdiv )
      _adj_fdiv_m32(*((_DWORD *)v15 + 47));
    else
      v10 = v10 / *((float *)v15 + 47);
    v21 = v10;
  }
  else
  {
    v9 = tan(v17);
    if ( _adjust_fdiv )
      _adj_fdiv_r(v15, v9);
    else
      v9 = v9 / (*(float *)(*((_DWORD *)v15 + 7) + 36) * v27 * *((float *)v15 + 47));
    v21 = v9;
  }
  v11 = sqrt(v21 * v21 + v23 * v23);
  if ( _adjust_fdiv )
    _adj_fdiv_m64(LODWORD(_T6029), HIDWORD(_T6029));
  else
    v11 = v11 / 2.0;
  v20 = v11;
  if ( _adjust_fdiv )
    _adj_fdivr_m64(LODWORD(_T6085), HIDWORD(_T6085));
  else
    v11 = 0.5 / v11;
  *(float *)(*((_DWORD *)v15 + 7) + 12) = v11;
  *(float *)(*((_DWORD *)v15 + 7) + 48) = -sin(v19);
  *(float *)(*((_DWORD *)v15 + 7) + 52) = cos(v19);
  *(float *)(*((_DWORD *)v15 + 7) + 56) = -sin(v22);
  *(float *)(*((_DWORD *)v15 + 7) + 60) = cos(v22);
  *(float *)(*((_DWORD *)v15 + 7) + 64) = sin(v25);
  *(float *)(*((_DWORD *)v15 + 7) + 68) = -cos(v25);
  *(float *)(*((_DWORD *)v15 + 7) + 72) = sin(v17);
  *(float *)(*((_DWORD *)v15 + 7) + 76) = -cos(v17);
  v12 = sqrt(*((float *)v15 + 83) * *((float *)v15 + 83) + *((float *)v15 + 82) * *((float *)v15 + 82) + *((float *)v15 + 81) * *((float *)v15 + 81));
  v28 = v12;
  if ( _adjust_fdiv )
    _adj_fdivr_m32(*((_DWORD *)v15 + 81));
  else
    v12 = *((float *)v15 + 81) / v12;
  *((float *)v15 + 81) = v12;
  v13 = *((float *)v15 + 82);
  if ( _adjust_fdiv )
    _adj_fdiv_m64(LODWORD(v28), HIDWORD(v28));
  else
    v13 = v13 / v28;
  *((float *)v15 + 82) = v13;
  v14 = *((float *)v15 + 83);
  if ( _adjust_fdiv )
    _adj_fdiv_m64(LODWORD(v28), HIDWORD(v28));
  else
    v14 = v14 / v28;
  *((float *)v15 + 83) = v14;
  *(_DWORD *)(*((_DWORD *)v15 + 7) + 16) = *((_DWORD *)v15 + 81);
  *(_DWORD *)(*((_DWORD *)v15 + 7) + 20) = *((_DWORD *)v15 + 82);
  *(_DWORD *)(*((_DWORD *)v15 + 7) + 24) = *((_DWORD *)v15 + 83);
}


The relevant lines are


Somewhere between here and the GPU the last value gets corrupted. It looks like last value is doing -cos(v22), instead of .. -cos(v17)
Cpu bug?