NuLA extended attributes mode inconsistent with real hardware about b-em HOT 10 CLOSED

What other test cases do we have for NULA attributes mode? If one of us was to make a change to B-Em to make these blocks no longer flash, how do we know we have not caused something else that should flash to stop flashing?

from b-em.

Unfortunately there are zero comprehensive tests of any kind for NULA so the original implementation was somewhat blind (I didn't even have the hardware yet so was going from the manual and PM's with RobC on Stardot.)

The closest thing to a test was the BASIC program I wrote to cycle through the extended MODEs and issues a series of COLOUR statements to check that the foregroud / background combinations are at least correct. See Stardot thread.

The pixel scrolling and left blanking was done using RobC's Shadow of the Beast demo until it looked right...

from b-em.

If it would help I could try to knock up a program to test the attribute modes and someone could run it on real hardware and take a screenshot for reference purposes. (I don't have any real hardware myself.) Let me know if you think this is useful and I'll see what I can do.

from b-em.

OK, here's a first^Wsecond attempt. The last part of this test tests flashing colours and it doesn't work properly on b-em; I think/hope that's a result of this bug, but the test program may be buggy. Barring bugs, the program describes what you should see on the screen for ease of verifying the output is correct.

*VNRESET
*VNVDU ON
MODE 101
REM Define physical colours 8-15 to use distinct colours so they can be
REM visually distinguished. For 9-15 we just use half-bright versions of
REM the standard colours. Physical colour 8 is normally black and we can't
REM use a half-bright version of that, so we use a purpley colour.
PROCphys(8,&5,&4,&C)
FOR I%=9 TO 15
C%=I%-8
R%=0
G%=0
B%=0
IF C% AND 1 THEN R%=&8
IF C% AND 2 THEN G%=&8
IF C% AND 4 THEN B%=&8
PROCphys(I%,R%,G%,B%)
NEXT
DIM name$(15)
RESTORE
FOR I%=0 TO 15
READ name$(I%)
NEXT

CLS
FOR I%=0 TO 7
PROCl2p(I%*2,0):PROCl2p(I%*2+1,I%+1):REM set pair I% to physical I%+1 on black
PROCpair(I%)
PRINT "Pair ";I%;" - ";name$(I%+1);" on black"
NEXT
PROCspace(7)

CLS
FOR I%=0 TO 7
PROCl2p(I%*2,0):PROCl2p(I%*2+1,I%+8):REM set pair I% to physical I%+8 on black
PROCpair(I%)
PRINT "Pair ";I%;" - ";name$(I%+8);" on black"
NEXT
PROCspace(7)
PRINTTAB(0,8);"Press SPACE repeatedly to enable flashing for each colour";
V%=0
FOR I%=0 TO 7
V%=(V% DIV 2)+8
REM PRINTTAB(0,I%+10),I%,~V%
IF I%<=3 THEN ?&FE22=&80+V% ELSE ?&FE22=&90+V%
IF I%=3 THEN V%=0
A%=I%+8
B%=A% EOR 7
PROCpair(I%)
PRINTTAB(0,I%);"Pair ";I%;" - flashing ";name$(A%);"/";name$(B%);" on black"
PROCspace(-1)
NEXT

END

DEF PROCl2p(logical%,physical%)
?&FE21=(logical%*16)+(physical% EOR 7)
ENDPROC

DEF PROCphys(physical%,red%,green%,blue%)
?&FE23=(physical%*16)+red%:?&FE23=(green%*16)+blue%
ENDPROC

DEF PROCpair(pair%)
COLOUR pair%+1
ENDPROC

DEF PROCspace(pair%)
IF pair%>=0 THEN PROCpair(pair%):PRINT "Press SPACE..."
*FX21
REPEAT UNTIL GET=32
ENDPROC

DATA black,red,green,yellow,blue,magenta,cyan,white,purple,half-red,half-green,half-yellow,half-blue,half-magenta,half-cyan,half-white

from b-em.

I don't have much time to look at this at the moment, but something that might be helpful - I remembered that I did add full NULA register dump to the b-em debugger with r vidproc so that should indicate what state it thinks the flash control is in:

VIDPROC registers :
Control=4A
Palette entries :
0=0 1=0 2=0 3=0 4=0 5=0 6=0 7=0
8=0 9=0 10=0 11=0 12=0 13=0 14=0 15=0
NULA palette :
0=000000 1=FF0000 2=00FF00 3=FFFF00 4=0000FF 5=FF00FF 6=00FFFF 7=FFFFFF
8=000000 9=FF0000 10=00FF00 11=FFFF00 12=0000FF 13=FF00FF 14=00FFFF 15=FFFFFF
NULA flash :
11111111
NULA registers :
Palette Mode=0 Horizontal Offset=0 Left Blank Size=0 Disable=0 Attribute Mode=0 Attribute Text=0

from b-em.

Thanks Kieran. It looks to me as though the NULA flash bits are being set as I'd expect using my test program. I think the patch below fixes things (it gives the behaviour I'd expect on the test program above and the newer test program) - but a) I'd appreciate some code review on this b) we definitely need someone to run the newer test program on real hardware to check it behaves as I expect before committing this change. I've posted to stardot asking for a volunteer to do b).

