Re: Double underlining and strikeout console patch

2023-01-30 Thread Crystal Kolipe
On Mon, Jan 30, 2023 at 09:15:51PM -0300, Crystal Kolipe wrote:
> But just because the original goal is well within sight, I didn't see that
> as a reason to stop.

Just to clarify, I mean a reason to stop writing further code for evaluation.

Not 'not a reason to stop', as in continue bloating the rasops code.



Re: Double underlining and strikeout console patch

2023-01-30 Thread Crystal Kolipe
On Mon, Jan 30, 2023 at 04:53:02PM -0700, Theo de Raadt wrote:
> Whoosh, you missed the point.
> 
> I've used xterm for decades and never used any of that.
> 
> If those operations turn into "no-op" or perform the minimum rendering
> change, it is still valid xterm escape processing.
> 
> There is a difference betwen parsing an escape sequence, and performing
> a discrete & specific action for it.
> 
> You are trying to say an escape sequence must render it exactly like
> a real xterm would.

No, I am certainly not saying that.

>  For example, that the double underline should show
> a double underline, it cannot show an underline, and if it doesn't, then
> I guess it should not be parsed? And create a  mess?  I think that's not
> right.

Of course it's not right.

Especially for the attributes, (such as double underline), which are _not_
defined in the xterm terminfo entry.  Because programs which parse terminfo
should not assume that they are available.

Attributes which _are_ defined in xterm terminfo, if we don't render those,
then programs which rightlyfully expect them to be rendered might
'create a mess':

E.G. an email program tries to display a deleted email using strike through,
and the text is actually all the same instead of being diferentiated.  That
gives a bad user experience.

There is a difference between saying that we support a fair subset of
xterm sequences, and saying that what is defined in the xterm terminfo
works adequately on wscons.  The two are not the same.

> The display operation can be exactly the same as another.  ALL of
> the various highlight requests could be parsed, and then ALL of them could
> do inverse video, and it be a valid xterm *parser*.

Yes, I agree.  The VGA driver does a similar thing, substituting light blue
for underline.

>  It is the parsing issue
> that matters for progress converting to xterm default.  Discrete maximum
> eyecandy is not as concerning but you are treating that as critical.

No I am not.  I wrote the patches for rendering these attributes for internal
use, and am sharing them because they are related to the other console
changes.

> If all these changes bloat the kernel size, or bloats the number of
> #ifdef SMALL_KERNEL required to make our install media work, then your
> position is quite different from the "switch to xterm" goals.

With what has already been committed, TERM=xterm already works better than
TERM=vt220.  Once the f-keys hunks went in, then it would be better than
TERM=pccon as well.

I have already been using TERM=xterm for a month, without most of the
'eyecandy' patches.

So I think that the first goal has already been achieved.  At least on my
machines it has.

But just because the original goal is well within sight, I didn't see that
as a reason to stop.



Re: Double underlining and strikeout console patch

2023-01-30 Thread Theo de Raadt
Whoosh, you missed the point.

I've used xterm for decades and never used any of that.

If those operations turn into "no-op" or perform the minimum rendering
change, it is still valid xterm escape processing.

There is a difference betwen parsing an escape sequence, and performing
a discrete & specific action for it.

You are trying to say an escape sequence must render it exactly like
a real xterm would.  For example, that the double underline should show
a double underline, it cannot show an underline, and if it doesn't, then
I guess it should not be parsed? And create a  mess?  I think that's not
right.  The display operation can be exactly the same as another.  ALL of
the various highlight requests could be parsed, and then ALL of them could
do inverse video, and it be a valid xterm *parser*.  It is the parsing issue
that matters for progress converting to xterm default.  Discrete maximum
eyecandy is not as concerning but you are treating that as critical.

If all these changes bloat the kernel size, or bloats the number of
#ifdef SMALL_KERNEL required to make our install media work, then your
position is quite different from the "switch to xterm" goals.

Crystal Kolipe  wrote:

