[Tuxnes-devel] Bug in pixels.h sprite rendering

[Jim U. <ji...@3e...>]

I've found what seems to me a definite bug in sprite
rendering in pixels.h, which works only by coincidence
on linux x86 platforms.  Here's a (shortened) version
of the current code:

for (s = 63; s >= 0; s--) {
	render sprites into 256-char linebuffer
}
for (x = 0; x < 256; x++) {
	if (linebuffer[x]) {
		ptr0[spriteram[s*4+3] + x] = endian_fix(palette[linebuffer[x]]);
	}
}

The line containing "ptr0[]" is wrong.  It's obvious
that if spriteram[s*4+3] > 0, then spriteram[s*4+3] + x
can exceed 256, the boundary of the line.  It turns out
that the "render sprites" code in the previous loop
itself takes care of putting all sprites on the current
line in the correct position in the linebuffer, so we
should be able to remove the reference to spriteram[].
In fact, if you replace the line with 

ptr0[x] = endian_fix(palette[linebuffer[x]]);

you get exactly the same behavior.  Why?  Because "s" is
always -1 during the second loop---due to the first loop!
We're referencing spriteram[-1*4+3] = spriteram[-1]
which is (probably) uninitialized, and is therefore
zero under an operating system such as linux.  If
you're not running under an OS, well, let's just say
that if you avoid a crash your sprites aren't in the
right place anyway.  Probably the error above is 
just a vestige of old code and it wasn't caught 'cause it
doesn't do much harm normally...

Patch attached.  This fixes 8, 16, and 32 bpp.  I can't
test 1, 4, or 24 bpp so I didn't make a patch, though I
can have a go at it if you like.  I will note that all
those #defines get really confusing and require careful
negotiation ;)

Hope this is really a bug and I'm not missing something,
Jim