MEGA65: sprite coordinates are latched (probably)

[lgblgblgb]

https://discord.com/channels/719326990221574164/805252939593416705/1093602744151052429

It seems, Xemu shows "broken" sprites. My theory: MEGA65 probably latches sprite coordinates when starting to render it in a given scanline, thus further changes of sprite coordinates does not affect the rest of the sprite rendering, while on Xemu it does. Below the screenshots. The "brakes at different Y offsets" are probably because the test program is written in BASIC, so it's the effect of "slowness" of the BASIC interpreter.

Paul: "This is likely true, and I think I had to make it that way, as else lots of things on real hardware broke. Maybe I even added a flag to toggle behaviour"

Test program

Test program: easterbunnydemo2.zip

Thanks to Nobato on Discord to mention the problem.

Result on MEGA65

Result in Xemu

Is latching controllable on MEGA65?

Quoting Paul again:

Paul: "This is likely true, and I think I had to make it that way, as else lots of things on real hardware broke. Maybe I even added a flag to toggle behaviour"

What I could found:

GS $D07A.3 VIC-IV:SPTR!CONT Continuously monitor sprite pointer, to allow changing sprite data source while a sprite is being drawn

This register bit may have some control over being latched or not. However the description suggests it's nothing to do with coordinates but the data source. The latter is interesting question as well (related to, but not this bug) however it's unclear if this coordinate latching is controlled with this bit as well, Paul stated:

"That's probably what I was thinking of"

In VHDL, that register bit is signal sprite_continuous_pointer_monitoring.

By checking the usage of this signal in viciv.vhdl - for me at least - it seems like more, it's only about the sprite data source, rather than the coordinates ...

So, is sprite coordinate latching is always on, or should be controllable?

I don't know ... It seems, what Paul thought about is not so much latching the sprite coordinates, but the sprite data. That is interesting as well to implement, for sure, but seems to be another issue/task than this one. From now, I'll assume, sprite coordinates are always latched, and it's not controllable to be turned on/off via a VIC-IV register or something.

The plan for this issue:

1. Let's maintain an internal flag for each sprites "being_rendered".
2. If the sprite Y coordinate matches the current scanline (surely this always means the corrected coordinate in scanline, from now on) then the "being_rendered" flag is turned on, with latching the sprite coordinate into internal variables from VIC registers. If "being_rendered" flag is already on, do not do anything.
3. "being_rendered" flag should be turned off at the last scanline of the sprite
4. During sprite rendering, the internal latched values should be used, instead of the VIC sprite coordinate registers.

I'm really not sure (probably test programs should be written!) what effect can be seen, if sprite enable/disable bit is rapidly altered during the rendering of a sprite, or only X coordinate of the sprite is altered but not Y.

Another question: what would happen if a sprite is not fully rendered in height yet, but sprite Y coordinate is changed by the CPU, in a way that it will be only some scanline more than the previous one. By latching values, surely the original "being_rendered" sprite won't be disturbed, but then, after rendering that sprite, the modified values are never reached to be latched, since it's already passed ... Or, then only the bottom half of the sprite is rendered with the new Y coord? Hmm ...

So the questions for myself in a shorter form:

1. Is X-coord is latched as well?
2. How sprite enable/disable effects latching meanwhile rendering and changing coords?
3. How sprite Y coord changes behaves to be set within the being rendered sprite (some scanline offset only) after finishing rendering at the original latched position?

Surely, other than coordinates, there is the mentioned data source change problem, and probably any other imaginable stuff like X/Y expansion of sprite, sprite width, mode, etc ... But for now, to have a handle'able issue at all, let's focus on coordinates only for now.

@hernandp Do you have anything to add here?

[hernandp]

Nothing to add in particular except what an interesting issue! I don’t remember what is the VIC2 behavior, it was possible to modify any position/size register in any scan line where the sprite sequencer was operating on?

Good catch

On Tue, 11 Apr 2023 at 06:07 LGB @.***> wrote:

https://discord.com/channels/719326990221574164/805252939593416705/1093602744151052429

It seems, Xemu shows "broken" sprites. My theory: MEGA65 probably latches sprite coordinates when starting to render it in a given scanline, thus further changes of sprite coordinates does not affect the rest of the sprite rendering, while on Xemu it does. Below the screenshots. The "brakes at different Y offsets" are probably because the test program is written in BASIC, so it's the effect of "slowness" of the BASIC interpreter.

