This patch fixes an acceleration-caused rendering problem for both 8bpp and 32bpp modes. The problem is that icon rendering in some OS'es was incorrect (purple background, etc.) in 8bpp but correct in 32bpp. Then an update to the code reversed the problem, making the rendering correct in 8bpp but incorrect in 32bpp. What this does is basically using the original two lines if the mode is 32bpp, and the new ones if 8bpp. The 4bpp and 16bpp modes are not affected as they always render correctly.
I came up with the solution and my friend SA1988 uploaded it to his site in .patch format: http://towg.tk/Emulators/video.patch.
S3 render fix patch
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: S3 render fix patch
Under what OSes does this occur? What drivers?
Re: S3 render fix patch
this mainly occurs on NT 3.1 with PCI S3 Trio64 drivers from 1995 (the only ones that enable 1024x768x32bpp under that OS), and I've tested this patch earlier, makes the background render correctly.
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: S3 render fix patch
I won't accept the patch as-is, as it's a hack rather than a proper fix (the S3 blitter has no knowledge whatsoever of the current display depth - that's a property of the RAMDAC, and isn't even on the same chip on 864 boards). I will look into the issue though.
Re: S3 render fix patch
I think I just found a better way to fix it:
Code: Select all
else if ((s3->accel.cmd & 0x600) == 0x200)
{
if ((s3->accel.cmd & 0x700) == 0x300)
{
s3_accel_start(2, 1, 0xffffffff, val >> 16, s3);
s3_accel_start(2, 1, 0xffffffff, val, s3);
}
else
{
s3_accel_start(2, 1, 0xffffffff, val, s3);
s3_accel_start(2, 1, 0xffffffff, val >> 16, s3);
}
}[/quote]
I tested, and it works correctly.
Edit: Windows 3.1 didn't have problems in 256 colors anyway. I need an install of Windows NT 3.1 to check. So disregard the above code until I investigate further.
Re: S3 render fix patch
So I logged some values (addr, val, accel.cmd, and all of accel.multifunc):
Windows 3.1 32bpp 640x80 (where writing the value's 16-bit components in reverse renders correctly):
Windows NT 3.1 8bpp 640x480 (where writing the value's 16-bit components in forward renders correctly):
Edit: Windows NT 3.1 32bpp 640x480:
The are the values in order:
These is my attempt at an exaplanation of them:
Unknown 0 is the same (0x001F) in both cases, so it's irrelevant here.
Unknown 1 is 0x0665 in the former case, and 0x0998 in the latter - 0x0665 in NT 3.1 at 32bpp. 0x3FF in Windows 3.1 at 32bpp if set to 1024x768.
Unknown 2 is 0x0240 in the former case, and 0x0040 in the latter, and in NT 3.1 at 32bpp.
Unknown 3 is 0x0007 in the former case, and 0x0000 in the latter, and in NT 3.1 at 32bpp.
It seems now that it's bit 0 Unknown 1 (multifunc[3]) that controls the order of the data sent. If it's set, it's sent in reverse, otherwise, it's sent normally.
Edit: This is how s3_accel_write_l in vid_s3.c is supposed to look like if you take in account what I discovered here:
Tested, and Windows 3.1 and Windows NT 3.1 now render fine in all supported modes.
Windows 3.1 32bpp 640x80 (where writing the value's 16-bit components in reverse renders correctly):
Code: Select all
Write S3 accel 000A007C 00000000 53B1 [001F] [0000] [0000] [0665] [027F] [0000] [0000] [0000] [0000] [0000] [0000] [0000] [0000] [0000] [0240] [0007]
Code: Select all
Write S3 accel 000A001C 00000000 53B1 [001F] [0000] [0000] [0998] [027F] [0000] [0000] [0000] [0000] [0000] [0000] [0000] [0000] [0000] [0040] [0000]
Code: Select all
Write S3 accel 000A0424 00C0C0C0 53B1 [0060] [0000] [0000] [0665] [027F] [0000] [0000] [0000] [0000] [0000] [0000] [0000] [0000] [0000] [0040] [0000]
Code: Select all
addr val accel.cmd accel.multifunc[0] accel.multifunc[1] accel.multifunc[2] accel.multifunc[3] accel.multifunc[4] accel.multifunc[5] accel.multifunc[6] accel.multifunc[7] accel.multifunc[8] accel.multifunc[9] accel.multifunc[0χa] accel.multifunc[0χb] accel.multifunc[0xc] accel.multifunc[0xd] accel.multifunc[0xe] accel.multifunc[0xf]
Code: Select all
Address Value Command [Unknown 0] [Zero] [Zero] [Unknown 1] [H. Resolution] [Zero] [Zero] [Zero] [Zero] [Zero] [Zero] [Zero] [Zero] [Zero] [Unknown 2] [Unknown 3]
Unknown 1 is 0x0665 in the former case, and 0x0998 in the latter - 0x0665 in NT 3.1 at 32bpp. 0x3FF in Windows 3.1 at 32bpp if set to 1024x768.
Unknown 2 is 0x0240 in the former case, and 0x0040 in the latter, and in NT 3.1 at 32bpp.
Unknown 3 is 0x0007 in the former case, and 0x0000 in the latter, and in NT 3.1 at 32bpp.
It seems now that it's bit 0 Unknown 1 (multifunc[3]) that controls the order of the data sent. If it's set, it's sent in reverse, otherwise, it's sent normally.
Edit: This is how s3_accel_write_l in vid_s3.c is supposed to look like if you take in account what I discovered here:
Code: Select all
void s3_accel_write_l(uint32_t addr, uint32_t val, void *p)
{
s3_t *s3 = (s3_t *)p;
// pclog("Write S3 accel l %08X %08X\n", addr, val);
if (addr & 0x8000)
{
s3_accel_write(addr, val, p);
s3_accel_write(addr + 1, val >> 8, p);
s3_accel_write(addr + 2, val >> 16, p);
s3_accel_write(addr + 3, val >> 24, p);
}
else
{
if (s3->accel.cmd & 0x100)
{
if ((s3->accel.multifunc[0xa] & 0xc0) == 0x80)
{
if (s3->accel.cmd & 0x1000)
val = ((val & 0xff000000) >> 24) | ((val & 0x00ff0000) >> 8) | ((val & 0x0000ff00) << 8) | ((val & 0x000000ff) << 24);
if ((s3->accel.cmd & 0x600) == 0x400)
s3_accel_start(32, 1, val, 0, s3);
else if ((s3->accel.cmd & 0x600) == 0x200)
{
s3_accel_start(16, 1, val >> 16, 0, s3);
s3_accel_start(16, 1, val, 0, s3);
}
else if (!(s3->accel.cmd & 0x600))
{
s3_accel_start(8, 1, val >> 24, 0, s3);
s3_accel_start(8, 1, val >> 16, 0, s3);
s3_accel_start(8, 1, val >> 8, 0, s3);
s3_accel_start(8, 1, val, 0, s3);
}
}
else
{
if ((s3->accel.cmd & 0x600) == 0x400)
s3_accel_start(4, 1, 0xffffffff, val, s3);
else if ((s3->accel.cmd & 0x600) == 0x200)
{
if (s3->accel.multifunc[3] & 0x1)
val = ((val & 0xffff0000) >> 16) | ((val & 0x0000ffff) << 16);
s3_accel_start(2, 1, 0xffffffff, val, s3);
s3_accel_start(2, 1, 0xffffffff, val >> 16, s3);
}
else if (!(s3->accel.cmd & 0x600))
{
if (s3->accel.multifunc[3] & 0x1)
val = ((val & 0xff000000) >> 24) | ((val & 0x00ff0000) >> 8) | ((val & 0x0000ff00) << 8) | ((val & 0x000000ff) << 24);
s3_accel_start(1, 1, 0xffffffff, val, s3);
s3_accel_start(1, 1, 0xffffffff, val >> 8, s3);
s3_accel_start(1, 1, 0xffffffff, val >> 16, s3);
s3_accel_start(1, 1, 0xffffffff, val >> 24, s3);
}
}
}
}
}
Re: S3 render fix patch
Alright, please disregard my previous post, I just found what seems to be the real fix.
In s3_accel_start in vid_s3.c, find this:
And replace it with:
For some reason, this code, as it originally was, caused the accelerator to expect all the words being sent in reverse order when in 32bpp. This fixes it, and makes everything render correctly in 32bpp. I even tested the game The Greens, for Windows 3.1, that TheCollector1995 reported rendered incorrectly with all the previous fixes to this render problem, and now it renders 100% correctly.
Edit: Just to make it clear, all other fixes mention in this thread should be disregarded, and if you had applied them already, remove them and only apply the fix mentioned in this post.
In s3_accel_start in vid_s3.c, find this:
Code: Select all
cpu_dat = (cpu_dat & 0xffff) | (s3->accel.dat_buf << 16);
Code: Select all
cpu_dat = ((cpu_dat & 0xffff) << 16) | s3->accel.dat_buf;
Edit: Just to make it clear, all other fixes mention in this thread should be disregarded, and if you had applied them already, remove them and only apply the fix mentioned in this post.