This is still too big for one go so I think it needs split further. I
suggest that first we get three diffs with the very easy stuff:
1) The two bug fixes
2) HPA/VPA/INDN/RIN
Some other comments on the larger diff:
- Why do you change RS_SCROLLBACK_SCREENS?
- Wouldn't it be better to make underline match rather than moving bits
about at runtime? Is that not possible?
- It seems odd that invisible will still show some elements like the
underline. I think unless we can support it fully it would be better not to
support it at all.
On Sat, 7 Jan 2023, 17:42 Crystal Kolipe, <kolip...@exoticsilicon.com>
wrote:
> # New escape sequences and two one-line bugfixes
>
> --- dev/wscons/wsemul_vt100_subr.c.dist Mon May 25 06:55:49 2020
> +++ dev/wscons/wsemul_vt100_subr.c Sat Jan 7 12:39:58 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,76 @@
> 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;
> + /*
> + * Sequences starting CSI 38 escape to a larger
> + * colourspace, typically either 256 colours or
> 24-bit.
> + *
> + * We support CSI 38;5;X;m to set colour X from a
> + * palette of 256.
> + */
> +#define EXIST_ARG2(i) ((edp->nargs-n)>=3)
> +#define ARG2_OR_DEF(i) (EXIST_ARG2(i) ? ARG(i+2) : 0)
> + case 38:
> + /*
> + * 38 followed by zero arguments is
> meaningless.
> + */
> + if (edp->nargs == n+1) {
> + break ;
> + }
> + /*
> + * 5 should normally be followed by a
> single
> + * argument, but zero arguments is also
> valid to
> + * set colour zero.
> + */
> + if (ARG(n+1)==5) {
> + flags |= WSATTR_WSCOLORS;
> + if (edp->scrcapabilities &
> + WSSCREEN_256COL) {
> + fgcol = ARG2_OR_DEF(n);
> + } else {
> + fgcol = (ARG2_OR_DEF(n) <
> 8 ? ARG2_OR_DEF(n)
> + : fgcol );
> + }
> + n+=(EXIST_ARG2(n) ? 2 : 1);
> + break;
> + }
> + /*
> + * 2 should introduce a sequence of three
> + * arguments, specifying RGB.
> + *
> + * We don't, (yet!), support setting
> colours by
> + * 24-bit RGB arguments and don't want to
> + * interpret these as regular SGR codes.
> + *
> + * If there are more then three, skip just
> + * three, otherwise skip all of them.
> + */
> + if (ARG(n+1)==2) {
> + n=(edp->nargs-n > 5 ? n+4 :
> + edp->nargs);
> + break;
> + }
> + /*
> + * Invalid code, I.E. not 2 or 5.
> + *
> + * We do what xterm does and just skip the
> + * single unrecognised argument, then
> allow any
> + * following arguments to be interpreted
> as SGR.
> + */
> + n++;
> + break;
> case 39:
> /* reset fg color */
> fgcol = WSCOL_WHITE;
> @@ -588,11 +702,46 @@
> flags |= WSATTR_WSCOLORS;
> bgcol = ARG(n) - 40;
> break;
> + case 48: /* set 8-bit background colour */
> + if (edp->nargs == n+1) {
> + break ;
> + }
> + if (ARG(n+1)==5) {
> + flags |= WSATTR_WSCOLORS;
> + if (edp->scrcapabilities &
> + WSSCREEN_256COL) {
> + bgcol = ARG2_OR_DEF(n);
> + } else {
> + bgcol = (ARG2_OR_DEF(n) <
> 8 ? ARG2_OR_DEF(n)
> + : bgcol );
> + }
> + n+=(EXIST_ARG2(n) ? 2 : 1);
> + break;
> + }
> + if (ARG(n+1)==2) {
> + n=(edp->nargs-n > 5 ? n+4 :
> + edp->nargs);
> + break;
> + }
> + n++;
> + break;
> case 49:
> /* reset bg color */
> bgcol = WSCOL_BLACK;
> if (fgcol == WSCOL_WHITE)
> flags &= ~WSATTR_WSCOLORS;
> + break;
> + case 90: case 91: case 92: case 93:
> + case 94: case 95: case 96: case 97:
> + /* bright foreground colour */
> + flags |= WSATTR_WSCOLORS;
> + fgcol = ARG(n) - 82;
> + break;
> + case 100: case 101: case 102: case 103:
> + case 104: case 105: case 106: case 107:
> + /* bright background colour */
> + flags |= WSATTR_WSCOLORS;
> + bgcol = ARG(n) - 92;
> break;
> default:
> #ifdef VT100_PRINTUNKNOWN
> --- dev/wscons/wsdisplayvar.h.dist Sun Sep 13 07:05:46 2020
> +++ dev/wscons/wsdisplayvar.h Fri Jan 6 20:09:43 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
> @@ -114,6 +119,7 @@
> #define WSSCREEN_HILIT 4 /* can highlight (however) */
> #define WSSCREEN_BLINK 8 /* can blink */
> #define WSSCREEN_UNDERLINE 16 /* can underline */
> +#define WSSCREEN_256COL 32 /* supports 256 colours */
> };
>
> /*
> --- dev/rasops/rasops.c.dist Thu Jul 23 06:17:03 2020
> +++ dev/rasops/rasops.c Sat Jan 7 13:55:14 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 Sat Jan 7 13:56:43 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,45 @@
> }
> }
>
> + /* 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;
> }
>