Paul: "This is likely true, and I think I had to make it that way, as else lots of things on real hardware broke. Maybe I even added a flag to toggle behaviour" Test program

Test program: easterbunnydemo2.zip https://github.com/lgblgblgb/xemu/files/11185140/easterbunnydemo2.zip

Thanks to Nobato on Discord to mention the problem. Result on MEGA65

[image: hardware] https://user-images.githubusercontent.com/884916/230765426-22a7ba1e-c32a-4bed-bafa-47cb4bff7939.jpg Result in Xemu

[image: xemu] https://user-images.githubusercontent.com/884916/230765434-82c7f3d5-ac7c-42e4-a722-8b41bf2f21c8.png Is latching controllable on MEGA65?

Quoting Paul again:

Paul: "This is likely true, and I think I had to make it that way, as else lots of things on real hardware broke. Maybe I even added a flag to toggle behaviour"

What I could found:

GS $D07A.3 VIC-IV:SPTR!CONT Continuously monitor sprite pointer, to allow changing sprite data source while a sprite is being drawn

This register bit may have some control over being latched or not. However the description suggests it's nothing to do with coordinates but the data source. The latter is interesting question as well (related to, but not this bug) however it's unclear if this coordinate latching is controlled with this bit as well, Paul stated:

"That's probably what I was thinking of"

In VHDL, that register bit is signal sprite_continuous_pointer_monitoring.

By checking the usage of this signal in viciv.vhdl - for me at least - it seems like more, it's only about the sprite data source, rather than the coordinates ... So, is sprite coordinate latching is always on, or should be controllable?

I don't know ... It seems, what Paul thought about is not so much latching the sprite coordinates, but the sprite data. That is interesting as well to implement, for sure, but seems to be another issue/task than this one. From now, I'll assume, sprite coordinates are always latched, and it's not controllable to be turned on/off via a VIC-IV register or something. The plan for this issue:

1. Let's maintain an internal flag for each sprites "being_rendered".
2. If the sprite Y coordinate matches the current scanline (surely this always means the corrected coordinate in scanline, from now on) then the "being_rendered" flag is turned on, with latching the sprite coordinate into internal variables from VIC registers. If "being_rendered" flag is already on, do not do anything.
3. "being_rendered" flag should be turned off at the last scanline of the sprite
4. During sprite rendering, the internal latched values should be used, instead of the VIC sprite coordinate registers.

I'm really not sure (probably test programs should be written!) what effect can be seen, if sprite enable/disable bit is rapidly altered during the rendering of a sprite, or only X coordinate of the sprite is altered but not Y.

@hernandp https://github.com/hernandp Do you have anything to add here?

— Reply to this email directly, view it on GitHub https://github.com/lgblgblgb/xemu/issues/374, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEFMBLVYQEDRYZXPJKM7ODXAUNOZANCNFSM6AAAAAAWYADIUY . You are receiving this because you were mentioned.Message ID: @.***>

[lgblgblgb]

@hernandp Not sure if VIC2 works this way, it's quite possible that it's a "MEGA65 thing" more, as Paul stated: "...and I think I had to make it that way, as else lots of things on real hardware broke..." So it's quite possible it's more a workaround on MEGA65 that it's implemented that way, and the bunny program above relies on this behaviour though. In any way, I guess MEGA65 must be followed for a MEGA65 emulator, even if it's maybe not what seems to be logical or "valid C64-herritage" or anything.

My planned modification is something like the patch above. It fixes the problem it seems, however it's somewhat ugly and it's still questionable if x-coord is also latched, or if the enable/disable sprite bit "resets" the latch and allow "re-latching", and things like that. Not even mentioning other possible things to be latched, maybe out of scope of this issue.

However, somehow I feel, this is not the right solution at all, that this latching is deferred to the render process, as it should happen at register write time more. But this raises many questions already with the structure of VIC emulation, and we want to optimize it early or not, etc etc ... Currently almost no processing/decision is done on register writing time, and most things done in rendering, which is not optimal, harder to follow and also "not as correct". However to modify on this scheme, some must be extra careful, like sprite pointer info is scattered throughout many registers, thus each modification on any of those registers should yield recalculation of the pointer. And ideally the renderer part should not make any calculation like the sprite pointer, but using the final "emulation specific" one already. Surely sprite pointer is only an example, the same applies for many-many things, including for example sprite coords (there, though, there is some rendering-time dependency still, ie latched while sprite is "active"). And so on.

