MEGA65: VIC-IV NCM mode emulation refinements

[lgblgblgb]

As @ki-bo stated in bug #328 :

Thanks! I just tested the hyppo branch build. If color $f is set, it uses the lower 4 bits of Colour RAM byte 1 as intended. That's good! But I think we need to look further.

One can also set bits 4-7 in Col RAM byte 1, and these bits are also utilised in NCM mode (on real Mega65). For this to be used, one must reset bit ATTR (5) of $d031. The current fix seems to ignore the Col RAM bits 4-7 in case Screen Memory sets color $f, as it seems.

Originally posted by @ki-bo in #328 (comment)

As that issue is more about the original "index 15 is foreground colour in NCM", let's move any further NCM based emulation problem here.

Surely the question is, if there are more fundamental problems here, or only still index 15 problem should be refined somewhat.

[ki-bo]

One more thing I just noticed: if ATTR (bit 5 of $d031) is set, the VIC-IV is still applying attribute effects to the NCM chars (eg. blinking). This is not implemented, yet. If an NCM char is recognised, Xemu will always render as if ATTR is unset.

[lgblgblgb]

I think+IIRC blink is interpreted that it's more a character mode only feature and nothing to do with gfx (though I have to admit, there is not always sense to introduce notions like "text" and "gfx" since there is no clear boundary, it's more correct to say, that from perspective how vic4.c is structured). Unfortunately to allow all the possibility of all bits/modes/etc/etc in all combination, is not always easy ... There are several limitations like this, for example I have never checked of classic VIC-II style bitmap mode can be somehow mixed with 16-bit addressing and/or allowing flip and other attribute stuffs. So most of these are implemented more at places "where it makes sense" to avoid complexity and major slow-down of emulation.

I would say, for sure, the goal should be a precise emulation. Thus "on demand" basis if something seems to be problematic, may need to be look after more deeply, though. At least in my practice, the "write a correct emulation in once" scenario never works, or never released ;) so an iterating process is more productive to always have "something" at least (like the open source motto: "release early, release often").

[ki-bo]

I agree the emulation needs to stick to typical use cases to keep the complexity and performance under control. And using the new char modes with the C65 attributes activated probably does not make sense in most cases. ;-)

I would still fix the issue ignoring the 16col palette pointer for pixel value $f, since this might be relevant for real programs wanting to go beyond 16 colours in that mode.

[lgblgblgb]

@ki-bo Sure thing, especially that the fix is kinda complexity/performance irrelevant, since Xemu has emulation at scanline basis/precision only, thus the calling parameters for the given "row" renderers happening only at every scanline, which is much more cheap than adding extra features per pixel for example.

For the first part, if you see function vic4_render_mono_char_row() in vic4.c you see how those attributes are handled. Basically to allow those to work in other row renderers as well, that logic needs to be ported there as well, either by placing into a function and calling from each, or inline'ing them more, for performance reasons, it surprisingly affect the performance in some cases to have decision like this.

But also note, that as this point, vic4.c as a whole is quite underoptimized, since it wouldn't worth it, it's more important to gain good emulation and compatibility first, rather than over-optimizing it, and making hard to fix bugs, or add missing emulated features in a sane way.

[lgblgblgb]

Excuse me, I'm a bit lost now with my very bad idea to have two issues opened ;) Can I close this? Is there no issue left? Or what is left is the "VIC3-style attribute bits" (like BLINK) not applied in NCM mode, but the fg/bg/index-15/.. issue is fully resolved now? Sorry for being a bit confused after all of this :)

[ki-bo]

Yes, I would agree to close it. #328 was fixed by your commit (respecting lower four bits of colour ram) and my commit added support for the upper four bits in colour ram. Your merge of the fix includes both #328 and this one.

The attribute support is not really relevant to implement, I suppose. So, again, let's just close it.