Possible bug related to scroll left and scroll right about chip8 HOT 5 CLOSED

Thank you.

Those instructions come from SuperCHIP and it's been a while since I studied the specs, so it took me a while to figure out, and even now I don't have a third party game to verify whether my understanding of the specs is correct.

I have attached a file scroll.txt
that just draws a little box on the screen and scrolls it left if you press 1 and right if you press 2 as a simple test.

I did commit aae87cf that contains the fix. It it is based on your proposed solution, but there was just one problem: In the SCR case the x variable runs from left to right, whereas in the SCL case x should run from right to left, so the for(x... lines should be changed as well. The code (from line 152) should look like this:

} else if(opcode == 0x00FB) {
  /* SCR */
  c8_resolution(&col, &row);
  col >>= 3;
  for(y = 0; y < row; y++) {
	  for(x = 0; x < col - 1; x++) {
		  pixels[y * col + x] = (pixels[y * col + x] >> 4) | (pixels[y * col + x + 1] << 4);
	  }
	  pixels[y * col + x] >>= 4;
  }
  screen_updated = 1;
} else if(opcode == 0x00FC) {
  /* SCL */
  c8_resolution(&col, &row);
  col >>= 3;
  for(y = 0; y < row; y++) {
	  for(x = col - 1; x > 0; x--) {
		  pixels[y * col + x] = (pixels[y * col + x] << 4) | (pixels[y * col + x - 1] >> 4);
	  }
	  pixels[y * col] <<= 4;
  }
  screen_updated = 1;

from chip8.

Glad to be of help!

I'm not sure if I'm fully grasping your solution.

Let's take SCR for example. From my understanding this instruction should right-shift by 4 all the pixels (or bits) in each row. However if we perform this operation from left to right wouldn't we risk overwriting as we go?

If we were to operate from right to left, we would obtain something like the following:

Follow the arrows from destination to source

       x-1                  x

   +--------+           +--------+
   |        |           |        |
-------     v        -------     v
1 1 0 1  1 0 0 0  |  1 0 1 1  0 0 0 1
         -------        ^     -------        ^
            |           |        |           |
            +-----------+        +-----------+

and end up in this situation:

x x x x  1 1 0 1  |  1 0 0 0  1 0 1 1

which should be the expected result.

from chip8.

Hi, yes, sorry, I typed my response a bit hastily yesterday and didn't even realise that I wasn't even testing my fix properly.

Part of the confusion is that the pixels in the pixels bit-map are stored in reverse order within each byte.

So if you were to look at a row of 16 pixels that are displayed like this:

  1   2      3    4
0101 0011 | 1010 1100

then it is actually stored like this in the pixels array like so:

~2   ~1     ~4   ~3
1100 1010 | 0011 0101 

You can look where the 0xDXYZ (DRW) instruction draws the pixels chip8.c:303 to see how this works:

int byte = ty * W + tx;
int bit = 1 << (byte & 0x07);
byte >>= 3;
pixels[byte] ^= bit;

You can also work through c8_get_pixel() to see how a pixel is read.

If the pixels were stored the other way round then it the second line would've had to read int bit = 0x80 >> (byte & 0x07).

I can't remember why I did it this way at the start. I think it seemed less complicated, but in retrospect I should've done it the other way around.

from chip8.

I've been looking through the code. There are some other issues I'd like to fix so I don't have anything to push right now.

from chip8.

I've pushed my most recent changes

from chip8.