Real fixes for the OP, to make it behave more like real H/W.

Turns out the fix added in the last commit was wrong. After doing some
tests on real H/W, I'm pretty sure that I have it correct now. :-)

diff --git a/src/blitter.cpp b/src/blitter.cpp
index 776bc8c7165e9081142207ae8f2a504f8f36b433..c4975ab14a00618762cf6f26805822033331238c 100644 (file)
--- a/src/blitter.cpp
+++ b/src/blitter.cpp
@@ -2774,7 +2774,7 @@ if (
 #else
 logBlit = true;
 #endif
-if (blit_start_log == 0)       // Wait for the signal...
+/*if (blit_start_log == 0)     // Wait for the signal...
        logBlit = false;//*/
 //temp, for testing...
 /*if (cmd != 0x49820609)

diff --git a/src/gui/debug/opbrowser.cpp b/src/gui/debug/opbrowser.cpp
index c4193d8798ce8438e0c183477aaf2970d2fa4c49..6b3ff3ed0af73398f0d953dff3418f4c14dd581d 100644 (file)
--- a/src/gui/debug/opbrowser.cpp
+++ b/src/gui/debug/opbrowser.cpp
@@ -7,7 +7,7 @@
 // JLH = James Hammons <jlhamm@acm.org>
 //
 // Who  When        What
-// ---  ----------  -------------------------------------------------------------
+// ---  ----------  -----------------------------------------------------------
 // JLH  12/01/2012  Created this file
 //
 
@@ -110,10 +110,9 @@ void OPBrowserWindow::DiscoverObjects(uint32_t address)
                        // Branch if YPOS < 2047 can be treated as a GOTO, so don't do any
                        // discovery in that case. Otherwise, have at it:
                        if ((lo & 0xFFFF) != 0x7FFB)
-                       // Recursion needed to follow all links! This does depth-first
-                       // recursion on the not-taken objects (N.B.: The object following
-                       // the branch object is at +16, not +8!)
-                               DiscoverObjects(address + 16);
+                               // Recursion needed to follow all links! This does depth-first
+                               // recursion on the not-taken objects
+                               DiscoverObjects(address + 8);
                }
 
                // Get the next object...
@@ -156,13 +155,15 @@ void OPBrowserWindow::DumpObjectList(QString & list)
 
                list += "<br>";
 
+               // Yes, the OP really determines bitmap/scaled bitmap address for the
+               // following phrases this way...!
                if (objectType == 0)
                        DumpFixedObject(list, OPLoadPhrase(address + 0),
-                               OPLoadPhrase(address + 8));
+                               OPLoadPhrase(address | 0x08));
 
                if (objectType == 1)
                        DumpScaledObject(list, OPLoadPhrase(address + 0),
-                               OPLoadPhrase(address + 8), OPLoadPhrase(address + 16));
+                               OPLoadPhrase(address | 0x08), OPLoadPhrase(address | 0x10));
 
                if (address == link)    // Ruh roh...
                {

diff --git a/src/op.cpp b/src/op.cpp
index dfc9ece092cf495f4f1f0a0ecaf7b503bc746db2..85bbdf185a3a1948ab6f54d064660dadc2297e99 100644 (file)
--- a/src/op.cpp
+++ b/src/op.cpp
@@ -558,14 +558,13 @@ WriteLog("    --> List end\n\n");
                {
                case OBJECT_TYPE_BITMAP:
                {
-//WAS:                 uint16_t ypos = (p0 >> 3) & 0x3FF;
                        uint16_t ypos = (p0 >> 3) & 0x7FF;
 // This is only theory implied by Rayman...!
-// It seems that if the YPOS is zero, then bump the YPOS value so that it coincides with
-// the VDB value. With interlacing, this would be slightly more tricky.
-// There's probably another bit somewhere that enables this mode--but so far, doesn't seem
-// to affect any other game in a negative way (that I've seen).
-// Either that, or it's an undocumented bug...
+// It seems that if the YPOS is zero, then bump the YPOS value so that it
+// coincides with the VDB value. With interlacing, this would be slightly more
+// tricky. There's probably another bit somewhere that enables this mode--but so
+// far, doesn't seem to affect any other game in a negative way (that I've
+// seen). Either that, or it's an undocumented bug...
 
 //No, the reason this was needed is that the OP code before was wrong. Any value
 //less than VDB will get written to the top line of the display!
@@ -575,9 +574,9 @@ WriteLog("    --> List end\n\n");
                        if (ypos == 0)
                                ypos = TOMReadWord(0xF00046, OP) / 2;                   // Get the VDB value
 #endif
-// Actually, no. Any item less than VDB will get only the lines that hang over VDB displayed.
-// Actually, this is incorrect. It seems that VDB value is wrong somewhere and that's
-// what's causing things to fuck up. Still no idea why.
+// Actually, no. Any item less than VDB will get only the lines that hang over
+// VDB displayed. Actually, this is incorrect. It seems that VDB value is wrong
+// somewhere and that's what's causing things to fuck up. Still no idea why.
 
                        uint32_t height = (p0 & 0xFFC000) >> 14;
                        uint32_t oldOPP = op_pointer - 8;
@@ -589,7 +588,9 @@ if (!inhibit)       // For OP testing only!
 // *** END OP PROCESSOR TESTING ONLY ***
                        if (halfline >= ypos && height > 0)
                        {
-                               uint64_t p1 = OPLoadPhrase(op_pointer);
+                               // Believe it or not, this is what the OP actually does...
+                               // which is why they're required to be on a dphrase boundary!
+                               uint64_t p1 = OPLoadPhrase(op_pointer | 0x08);
                                op_pointer += 8;
 //WriteLog("OP: Writing halfline %d with ypos == %d...\n", halfline, ypos);
 //WriteLog("--> Writing %u BPP bitmap...\n", op_bitmap_bit_depth[(p1 >> 12) & 0x07]);
@@ -621,23 +622,17 @@ if (!inhibit)     // For OP testing only!
                                OPStorePhrase(oldOPP, p0);
                        }
 //WriteLog("\t\tOld OP: %08X -> ", op_pointer);
-//Temp, for testing...
-//No doubt, this type of check will break all kinds of stuff... !!! FIX !!!
-//And it does! !!! FIX !!!
-//Let's remove this "fix" since it screws up more than it fixes.
-/*     if (op_pointer > ((p0 & 0x000007FFFF000000LL) >> 21))
-               return;*/
-
-// NOTE: The link address only replaces bits 3-21 in the OLP, and this replaces
-//       EVERYTHING. !!! FIX !!! [DONE]
-#warning "!!! Link address is not linked properly for all object types !!!"
-#warning "!!! Only BITMAP is properly handled !!!"
-                       op_pointer &= 0xFFC00007;
-                       op_pointer |= (p0 & 0x000007FFFF000000LL) >> 21;
+
+                       // OP bottom 3 bits are hardwired to zero. The link address reflects
+                       // this, so we only need the top 19 bits of the address (which is
+                       // why we only shift 21, and not 24).
+                       op_pointer = (p0 & 0x000007FFFF000000LL) >> 21;
+
 //WriteLog("New OP: %08X\n", op_pointer);
-//kludge: Seems that memory access is mirrored in the first 8MB of memory...
-if (op_pointer > 0x1FFFFF && op_pointer < 0x800000)
-       op_pointer &= 0xFF1FFFFF;       // Knock out bits 21-23
+                       //kludge: Seems that memory access is mirrored in the first 8MB of
+                       // memory...
+                       if (op_pointer > 0x1FFFFF && op_pointer < 0x800000)
+                               op_pointer &= 0xFF1FFFFF;       // Knock out bits 21-23
 
                        break;
                }
