Incorrect rendering of free disk space in Windows 98 with Voodoo 3

[Vort]

After My Computer is opened and disk C is selected in Windows 98 SE with Voodoo 3 (driver 1.07.00-WHQL), graphical artifacts on pie chart appear:

No artifacts appear with Cirrus card or with Voodoo 3 when hardware acceleration is partially disabled ():

Version: 890cc3a

[Vort]

Most of the artifacts can be eliminated by implementing src_x and src_y registers in a way, described by documentation: "The values in the srcXY are signed, however for blts srcXY must contain only positive values."

diff --git a/bochs/iodev/display/voodoo_data.h b/bochs/iodev/display/voodoo_data.h
index 5f5a29cb1..4ee3fbf64 100644
--- a/bochs/iodev/display/voodoo_data.h
+++ b/bochs/iodev/display/voodoo_data.h
@@ -1765,8 +1765,8 @@ struct _banshee_info
     Bit8u  src_fmt;
     Bit16u src_pitch;
     Bit8u  src_swizzle;
-    Bit16u src_x;
-    Bit16u src_y;
+    Bit16s src_x;
+    Bit16s src_y;
     Bit16u src_w;
     Bit16u src_h;
     Bit32u dst_base; 

Here is how chart looks after such change:

[Vort]

Also src_x and src_y needed sign extensions. Prettified dst_x and dst_y extensions along the way. This patch fixes problem for me:

diff --git a/bochs/iodev/display/banshee.cc b/bochs/iodev/display/banshee.cc
index db7da9b08..165ad3726 100644
--- a/bochs/iodev/display/banshee.cc
+++ b/bochs/iodev/display/banshee.cc
@@ -1667,8 +1667,8 @@ void bx_banshee_c::blt_reg_write(Bit8u reg, Bit32u value)
       BLT.src_h = (BLT.reg[reg] >> 16) & 0x1fff;
       break;
     case blt_srcXY:
-      BLT.src_x = BLT.reg[reg] & 0x1fff;
-      BLT.src_y = (BLT.reg[reg] >> 16) & 0x1fff;
+      BLT.src_x = (Bit32s)BLT.reg[reg] << 19 >> 19;
+      BLT.src_y = (Bit32s)BLT.reg[reg] << 3 >> 19;
       break;
     case blt_colorBack:
       BLT.bgcolor[0] = BLT.reg[reg] & 0xff;
@@ -1687,16 +1687,8 @@ void bx_banshee_c::blt_reg_write(Bit8u reg, Bit32u value)
       BLT.dst_h = (BLT.reg[reg] >> 16) & 0x1fff;
       break;
     case blt_dstXY:
-      if ((BLT.reg[reg] >> 15) & 1) {
-        BLT.dst_x = (Bit16s)(BLT.reg[reg] & 0xffff);
-      } else {
-        BLT.dst_x = BLT.reg[reg] & 0x1fff;
-      }
-      if (BLT.reg[reg] >> 31) {
-        BLT.dst_y = (Bit16s)(BLT.reg[reg] >> 16);
-      } else {
-        BLT.dst_y = (BLT.reg[reg] >> 16) & 0x1fff;
-      }
+      BLT.dst_x = (Bit32s)BLT.reg[reg] << 19 >> 19;
+      BLT.dst_y = (Bit32s)BLT.reg[reg] << 3 >> 19;
       break;
     case blt_command:
       old_cmd = BLT.cmd;
diff --git a/bochs/iodev/display/voodoo_data.h b/bochs/iodev/display/voodoo_data.h
index 1711d3c54..37403d5ad 100644
--- a/bochs/iodev/display/voodoo_data.h
+++ b/bochs/iodev/display/voodoo_data.h
@@ -1765,8 +1765,8 @@ struct _banshee_info
     Bit8u  src_fmt;
     Bit16u src_pitch;
     Bit8u  src_swizzle;
-    Bit16u src_x;
-    Bit16u src_y;
+    Bit16s src_x;
+    Bit16s src_y;
     Bit16u src_w;
     Bit16u src_h;
     Bit32u dst_base; 

[stlintel]

BLT.dst_x = (Bit32s)BLT.reg[reg] << 19 >> 19 looks weird and also might be not that portable. If you want to sign extend BLT.reg[reg] I think better to do it manually by ORing with mask according to sign bit

