Matrix Brandy BASIC V for Linux with SDL: V1.21.10 released

discuss PC<>Acorn file transfer issues & the use of FDC, XFER, Omniflop/disk etc.
Post Reply
Soruk
Posts: 105
Joined: Mon Jul 09, 2018 10:31 am
Location: Basingstoke, Hampshire
Contact:

Matrix Brandy BASIC V for Linux with SDL: V1.21.10 released

Post by Soruk » Thu Jul 19, 2018 11:13 am

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 V, where I've been working to fix bugs and add missing features. The biggest chunk to date has been proper Mode 7 support, and it's now rendering everything I can throw at it in exactly the same way as RISC OS 3.7 does in RPCEmu - and since RISC OS's Mode 7 is in RISC OS itself I don't have to worry about the accuracy of RPCEmu - unlike BeebEm. I'm also looking to fix as many bugs as I can that I stumble upon (mainly from code snippets on this forum intended for the BBC) and try to implement as much of the BASIC V language that currently complains it's unsupported. So far I have reading the mouse position and buttons working, and INKEY with negative parameters partly working. So far I have got pretty much everything from this thread viewtopic.php?f=2&t=14919 working.

My development and target platform is Linux, including the Raspberry Pi.

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

Update 20-08-2018: Matrix Brandy V1.21.10 has been released. Changelog on the website.
Last edited by Soruk on Mon Aug 20, 2018 8:25 am, edited 11 times in total.

Phlamethrower
Posts: 72
Joined: Fri Nov 24, 2017 1:35 pm
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Phlamethrower » Thu Jul 19, 2018 8:33 pm

Soruk wrote:
Thu Jul 19, 2018 11:13 am
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).

User avatar
jgharston
Posts: 3132
Joined: Thu Sep 24, 2009 11:22 am
Location: Whitby/Sheffield
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by jgharston » Thu Jul 19, 2018 9:59 pm

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/

Code: Select all

$ bbcbasic
PDP11 BBC BASIC IV Version 0.25
(C) Copyright J.G.Harston 1989,2005-2015
>_

Soruk
Posts: 105
Joined: Mon Jul 09, 2018 10:31 am
Location: Basingstoke, Hampshire
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Soruk » Fri Jul 20, 2018 8:22 am

Phlamethrower wrote:
Thu Jul 19, 2018 8:33 pm
Soruk wrote:
Thu Jul 19, 2018 11:13 am
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.)
Last edited by Soruk on Fri Jul 20, 2018 8:22 am, edited 1 time in total.

Soruk
Posts: 105
Joined: Mon Jul 09, 2018 10:31 am
Location: Basingstoke, Hampshire
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Soruk » Fri Jul 20, 2018 10:01 am

jgharston wrote:
Thu Jul 19, 2018 9:59 pm
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.
Last edited by Soruk on Fri Jul 20, 2018 2:12 pm, edited 2 times in total.

Coeus
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Fri Jul 20, 2018 10:15 pm

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
Posts: 105
Joined: Mon Jul 09, 2018 10:31 am
Location: Basingstoke, Hampshire
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Soruk » Fri Jul 20, 2018 10:49 pm

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.
Attachments
remtest.zip
Zip file with remtest.ffb inside
(184 Bytes) Downloaded 7 times
Last edited by Soruk on Fri Jul 20, 2018 10:51 pm, edited 1 time in total.

Coeus
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Sun Jul 22, 2018 10:35 am

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

Coeus
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Sun Jul 22, 2018 11:32 am

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;
Last edited by Coeus on Sun Jul 22, 2018 11:35 am, edited 1 time in total.

scruss
Posts: 34
Joined: Sun Jul 01, 2018 3:12 pm
Location: Toronto
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by scruss » Sun Jul 22, 2018 1:42 pm

Soruk wrote:
Thu Jul 19, 2018 11:13 am
… 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
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Sun Jul 22, 2018 6:45 pm

scruss wrote:
Sun Jul 22, 2018 1:42 pm
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
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Sun Jul 22, 2018 7:55 pm

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:
Screenshot from 2018-07-22 20-51-29.png
The Brandy output has some extra white dots in the black areas.