@@ -659,10 +654,10 @@ if (!inhibit)     // For OP testing only!
 // *** END OP PROCESSOR TESTING ONLY ***
                        if (halfline >= ypos && height > 0)
                        {
-                               uint64_t p1 = OPLoadPhrase(op_pointer);
-                               op_pointer += 8;
-                               uint64_t p2 = OPLoadPhrase(op_pointer);
-                               op_pointer += 8;
+                               // Believe it or not, this is what the OP actually does...
+                               uint64_t p1 = OPLoadPhrase(op_pointer | 0x08);
+                               uint64_t p2 = OPLoadPhrase(op_pointer | 0x10);
+                               op_pointer += 16;
 //WriteLog("OP: %08X (%d) %08X%08X %08X%08X %08X%08X\n", oldOPP, halfline, (uint32_t)(p0>>32), (uint32_t)(p0&0xFFFFFFFF), (uint32_t)(p1>>32), (uint32_t)(p1&0xFFFFFFFF), (uint32_t)(p2>>32), (uint32_t)(p2&0xFFFFFFFF));
                                OPProcessScaledBitmap(p0, p1, p2, render);
 
@@ -769,7 +764,16 @@ OP: Scaled bitmap 4x? 4bpp at 34,? hscale=80 fpix=0 data=000756E8 pitch 1 hflipp
 //WriteLog(" [after]: rem=%02X, vscale=%02X\n", remainder, vscale);
                        }
 
+                       // OP bottom 3 bits are hardwired to zero. The link address reflects
+                       // this, so we only need the top 19 bits of the address (which is
+                       // why we only shift 21, and not 24).
                        op_pointer = (p0 & 0x000007FFFF000000LL) >> 21;
+
+                       //kludge: Seems that memory access is mirrored in the first 8MB of
+                       // memory...
+                       if (op_pointer > 0x1FFFFF && op_pointer < 0x800000)
+                               op_pointer &= 0xFF1FFFFF;       // Knock out bits 21-23
+
                        break;
                }
                case OBJECT_TYPE_GPU:
@@ -797,10 +801,6 @@ OP: Scaled bitmap 4x? 4bpp at 34,? hscale=80 fpix=0 data=000756E8 pitch 1 hflipp
                        uint8_t  cc   = (p0 >> 14) & 0x03;
                        uint32_t link = (p0 >> 21) & 0x3FFFF8;
 
-                       // If no branch is taken, we need to ensure that it goes to the
-                       // next object (it doesn't go +8, but +16 to following object)
-                       op_pointer += 8;
-
 //                     if ((ypos!=507)&&(ypos!=25))
 //                             WriteLog("\t%i%s%i link=0x%.8x\n",halfline,condition_to_str[cc],ypos>>1,link);
                        switch (cc)