> On Tue, Jan 31, 2023 at 12:03:53AM +0100, Christian Weisgerber wrote:
> > Crystal Kolipe:
> > 
> > > Here is the latest version of the double underline and strikeout parts of 
> > > my
> > > console patchset.
> > 
> > I'm sorry, but I gotta ask: Who or what uses something like this??
> 
> It's useful on displays that lack other forms of attributes, such as a low-
> resolution monochrome LCD.  If you can't do bold, dim, or colour.
> 
> And I mean, obviously something like ssdfb(4) is not going to run X very well,
> is it?
> 
> > Offhand, I don't even know if xterm can do it.
> 
> It can, but at smaller font sizes it looks the same as single underline.
> 
> echo '^[[21This is double underlined'
> 
> > If you want this
> > kind of typographic detail, shouldn't you be using a graphics canvas
> > rather than a character-cell terminal?
> 
> Why?  The console already supports 'attributes', such as bold and
> underlining.  Why would necessarily we want one and not another?
> 
> The original idea behind this patchset was to bring wscons to the point where
> the default emulation can change from vt220 to xterm.  That has very obvious
> advantages, and I'm assuming that you wouldn't automatically be against such
> a change.
> 
> But if we're about to tell people, 'OK, now you can set TERM=xterm, and gain
> improved performance on the console', then users are going to notice
> features that xterm provides, but that wscons does not.
> 
> I'm creating sample implementations of the xterm features that I and my
> collegues here use in the first instance.  Whether any particular one is
> deemed to be suitable or appropriate to include in OpenBSD is an open 
> question.
> 
> But at least you've got code to try.
> 
> I've been using 256 colours, strikeout, dim, invisible, true bold, italic,
> and more on the console for the last few weeks, (some of it for longer), and
> personally I think it's useful in email clients, status reporting scripts,
> and more.
> 



Re: Double underlining and strikeout console patch

2023-01-30 Thread Crystal Kolipe
On Tue, Jan 31, 2023 at 12:03:53AM +0100, Christian Weisgerber wrote:
> Crystal Kolipe:
> 
> > Here is the latest version of the double underline and strikeout parts of my
> > console patchset.
> 
> I'm sorry, but I gotta ask: Who or what uses something like this??

It's useful on displays that lack other forms of attributes, such as a low-
resolution monochrome LCD.  If you can't do bold, dim, or colour.

And I mean, obviously something like ssdfb(4) is not going to run X very well,
is it?

> Offhand, I don't even know if xterm can do it.

It can, but at smaller font sizes it looks the same as single underline.

echo '^[[21This is double underlined'

> If you want this
> kind of typographic detail, shouldn't you be using a graphics canvas
> rather than a character-cell terminal?

Why?  The console already supports 'attributes', such as bold and
underlining.  Why would necessarily we want one and not another?

The original idea behind this patchset was to bring wscons to the point where
the default emulation can change from vt220 to xterm.  That has very obvious
advantages, and I'm assuming that you wouldn't automatically be against such
a change.

But if we're about to tell people, 'OK, now you can set TERM=xterm, and gain
improved performance on the console', then users are going to notice
features that xterm provides, but that wscons does not.

I'm creating sample implementations of the xterm features that I and my
collegues here use in the first instance.  Whether any particular one is
deemed to be suitable or appropriate to include in OpenBSD is an open question.

But at least you've got code to try.

I've been using 256 colours, strikeout, dim, invisible, true bold, italic,
and more on the console for the last few weeks, (some of it for longer), and
personally I think it's useful in email clients, status reporting scripts,
and more.



Re: Double underlining and strikeout console patch

2023-01-30 Thread Christian Weisgerber
Crystal Kolipe:

> Here is the latest version of the double underline and strikeout parts of my
> console patchset.

I'm sorry, but I gotta ask: Who or what uses something like this??

Offhand, I don't even know if xterm can do it.  If you want this
kind of typographic detail, shouldn't you be using a graphics canvas
rather than a character-cell terminal?

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Double underlining and strikeout console patch

2023-01-30 Thread Crystal Kolipe
Here is the latest version of the double underline and strikeout parts of my
console patchset.

Previously, the double underline was always painted two pixels above the
normal underline.  This worked well for most font sizes, but to improve the
visual effect with the smallest and largest fonts the offset now depends on
the font height.

