Matrix Brandy BASIC VI for console and SDL1.2: V1.23.1 released

[Soruk]

Hi,

As I've mentioned elsewhere on the forum where I've found some suitable test harness samples, I've recently started hacking on the source code for Brandy BASIC V by David Daniels. Sadly his Sourceforge repo has not seen any updates since 2015 and submitted patches have gone ignored.

So I've decided to fork it, and created Matrix Brandy BASIC.

Some of the new/missing features added (this list updated since I first started the thread):
* MODE 7
* Networking
* 8-bit and 64-bit integers
* INKEY(-ve) - partial
* Enhanced graphics
* 24-bit colour
* Mouse support
* Sound
* Some new BASIC features inspired by BBCSDL, Basalt and JGHarston's additions
* Host library calls
* ...and of course the obligatory bug fixes (and likely new ones inserted!)

Supported platforms (that I can test on):

Code: Select all

,--------------------------------.
|       OS       | SDL | Console |
|--------------------------------|
| Linux x86      |  X  |    X    |
| Linux x86-64   |  X  |    X    |
| Linux ARM-32   |  X  |    X    |
| Linux ARM-64   |  X  |    X    |
| Minix 3        |  -  |    X    |
| Solaris        |  X  |    X    | (Testing using Solaris 11.4 x86_64 from Oracle's prebuilt VirtualBox VM library)
| Windows x86    |  1  |    1    |
| Windows x86-64 |  1  |    1    |
| MacOS Catalina |  2  |    X    | (virtualised hackintosh)
| RISC OS 3.5+   |  -  |    3    |
`--------------------------------'

Notes:
1) Windows builds known to work on Windows 7 and Windows 10 (tested on Win10 64-bit). Appears to work reasonably well on Windows XP SP3 (32-bit). Known NOT to work on Windows 2000. My builds are built on Windows 7 32-bit and 64-bit. The supplied makefiles use Cygwin as a build environment but uses the MinGW toolset so not reliant on any Cygwin component at runtime.
2) Requires a recent development version of SDL 1.2 (a working snapshot available on the Matrix Brandy website).
3) Provides full access to the native RISC OS VDU driver, including graphics and MODE 7. It runs as a single-tasking application, but can multitask with the help of applications such as !GraphTask. A VFP build is also available for RISC OS on the Raspberry Pi.

Building and running against sdl12-compat (the SDL1.2 compatibility layer on top of SDL2) is tested and works fine, indeed the builds in my Yum/DNF repositories for EL9 and Fedora are all built against this.

Other UNIX-type platforms should also work, OpenBSD 6.7 is reported to work fine.

The console builds are great for using BBC BASIC as a scripting language, indeed you can use the standard Unix #!/path/to/sbrandy as the first line with your program (with or without line numbers) on the lines below it, and make your script executable and run it like any other shell, Perl or Python (etc) script.

My development and target platform is Linux, including the Raspberry Pi, although I am also targeting Windows and more recently MacOS X and RISC OS.

Unlike BBCSDL, Matrix Brandy does not use an IDE, the SDL builds offer a BBC Micro-like screen which is still command-driven, and immediate-mode programming is always available. The two text builds available offer differing text capabilities, "tbrandy" is the more fully-featured version which offers some colour capability via ANSI codes (and optionally, graphics via Tektronix control codes), while "sbrandy" is very minimal and ideal for cron and CGI scripting.

The source code is always available for download from http://brandy.matrixnetwork.co.uk/ and now on Github at https://github.com/soruk42/MatrixBrandy - and patches are most welcome!

Additional discussion that doesn't fit the Stardot theme is welcome over at The Distillery.

Update 21-03-2023: Matrix Brandy VI 1.22.15 has been released. Changelog on the website.

[Phlamethrower]

submitted patches have gone ignored

Sure looks that way!

Over ten years ago I found a couple of bugs and submitted some patches (or at least I hope I did). It looks like one of them (fix RND(N) to return numbers in the range 1 to N) has made it in to your version, but the other one hasn't.

In fileio.c, fileio_openup() needs fixing to return 0 if the file couldn't be opened, the same as fileio_openin() does. Make sure you're looking at the non-RISC OS version of the functions (they're about halfway down the file, after the RISC OS ones).

If in doubt, testing with Bob and Trev: Resurrection should hopefully reveal the bugs pretty quickly (since that's what I was working on when I originally found them).

[jgharston]

Some years ago I made a fork and updated and corrected a lot of the MOS interface implementation, and like the previous post none of it got merged into the main branch (if I'm using theose terms correctly). I was planning on sitting down and merging in your teletext code, but got distracted by some PDP11 stuff: http://mdfs.net/Software/BBCBasic/Brandy/

[Soruk]

submitted patches have gone ignored

Sure looks that way!

Over ten years ago I found a couple of bugs and submitted some patches (or at least I hope I did). It looks like one of them (fix RND(N) to return numbers in the range 1 to N) has made it in to your version, but the other one hasn't.

In fileio.c, fileio_openup() needs fixing to return 0 if the file couldn't be opened, the same as fileio_openin() does. Make sure you're looking at the non-RISC OS version of the functions (they're about halfway down the file, after the RISC OS ones).

If in doubt, testing with Bob and Trev: Resurrection should hopefully reveal the bugs pretty quickly (since that's what I was working on when I originally found them).

Thank you for the pointer - OPENUP fixed, and now behaving the same as RISC OS 3.7 (tested against RPCEmu).

I hadn't seen you had done a patch as well for RND(N) - I'd spotted the problem back in 2015 and also submitted a patch (here - https://sourceforge.net/p/brandy/patches/14/ )

(Edit: typo. My typing is even worse before coffee.)

[Soruk]

Some years ago I made a fork and updated and corrected a lot of the MOS interface implementation, and like the previous post none of it got merged into the main branch (if I'm using theose terms correctly). I was planning on sitting down and merging in your teletext code, but got distracted by some PDP11 stuff: http://mdfs.net/Software/BBCBasic/Brandy/

I'm having a go at merging your Banana (diffed against 1.20.1) into Matrix, so far so good. Just need to run a barrage of tests against it.

Update: A bug in the Banana version of tokens.c:reformat(), where the token sometimes gets re-inserted on the end of a line.
It's easy to replicate. On RISC OS, save the following program:

Code: Select all

10 REM =========

and load it into Banana Brandy. A spurious REM appears on the end of the line. It's also not restricted to REM, it also happens with other commands, I've noticed RESTORE as well. This is going to take a bit of hunting to work out what's going on here.

Update 2: Struggling to unravel this - so, not the nicest workaround, but if ftype == 2 it uses JGH's code, else it uses the mainline code.

[Coeus]

I don't have access to RiscOS so I can't jump in and look but if you want a second pair of eyes on this and can attach that test file as saved from RiscOS I will see if I can take a look.

[Soruk]

Thank you, here it is.

It doesn't trip up my released version (Matrix Brandy) but it is tripping up JGH's Banana Brandy version. I have mostly completed the merge but I want to run more tests before I upload V1.21.5. I've had to zip it up, the forum didn't like the unzipped file.

Since Richard Russell's code isn't open source I'm not including any files from his BBCSDL release - but you can download that free from his website, and in there are some .bbc files which (appear to) load fine in Banana.

[Coeus]

So the first thing I notice is that banana is not 64 bit compatible as it is assigning pointers to 32 bit integers.

[Coeus]

Ok, so I have worked out what was happenning. Referring to tokens.c in JGH's banana120.zip at lines 2171 to 2170 REM is handled as a special case:

Code: Select all

        if (token == ACORN_REM || token == ACORN_DATA) { /* REM or DATA - Copy rest of line */
          p = onebyte_token[token-ACORNONE_LOWEST];
          strcpy(cp, p);
          cp+=strlen(p);
          tp++;
          while (*tp != ACORN_ENDLINE) {
            *cp = *tp;
            cp++;
            tp++;
          }

so the strcpy inserts the word REM and the loop copies to the end of the line. After the multi-branch if/else control then ends up at the code which is responsible for possibly inserting spaces around the expanded token with the comment starting on line 2238:

Code: Select all

      if (cp  !=  &line[0] && isalnum(*(cp-1))) {       /* If keyword is preceded by a letter or a digit, add a blank */
        *cp = ' ';
        cp++;
      }
      strcpy(cp, p);
      cp+=strlen(p);

. On line 2252 another strcpy also inserts REM, this time after the rest of the line. The REM handling is an odd case because all the other branches of that if/else finish by having set p to the correct entry in the token table but not actually inserting the expanded version.

As there is no further processing of the source (Acorn tokenised) line to be done after the REM-specific copy loop has finished, but we do need to run the re-tokenise into brandy internal form on line 2271:

Code: Select all

  *cp = NUL;    /* Complete the line */
  tokenize(line, tokenbuf, HASLINE, FALSE);
  return get_linelen(tokenbuf);

the easiest fix is to exit early from the loop on line 2153:

Code: Select all

  while (token != ACORN_ENDLINE) {

so here's that as a patch:

Code: Select all

--- banana-orignal/src/tokens.c	2014-04-05 17:41:14.000000000 +0100
+++ banana-patched/src/tokens.c	2018-07-22 12:18:32.109522256 +0100
@@ -2178,6 +2178,7 @@
             cp++;
             tp++;
           }
+          break;
         } else {      /* Tokens */
           if (token == 0xCDu) {                                           /* CD    */
             p=tp+1;

[scruss]

… I've recently started hacking on the source code for Brandy BASIC V by David Daniels

Thank you!

I'm not in a position to submit useful patches, but Brandy has always had a vexing relationship with its launching terminal. If you launch brandy & or put the job in the background with bg, it loses keyboard access. This is probably baked deeply into the code, so may likely be more bother than it's worth to fix. As is, Brandy can't launch without an active terminal controlling it.

[Coeus]

I'm not in a position to submit useful patches, but Brandy has always had a vexing relationship with its launching terminal. If you launch brandy & or put the job in the background with bg, it loses keyboard access. This is probably baked deeply into the code, so may likely be more bother than it's worth to fix. As is, Brandy can't launch without an active terminal controlling it.

Having looked at the keyboard module it seems it will accept key events from SDL or characters arriving from the controlling terminal. That means it responds whether I focus the brandy window or the terminal window from which I started it.

That does seem a little strange. Normally I'd expect code that uses SDL for output to use SDL alone for keyboard events, but there is also some code to deal with the case stdin has been redirected to come from a file and this may be much more useful.

The immediate issue with running brandy in the background is that the kernel sends it SIGTTOU when it tries to alter the settings on its controlling terminal as part of the keyboard initialisation. The default action of that signal is, quite reasonably, to stop the program so you can finish doing whatever is in the foreground on that terminal then bring the background job to the foreground to interact with it. Setting the action for this signal, and SIGTTIN, to ignore means attempt to perform operations on the terminal now fail instead but brandy seems to be ok with that as it still has SDL keyboard events so this patch will enable background working:

Code: Select all

--- brandy-1.21.2-orig/src/keyboard.c	2018-07-03 11:27:00.000000000 +0100
+++ brandy-1.21.2-patched/src/keyboard.c	2018-07-22 19:32:55.911532692 +0100
@@ -160,6 +160,7 @@
 /* ==================================================================== */
 
 #include <stdlib.h>
+#include <signal.h>
 
 #if defined(TARGET_LINUX) | defined(TARGET_NETBSD) | defined(TARGET_MACOSX)\
  | defined(TARGET_DJGPP) | defined(TARGET_FREEBSD) | defined(TARGET_OPENBSD)\
@@ -1561,6 +1562,8 @@
     basicvars.runflags.inredir = TRUE;
     return TRUE;
   }
+  (void) signal(SIGTTIN, SIG_IGN);
+  (void) signal(SIGTTOU, SIG_IGN);
   origtty = tty;                /* Preserve original settings for later */
 #ifdef TARGET_LINUX
   tty.c_lflag &= ~(XCASE|ECHONL|NOFLSH);

[Coeus]

There appears to be a small discrepancy in the output of the Mexican Hat program in the short BASIC type-ins thread between B-Em and Brandy. Here's the program:

Code: Select all

   10 MODE 0
   20VDU23,224,&AA,&55,&AA,&55,&AA,&55,&AA,&55
   30FOR Y%=0 TO 31
   40FOR X%=0 TO 79
   50IF (X% AND 8)<>(Y% AND 4)*2 THEN VDU 31,X%,Y%,224
   60NEXT
   70NEXT
   80VDU 30
   90ANGLE%=40
  100S%=20
  110C=COS(RAD(ANGLE%))
  120S=SIN(RAD(ANGLE%))
  130VDU29,640;512;
  140FOR X%=-500 TO 500 STEP S%
  150FOR Y%=500 TO -500 STEP -S%
  160PROCP(4,X%,Y%,FNA(X%,Y%))
  170PROCP(4,X%,Y%+S%,FNA(X%,Y%+S%))
  180PROCP(87,X%+S%,Y%,FNA(X%+S%, Y%))
  190PROCP(87,X%+S%,Y%+S%,FNA(X%+S%, Y%+S%))
  200PROCP(4,X%,Y%,FNA(X%,Y%))
  210PROCP(5,X%+S%,Y%,FNA(X%+S%, Y%))
  220PROCP(5,X%+S%,Y%+S%,FNA(X%+S%, Y%+S%))
  230PROCP(5,X%,Y%,FNA(X%,Y%))
  240NEXT Y%
  250NEXT X%
  260END
  270DEF PROCP(K%,X%,Y%,Z%)
  280PLOT K%,X%*C+Y%*S,(-X%*S+Y%*C) DIV 3+Z%
  290ENDPROC
  300DEF FNA(X%,Y%)
  310D%=SQR(X%*X%+Y%*Y%)
  320=SIN(RAD(D%))*200

and here are the screendumps side by side, Brandy on the left, B-Em on the right:

The Brandy output has some extra white dots in the black areas.

[Soruk]

Another one that seems to trip up Banana is "RESTORE" - I can't see any code that's specific to that token. Which also makes me a bit worried that there's other tokens that'll get mis-handled. Test case saved from RISC OS attached.

That hat doodle is another odd one - I've got that typed in here too - but I'll try with a cut'n'paste of what you have here as it's possible I have a typo as I'm seeing something different to either of your screenshots!

Edit: Yup, a typo. But yes, I'm seeing the same artifacting you're seeing, which I'm not seeing in RPCEmu (RISC OS 3.7).

[Coeus]

Another one that seems to trip up Banana is "RESTORE" - I can't see any code that's specific to that token.

No there's nothing special about RESTORE but there is something special about a tokenised line number which immediately follows it. Once again the code that handles the line number is sufficient without the general piece of code to insert an expanded token, possibly surrounded by spaces, so that code was being run in error. RESTORE happened to be the last token chosen from the table.

So here's another patch - this re-arranges the ifs so that the "insert a token maybe surrounded by spaces" is only run for the non-special cases. This supersedes the REM-specific patch.

Code: Select all

--- banana-orignal/src/tokens.c	2014-04-05 17:41:14.000000000 +0100
+++ banana-patched/src/tokens.c	2018-07-23 14:39:09.847630979 +0100
@@ -2162,79 +2162,69 @@
           tp++;
         } while (token != '\"' && *tp != ACORN_ENDLINE);
       }
-    } else {
-      if (token == ACORN_LINENUM) {
-        count = sprintf(cp, "%d", expand_linenum(tp+1));
-        cp+=count;
-        tp+=ACORN_LINESIZE;
-      } else {
-        if (token == ACORN_REM || token == ACORN_DATA) { /* REM or DATA - Copy rest of line */
+    } else if (token == ACORN_LINENUM) {
+      count = sprintf(cp, "%d", expand_linenum(tp+1));
+      cp+=count;
+      tp+=ACORN_LINESIZE;
+    } else if (token == ACORN_REM || token == ACORN_DATA) { /* REM or DATA - Copy rest of line */
+      p = onebyte_token[token-ACORNONE_LOWEST];
+      strcpy(cp, p);
+      cp+=strlen(p);
+      tp++;
+      while (*tp != ACORN_ENDLINE) {
+        *cp = *tp;
+        cp++;
+        tp++;
+      }
+    } else {      /* Tokens */
+      if (token == 0xCDu) {                                           /* CD    */
+        p=tp+1;
+        while(*p == ' ') p++;
+        if (*p == CR || *p == ':') {
           p = onebyte_token[token-ACORNONE_LOWEST];
-          strcpy(cp, p);
-          cp+=strlen(p);
-          tp++;
-          while (*tp != ACORN_ENDLINE) {
-            *cp = *tp;
-            cp++;
-            tp++;
-          }
-        } else {      /* Tokens */
-          if (token == 0xCDu) {                                           /* CD    */
-            p=tp+1;
-            while(*p == ' ') p++;
-            if (*p == CR || *p == ':') {
-              p = onebyte_token[token-ACORNONE_LOWEST];
-            } else {
-              p = bbcbyte_token[token-ACORN_OTHER];
-            }
-          } else {
-            if (token >= RUSSELL_LOWEST && token <= RUSSELL_HIGHEST) {    /* 01-10 */
-              p = lowbyte_token[token-RUSSELL_LOWEST];
-            } else {
-              if (token < ACORN_OTHER || token > ACORN_TWOBYTE) {         /* 7F-C5, C9-FF */ 
-                p = onebyte_token[token-ACORNONE_LOWEST];
+        } else {
+          p = bbcbyte_token[token-ACORN_OTHER];
+        }
+      } else if (token >= RUSSELL_LOWEST && token <= RUSSELL_HIGHEST) {    /* 01-10 */
+        p = lowbyte_token[token-RUSSELL_LOWEST];
+      } else if (token < ACORN_OTHER || token > ACORN_TWOBYTE) {         /* 7F-C5, C9-FF */ 
+        p = onebyte_token[token-ACORNONE_LOWEST];
+      } else if (ftype == 2) {
+        p = winbyte_token[token-ACORN_OTHER];
+      } else {
+        token2 = *(tp+1);
+        if (token2 < ACORNTWO_LOWEST) {
+          p = bbcbyte_token[token-ACORN_OTHER];                 /* Cx <8E  */
+        } else {
+          switch (token) {
+            case ACORN_TWOBYTE:                                 /* C8 nn   */
+              if (token2>ACORNTWO_HIGHEST) {
+                p = bbcbyte_token[token2-ACORN_OTHER];          /* C8      */
               } else {
-                if (ftype == 2) {
-                  p = winbyte_token[token-ACORN_OTHER];
-                } else {
-                  token2 = *(tp+1);
-                  if (token2 < ACORNTWO_LOWEST) {
-                    p = bbcbyte_token[token-ACORN_OTHER];                 /* Cx <8E  */
-                  } else {
-                    switch (token) {
-                      case ACORN_TWOBYTE:                                 /* C8 nn   */
-                        if (token2>ACORNTWO_HIGHEST) {
-                          p = bbcbyte_token[token2-ACORN_OTHER];          /* C8      */
-                        } else {
-                          p = twobyte_token[token2-ACORNTWO_LOWEST];      /* C8 8E+n */
-                          tp++;
-                          break;
-                        }
-                      case ACORN_COMMAND:                                 /* C7 nn   */
-                        if (token2>ACORNCMD_HIGHEST) {
-                          p = bbcbyte_token[token2-ACORN_OTHER];          /* C7      */
-                        } else {
-                          p = command_token[token-ACORNCMD_LOWEST];       /* C7 8E+n */
-                          tp++;
-                          break;
-                        }
-                      case ACORN_OTHER:                                   /* C6 nn   */
-                        if (token2>ACORNOTH_HIGHEST) {
-                          p = bbcbyte_token[token2-ACORN_OTHER];          /* C6      */
-                        } else {
-                          p = other_token[token-ACORNOTH_LOWEST];
-                          tp++;
-                          break;
-                        }
-                    } /* switch */
-                  }
-                }
+                p = twobyte_token[token2-ACORNTWO_LOWEST];      /* C8 8E+n */
+                tp++;
+                break;
               }
-            }
-          }
-          tp++;
+            case ACORN_COMMAND:                                 /* C7 nn   */
+              if (token2>ACORNCMD_HIGHEST) {
+                p = bbcbyte_token[token2-ACORN_OTHER];          /* C7      */
+              } else {
+                p = command_token[token-ACORNCMD_LOWEST];       /* C7 8E+n */
+                tp++;
+                break;
+              }
+            case ACORN_OTHER:                                   /* C6 nn   */
+              if (token2>ACORNOTH_HIGHEST) {
+                p = bbcbyte_token[token2-ACORN_OTHER];          /* C6      */
+              } else {
+                p = other_token[token-ACORNOTH_LOWEST];
+                tp++;
+                break;
+              }
+          } /* switch */
         }
       }
+      tp++;
 /*
 ** Because the code expands tokenised Acorn Basic to text then
 ** retokenises it, blanks are added around keywords if there are

[Soruk]

You, sir, are a genius. Thank you! I had totally missed the fact line numbers themselves were being tokenised.

I'll merge this into Matrix Brandy - then, assuming my test suite doesn't collapse in a heap, I think 1.21.5 will be ready.

Then, I will see what's going on with the artifacting in the hat doodle.

[Coeus]

Here's another little patch, this time against 1.21.5 which fixes the compiler warnings I am getting from gcc 8.1.1.

Code: Select all

--- brandy-1.21.5-orignal/src/graphsdl.h	2018-07-23 21:14:26.031982341 +0100
+++ brandy-1.21.5-patched/src/graphsdl.h	2018-07-23 21:10:44.438623115 +0100
@@ -0,0 +1,6 @@
+#ifndef GRAPHSDL_INC
+#define GRAPHSDL_INC
+
+extern void get_sdl_mouse(int32 values[]);
+
+#endif
--- brandy-1.21.5-orignal/src/graphsdl.c	2018-07-20 12:26:01.000000000 +0100
+++ brandy-1.21.5-patched/src/graphsdl.c	2018-07-23 21:11:19.198625886 +0100
@@ -42,6 +42,7 @@
 #include "scrcommon.h"
 #include "screen.h"
 #include "mos.h"
+#include "graphsdl.h"
 
 /*
 ** Notes
--- brandy-1.21.5-orignal/src/mos.h	2018-07-20 23:06:55.000000000 +0100
+++ brandy-1.21.5-patched/src/mos.h	2018-07-23 21:05:54.215293016 +0100
@@ -36,6 +36,7 @@
 extern int32 mos_rdtime(void);
 extern void  mos_wrtime(int32);
 extern void  mos_rdrtc(char *);
+extern void  mos_wrrtc(char *);
 extern void  mos_call(int32, int32, int32 []);
 extern int32 mos_usr(int32);
 extern void  mos_sys(int32, int32[], int32[], int32*);
@@ -51,6 +52,7 @@
 extern void  mos_mouse(int32 []);
 extern void  mos_wrbeat(int32);
 extern int32 mos_rdbeat(void);
+extern int32 mos_rdbeats(void);
 extern void  mos_wrtempo(int32);
 extern int32 mos_rdtempo(void);
 extern void  mos_voice(int32, char *);
--- brandy-1.21.5-orignal/src/mos.c	2018-07-23 08:29:06.000000000 +0100
+++ brandy-1.21.5-patched/src/mos.c	2018-07-23 21:12:09.931964193 +0100
@@ -78,6 +78,10 @@
 #include <unistd.h>
 #endif
 
+#ifdef USE_SDL
+#include "graphsdl.h"
+#endif
+
 int check_command(char *text);
 
 /* Address range used to identify emulated calls to the BBC Micro MOS */
@@ -1213,7 +1217,7 @@
   clen=strlen(command) + 256;
   cmdbufbase=malloc(clen);
   cmdbuf=cmdbufbase;
-  strncpy(cmdbuf, command, clen-1);
+  memcpy(cmdbuf, command, clen-1);
 
   if (!basicvars.runflags.ignore_starcmd) {
 /*

The last change - the strncpy to memcpy may not be so obvious. gcc warns about using strncpy when the limit is calculated from the length of the source because it assumes it should be related to the destination and is a protection against buffer overrun. But, specifically because we know the length of the source string we can copy that exact number of characters rather than worrying about the NUL at the end, hence mempcy works just as well and doesn't cause the warning.

[Soruk]

Thank you! I've also amended the makefile to reflect the new graphsdl.h file being used by graphsdl.c and mos.c

[Coeus]

Ok, I have discovered what appears to be another bug. I thought, with respect to the graphic artefact in the Mexican hat I would try to prove if this was the VDU emulation doing something different, or some difference in the arithmetic in the BASIC interpreter. To do this I ran the original Mexican Hat on B-Em and captured the bytes sent by the VDU driver into a spool file and then used a short C program on Linux to turn these into aseries of VDU statements in a BASIC program. When run from within brandy, that program should "replay" what was sent to the VDU drivers by the original program running on B-Em.

When run, the program gets as far as drawing the chequerboard and the crashes with a segmentation fault:

Code: Select all

Thread 1 "brandy" received signal SIGSEGV, Segmentation fault.
0x000055555557bf6c in exec_vdu () at src/iostate.c:1668
1668	      if (*basicvars.current == ',')
(gdb) bt
#0  0x000055555557bf6c in exec_vdu () at src/iostate.c:1668
#1  0x000055555556f046 in exec_statements (lp=<optimized out>)
    at src/statement.c:391
#2  0x000055555556f12f in run_program (lp=0x7ffff7ea8014 "\001")
    at src/statement.c:430
#3  0x000055555556f046 in exec_statements (lp=<optimized out>)
    at src/statement.c:391
#4  0x000055555556f1c8 in exec_thisline () at src/statement.c:467
#5  0x000055555558d215 in interpret_line () at src/brandy.c:297
#6  run_interpreter () at src/brandy.c:338
#7  0x0000555555561bc5 in main (argc=3, argv=0x7fffffffe6f8) at src/brandy.

I will see what I can do later, but in the meantime I attach the generated BASIC program that causes the crash. This needs brandy to be started with -s 1M

[Coeus]

Here's a patch that fixes this:

Code: Select all

--- brandy-1.21.5-orignal/src/graphsdl.c	2018-07-20 12:26:01.000000000 +0100
+++ brandy-1.21.5-patched/src/graphsdl.c	2018-07-27 18:46:23.275668164 +0100
@@ -2086,7 +2087,7 @@
   int32 x, y;
   x = vduqueue[1]+vduqueue[2]*256;
   if (x > 0x7FFF) x = -(0x10000-x);	/* X is negative */
-  y = vduqueue[3]+vduqueue[3]*256;
+  y = vduqueue[3]+vduqueue[4]*256;
   if (y > 0x7FFF) y = -(0x10000-y);	/* Y is negative */
   emulate_plot(vduqueue[0], x, y);	/* vduqueue[0] gives the plot code */
 }
@@ -3868,9 +3869,13 @@
 
   /* set highest and lowest points to visit */
   for (i = 0; i < n; i++) {
-    if (y[i] > high)
+    if (y[i] > MAX_YRES)
+      y[i] = high = MAX_YRES;
+    else if (y[i] > high)
       high = y[i];
-    if (y[i] < low)
+    if (y[i] < 0)
+      y[i] = low = 0;
+    else if (y[i] < low)
       low = y[i];
   }

The segmentation fault was caused because buff_convex_poly does not check that the passed-in values of X and Y will not exceed the bounds of the arrays geom_left and geom_right. Exceeding the bounds of these arrays corrupts the state of the main structure basic_variables and that then causes the crash. This is fixed by the second change in the patch (although I fixed it first):

Code: Select all

@@ -3868,9 +3869,13 @@
 
   /* set highest and lowest points to visit */
   for (i = 0; i < n; i++) {
-    if (y[i] > high)
+    if (y[i] > MAX_YRES)
+      y[i] = high = MAX_YRES;
+    else if (y[i] > high)
       high = y[i];
-    if (y[i] < low)
+    if (y[i] < 0)
+      y[i] = low = 0;
+    else if (y[i] < low)
       low = y[i];
   }

The reason the y dimension was out of bounds in the first place is because, when calculating the 16bit y co-ordinate from two values in the VDU queue, it was using byte 3 twice instead of using 3 and 4. This is what the other part fixes:

Code: Select all

@@ -2086,7 +2087,7 @@
   int32 x, y;
   x = vduqueue[1]+vduqueue[2]*256;
   if (x > 0x7FFF) x = -(0x10000-x);	/* X is negative */
-  y = vduqueue[3]+vduqueue[3]*256;
+  y = vduqueue[3]+vduqueue[4]*256;
   if (y > 0x7FFF) y = -(0x10000-y);	/* Y is negative */
   emulate_plot(vduqueue[0], x, y);	/* vduqueue[0] gives the plot code */
 }

And, after that, the result of the test is that the graphics artefact is in the plot emulation, not in the calculations in running the BASIC program itself.

[Soruk]

Thank you - looking at the PLOT routines is definitely on my roadmap. In the meantime, I've just pushed out V1.21.6, which includes your patch, along with a little bit of extra functionality (and something that seems to be a "thing" at the moment - a Telstar client!)

[jgharston]

Wow, I turn my back for a bit to fix a deep-down bug in PDP BASIC (turns out it was fine, it was that MUL was implemented incorrectly in the emulator), and a huge swath of bugfixes and merges for Brandy. How are you managing that in this heat, it just switches me off!

[hoglet]

Wow, I turn my back for a bit to fix a deep-down bug in PDP BASIC (turns out it was fine, it was that MUL was implemented incorrectly in the emulator), and a huge swath of bugfixes and merges for Brandy. How are you managing that in this heat, it just switches me off!

Jonathan, have you tried MUL on the PDP-11 Co Pro in either PiTubeDirect or the Matchbox?

(There is also a rather nice debugger in PiTubeDirect which is worth trying if you have a serial cable)

Dave

[fordp]

I should have a go a t a bit of hacking. Most of the other projects are on the stardot github could the code at least be mirrored there?

[Soruk]

Is there a pointer on how to get set up on the Stardot github? Currently I'm hosting on my personal git server. I can set up remote SSH git access to it if there is interest.

[Elminster]

Is there a pointer on how to get set up on the Stardot github? Currently I'm hosting on my personal git server. I can set up remote SSH git access to it if there is interest.

You mean to get added? Ages ago we used this thread. Not sure if that is still the way we do it.

viewtopic.php?f=55&t=12203&start=30&hil ... dot+GitHub

[Deleted User 9295]

The Brandy output has some extra white dots in the black areas.

For comparison, here is the output from BBCSDL (running in Windows):



This is performing the calculations at 80-bit floating-point precision, compared with Brandy's 64-bits and the Beeb's 40-bits. I don't know what graphics package Brandy is using, but BBCSDL is using SDL_gfxPrimitives.c.

Richard.

[Coeus]

Is the way brandy writes variables to a file intended to be compatible with the way BASIC 2 and BASIC 4 do? If I write a file in brandy containg a mix of integer and floating point I get a type mismatch error when trying to read the file from BASIC 2. Looking at viewtopic.php?f=54&p=208672#p208434 it looks like brandy is not putting the leading &FF before each floating point value it writes.

[Coeus]

I have found another small twist in the artefacts. I ran a modified version of the mexican hat program to capture the VDU sequence generated:

Code: Select all

   10*SPOOL mexvdu
   20 MODE 0
   30VDU23,224,&AA,&55,&AA,&55,&AA,&55,&AA,&55
   40FOR Y%=0 TO 31
   50FOR X%=0 TO 79
   60IF (X% AND 8)<>(Y% AND 4)*2 THEN VDU 31,X%,Y%,224
   70NEXT
   80NEXT
   90VDU 30
  100ANGLE%=40
  110S%=20
  120C=COS(RAD(ANGLE%))
  130S=SIN(RAD(ANGLE%))
  140VDU29,640;512;
  150FOR X%=-500 TO 500 STEP S%
  160FOR Y%=500 TO -500 STEP -S%
  170PROCP(4,X%,Y%,FNA(X%,Y%))
  180PROCP(4,X%,Y%+S%,FNA(X%,Y%+S%))
  190PROCP(87,X%+S%,Y%,FNA(X%+S%, Y%))
  200PROCP(87,X%+S%,Y%+S%,FNA(X%+S%, Y%+S%))
  210PROCP(4,X%,Y%,FNA(X%,Y%))
  220PROCP(5,X%+S%,Y%,FNA(X%+S%, Y%))
  230PROCP(5,X%+S%,Y%+S%,FNA(X%+S%, Y%+S%))
  240PROCP(5,X%,Y%,FNA(X%,Y%))
  250NEXT Y%
  260NEXT X%
  270*SPOOL
  280END
  290DEF PROCP(K%,X%,Y%,Z%)
  300PLOT K%,X%*C+Y%*S,(-X%*S+Y%*C) DIV 3+Z%
  310ENDPROC
  320DEF FNA(X%,Y%)
  330D%=SQR(X%*X%+Y%*Y%)
  340=SIN(RAD(D%))*200

so this creates a file mexvdu which contains all the bytes sent to the VDU driver to draw the hat. I was running this with B-Em emulating a Master Turbo and here is the screen output:

which looks clean but may actually be missing something.
Then I tried replaying the file of VDU bytes using *PRINT (which, unlike *TYPE, is a binary copy and doesn't expand control characters etc) on a Model B. This loooked very slighly different - look at the bottom centre near the turn-up.

Just to check this wasn't something corrupting the stream of VDU bytes I replayed that on the Master again and got the same image as the first time:

So, already, the BBC B with MOS 1.20 and the Master with 3.20 have slighly differences in what the VDU drivers do with the same data stream. With the aid of a small C program:

Code: Select all

#include <stdio.h>

int main(int argc, char **argv)
{
    int line = 0;
    int x = 0, ch;

    while ((ch = getchar()) != EOF) {
        ch &= 0xff;
        if (x == 0) {
            printf("%uVDU%u", ++line, ch);
            x = 1;
        }
        else {
            printf(",%u", ch);
            if (++x == 20) {
                putchar('\n');
                x = 0;
            }
        }
    }
    putchar('\n');
}

I was able to convert the stream of bytes into a BASIC program which consists entirely of VDU statements in sequence. Runninbg this on Brandy gives an image that looks different again:

so this is getting three different results using the same set of data and different again from BBC BASIC for SDL from Richard Russel (see above). The points were only calculated once on the BBC Master with Acorn 5 byte FP. So this does suggest the difference is in the way PLOT is implemented, either in the VDU drivers or in the more modern BASICs.

Interestingly, translating the BASIC into C and using a library called cairo gives this PDF version at it looks like, rather than leaving some spurious dots as the fault, the fault may be partially erasing lines that are supposed to be there and are there on the PDF version when working at a higher resolution.

[Deleted User 9295]

So this does suggest the difference is in the way PLOT is implemented, either in the VDU drivers or in the more modern BASICs.

That's to be expected, in my opinion. An emulator (like BeebEm) should of course produce identical results to the original Beeb, but a native implementation of BBC BASIC on a different platform is going to be leveraging the graphics drivers of that platform (for performance reasons, if nothing else). So for example BBC BASIC for Windows uses the Windows GDI subsystem to draw graphics, BBC BASIC for SDL 2.0 uses the SDL_gfx subsystem (which delegates to OpenGL) etc. One would expect the results to be quite similar, but not pixel-identical (not least because the pixels themselves may be a different size/shape).

Richard.

[Coeus]

...BBC BASIC for SDL 2.0 uses the SDL_gfx subsystem (which delegates to OpenGL) etc. One would expect the results to be quite similar, but not pixel-identical (not least because the pixels themselves may be a different size/shape).

I did check and didn't see anything that suggests Brandy uses SDL_gfx but then Brandy uses SDL 1 - perhaps SDF_gfx is an SDL 2 feature. It is hard to be sure what is available in SDL 1 as the documentation doesn't seem to be available. Maybe that is a deliberate ploy to get people to migrate and that is something to think about with Brandy.

On the question of pixel size and shape this raises and interesting question. In an emulator, obviously the number of pixels has to match the original hardware so that routines written in assembler, either in the OS or in games, get the expected results, but I am not sure if a compatible BASIC needs to do the same. I'd be tempted, for example, to use the same number of host pixels as the logical number of pixels in the RiscOS screen modes rather than the number of physical pixels, i.e. 1280x1024 in most modes unless the host doesn't have that many pixels available.

It rather depends on what one sees as the main use case. In my view, if one wants a complete nostalgia trip an emulator may be more appropriate where the old system is often implemented "warts and all" whereas a host implementaton of a compatible BASIC is to run programs from back in the day but also recapture the interactive development and easy access to graphics, sound etc. on modern machines. But then it's Soruk's decision for Brandy as he is now maintaining it.