fbergama / pigfx

PiGFX is a bare metal kernel for the Raspberry Pi that implements a basic ANSI terminal emulator with the additional support of some primitive graphics functions.
MIT License
278 stars 62 forks source link

Feature (Reverse esc [7m) / Possible issue USB? #51

Closed lindoran closed 3 years ago

lindoran commented 3 years ago

I know because we have color, a lot of the character modes are kind of out of scope for the project. Would it be hard to add ( esc [7m ) for reverse colors (at least with text), just simply a code to flip the foreground and back ground colors? This would help with some programs that use it (or something like it). The trouble is a lot of set up programs for binary modification under CP/M have a 6 character limit, so you can not use color control codes to simulate this as the minimum characters needed to do a color change is 9-11 characters depending on how many characters your color code is.

Another issue. USB keyboards tend to lock up when left idle or if you hold down a button in auto repeat for a long period of time (like over 50 seconds). the heartbeat light stays on but the terminal is otherwise unresponsive. this is with the pi-zero 1.3 (not w). PS/2 doesn't seam to have the same issue (this is with PS/2 hooked up direct being powered with 3v (no level shift needed) --- at first i thought the issue persisted after switching to PS/2 until i shut off usb in the config file; but I really didn't do much testing beyond switching to PS/2 and shutting off USB.

chregu82 commented 3 years ago

This command (ESC[7m is described as "Turn reverse video on". Does anyone know what this actually should do exactly? I don't want to implement a VT100 command the wrong way. I'll have a look at the keyboard issue. At the moment I don't have much time for pigfx.

lindoran commented 3 years ago

After I get back from my doctor appointment I'll have a look at a few terminal emulators and I'll reach out on z80 projects right now and see if somebody can look at the actual hardware.

On Thu, Jan 14, 2021, 4:43 AM Christian Lehner notifications@github.com wrote:

This command (ESC[7m is described as "Turn reverse video on". Does anyone know what this actually should do exactly? I don't want to implement a VT100 command the wrong way. I'll have a look at the keyboard issue. At the moment I don't have much time for pigfx.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/fbergama/pigfx/issues/51#issuecomment-760113600, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQIK5ZB5H5EY5FAOJBU426TSZ3DEHANCNFSM4V6A5WFQ .

lindoran commented 3 years ago

https://en.wikipedia.org/wiki/Reverse_video#/media/File:Visicalc.png

here is an example of reverse video from wikipedia taken from the article:

https://en.wikipedia.org/wiki/ANSI_escape_code

another hiccup I ran into while researching this is; on occasion the use of the intensity code was expected where they were using "reverse". this is problematic likely as PiGFX only supports 1 character set; I'd defer to you as to weather or not that character set supports lettering modes like bold and light modes. one solution may be to implement a method for picking an "off mode" color; with a control code and when the terminal sees bold/light it would switch to those off mode colors... I was looking at the code for the control code parsing. It doesn't look like it would be too hard to use the function gfx_swap_fg_bg() to implement reverse but I was still trying to figure out the syntax for cmd_params_size & cmd_params array when you initially responded ... Im very rough at C but i think we need to add: if( state->cmd_params_size == 1 && state->cmd_params[0] == 7 ) { gfx_swap_fg_bg(); goto back_to_normal; }

to general General 'ESC[' ANSI/VT100 commands under 'm' in the case statement.

On Thu, Jan 14, 2021 at 7:56 AM dave collins lindoranster@gmail.com wrote:

After I get back from my doctor appointment I'll have a look at a few terminal emulators and I'll reach out on z80 projects right now and see if somebody can look at the actual hardware.

On Thu, Jan 14, 2021, 4:43 AM Christian Lehner notifications@github.com wrote:

This command (ESC[7m is described as "Turn reverse video on". Does anyone know what this actually should do exactly? I don't want to implement a VT100 command the wrong way. I'll have a look at the keyboard issue. At the moment I don't have much time for pigfx.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/fbergama/pigfx/issues/51#issuecomment-760113600, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQIK5ZB5H5EY5FAOJBU426TSZ3DEHANCNFSM4V6A5WFQ .

-- if only there was an endless river of information, connecting the world's computer systems; simply cataloging and indexing useful information.... sigh oh well.

chregu82 commented 3 years ago

One thing that I'm not sure about is if the whole screen gets inverted or only what is drawn after the command. What is correct?

lindoran commented 3 years ago

so, just the character is flipped; as if you were putting high lighter on a piece of paper, just the characters that are highlighted would have the changed background. (for esc [7m). for esc [1m, the intensity of the character changes, so on a crt it looks BOLD.. been looking at PuTTY for the last hour or so. how it handles these two codes are it uses a off white (prob gray 254 or 253) color for the default text color set by esc [m and then uses white (color 15 or 255) for esc [1m. I do not know how this works with different default colors, it my implement some form of color 'shift'. The reverse text code esc [7m highlights the letters after the code is called but leaves the ones on the screen that were called before. it'd be nice if there was a way to set a default off color for 1m, by calling a [= code, i have no idea how much extra work that would be.... I was going to look at the source code for a ansi terminal next to se exactly how they do it; its one of those things that are drastically different and there's a lot of license because you are emulating a CRT; and you'll have to kind of approximate the output to look similar... IE on a CRT you would actually just up the intensity for 1m, ... highlight (7m) would just invert the character bitmap 1 would be 0 and 0 would be 1... etc. that was long and rambbly but hopefully it helps.

lindoran commented 3 years ago

I think I may have found a bug in the type selection case statement for the 'm' type codes. the following changes had issues until I changed the if statement for the base code. I was matching it to the case statement for the 'K' series codes and found that the base code for K was coded more or less the way that I changed it to, and the below "sort of" works. The main issue I'm running into now when the terminal resets the color doesn't reset to 8 (at least until you call esc [m). I did change the general init statement at the top of the file ( gfx_set_env ) to reflect the new default color of 8. I'd really to set a code in esc [= (or one of the color codes) to change the default color to change to the esc [1m uses, and maybe the default color. Come to think of it it might be cool if you could set them in the pigfx.txt file.... hope this helps.

made the following changes to gfx.c under the case statement for VT100 - 'm' :

      case 'm':
        //if( state->cmd_params_size == 1 && state->cmd_params[0]==0 )
        if( state->cmd_params_size == 0)
        {
            gfx_set_bg(0);
            gfx_set_fg(8);
            goto back_to_normal;
        }
    if( state->cmd_params_size == 1 && state->cmd_params[0] == 7 )
        {
     gfx_swap_fg_bg();
     goto back_to_normal;
         }
     if( state->cmd_params_size == 1 && state->cmd_params[0] == 1 )
          {
             gfx_set_fg(15);
             goto back_to_normal;
          }

PXL_20210115_034908769 PXL_20210115_034847816 PXL_20210115_035018013

lindoran commented 3 years ago

another update; TLDR it was not a bug but a documentation issue. The expression for 'esc [m' was coded correct for the alternate vt100 code for 'Turn off character attributes' which is esc [0m. the terminal documentation in /doc state to use esc [m but the code is written for esc [0m so on my copy I implemented :

             if( state->cmd_params_size == 0  ||  (state->cmd_params_size == 1 && state->cmd_params[0] == 0 ))
        {
                   gfx_set_bg(0);
                   gfx_set_fg(8);
                   goto back_to_normal;
                }

             if( state->cmd_params_size == 1 && state->cmd_params[0] == 7 )
        {
            gfx_swap_fg_bg();
            goto back_to_normal;
        }

              if( state->cmd_params_size == 1 && state->cmd_params[0] == 1 )
                {
           gfx_set_fg(15);
                   goto back_to_normal;
                }

which works well enough (it just implements both). I also figured out how to change the starting color -- its changed to gray after gfx_set_drawing_mode(drawingNORMAL), right before calling the main terminal loop; on start up in pigfx.c, I just changed it to dark-gray for 8. This all of course leaves the cursor as white.... I don't quite understand the coding for the cursor, I'm still a novice with C.

chregu82 commented 3 years ago

The cursor is always what is under the cursor bitwise inverted. So if the pixel is 0b00000000 (black), the cursor will be 0x11111111 (white). I made a new version in the develop branch. I implemented to reverse mode and several other [m commands. I also added a command for defining the default background and foreground color. Please have a look at it and tell me if it works.

lindoran commented 3 years ago

The codes seam to work exactly as specified in: https://en.wikipedia.org/wiki/ANSI_escape_code#SGR_parameters

And will absolutely work for what I need them for in place of [1m and in fact are a much better choice.

It would also be good put in the rest of the 3/4 bit coding for 90 - 107 for the high intensity colors. it actually doesn't look to hard to put those in, I'm going to play around with that this afternoon if I have time :). 8 bit codeing looks a bit more tricky to fully implement however. another note, right before the terminal starts working on received data it sets the color to GRAY in entry_point (in pigfx.c), but the default color is white? I'm wondering if that was just a oversight. Thank you so much for your continued attention to this project, I really appreciate all you do!

lindoran commented 3 years ago

I just re-read the Wikipedia page and 8 bit was the standard you already put in place; I was confused when I read it but i understand now (sorry about that) ... so basically the only change I would make is adding the high intensity color selection; which seems simple enough, just add a case selection for those codes and then subtract, just like you did with the standard intensity colors... again thanks so much for taking a look at this :-) I always recommend this for project builds over ESP32 based terminals because I think the barrier to entry is MUCH lower...

chregu82 commented 3 years ago

I added the high intensity color selection. PiGFX always used foregound color gray. However the standard foregound color has been white. I changed this now to gray. It makes no sense that used color is gray and default color is white. I'm not sure though what was correct for a real terminal. Google didn't help me either.

lindoran commented 3 years ago

I don't have a clue what the default color on a VT340 would have been (again google no help, Wikipedia as well) -- in the pictures that I've seen of color terminals (actual hardware) a lot of them do seem to use a gray (or normal gray #7) as the color. I think its probably right, i believe Xterm uses this as the normal color as well. --

thing 1. I created a pull request, I found a small typo in GFX.C were the high intensity background color subtractive arithmetic would select the wrong color, it's just subtracting 10 more as the background color range is 100 - 107, so you would want to subtract slightly more to get the correct range of 8-15.

thing 2. -- (TLDR its silly to support both types of intensity color selection, what are your thoughts? ) in testing I found another thing that probably needs thinking through. of the software (at least for CPM that supports color in this way, really the only one i found was gorrilla.bas / gorilla.com. The most recent version is written in Turbo Modula-2, its repo is : https://github.com/sblendorio/gorilla-cpm . I found that for the ANSI terminal definition the program uses the codes for intensity selection 1m, 2m and 22m instead of the 90-107 range. It also seams to use underline and blink ( both sets of codes for that blink, no blink, underline, no underline), which is a whole other can of worms. It seams silly to support both right? -- for one the terminal would have to keep track of the intensity state, and the last color changed; and then when called upon to draw something to the screen it would have to draw the appropriate color. The question that needs to be asked: PiGFX already supports 256 colors through the 8 bit syntax and the full range of 3/4 bit through individual color selection codes (which I think everyone can agree are far better, its less control codes to send to change the color 1 vs 2); does it really need the ability to support a third type of color selection? As for underline and blink would it be nice? maybe ... but I've always looked at PiGFX as what can it do? its a more modern terminal made better by support for key things on the older systems, to me a third type of color selection is kind of feature creep. What are your thoughts? Also I tested it out under the changes I made and it all seams to work correctly :D, thank you again!

chregu82 commented 3 years ago

The initial goal of PiGFX was to be a VT100 compatible terminal emulator. So I think it would be nice if we support as much of these commands as possible. I don't think it's such a big deal to keep track of the attributes that are set to a character, although things like underline and blink are not that easy to do.

lindoran commented 3 years ago

Had some time today; i did a pull request to add the legacy 4 byte intensity codes; this fixes gorilla.bas and it's ports. also fixed a bug? (well -- supported a bug that is a feature lol) where by witch 7m is called and it's not needed so now the terminal only switches to and from reverse text when it's needed. also had a thought about underline for a quick and dirty add. what if we add a terminal state that is underline to CTX and then when the characters are rendered simply make the bottom two or one rows of the character bitmap the foreground color? this still doesn't solve the issue with blink; however it does allow for underline.... I'm still trying to figure out how the terminal draws a character so I was wondering if that's a difficult add or not. blink is most likely more difficult if the terminal just leaves the characters on the screen similar to the way that bitmaps are drawn (not as sprites); if there was a way to draw the characters to the screen but somehow differentiate them from the rest of the drawn bitmaps we could simply toggle the foreground color to the background (similar to the way the cursor blinks -- but in this case the character would be blocked out as a solid block of background character.)

chregu82 commented 3 years ago

I would also try adding an underline to the character at the time it gets drawn. This is static and therfore should work fine. It could be a little more complicated to do it on different fonts. Implementing blinking would probably very painful. You would have to do keep 2 bitmaps of the character and redraw them every time the blink state changes. Another possibility could be what the cursor does, inverting the bits at the pixels which should be blinking. However you would somehow need to keep track of the blinking areas.

lindoran commented 3 years ago

Thinking more on this. Do you think it's a good point to push the changes in development branch to main? I know In my last push I did do the documentation updates but I didn't change the revision number. what do you think? Or should it wait till underline / blink is implemented?

chregu82 commented 3 years ago

I'm going to push it back to main soon.