is it sign-extension of bit13 and above ? if (BLT.reg[reg] & 0x1000) BLT.reg[reg] |= 0xE000;

[Vort]

is it sign-extension of bit13 and above ?

It should be extended into dst_x, not into itself.

... << 19 >> 19 looks weird ...

<< 19 >> 19 is compact, both in source code (1 line) and assembly (2 instructions):

Do you think this code is better?

      BLT.dst_x = BLT.reg[reg] & 0x1000 ? BLT.reg[reg] | 0xE000 : BLT.reg[reg];
      BLT.dst_y = BLT.reg[reg] & 0x10000000 ? BLT.reg[reg] >> 16 | 0xE000 : BLT.reg[reg] >> 16;

also might be not that portable

If that's the case, then, sadly, less compact code should be used, but I don't know which platforms may have problems with it.

upd. Proper code should use not only |, but also &, adding even more complexity.

[Vort]

Here is solution, which is portable, fast and readable (source):

diff --git a/bochs/iodev/display/banshee.cc b/bochs/iodev/display/banshee.cc
index e489d12b1..e6e3e4977 100644
--- a/bochs/iodev/display/banshee.cc
+++ b/bochs/iodev/display/banshee.cc
@@ -108,6 +108,13 @@ const Bit8u banshee_iomask[256] = {4,0,0,0,7,1,3,1,7,1,3,1,7,1,3,1,7,1,3,1,
 const Bit8u pxconv_table[16] = {0x3a,0x02,0x00,0x38,0x38,0x38,0x00,0x00,
                                 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00};

+template <typename T, unsigned B>
+inline T signextend(const T x)
+{
+  struct { T x : B; } s;
+  return s.x = x;
+}
+
 #include "voodoo_types.h"
 #include "voodoo_data.h"
 #include "voodoo_main.h"
@@ -1759,8 +1766,8 @@ void bx_banshee_c::blt_reg_write(Bit8u reg, Bit32u value)
       BLT.src_h = (BLT.reg[reg] >> 16) & 0x1fff;
       break;
     case blt_srcXY:
-      BLT.src_x = BLT.reg[reg] & 0x1fff;
-      BLT.src_y = (BLT.reg[reg] >> 16) & 0x1fff;
+      BLT.src_x = signextend<Bit16s, 13>(BLT.reg[reg]);
+      BLT.src_y = signextend<Bit16s, 13>(BLT.reg[reg] >> 16);
       break;
     case blt_colorBack:
       BLT.bgcolor[0] = BLT.reg[reg] & 0xff;
@@ -1779,16 +1786,8 @@ void bx_banshee_c::blt_reg_write(Bit8u reg, Bit32u value)
       BLT.dst_h = (BLT.reg[reg] >> 16) & 0x1fff;
       break;
     case blt_dstXY:
-      if ((BLT.reg[reg] >> 15) & 1) {
-        BLT.dst_x = (Bit16s)(BLT.reg[reg] & 0xffff);
-      } else {
-        BLT.dst_x = BLT.reg[reg] & 0x1fff;
-      }
-      if (BLT.reg[reg] >> 31) {
-        BLT.dst_y = (Bit16s)(BLT.reg[reg] >> 16);
-      } else {
-        BLT.dst_y = (BLT.reg[reg] >> 16) & 0x1fff;
-      }
+      BLT.dst_x = signextend<Bit16s, 13>(BLT.reg[reg]);
+      BLT.dst_y = signextend<Bit16s, 13>(BLT.reg[reg] >> 16);
       break;
     case blt_command:
       old_cmd = BLT.cmd;
diff --git a/bochs/iodev/display/voodoo_data.h b/bochs/iodev/display/voodoo_data.h
index 1a6d96500..4528af006 100644
--- a/bochs/iodev/display/voodoo_data.h
+++ b/bochs/iodev/display/voodoo_data.h
@@ -1765,8 +1765,8 @@ struct _banshee_info
     Bit8u  src_fmt;
     Bit16u src_pitch;
     Bit8u  src_swizzle;
-    Bit16u src_x;
-    Bit16u src_y;
+    Bit16s src_x;
+    Bit16s src_y;
     Bit16u src_w;
     Bit16u src_h;
     Bit32u dst_base;