Is the negation of the sprite collision mask correct?

[Yazwh0]

I'm trying to understand how sprite collision works, and the negation of the mask on this line looks wrong.

I cant think of a reason why it would be there, but I don't really know the intent of how the collision is to be used either.

vera-module/fpga/source/graphics/sprite_renderer.v

Line 328 in 1a87b77

(!pixel_is_transparent && sprite_collision_mask_r != 4'b0) ? (linebuf_rddata[15:12] & ~sprite_collision_mask_r) : 4'b0;

[Yazwh0]

This is indeed different to the emulators currently work.

The example below sets the palette offset for the moving sprite to the collision mask in the ISR. The moving sprite has a mask of $f, while the three blocks are $1, $2, $4. So when they collide in the emulators we get the or of those masks moved into the offset. In the hardware it looks like we get the inverse instead.

Should the ~ be there?

Hardware:
https://cdn.discordapp.com/attachments/629863245934755860/1064297468080160860/2023-01-15_14-36-12q.mp4

Emulators:
https://cdn.discordapp.com/attachments/1029612422807433269/1064291499120009358/spritecollisions.gif

[mooinglemur]

I suppose it would depend on @fvdhoef 's intent for the meaning of "mask". By some definitions, such as netmask, setting a mask bit to 1 masks (excludes) those bits from consideration. That's what I see here in the Verilog and that's what I understand as a strict definition of the word mask.

However, the X16 emulators have it the other way around, where a bit set to 0 excludes it from consideration in the collision, and while that might be more intuitive without explicit documentation saying otherwise, that's not what the hardware is doing.

So I'm hoping to get a sense of the actual original intent, and if the VERA's current hardware behavior is what was intended, I'm going to push for a change in the emulators to match.

[Yazwh0]

Additionally, the sprite collision mask in the line buffer is written to without the inversion:

vera-module/fpga/source/graphics/sprite_renderer.v

Line 319 in 1a87b77

assign linebuf_wrdata = {linebuf_rddata[15:12] | sprite_collision_mask_r, 2'b0, sprite_z_r, cur_pixel_color};

[fvdhoef]

My intents are as follows:
Each sprite can be assigned a 4-bit collision mask. For simple cases you could only use one of these bits. If 2 sprites have overlapping non-transparant pixels which have the same collision mask bit set it will indicate this in the Sprite collissions field in the ISR register.

Imagine that you have a r-type style shoot 'm up game. You'd assign the sprites of your space ship mask 0001, the mask of enemy ships 0010. Bullets that you fire 0010 (the same as the enemy ships) and enemy bullets 0001. Now you can just move the sprites along the screen and when an enemy bullet hits your space ship the ISR contains 0001xxxx. When your bullet hits an enemy ship the ISR will be 0010xxxx. If both occur at the same time the ISR will be 0011xxxx. You could also have a bullet that hits both type of ships, for this the collision mask would be 0011.

So basically the collision mask gives you the possibility to group your sprites into 4 categories of objects, for which the collision mask will indicate how they will collide with each other.

[mooinglemur]

Thanks, Frank. I think there's still some confusion, and I think the behavior is even more confused than I originally thought. So I wrote a simple BASIC program to demonstrate the output from the ISR register under certain sprite configurations.

10-25) We initialize two overlapping sprites (from the same memory location, very likely to contain overlapping pixels, but they're disabled at first.
30) We acknowledge the SPRCOL bit 2 in ISR, just in case it was set.
35) Then we enable the sprite layer.
40-50) We enable those two sprites.
60) After a small delay...
70) we see what we expect, no sprite collisions ISR = 00001010
80) We set the first mask bit on sprite 0
90) After a small delay...
95) we also see what we expect. Only one sprite has a collision mask set and we get ISR = 00001010
100) We set the first mask bit on sprite 1
110) After a small delay... (definitely more than a frame)
120) WHOA, this is not what we expected. ISR = 00001010 still. We have two overlapping sprites with the same mask config. Weird.
130) Let's change sprite 1's mask. Clear the first bit, set the second bit isntead.
140) Small delay
150) WEIRD! Now the SPRCOL shows up, but it's for sprite 0's mask and sprite 0's only. ISR = 00011110
160) Let's change sprite 0's mask to match sprite 1's mask. Now they both have the second bit set.
170) Small delay
180) Huh?! The SPRCOL bits are gone (but of course we didn't ever ack it in ISR, so bit 2 is still set from before)

I double-checked the state of ISR and the two sprites by hand after the program finished, and I hope you're as confused as I am :)

I'm certain this VERA is current with the current state of the rev4 branch.

[Yazwh0]

My intents are as follows: Each sprite can be assigned a 4-bit collision mask. For simple cases you could only use one of these bits. If 2 sprites have overlapping non-transparant pixels which have the same collision mask bit set it will indicate this in the Sprite collissions field in the ISR register.

Imagine that you have a r-type style shoot 'm up game. You'd assign the sprites of your space ship mask 0001, the mask of enemy ships 0010. Bullets that you fire 0010 (the same as the enemy ships) and enemy bullets 0001. Now you can just move the sprites along the screen and when an enemy bullet hits your space ship the ISR contains 0001xxxx. When your bullet hits an enemy ship the ISR will be 0010xxxx. If both occur at the same time the ISR will be 0011xxxx. You could also have a bullet that hits both type of ships, for this the collision mask would be 0011.

So basically the collision mask gives you the possibility to group your sprites into 4 categories of objects, for which the collision mask will indicate how they will collide with each other.

Awesome, thats how I thought it should work. It does mean that the ~ above is incorrect.

[fvdhoef]

I don't have the means to test it out at the moment. So if one of you could actually modify this and test this on real hardware, that would be awesome.

[jburks]

I can make the change and test. I think the inversion to sprite_collision_mask_r is incorrect here. If sprite 1 has collision mask 4'b0001 and sprite 2 has collision mask 4'b1111 (it collides with anything that can be collided with), then when a colliding pixel of sprite 2 is drawn we get:

(linebuf_rddata[15:12] & ~sprite_collision_mask_r) => 4'b0001 & ~4'b1111 => 4'b0001 & 4'b0000 => 0 indicating no collision.