This version of the patchset updates a few things since the one posted early
on Wednesday:

NEW - Fix two off-by-ones that caused spurious trailing null characters when
      requesting the terminal IDs.  (Noticed by Paul running tmux.)

NEW - Discard any parameters passed to a CSI 38 m or CSI 48 m sequence.
      See below for more discussion on this.

NEW - Tweaked dim attribute to work correctly on byte-swapped framebuffers.

* Control sequences for dim text, invisible text, strike through, italic, and
  double underline attributes are now recognised and set flags in wscons.
* Rendering of dim, invisible, struck, and double underlined text is now
  supported in rasops32.c, so users of 32bpp displays will see these text
  effects on the console.
* The default number of scrollback screens has been increased from five to
  twenty.
* F1 - F4 and F13 - F24 now send different sequences.
* With numlock OFF, keypad 5 is now 'begin' rather than unused.
* Home, End, keypad home, and keypad end now send different sequences.
* A new keysym has been added - KS_Backtab.
* Shift-TAB is now defined as KS_Backtab and sends ESC [ Z.
* Shift-F1 - Shift-F12 now send F13 - F24.
* Five new escape sequences added for cursor movement.

Off by ones:

These can actually be triggered with the existing console code, without even
applying any of my patches, and stem from using sizeof with a string constant,
which returns the length of the string _including_ the terminating null.

CSI 38 m and CSI 48 m sequences:

The 30-37 codes set one of eight fixed foreground colours, the 40-47 codes set
the background in the same way.  Codes 39 and 49 reset to the default colours.

Codes 38 and 48 have become used as an escape to a larger colour space,
typically 256 colours or 24-bit colour.

A typical invocation to select colour 100 from a 256 colour palette might be:

CSI 38;5;100m

Such sequences are not defined in the terminfo entry for xterm.  However, some
programs use them, assuming that they are supported.

The current wscons code ignores the unimplemented 38, but then proceeds to
interpret 5 and 100 as further SGR codes.

In many cases, this doesn't much of a problem, because although 5 enables
blink, none of the rasops code supports blink anyway.  However, the last value
can potentially set any SGR attribute.  So, for example:

CSI 38;5;4m

...will not select colour 4 as the program is expecting, but instead turn on
underlining.

To avoid this, the lastest version of my patch simply discards all values
after a 38 or 48, until the trailing 'm'.

This works to avoid setting unwanted attributes, until such time as 256 colour
support is added, (if this is even desired).

--- dev/wscons/wsemul_vt100_keys.c.dist Sat Mar 14 00:38:50 2015
+++ dev/wscons/wsemul_vt100_keys.c      Mon Jan  2 16:01:42 2023
@@ -37,11 +37,9 @@
 #include <dev/wscons/wsemulvar.h>
 #include <dev/wscons/wsemul_vt100var.h>
 
+#define vt100_fkeys_len(x) (5+(x>=8)+(x>=12))
+
 static const u_char *vt100_fkeys[] = {
-       "\033[11~",     /* F1 */
-       "\033[12~",
-       "\033[13~",             /* F1-F5 normally don't send codes */
-       "\033[14~",
        "\033[15~",     /* F5 */
        "\033[17~",     /* F6 */
        "\033[18~",
@@ -50,18 +48,18 @@
        "\033[21~",
        "\033[23~",     /* VT100: ESC */
        "\033[24~",     /* VT100: BS */
-       "\033[25~",     /* VT100: LF */
-       "\033[26~",
-       "\033[28~",     /* help */
-       "\033[29~",     /* do */
-       "\033[31~",
-       "\033[32~",
-       "\033[33~",
-       "\033[34~",     /* F20 */
-       "\033[35~",
-       "\033[36~",
-       "\033[37~",
-       "\033[38~"
+       "\033[1;2P",    /* VT100: LF */
+       "\033[1;2Q",
+       "\033[1;2R",    /* help */
+       "\033[1;2S",    /* do */
+       "\033[15;2~",
+       "\033[17;2~",
+       "\033[18;2~",
+       "\033[19;2~",   /* F20 */
+       "\033[20;2~",
+       "\033[21;2~",
+       "\033[23;2~",
+       "\033[24;2~"
 };
 
 static const u_char *vt100_pfkeys[] = {
@@ -96,14 +94,22 @@
                    edp->translatebuf, edp->flags & VTFL_UTF8));
        }
 