Soruk
Posts: 105
Joined: Mon Jul 09, 2018 10:31 am
Location: Basingstoke, Hampshire
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Soruk » Mon Jul 23, 2018 7:38 am

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).
Attachments
restoretest.zip
Another banana choker.
(210 Bytes) Downloaded 5 times
Last edited by Soruk on Mon Jul 23, 2018 7:44 am, edited 1 time in total.

Coeus
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Mon Jul 23, 2018 1:44 pm

Soruk wrote:
Mon Jul 23, 2018 7:38 am
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
Posts: 105
Joined: Mon Jul 09, 2018 10:31 am
Location: Basingstoke, Hampshire
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Soruk » Mon Jul 23, 2018 2:57 pm

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
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Mon Jul 23, 2018 8:22 pm

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.
Last edited by Coeus on Mon Jul 23, 2018 8:22 pm, edited 1 time in total.

Soruk
Posts: 105
Joined: Mon Jul 09, 2018 10:31 am
Location: Basingstoke, Hampshire
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Soruk » Tue Jul 24, 2018 10:06 am

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

Coeus
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Fri Jul 27, 2018 12:06 am

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
Attachments
mexhat.zip
(82.43 KiB) Downloaded 4 times
Last edited by Coeus on Fri Jul 27, 2018 2:46 pm, edited 1 time in total.

Coeus
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Fri Jul 27, 2018 6:03 pm

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.
Last edited by Coeus on Fri Jul 27, 2018 6:09 pm, edited 2 times in total.

Soruk
Posts: 105
Joined: Mon Jul 09, 2018 10:31 am
Location: Basingstoke, Hampshire
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Soruk » Fri Jul 27, 2018 8:38 pm

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!)

User avatar
jgharston
Posts: 3132
Joined: Thu Sep 24, 2009 11:22 am
Location: Whitby/Sheffield
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by jgharston » Fri Jul 27, 2018 9:10 pm

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!

Code: Select all

$ bbcbasic
PDP11 BBC BASIC IV Version 0.25
(C) Copyright J.G.Harston 1989,2005-2015
>_

User avatar
hoglet
Posts: 7361
Joined: Sat Oct 13, 2012 6:21 pm
Location: Bristol
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by hoglet » Sat Jul 28, 2018 7:17 am

jgharston wrote:
Fri Jul 27, 2018 9:10 pm
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

User avatar
fordp
Posts: 958
Joined: Sun Feb 12, 2012 9:08 pm
Location: Kent, England
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by fordp » Sat Jul 28, 2018 12:53 pm

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?
FordP (Simon Ellwood)
Time is an illusion. Lunchtime, doubly so!

Soruk
Posts: 105
Joined: Mon Jul 09, 2018 10:31 am
Location: Basingstoke, Hampshire
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Soruk » Sat Jul 28, 2018 6:22 pm

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.

User avatar
Elminster
Posts: 2649
Joined: Wed Jun 20, 2012 8:09 am
Location: Essex, UK
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Elminster » Sat Jul 28, 2018 7:05 pm

Soruk wrote:
Sat Jul 28, 2018 6:22 pm
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

User avatar
Richard Russell
Posts: 368
Joined: Sun Feb 27, 2011 10:35 am
Location: Downham Market, Norfolk
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Richard Russell » Sun Jul 29, 2018 11:31 am

Coeus wrote:
Sun Jul 22, 2018 7:55 pm
The Brandy output has some extra white dots in the black areas.
For comparison, here is the output from BBCSDL (running in Windows):

Image

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
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Sun Jul 29, 2018 3:11 pm

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
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Mon Jul 30, 2018 1:26 am

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:
master.png
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.
bbcb.png
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:
masreplay.png
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:
brandy.png
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.
Attachments
mc.pdf
(86.44 KiB) Downloaded 6 times
Last edited by Coeus on Mon Jul 30, 2018 1:37 am, edited 1 time in total.

User avatar
Richard Russell
Posts: 368
Joined: Sun Feb 27, 2011 10:35 am
Location: Downham Market, Norfolk
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Richard Russell » Mon Jul 30, 2018 8:21 am

Coeus wrote:
Mon Jul 30, 2018 1:26 am
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
Posts: 900
Joined: Mon Jul 25, 2016 11:05 am
Contact:

Re: Matrix Brandy BASIC V for Linux with SDL

Post by Coeus » Mon Jul 30, 2018 9:08 am

Richard Russell wrote:
Mon Jul 30, 2018 8:21 am
...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.

Post Reply