Preliminary patch:

diff --git a/targets/mega65/vic4.c b/targets/mega65/vic4.c
index 717ec02..270be2c 100644
--- a/targets/mega65/vic4.c
+++ b/targets/mega65/vic4.c
@@ -75,6 +75,7 @@ static Uint8 vic_pixel_readback_result[4];
 static Uint8 vic_color_register_mask = 0xFF;
 static Uint32 *used_palette;                                   // normally the same value as "palette" from vic4_palette.c but GOTOX RRB token can modify this! So this should be used
 static int EFFECTIVE_V400;
+static Uint8 sprite_is_being_rendered[8];

 // TODO: not really implemented just here ...
 static int etherbuffer_is_io_mapped = 0;
@@ -219,6 +220,7 @@ static XEMU_INLINE void pixel_readback ( void )
 // Do NOT call this function from vic4.c! It must be used by the emulator's main loop!
 void vic4_close_frame_access ( void )
 {
+       memset(sprite_is_being_rendered, 0, sizeof sprite_is_being_rendered);
        DEBUG("FRAME CLOSED" NL);
        // Debug pixel-read back feature of MEGA65
        pixel_readback();
@@ -1102,13 +1104,20 @@ static XEMU_INLINE void vic4_do_sprites ( void )
        // In multicolor mode (MCM=1), the bit combinations "00" and "01" belong to the background
        // and "10" and "11" to the foreground whereas in standard mode (MCM=0),
        // cleared pixels belong to the background and set pixels to the foreground.
+       static int sprite_x_latch[8], sprite_y_latch[8];
        const int reg_tiling = REG_SPRTILEN;
        for (int sprnum = 7; sprnum >= 0; sprnum--) {
                if (REG_SPRITE_ENABLE & (1 << sprnum)) {
                        const int y_adjust = SPRITE_V400(sprnum) ? 0 : (REG_SPRITE_Y_ADJUST - 2);
                        const int spriteHeight = SPRITE_EXTHEIGHT(sprnum) ? REG_SPRHGHT : 21;
-                       const int x_display_pos = (REG_SPR640 ? 1 : 2) * SPRITE_POS_X(sprnum) + (REG_SPR640 ? 1 : SPRITE_FIRST_X);
-                       const int y_display_pos = ((SPRITE_V400(sprnum) ? 1 : 2) * (SPRITE_POS_Y(sprnum) - y_adjust)) ;
+                       int x_display_pos, y_display_pos;
+                       if (sprite_is_being_rendered[sprnum]) {
+                               x_display_pos = sprite_x_latch[sprnum];
+                               y_display_pos = sprite_y_latch[sprnum];
+                       } else {
+                               x_display_pos = (REG_SPR640 ? 1 : 2) * SPRITE_POS_X(sprnum) + (REG_SPR640 ? 1 : SPRITE_FIRST_X);
+                               y_display_pos = ((SPRITE_V400(sprnum) ? 1 : 2) * (SPRITE_POS_Y(sprnum) - y_adjust)) ;
+                       }

                        int sprite_row_in_raster = ycounter - y_display_pos;

@@ -1119,6 +1128,13 @@ static XEMU_INLINE void vic4_do_sprites ( void )
                                sprite_row_in_raster = sprite_row_in_raster >> 1;

                        if (sprite_row_in_raster >= 0 && sprite_row_in_raster < spriteHeight) {
+                               if (sprite_row_in_raster == 0 && !sprite_is_being_rendered[sprnum]) {
+                                       sprite_is_being_rendered[sprnum] = 1;
+                                       sprite_x_latch[sprnum] = x_display_pos;
+                                       sprite_y_latch[sprnum] = y_display_pos;
+                               } else if (sprite_row_in_raster >= spriteHeight - 1) {
+                                       sprite_is_being_rendered[sprnum] = 0;
+                               }
                                const int widthBytes = SPRITE_EXTWIDTH(sprnum) ? 8 : 3;
                                // Mask-out bits 0-3, 23-19 if SPRPTR16 enabled