andkrau / NuanceResurrection

Other
56 stars 5 forks source link

Fix texturing in MGL w/DynamicCompiler #26

Closed cubanismo closed 7 months ago

cubanismo commented 7 months ago

MGL (The OpenGL-like library for Nuon developers, included in the Nuon SDK) uses the following sequence when loading the shift value used for the fixed-point multiplies that calculate its texture s/t texture coordinate starting points for a line of rasterization (From raster.s in the MGL source code included in the Nuon SDK)

ld_s    (_MPETextureParameter), v7[2]           ; V7[2] holds t shift (and texture parameter)
...
lsr     #08, v7[2], v7[1]                       ; V7[1] holds s shift
...
mul     v6[2], v5[0], >>v7[1], v5[0]            ; Calculate starting s
...
mul     v6[2], v5[1], >>v7[2], v5[1]            ; Calculate starting t

Note that both shift values (v7[2] and v7[1]) are loaded in from the same scalar, _MPETextureParameter, which the C code sets up as follows in pipeline.c:

DMAToMPE(mpeIndex, MPETextureInfo, tp->mpeInfo, 3);

MPETextureInfo is a 3-scalar array, the third entry of which is MPETextureParameter, as defined in mpedata.s. It is set up when creating MGL textures thusly in mgl.c:

tp->mpeInfo[0] = uvxyctl | (tile(width) << 16) | (tile(height) << 12) | width;      // uvctl, xyctl
tp->mpeInfo[1] = (clutSize > 0) ? ((GLuint)MPETextureCache + clutOffset) : 0;       // clutbase
tp->mpeInfo[2] =
    ((45+GLXYZWCLIPSHIFT-GLMINZSHIFT-16+GLTEXCOORDSHIFT-textureShift(width))<<8) |  // s, t shifts
    (45+GLXYZWCLIPSHIFT-GLMINZSHIFT-16+GLTEXCOORDSHIFT-textureShift(height));

So we can see MPETextureParameter will contain two shift values for s and t, with the s value shifted left 8 bits as expected.

However, note that while the assembly code shifts the s shift down to the right when loading it into the v7[1] scalar register, leaving only zeros to its left, it does not mask those bits off from v7[2]. This works on hardware and in the inline- interpreted/non-dynamic-compiler path of Nuance because the shift applied by the multiply instruction has a range of [-32, 63]. The hardware and Nuance, in ExecuteMUL.cpp, mask the shift register's bits with 0x7f when loading it. In EmitMUL.cpp however, the Emit_MULScalarShiftScalar function, which appears to have been largely cut- and-pasted from the Emit_MULScalarShiftAcshift function that takes its shift value from the already-masked acshift register, the masking is omotted, and hence applies a very large right shift to the multiplication result, effectively zeroing it out.

The fix of course is to emit an additional ANDIR instruction in this path to mask the shift scalar as is done in the interpreted path.

This fixes rendering in some of my code, as well as in the "room" MGL example included in the Nuon SDK.

Also, attaching a small test program that demonstrates the bug:

mgl-test.zip

And a screenshot of the good and bad states:

Bad: onetri-bad

Good: onetri-good

toxieainc commented 7 months ago

Nice!!

toxieainc commented 7 months ago

I also ported this fix to the other occurrences..