Index: rasops/rasops.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops.c,v
retrieving revision 1.69
diff -u -p -r1.69 rasops.c
--- rasops/rasops.c 18 Jan 2023 11:08:49 -  1.69
+++ rasops/rasops.c 30 Jan 2023 12:40:55 -
@@ -561,7 +561,7 @@ rasops_pack_cattr(void *cookie, int fg, 
if ((flg & WSATTR_HILIT) != 0 && fg < 8)
fg += 8;
 
-   *attr = (bg << 16) | (fg << 24) | (flg & WSATTR_UNDERLINE);
+   *attr = (bg << 16) | (fg << 24) | flg;
return (0);
 }
 
@@ -585,7 +585,7 @@ rasops_pack_mattr(void *cookie, int fg, 
bg = swap;
}
 
-   *attr = (bg << 16) | (fg << 24) | (flg & WSATTR_UNDERLINE);
+   *attr = (bg << 16) | (fg << 24) | flg;
return (0);
 }
 
Index: rasops/rasops32.c
===
RCS file: /cvs/src/sys/dev/rasops/rasops32.c,v
retrieving revision 1.13
diff -u -p -r1.13 rasops32.c
--- rasops/rasops32.c   18 Jan 2023 11:08:49 -  1.13
+++ rasops/rasops32.c   30 Jan 2023 12:40:55 -
@@ -202,12 +202,51 @@ rasops32_putchar(void *cookie, int row, 
}
}
 
-   /* Do underline a pixel at a time */
-   if ((attr & WSATTR_UNDERLINE) != 0) {
+
+   /* Do underline one pixel at a time.
+*
+* Double underline requires single underline to be painted as well.
+*/
+
+   if ((attr & (WSATTR_UNDERLINE | WSATTR_DOUBLE_UNDERLINE)) != 0) {
rp -= step;
for (cnt = 0; cnt < width; cnt++)
((int *)rp)[cnt] = f;
}
+
+   /*
+* Double underline now just needs to paint the second underline a few
+* rows up.  The exact height is relative to the font height, to create
+* a visually pleasing effect regardless of display resolution.
+*/
+#define FONT_HEIGHT ri->ri_font->fontheight
+#define DOUBLE_UL_SHIFT (FONT_HEIGHT < 12 ? 0 : (FONT_HEIGHT <= 32 ? 1 : 2))
+
+   if ((attr & WSATTR_DOUBLE_UNDERLINE)!=0) {
+   rp-=(step << DOUBLE_UL_SHIFT);
+   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 << DOUBLE_UL_SHIFT);
+   }
+
+   /*
+* Reset pointer to ensure that strikethough appears at a consistent
+* height if combined with single underlining.
+*/
+
+   if ((attr & WSATTR_STRIKE) != 0) {
+   if ((attr & WSATTR_UNDERLINE) != 0) {
+   rp+=step ;
+   }
+   rp -= (1 + (ri->ri_font->fontheight >> 1)) * step;
+   for (cnt=0; cnt< width; cnt++)
+   ((int *)rp)[cnt]=f;
+   }
 
return 0;
 }
Index: wscons/wsdisplayvar.h
===
RCS file: /cvs/src/sys/dev/wscons/wsdisplayvar.h,v
retrieving revision 1.38
diff -u -p -r1.38 wsdisplayvar.h
--- wscons/wsdisplayvar.h   13 Sep 2020 10:05:46 -  1.38
+++ wscons/wsdisplayvar.h   30 Jan 2023 12:40:55 -
@@ -94,11 +94,13 @@ struct wsdisplay_emulops {
 #define WSCOL_CYAN 6
 #define WSCOL_WHITE7
 /* 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_WSCOLORS16
+#define WSATTR_DOUBLE_UNDERLINE32
+#define WSATTR_STRIKE  64
 };
 
 #defineWSSCREEN_NAME_SIZE  16
Index: wscons/wsemul_vt100_subr.c
===
RCS file: /cvs/src/sys/dev/wscons/wsemul_vt100_subr.c,v
retrieving revision 1.29
diff -u -p -r1.29 wsemul_vt100_subr.c
--- wscons/wsemul_vt100_subr.c  12 Jan 2023 20:39:37 -  1.29
+++ wscons/wsemul_vt100_subr.c  30 Jan 2023 12:40:55 -
@@ -597,17 +597,27 @@ wsemul_vt100_handle_csi(struct wsemul_vt
case 7: /* reverse */
flags |= WSATTR_REVERSE;
break;
+   case 9: /* strikeout */
+   flags |= WSATTR_STRIKE;
+   break;
+   case 21: /* double underline