RFC: S3/Virge off-by-ones?

Post new patches here!
Post Reply
rene
Posts: 7
Joined: Wed 05 Sep, 2018 1:30 pm

RFC: S3/Virge off-by-ones?

Post by rene » Sat 03 Nov, 2018 6:45 pm

Hi there, opened a new thread under Patches. As mentioned in my previous s3/virge thread writing some new bare metal virge code I realised at least the hardware clipping appears to be off-by-one in the top-most and right-most line of pixels. I did some initial trivial tweaks that fix my test case, but crash at least w/ Descent 2. As I mentioned the virge.pdf SPEC says last pixel drawn, so I hope I read this right, but looking closely in Descent also appears to have the top scan line missing, and the right filled with random, e.g. not correct pixels, WIP: https://svn.exactcode.de/t2/trunk/packa ... irge.patch

given clip 0,0 639, 479 and a triangle start at 479 y_count 480 this
clipped off-by-one?

- René Rebe <rene@exactcode.de>

--- ./src/vid_s3_virge.c.vanilla 2018-11-02 21:34:23.001923446 +0100
+++ ./src/vid_s3_virge.c 2018-11-02 21:36:33.253917430 +0100
@@ -2975,8 +2975,8 @@
z_offset -= s3d_tri->z_str;
y_count -= diff_y;
}
- if ((state->y - y_count) < s3d_tri->clip_t)
- y_count = state->y - s3d_tri->clip_t;
+ if ((state->y + 1 - y_count) < s3d_tri->clip_t)
+ y_count = state->y - s3d_tri->clip_t + 1;
}

for (; y_count > 0; y_count--)
@@ -2990,7 +2990,7 @@
xe--;
}

- if (x != xe && ((x_dir > 0 && x < xe) || (x_dir < 0 && x > xe)))
+ if ((x_dir > 0 && x <= xe) || (x_dir < 0 && x >= xe))
{
uint32_t dest_addr, z_addr;
int dx = (x_dir > 0) ? ((31 - ((state->x1-1) >> 15)) & 0x1f) : (((state->x1-1) >> 15) & 0x1f);
@@ -3069,7 +3069,7 @@
dest_addr = dest_offset + (x * (bpp + 1));
z_addr = z_offset + (x << 1);

- for (; x != xe; x = (x + x_dir) & 0xfff)
+ for (; (x_dir > 0 && x <= xe) || (x_dir < 0 && x >= xe); x = (x + x_dir) & 0xfff)
{
int update = 1;
uint16_t src_z = 0;

rene
Posts: 7
Joined: Wed 05 Sep, 2018 1:30 pm

Re: RFC: S3/Virge off-by-ones?

Post by rene » Sun 23 Dec, 2018 12:09 pm

The non mip-map linear flat texture case also needs to use the src-stride instead of the mipmap level: https://www.youtube.com/watch?v=l3lH0cIZUSA

As my original patch got not much attention, I did not yet code a pcem patch (should be trivial). I also still need to figure out all the details of the aforementioned clipping off-by-one.

Anyone interested in my continuing with this and working on patches?

User avatar
SarahWalker
Site Admin
Posts: 1702
Joined: Thu 24 Apr, 2014 4:18 pm

Re: RFC: S3/Virge off-by-ones?

Post by SarahWalker » Sun 23 Dec, 2018 12:23 pm

Sorry, I've been busy working on the new recompiler, and have neglected patches a bit in the last few months. I'll try to look at this early in the new year.

rene
Posts: 7
Joined: Wed 05 Sep, 2018 1:30 pm

Re: RFC: S3/Virge off-by-ones?

Post by rene » Tue 25 Dec, 2018 1:30 pm

Merry Christmas and thanks for the reply! just let me know if you prefer working on this yourself, of if you would appreciate more revised patches.

User avatar
SarahWalker
Site Admin
Posts: 1702
Joined: Thu 24 Apr, 2014 4:18 pm

Re: RFC: S3/Virge off-by-ones?

Post by SarahWalker » Thu 27 Dec, 2018 9:12 am

I'm not looking at the Virge at the moment (kind of totally focused on recompiler stuff), so any patches are very welcome! :)

User avatar
SarahWalker
Site Admin
Posts: 1702
Joined: Thu 24 Apr, 2014 4:18 pm

Re: RFC: S3/Virge off-by-ones?

Post by SarahWalker » Tue 15 Jan, 2019 9:34 pm

Committed a change based on the clipping fixes at rev 1209. Please let me know if this is still wrong!

Post Reply