Weird colors with fvdi aranym

[faucon2001]

FVDI latest builds for Aranym ( I have tested every version in the repository) give wrong colors with phweather and zview. See Screenshot : https://ibb.co/Q82JFzL tested with aranym 1.1.0, emutos 1.1.1, fvdi 0.96a-* and Mint 1.19.1a9

[mfro0]

Can you say when this first occured for you?

I looked all the way back until summer last year and couldn't find a commit that would affect the Aranym driver or palette/colour handling.

[chrisridd]

I'm seeing possibly similar problems. Using the ancient FVDI and NVDI5.PAL from François's miniPack, the resulting palette looks OK. Index 0 is 1000/1000/1000 for instance. But using master, the palette starts 1000/0/0 and indeed all the window backgrounds are a nice bright red.

Here's the palette from the miniPack. It seems to me to be starting 1000/1000/1000. The other files are printing vq_color(0..255, 0..1, &rgb)

badpalette.zip

I wonder if @th-otto's change bcdbbbae73 from 10 May 2020 is relevant. Before, it seems like palette files without a "PA01" header would actually fail to load. So if I add those 4 bytes to the start of the file, the colours on master now look OK.

So to me it looks like the palette file loading was broken before, and is more correct now - with the downside that old palette files will now look different. WDYT? Can @faucon2001 check if FVDI is trying to load a custom palette before which was malformed?

I did start trying to bisect to find the commit changing this, but unfortunately I can only build back as far as June 18 2020, and that was still "red".

François's FVDI claims to be 0.968 so must predate 654b9631e2 (15 May 2020), and that's as far as my detective skills went.

[chrisridd]

I found a bug in bcdbbba that's causing my strange colours. #14 is open to fix it.

[mfro0]

Is it ok to close this one, then?

[chrisridd]

If @faucon2001 could check a build with my palette fix in, that'd be best. It might be a different problem.

[faucon2001]

I am currently using a fairly old version of fvdi : 0,968b3 from 2014. So it's difficult to tell you when this bug appears.

I have tested this build https://tho-otto.de/snapshots/fvdi/fvdi-0.96a-ft2.10.2-eb2-020.zip I have updated fvdi.prg and aranym.sys and it still shows wrong colors. But, if I update only fvdi.prg and leave the old aranym.sys, colors are correct.

[chrisridd]

Can you upload a similar screenshot which looks right? Is it just phweather and zview?

[faucon2001]

With wrong colors : https://ibb.co/3FDh2rV With good colors : https://ibb.co/rsbrxPy I have noticed the same wrong color issue with Netsurf and Highwire.

[chrisridd]

Using aranym in 800x608x32 I can reproduce this with the zview 1.0 binary and the 1.0.2 binary from github, just opening the "Browser" window.

But if I just change fvdi.sys to use:

01r aranym.sys mode 1024x768x24@72 assumenf

the problem disappears. I guess the "x24" is the important part. Can you try using x24?

[chrisridd]

Zview's interesting, it doesn't use vr_trnfm() at all and instead it directly builds device-specific MFDBs and passes them to vro_cpyfm() or vrt_cpyfm(). I got zview to open a tiny PNG file with 16 solid blue followed by 16 solid white pixels and dumped out the MFDB. The bytes I saw started:

ff000000ff000000ff000000ff000000[...etc]

I'd have expected a sequence of 00 00 00 ff, so for some reason zview is screwing up the channel order. Indeed, the screen looks to have black pixels instead of blue.

The white pixels looked like this:

ffffff00ffffff00ffffff00ffffff00[...etc]

Again, I've have expected 00 ff ff ff. The white pixels look yellow, which would be ff ff 00. So that's consistent.

(It looks like true-colour MFDBs are actually stored as packed pixels, not plane-by-plane. Not seen this documented anywhere, or if this is peculiar to the aranym driver.)

Zview is using vq_scrninfo() so I wonder if we're reporting something different there in 32-bit mode.

[chrisridd]

Got it! Apologies for the tedious debugging thread...

The drivers/aranym/spec.c file's set_scrninfo() changed in e125ee4c and introduced this regression.

Before, the driver set (for 32bpp) mode[6] which is described as "usual bit order". mode[6] was:

{32, CHECK_PREVIOUS | CHUNKY | TRUE_COLOUR, {r_32,  g_32,  b_32,  none, none, none}, 0, 2, 2, 1}

In e125ee4c there's a new "scrninfo fb" option. The default for fb_scrninfo is 0, but by passing that string you can set fb_scrninfo to 1. All fine. Now though:

• fb_scrninfo == 1: (32bpp) returns mode[9]
• fb_scrninfo == 0: (32bpp) returns mode[6]

Fine again, except here's the new mode[6] (selected by default):

{ 32, CHECK_PREVIOUS | CHUNKY | TRUE_COLOUR, { r_32f, g_32f, b_32f, none, none, none }, 0, 2, 2, 1 },

Note the r/g/b bit arrays are different from before. The contents of all the r/g/b arrays has not changed.

What's the right fix @mfro0 ? I don't know the history behind r_32 vs r_32f etc, so while swapping the new mode[6] to use r_32/g_32/b_32 and mode[9] to use r_32f/g_32f/b_32f would "work", I don't know what it'll break. It also looks odd - mode[4] and mode[5] use the "f" arrays.

Or should the r_32 array contain the values from r_32f and vice-versa? Again, what would that break? I'll do a PR for that, obviously feel free to reject it. Maybe @th-otto can comment?

[vinriviere]

The drivers/aranym/spec.c file's set_scrninfo() changed in e125ee4 and introduced this regression.

Good catch, Chris! That commit message was "Make some variables static/const". But it also changed other things, namely tab/space changes, which makes the commit unreadable. And hidden into that mess, there was that "packed pixels, software clut (none), fb layout" thing that introduced the regression. Very hard to find, when hidden in such a way.

We will never say it enough: 1 functional change = 1 commit

[chrisridd]

Well, mistakes always happen however careful you are.

Regarding the whitespace/formatting changes, perhaps there should be a single "Big Bang" mechanical reformat of everything, and an automated PR/merge check that the format's still intact? It does IME make following code changes across the reformat a bit trickier, but no pain no gain?

Clang has a pretty good formatting tool. Commit your .clang-format file to git (maybe some bike shedding here to decide on the style) and away you go. I don't know how it handles 68k assembler. Several IDEs appear to support clang-format as well.

[faucon2001]

Thanks Chris. It works well now.

[chrisridd]

Awesome!

[mfro0]

fixed with 1a6851d