search

lgblgblgb / xemu

Emulations (running on Linux/Unix/Windows/macOS, utilizing SDL2) of some - mainly - 8 bit machines, including the Commodore LCD, Commodore 65, and the MEGA65 as well.
https://github.com/lgblgblgb/xemu/wiki
GNU General Public License v2.0
207 stars 32 forks source link

MEGA65: VIC-IV emulation problem, always limiting char_fgcolor to 4 bits is bad #370

Closed lgblgblgb closed 1 year ago

lgblgblgb commented 1 year ago

See on Discord: https://discord.com/channels/719326990221574164/781481205639020554/1075744677082452080

Basically: VIC-IV emulation code has this line: const Uint8 char_fgcolor = color_data & 0xF; and then char_fgcolor is used at several places, even in FCM (256 colours). Though it seems it's incorrect.

Though I can fix this with the "and mask", I guess one solution is more simple: let's forget about this char_fgcolor totally and remove it. Instead, every references of char_fgcolor must be replaced with color_data surely with the appropriate "and mask"! Surely it must not be forgotten that color_data is a 16 bit variable! This even simplifies things, like this:

(used_palette + (color_data & 0xF0))[char_fgcolor] becomes simply this: used_palette[color_data & 0xFF]

My patch is at: https://github.com/lgblgblgb/xemu/commit/e93802dd5f5fe23bdd9df857fc612fc884f7b6c4

@hernandp If you have some time, please have a look on this, if you have any objections. Committed to the next branch. Otherwise I'll close this issue. However some minor side note: maybe all the "FIXME" and "TODO" around the changes should be reviewed some time as well, and either those comments should be removed if everything is OK, or the problems should be addressed if not OK.

hernandp commented 1 year ago

hi @lgblgblgb it is clear than ANDing the foreground color to 0xF seems to be a legacy from the first VIC-IV implementation work -- It worked until guys started playing hard with FCM! I looked at your patch and seems completely reasonable, with the exception (and you realized the same) of text-mode.. Look at https://github.com/MEGA65/mega65-core/blob/74d636b1766dcab1923af439b9fd6d7247ae58c9/src/vhdl/viciv.vhdl#L4259

I need to look this closely.

hernandp commented 1 year ago

I think that your 0xF mask in the !BMM branch is correct. Agree with your patch :+1:

lgblgblgb commented 1 year ago

Ok, thanks for the feedback. Honestly, I am lazy and I haven't been able to find the willpower yet to dig into the VHDL of VIC-IV specific things, easier to ask ... errmm :-O Closing now then.