-       if (in >= KS_f1 && in <= KS_f24) {
-               *out = vt100_fkeys[in - KS_f1];
-               return (5);
+       if (in >= KS_f1 && in <= KS_f4) {
+               *out = vt100_pfkeys[in - KS_f1];
+               return (3);
        }
-       if (in >= KS_F1 && in <= KS_F24) {
-               *out = vt100_fkeys[in - KS_F1];
-               return (5);
+       if (in >= KS_F1 && in <= KS_F4) {
+               *out = vt100_pfkeys[in - KS_F1];
+               return (3);
        }
+       if (in >= KS_f5 && in <= KS_f24) {
+               *out = vt100_fkeys[in - KS_f5];
+               return vt100_fkeys_len(in - KS_f5);
+       }
+       if (in >= KS_F5 && in <= KS_F24) {
+               *out = vt100_fkeys[in - KS_F5];
+               return vt100_fkeys_len(in - KS_F5);
+       }
        if (in >= KS_KP_F1 && in <= KS_KP_F4) {
                *out = vt100_pfkeys[in - KS_KP_F1];
                return (3);
@@ -148,12 +154,12 @@
        }
        switch (in) {
            case KS_Help:
-               *out = vt100_fkeys[15 - 1];
+               *out = vt100_fkeys[15 - 1 + 4]; /* vt100_fkeys starts at F5 */
                return (5);
            case KS_Execute: /* "Do" */
-               *out = vt100_fkeys[16 - 1];
+               *out = vt100_fkeys[16 - 1 + 4]; /* vt100_fkeys starts at F5 */
                return (5);
-           case KS_Find:
+           case KS_Find:                       /* Not defined in xterm 
terminfo */
                *out = "\033[1~";
                return (4);
            case KS_Insert:
@@ -163,7 +169,7 @@
            case KS_KP_Delete:
                *out = "\033[3~";
                return (4);
-           case KS_Select:
+           case KS_Select:                     /* Not defined in xterm 
terminfo */
                *out = "\033[4~";
                return (4);
            case KS_Prior:
@@ -174,14 +180,27 @@
            case KS_KP_Next:
                *out = "\033[6~";
                return (4);
+           case KS_Backtab:
+               *out = "\033[Z";
+               return (3);
+           /*
+            * Unlike insert, delete, page up, and page down, we purposely don't
+            * send the same sequence of \033OE for the non-keypad 'begin' key.
+            *
+            * This is because the terminfo xterm entry is mapping this to kb2,
+            * which is defined as 'centre of keypad'.
+            */
+           case KS_KP_Begin:
+               *out = "\033OE";
+               return (3);
            case KS_Home:
            case KS_KP_Home:
-               *out = "\033[7~";
-               return (4);
+               *out = "\033OH";
+               return (3);
            case KS_End:
            case KS_KP_End:
-               *out = "\033[8~";
-               return (4);
+               *out = "\033OF";
+               return (3);
            case KS_Up:
            case KS_KP_Up:
                if (edp->flags & VTFL_APPLCURSOR)
--- dev/wscons/wsemul_vt100_subr.c.dist Mon May 25 06:55:49 2020
+++ dev/wscons/wsemul_vt100_subr.c      Thu Jan  5 12:29:31 2023
@@ -231,7 +231,7 @@
        switch (A3(edp->modif1, edp->modif2, c)) {
        case A3('>', '\0', 'c'): /* DA secondary */
                wsdisplay_emulinput(edp->cbcookie, WSEMUL_VT_ID2,
-                   sizeof(WSEMUL_VT_ID2));
+                   sizeof(WSEMUL_VT_ID2)-1);
                break;
 
        case A3('\0', '\0', 'J'): /* ED selective erase in display */
@@ -461,6 +461,9 @@
                edp->ccol -= min(DEF1_ARG(0), edp->ccol);
                edp->flags &= ~VTFL_LASTCHAR;
                break;
+       case 'G': /* HPA */
+               edp->ccol = min(DEF1_ARG(0)-1, edp->ncols-1);
+               break;
        case 'H': /* CUP */
        case 'f': /* HVP */
                if (edp->flags & VTFL_DECOM)
@@ -502,16 +505,37 @@
                WSEMULOP(rc, edp, &edp->abortstate, erasecols,
                    ERASECOLS(NCOLS - n, n, edp->bkgdattr));
                break;
+       case 'S': /* INDN */
+               wsemul_vt100_scrollup (edp,DEF1_ARG(0));
+               break;
+       case 'T': /* RIN */
+               wsemul_vt100_scrolldown (edp,DEF1_ARG(0));
+               break;
        case 'X': /* ECH erase character */
                n = min(DEF1_ARG(0), COLS_LEFT + 1);
                WSEMULOP(rc, edp, &edp->abortstate, erasecols,
                    ERASECOLS(edp->ccol, n, edp->bkgdattr));
                break;
+       case 'Z': /* KCBT */
+               if (!edp->ccol)
+                       break;
+               if (edp->tabs) {
+                       for (n=edp->ccol-1 ; n>0; n--)
+                               if (edp->tabs[n])
+                                       break;
+               } else {
+                       n=((edp->ccol - 1) & 8);
+               }
+               edp->ccol=n;
+               break;
        case 'c': /* DA primary */
                if (ARG(0) == 0)
                        wsdisplay_emulinput(edp->cbcookie, WSEMUL_VT_ID1,
-                           sizeof(WSEMUL_VT_ID1));
+                           sizeof(WSEMUL_VT_ID1)-1);
                break;
+       case 'd': /* VPA */
+               edp->crow = min(DEF1_ARG(0)-1, edp->nrows-1);
+               break;
        case 'g': /* TBC */
                if (edp->tabs != NULL)
                        switch (ARG(0)) {
@@ -549,6 +573,12 @@
                        case 1: /* bold */
                                flags |= WSATTR_HILIT;
                                break;
+                       case 2: /* dim */
+                               flags |= WSATTR_DIM;
+                               break;
+                       case 3: /* italic */
+                               flags |= WSATTR_ITALIC;
+                               break;
                        case 4: /* underline */
                                flags |= WSATTR_UNDERLINE;
                                break;
@@ -558,11 +588,31 @@
                        case 7: /* reverse */
                                flags |= WSATTR_REVERSE;
                                break;
-                       case 22: /* ~bold VT300 only */
+                       /*
+                        * Invisible text only makes the _glyph_ invisible.
+                        *
+                        * Other active attributes such as underlining and
+                        * strikeout are still displayed in the character cell.
+                        */
+                       case 8: /* invisible */
+                               flags |= WSATTR_INVISIBLE;
+                               break;
+                       case 9: /* strike */
+                               flags |= WSATTR_STRIKE;
+                               break;
+                       case 21: /* double underline */
+                               flags |= WSATTR_DOUBLE_UNDERLINE;
+                               break;
+                       case 22: /* ~bold ~dim VT300 only */
                                flags &= ~WSATTR_HILIT;
+                               flags &= ~WSATTR_DIM;
                                break;
+                       case 23: /* ~italic */
+                               flags &= WSATTR_ITALIC;
+                               break;
                        case 24: /* ~underline VT300 only */
                                flags &= ~WSATTR_UNDERLINE;
+                               flags &= ~WSATTR_DOUBLE_UNDERLINE;
                                break;
                        case 25: /* ~blink VT300 only */
                                flags &= ~WSATTR_BLINK;
@@ -570,12 +620,36 @@
                        case 27: /* ~reverse VT300 only */
                                flags &= ~WSATTR_REVERSE;
                                break;
+                       case 28: /* ~invisible */
+                               flags &= ~WSATTR_INVISIBLE;
+                               break;
+                       case 29: /* ~strike */
+                               flags &= ~WSATTR_STRIKE;
+                               break;
                        case 30: case 31: case 32: case 33:
                        case 34: case 35: case 36: case 37:
                                /* fg color */
                                flags |= WSATTR_WSCOLORS;
                                fgcol = ARG(n) - 30;
                                break;
+                       /*
+                        * Some software naively assumes that sequences such
+                        * as CSI 38;5;Xm are available without really
+                        * checking the terminal capabilities.
+                        *
+                        * If we don't swallow the parameters following the
+                        * 38, they get incorrectly interpreted as 'CSI m'
+                        * sequences on their own.
+                        *
+                        * So an attempt to set colour 1 ends up setting bold
+                        * and blink, because wscons treats CSI 38;5;1m as
+                        * CSI 5;1m.
+                        *
+                        * A similar problem exists for CSI 48m sequences.
+                        */
+                       case 38:
+                               n=edp->nargs;
+                               break;
                        case 39:
                                /* reset fg color */
                                fgcol = WSCOL_WHITE;
@@ -587,6 +661,9 @@
                                /* bg color */
                                flags |= WSATTR_WSCOLORS;
                                bgcol = ARG(n) - 40;
+                               break;
+                       case 48:
+                               n=edp->nargs;
                                break;
                        case 49:
                                /* reset bg color */
--- dev/wscons/wsksymdef.h.dist Mon Sep 20 14:32:39 2021
+++ dev/wscons/wsksymdef.h      Mon Jan  2 15:48:18 2023
@@ -626,6 +626,7 @@
 #define KS_Open                        0xf393
 #define KS_Paste               0xf394
 #define KS_Cut                 0xf395
+#define KS_Backtab             0xf396
 
 #define KS_Menu                        0xf3c0
 #define KS_Pause               0xf3c1
--- dev/wscons/wsdisplayvar.h.dist      Sun Sep 13 07:05:46 2020
+++ dev/wscons/wsdisplayvar.h   Wed Jan  4 09:24:12 2023
@@ -94,11 +94,16 @@
 #define WSCOL_CYAN     6
 #define WSCOL_WHITE    7
 /* flag values: */
-#define WSATTR_REVERSE 1
-#define WSATTR_HILIT   2
-#define WSATTR_BLINK   4
-#define WSATTR_UNDERLINE 8
-#define WSATTR_WSCOLORS 16
+#define WSATTR_REVERSE         1
+#define WSATTR_HILIT           2
+#define WSATTR_BLINK           4
+#define WSATTR_UNDERLINE       8
+#define WSATTR_WSCOLORS        16
+#define WSATTR_DIM             32
+#define WSATTR_STRIKE          64
+#define WSATTR_DOUBLE_UNDERLINE        128
+#define WSATTR_INVISIBLE       256
+#define WSATTR_ITALIC          512
 };
 
 #define        WSSCREEN_NAME_SIZE      16
--- dev/pckbc/wskbdmap_mfii.c.dist      Sat May  1 13:11:16 2021
+++ dev/pckbc/wskbdmap_mfii.c   Mon Jan  2 13:51:12 2023
@@ -59,7 +59,7 @@
     KC(12),                    KS_minus,       KS_underscore,
     KC(13),                    KS_equal,       KS_plus,
     KC(14),  KS_Cmd_ResetEmul, KS_Delete,
-    KC(15),                    KS_Tab,
+    KC(15),                    KS_Tab,         KS_Backtab,
     KC(16),                    KS_q,
     KC(17),                    KS_w,
     KC(18),                    KS_e,
@@ -103,16 +103,16 @@
     KC(56),  KS_Cmd2,          KS_Alt_L,
     KC(57),                    KS_space,
     KC(58),                    KS_Caps_Lock,
-    KC(59),  KS_Cmd_Screen0,   KS_f1,
-    KC(60),  KS_Cmd_Screen1,   KS_f2,
-    KC(61),  KS_Cmd_Screen2,   KS_f3,
-    KC(62),  KS_Cmd_Screen3,   KS_f4,
-    KC(63),  KS_Cmd_Screen4,   KS_f5,
-    KC(64),  KS_Cmd_Screen5,   KS_f6,
-    KC(65),  KS_Cmd_Screen6,   KS_f7,
-    KC(66),  KS_Cmd_Screen7,   KS_f8,
-    KC(67),  KS_Cmd_Screen8,   KS_f9,
-    KC(68),  KS_Cmd_Screen9,   KS_f10,
+    KC(59),  KS_Cmd_Screen0,   KS_f1,          KS_f13,
+    KC(60),  KS_Cmd_Screen1,   KS_f2,          KS_f14,
+    KC(61),  KS_Cmd_Screen2,   KS_f3,          KS_f15,
+    KC(62),  KS_Cmd_Screen3,   KS_f4,          KS_f16,
+    KC(63),  KS_Cmd_Screen4,   KS_f5,          KS_f17,
+    KC(64),  KS_Cmd_Screen5,   KS_f6,          KS_f18,
+    KC(65),  KS_Cmd_Screen6,   KS_f7,          KS_f19,
+    KC(66),  KS_Cmd_Screen7,   KS_f8,          KS_f20,
+    KC(67),  KS_Cmd_Screen8,   KS_f9,          KS_f21,
+    KC(68),  KS_Cmd_Screen9,   KS_f10,         KS_f22,
     KC(69),                    KS_Num_Lock,
     KC(70),                    KS_Hold_Screen,
     KC(71),                    KS_KP_Home,     KS_KP_7,
@@ -128,8 +128,8 @@
     KC(81),                    KS_KP_Next,     KS_KP_3,
     KC(82),                    KS_KP_Insert,   KS_KP_0,
     KC(83),                    KS_KP_Delete,   KS_KP_Decimal,
-    KC(87),  KS_Cmd_Screen10,  KS_f11,
-    KC(88),  KS_Cmd_Screen11,  KS_f12,
+    KC(87),  KS_Cmd_Screen10,  KS_f11,         KS_f23,
+    KC(88),  KS_Cmd_Screen11,  KS_f12,         KS_f24,
     KC(91),                    KS_f13,
     KC(92),                    KS_f14,
     KC(93),                    KS_f15,
--- dev/rasops/rasops.c.dist    Thu Jul 23 06:17:03 2020
+++ dev/rasops/rasops.c Thu Jan  5 19:19:50 2023
@@ -141,7 +141,7 @@
        uint32_t rs_defattr;
 
        int rs_sbscreens;
-#define RS_SCROLLBACK_SCREENS 5
+#define RS_SCROLLBACK_SCREENS 20
        int rs_dispoffset;      /* rs_bs index, start of our actual screen */
        int rs_visibleoffset;   /* rs_bs index, current scrollback screen */
 };
@@ -568,7 +568,19 @@
        if ((flg & WSATTR_HILIT) != 0)
                fg += 8;
 
-       flg = ((flg & WSATTR_UNDERLINE) ? 1 : 0);
+       /*
+        * The rapops code expects a different layout of the bitfields in flg:
+        *
+        * WSATTR_UNDERLINE is moved to bit 0
+        * Bits 1 and 2 are re-purposed to indicate whether the foreground and
+        * background are grey or not, (I.E. red==green==blue).
+        * 
+        * This was probably a convenient hack when we only had to deal with
+        * underlining, but further attributes should maintain their original
+        * bit positions for consistency between the wscons and rasops code.
+        */
+
+       flg = ((flg & 0xfff8) | (flg & WSATTR_UNDERLINE ? 1 : 0));
 
        if (rasops_isgray[fg])
                flg |= 2;
--- dev/rasops/rasops32.c.dist  Mon Jul 20 09:40:45 2020
+++ dev/rasops/rasops32.c       Fri Jan  6 08:08:09 2023
@@ -93,11 +93,44 @@
 
        b = ri->ri_devcmap[(attr >> 16) & 0xf];
        f = ri->ri_devcmap[(attr >> 24) & 0xf];
+
+       /*
+        * Implement dim by shifting each of the red, green and blue values by
+        * one bit.
+        *
+        * Since we are shifting each channel equally, this works for both RGB
+        * and byte-swapped BGR formats with exactly 8 bits per channel,
+        * I.E. 0xXXRRGGBB, and 0xBBGGRRXX.
+        *
+        * The unused byte is always set to 0x00 for 32bpp in
+        * rasops_init_devcmap, so will be unaffected by the shift.
+        *
+        * XXX If we ever support 32-bit devices that are not 8 bits per channel
+        * then this code will need to change.
+        */
+
+       if ((attr & WSATTR_DIM)!=0) {
+               f=(f>>1) & 0x7F7F7F7F;
+               }
+
        u.d[0][0] = b; u.d[0][1] = b;
        u.d[1][0] = b; u.d[1][1] = f;
        u.d[2][0] = f; u.d[2][1] = b;
        u.d[3][0] = f; u.d[3][1] = f;
 
+       /*
+        * Implement 'invisible' attribute by changing the character to a space.
+        *
+        * We need to do this rather than just ignoring the character or filling
+        * the bits with 0x00 and returning as a special case, because effects
+        * such as underline and strikethrough are still rendered on invisible
+        * characters, (at least they are in xterm).
+        */
+
+       if ((attr & WSATTR_INVISIBLE)!=0) {
+               uc=' ';
+               }
+
        if (uc == ' ') {
                while (height--) {
                        /* the general, pixel-at-a-time case is fast enough */
@@ -202,12 +235,44 @@
                }
        }
 
+       /* Double underline relies on normal underlining being done as well. */
+
        /* Do underline a pixel at a time */
-       if ((attr & 1) != 0) {
+       if ((attr & (WSATTR_DOUBLE_UNDERLINE | 1)) != 0) {
                rp -= step;
                for (cnt = 0; cnt < width; cnt++)
                        ((int *)rp)[cnt] = f;
        }
+
+       /*
+        * Double underline now just needs to paint the second underline two
+        * rows up. */
+
+       if ((attr & WSATTR_DOUBLE_UNDERLINE)!=0) {
+               rp-=(step << 1);
+               for (cnt=0; cnt< width; cnt++)
+                       ((int *)rp)[cnt]=f;
+               /*
+                * Reset row pointer to ensure that strikethough appears at a
+                * consistent height if combined with double underlining.
+                */
+
+               rp+=(step << 1);
+               }
+
+       /*
+        * Reset pointer to ensure that strikethough appears at a consistent
+        * height if combined with single underlining.
+        */
+
+       if ((attr & WSATTR_STRIKE)!=0) {
+               if ((attr & 1)==1) {
+                       rp+=step ;
+                       }
+               rp -= (1+ri->ri_font->fontheight/2)*step;
+               for (cnt=0; cnt< width; cnt++)
+                       ((int *)rp)[cnt]=f;
+               }
 
        return 0;
 }

Reply via email to