diff --git a/src/video.c b/src/video.c
index ca0a42c..1b6ff70 100644
--- a/src/video.c
+++ b/src/video.c
@@ -146,7 +146,7 @@ void videoula_write(uint16_t addr, uint8_t val)
                         {
                                 for (c = 0; c < 16; c++)
                                 {
-                                        if ((ula_palbak[c] & 8) && nula_flash[c&7]) ula_pal[c] = nula_collook[ula_palbak[c] & 15];
+                                        if ((ula_palbak[c] & 8) && nula_flash[(ula_palbak[c] & 7) ^ 7]) ula_pal[c] = nula_collook[ula_palbak[c] & 15];
                                         else                 ula_pal[c] = nula_collook[(ula_palbak[c] & 15) ^ 7];
                                 }
                         }
@@ -171,7 +171,7 @@ void videoula_write(uint16_t addr, uint8_t val)
                 c = ula_palbak[val >> 4];
                 ula_palbak[val >> 4] = val & 15;
                 ula_pal[val >> 4] = nula_collook[(val & 15) ^ 7];
-                if ((val & 8) && (ula_ctrl & 1) && nula_flash[val - 8]) ula_pal[val >> 4] = nula_collook[val & 15];
+                if ((val & 8) && (ula_ctrl & 1) && nula_flash[val&7]) ula_pal[val >> 4] = nula_collook[val & 15];
         }
         break;
 
@@ -277,7 +277,7 @@ void videoula_write(uint16_t addr, uint8_t val)
                         for (c = 0; c < 16; c++)
                         {
                                 ula_pal[c] = nula_collook[(ula_palbak[c] & 15) ^ 7];
-                                if ((ula_palbak[c] & 8) && (ula_ctrl & 1) && nula_flash[ula_palbak[c] - 8]) ula_pal[c] = nula_collook[ula_palbak[c] & 15];
+                                if ((ula_palbak[c] & 8) && (ula_ctrl & 1) && nula_flash[(ula_palbak[c] & 7) ^ 7]) ula_pal[c] = nula_collook[ula_palbak[c] & 15];
                         }
                         mode7_need_new_lookup = 1;
                 }

from b-em.

Hi Guys,

I'm sure you are aware of this, but VideoNuLA now implements three different attribute based modes:

• normal attribute mode (reg 6 = 1, reg 7 = 0)
• text attribute mode (reg 6 = 1, reg 7 = 1)
• spectrum attribute mode (reg 6 = 2, reg 7 = don't care I think)

It looks like the B-Em emulates normal and text attribute mode, but not the spectrum attribute mode.

Probably we should create a separate issue to track this.

Dave

from b-em.

Rob has confirmed over on stardot (http://stardot.org.uk/forums/viewtopic.php?f=3&t=12150&start=510#p197338) that the new test program behaves as expected on real hardware, which gives me more confidence the fix I posted above is correct. It would still be good if someone could give it a look over though.

I think the title of this issue is misleading; I believe the problem is really in the handling of flashing colours in general, it just happens that I found it while using attribute mode. Should/can I change it?

from b-em.

Steve, I applied your patch to B-Em master and ran the test program. What I see is what the test programs says I should see so your patch is a success. I'll close this and open another one for possible future implementation of Spectrum attributes mode.

from b-em.

Issue #52 opened to track Spectrum attributes.

from b-em.