[PATCH] Support 24-bit terminal colors.

[PATCH] Support 24-bit terminal colors.

[Rami Ylimäki]

From: Rami Ylimäki <rjy@iki.fi>

Based on previous work by Rüdiger Sonderfeld, Christian Hopps and
Charles Strahan.

Currently it's not possible to detect whether terminal supports 24-bit
colors. Therefore following methods can be used to force 24-bit mode on:

emacs -nw --color=rgb
emacsclient -nw -F "((tty-color-mode . rgb))"

* lisp/server.el (server-process-filter): Pass 'tty-color-mode frame
  parameter from command line to server-create-tty-frame.
  (server-create-tty-frame): Pass 'tty-color-mode frame parameter to
  make-frame.

* lisp/term/tty-colors.el (tty-color-mode-alist): Add 'rgb entry to
  support 24-bit terminals. This enables the --color=rgb option as well.

* lisp/term/xterm.el (xterm-register-default-colors): Define all named
  tty colors. Add xterm-rgb-support-b to check whether 24-bit mode has
  been forced on.

* src/dispextern.h: Define bit FACE_TTY_NONINDEXED_COLOR to separate
  indexed and rgb component based tty color encodings. The rgb component
  based color "index" is simply represented by a 24-bit rgb triplet with
  FACE_TTY_NONINDEXED_COLOR set.

* src/termchar.h (tty_display_info): Remove TN_max_pairs field
  describing maximum number of color pairs. The field is unused and
  would also be inconvenient with 24-bit colors. Add new tty control
  functions TS_set_rgb_foreground and TS_set_rgb_background for
  outputting rgb component based tty colors.

* src/term.c (turn_on_face): Output indexed or rgb component based tty
  colors by detecting whether FACE_TTY_NONINDEXED_COLOR has been set.
  (tty_default_color_capabilities): Add TS_set_rgb_foreground and
  TS_set_rgb_background.
  (tty_setup_colors): Initialize tty_display_info for 24-bit terminals.

* src/xfaces.c (tty_lookup_color): Use rgb component based tty color
  encoding on 24-bit terminals.
  (map_tty_color): Use rgb component based tty color encoding for face
  foreground and background on 24-bit terminals.
---
 doc/man/emacs.1.in      |  2 +-
 lisp/server.el          | 14 ++++++--
 lisp/term/tty-colors.el |  3 +-
 lisp/term/xterm.el      | 16 +++++++++
 src/dispextern.h        |  3 ++
 src/term.c              | 89 +++++++++++++++++++++++++++++++++++--------------
 src/termchar.h          |  7 ++--
 src/xfaces.c            | 51 ++++++++++++++++++++++++++--
 8 files changed, 149 insertions(+), 36 deletions(-)

diff --git a/doc/man/emacs.1.in b/doc/man/emacs.1.in
index 3b1566f..79d48fb 100644
--- a/doc/man/emacs.1.in
+++ b/doc/man/emacs.1.in
@@ -252,7 +252,7 @@ Set additional X resources.
 Override color mode for character terminals;
 .I mode
 defaults to "auto", and can also be "never", "auto", "always",
-or a mode name like "ansi8".
+or a mode name like "ansi8". Use "rgb" for 24-bit colors.
 .TP
 .BI \-bw " pixels\fR,\fP " \-\-border\-width " pixels"
 Set the
diff --git a/lisp/server.el b/lisp/server.el
index 5300984..5ecd483 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -792,11 +792,13 @@ This handles splitting the command if it would be bigger than
       (setq prefix "-print-nonl "))
     (server-send-string proc (concat prefix qtext "\n"))))
 
-(defun server-create-tty-frame (tty type proc)
+(defun server-create-tty-frame (tty type color-mode proc)
   (unless tty
     (error "Invalid terminal device"))
   (unless type
     (error "Invalid terminal type"))
+  (unless color-mode
+    (error "Invalid terminal color mode"))
   (add-to-list 'frame-inherited-parameters 'client)
   (let ((frame
          (server-with-environment
@@ -812,6 +814,7 @@ This handles splitting the command if it would be bigger than
            (make-frame `((window-system . nil)
                          (tty . ,tty)
                          (tty-type . ,type)
+                         (tty-color-mode . ,color-mode)
                          ;; Ignore nowait here; we always need to
                          ;; clean up opened ttys when the client dies.
                          (client . ,proc)
@@ -1055,6 +1058,8 @@ The following commands are accepted by the client:
 		(coding-system (and (default-value 'enable-multibyte-characters)
 				    (or file-name-coding-system
 					default-file-name-coding-system)))
+		(tty-color-mode 'default)
+
 		nowait     ; t if emacsclient does not want to wait for us.
 		frame      ; Frame opened for the client (if any).
 		display    ; Open frame on this display.
@@ -1089,7 +1094,10 @@ The following commands are accepted by the client:
                  (let ((alist (pop args-left)))
                    (if coding-system
                        (setq alist (decode-coding-string alist coding-system)))
-                   (setq frame-parameters (car (read-from-string alist)))))
+                   (setq frame-parameters (car (read-from-string alist)))
+                   (let ((color-mode (cdr (assoc 'tty-color-mode frame-parameters))))
+                     (if (assoc color-mode tty-color-mode-alist)
+                         (setq tty-color-mode color-mode)))))
 
                 ;; -display DISPLAY:
                 ;; Open X frames on the given display instead of the default.
@@ -1241,7 +1249,7 @@ The following commands are accepted by the client:
 						       frame-parameters))
 		   ;; When resuming on a tty, tty-name is nil.
 		   (tty-name
-		    (server-create-tty-frame tty-name tty-type proc))))
+		    (server-create-tty-frame tty-name tty-type tty-color-mode proc))))
 
             (process-put
              proc 'continuation
diff --git a/lisp/term/tty-colors.el b/lisp/term/tty-colors.el
index a886950..b9b69cd 100644
--- a/lisp/term/tty-colors.el
+++ b/lisp/term/tty-colors.el
@@ -764,7 +764,8 @@
     (auto . 0)
     (ansi8 . 8)
     (always . 8)
-    (yes . 8))
+    (yes . 8)
+    (rgb . 16777216))
   "An alist of supported standard tty color modes and their aliases.")
 
 (defun tty-color-alist (&optional _frame)
diff --git a/lisp/term/xterm.el b/lisp/term/xterm.el
index 01c0113..e0f3eb5 100644
--- a/lisp/term/xterm.el
+++ b/lisp/term/xterm.el
@@ -905,6 +905,14 @@ hitting screen's max DCS length."
   "Convert an 8-bit primary color value PRIM to a corresponding 16-bit value."
   (logior prim (lsh prim 8)))
 
+(defun xterm-rgb-support-p ()
+  "Check whether 24-bit colors have been forced on."
+  (or
+   ;; emacs -nw --color=rgb
+   (eql (cdr (assoc 'tty-color-mode default-frame-alist)) 'rgb)
+   ;; emacsclient -nw -F "((tty-color-mode . rgb))"
+   (eql (cdr (assoc 'tty-color-mode (frame-parameters))) 'rgb)))
+
 (defun xterm-register-default-colors (colors)
   "Register the default set of colors for xterm or compatible emulator.
 
@@ -930,6 +938,14 @@ versions of xterm."
     ;; are more colors to support, compute them now.
     (when (> ncolors 0)
       (cond
+       ((xterm-rgb-support-p) ; 24-bit xterm
+	;; all named tty colors
+	(let ((idx (length xterm-standard-colors)))
+	  (mapc (lambda (color)
+		  (unless (assoc (car color) xterm-standard-colors)
+		    (tty-color-define (car color) idx (cdr color))
+		    (setq idx (1+ idx))))
+		color-name-rgb-alist)))
        ((= ncolors 240)	; 256-color xterm
 	;; 216 non-gray colors first
 	(let ((r 0) (g 0) (b 0))
diff --git a/src/dispextern.h b/src/dispextern.h
index f2c42de..35d2106 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1751,6 +1751,9 @@ struct face
 
 #define FACE_TTY_DEFAULT_BG_COLOR ((unsigned long) -3)
 
+/* Color index bit indicating absence of palette.  */
+#define FACE_TTY_NONINDEXED_COLOR ((unsigned long) (1 << 24))
+
 /* True if COLOR is a specified (i.e., nondefault) foreground or
    background color for a tty face.  */
 
diff --git a/src/term.c b/src/term.c
index d54ff11..dc6028d 100644
--- a/src/term.c
+++ b/src/term.c
@@ -1891,6 +1891,43 @@ produce_glyphless_glyph (struct it *it, Lisp_Object acronym)
    ? (tty->TN_no_color_video & (ATTR)) == 0             \
    : 1)
 
+static char *
+tparam_color (struct tty_display_info *tty, unsigned long color, bool is_foreground)
+{
+  const char *ts = is_foreground
+		   ? tty->TS_set_foreground
+		   : tty->TS_set_background;
+  return ts ? tparam (ts, NULL, 0, color, 0, 0, 0) : NULL;
+}
+
+static char *
+tparam_rgb (struct tty_display_info *tty, unsigned long color, bool is_foreground)
+{
+  const char *ts = is_foreground
+		   ? tty->TS_set_rgb_foreground
+		   : tty->TS_set_rgb_background;
+  const int red = (color >> 16) & 0xFF;
+  const int green = (color >> 8) & 0xFF;
+  const int blue = color & 0xFF;
+  return ts ? tparam (ts, NULL, 0, red, green, blue, 0) : NULL;
+}
+
+static void
+turn_on_color (struct tty_display_info *tty, unsigned long color, bool is_foreground)
+{
+  if (face_tty_specified_color (color))
+    {
+      char *p = color & FACE_TTY_NONINDEXED_COLOR
+		? tparam_rgb (tty, color, is_foreground)
+		: tparam_color (tty, color, is_foreground);
+      if (p)
+	{
+	  OUTPUT (tty, p);
+	  xfree (p);
+        }
+    }
+}
+
 /* Turn appearances of face FACE_ID on tty frame F on.
    FACE_ID is a realized face ID number, in the face cache.  */
 
@@ -1930,24 +1967,8 @@ turn_on_face (struct frame *f, int face_id)
 
   if (tty->TN_max_colors > 0)
     {
-      const char *ts;
-      char *p;
-
-      ts = tty->standout_mode ? tty->TS_set_background : tty->TS_set_foreground;
-      if (face_tty_specified_color (fg) && ts)
-	{
-          p = tparam (ts, NULL, 0, fg, 0, 0, 0);
-	  OUTPUT (tty, p);
-	  xfree (p);
-	}
-
-      ts = tty->standout_mode ? tty->TS_set_foreground : tty->TS_set_background;
-      if (face_tty_specified_color (bg) && ts)
-	{
-          p = tparam (ts, NULL, 0, bg, 0, 0, 0);
-	  OUTPUT (tty, p);
-	  xfree (p);
-	}
+      turn_on_color (tty, fg, !tty->standout_mode);
+      turn_on_color (tty, bg, tty->standout_mode);
     }
 }
 
@@ -2050,11 +2071,12 @@ TERMINAL does not refer to a text terminal.  */)
    to work around an HPUX compiler bug (?). See
    http://lists.gnu.org/archive/html/emacs-devel/2007-08/msg00410.html  */
 static int default_max_colors;
-static int default_max_pairs;
 static int default_no_color_video;
 static char *default_orig_pair;
 static char *default_set_foreground;
 static char *default_set_background;
+static char *default_set_rgb_foreground;
+static char *default_set_rgb_background;
 
 /* Save or restore the default color-related capabilities of this
    terminal.  */
@@ -2067,8 +2089,9 @@ tty_default_color_capabilities (struct tty_display_info *tty, bool save)
       dupstring (&default_orig_pair, tty->TS_orig_pair);
       dupstring (&default_set_foreground, tty->TS_set_foreground);
       dupstring (&default_set_background, tty->TS_set_background);
+      dupstring (&default_set_rgb_foreground, tty->TS_set_rgb_foreground);
+      dupstring (&default_set_rgb_background, tty->TS_set_rgb_background);
       default_max_colors = tty->TN_max_colors;
-      default_max_pairs = tty->TN_max_pairs;
       default_no_color_video = tty->TN_no_color_video;
     }
   else
@@ -2076,8 +2099,9 @@ tty_default_color_capabilities (struct tty_display_info *tty, bool save)
       tty->TS_orig_pair = default_orig_pair;
       tty->TS_set_foreground = default_set_foreground;
       tty->TS_set_background = default_set_background;
+      tty->TS_set_rgb_foreground = default_set_rgb_foreground;
+      tty->TS_set_rgb_background = default_set_rgb_background;
       tty->TN_max_colors = default_max_colors;
-      tty->TN_max_pairs = default_max_pairs;
       tty->TN_no_color_video = default_no_color_video;
     }
 }
@@ -2097,9 +2121,10 @@ tty_setup_colors (struct tty_display_info *tty, int mode)
     {
       case -1:	 /* no colors at all */
 	tty->TN_max_colors = 0;
-	tty->TN_max_pairs = 0;
 	tty->TN_no_color_video = 0;
-	tty->TS_set_foreground = tty->TS_set_background = tty->TS_orig_pair = NULL;
+	tty->TS_orig_pair = NULL;
+	tty->TS_set_foreground = tty->TS_set_background = NULL;
+	tty->TS_set_rgb_foreground = tty->TS_set_rgb_background = NULL;
 	break;
       case 0:	 /* default colors, if any */
       default:
@@ -2107,6 +2132,7 @@ tty_setup_colors (struct tty_display_info *tty, int mode)
 	break;
       case 8:	/* 8 standard ANSI colors */
 	tty->TS_orig_pair = "\033[0m";
+	tty->TS_set_rgb_foreground = tty->TS_set_rgb_background = NULL;
 #ifdef TERMINFO
 	tty->TS_set_foreground = "\033[3%p1%dm";
 	tty->TS_set_background = "\033[4%p1%dm";
@@ -2115,9 +2141,21 @@ tty_setup_colors (struct tty_display_info *tty, int mode)
 	tty->TS_set_background = "\033[4%dm";
 #endif
 	tty->TN_max_colors = 8;
-	tty->TN_max_pairs = 64;
 	tty->TN_no_color_video = 0;
 	break;
+      case 16777216:
+	tty->TS_orig_pair = "\033[0m";
+	tty->TS_set_foreground = tty->TS_set_background = NULL;
+#ifdef TERMINFO
+	tty->TS_set_rgb_foreground = "\033[38;2;%p1%d;%p2%d;%p3%dm";
+	tty->TS_set_rgb_background = "\033[48;2;%p1%d;%p2%d;%p3%dm";
+#else
+	tty->TS_set_rgb_foreground = "\033[38;2;%d;%d;%dm";
+	tty->TS_set_rgb_background = "\033[48;2;%d;%d;%dm";
+#endif
+	tty->TN_max_colors = 16777216;
+	tty->TN_no_color_video = 0;
+        break;
     }
 }
 
@@ -4137,7 +4175,6 @@ use the Bourne shell command 'TERM=...; export TERM' (C-shell:\n\
 	}
 
       tty->TN_max_colors = tgetnum ("Co");
-      tty->TN_max_pairs = tgetnum ("pa");
 
       tty->TN_no_color_video = tgetnum ("NC");
       if (tty->TN_no_color_video == -1)
@@ -4514,6 +4551,8 @@ bigger, or it may make it blink, or it may do nothing at all.  */);
   default_orig_pair = NULL;
   default_set_foreground = NULL;
   default_set_background = NULL;
+  default_set_rgb_foreground = NULL;
+  default_set_rgb_background = NULL;
 #endif /* !DOS_NT */
 
   encode_terminal_src = NULL;
diff --git a/src/termchar.h b/src/termchar.h
index 35b30fb..81fe96e 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -149,10 +149,6 @@ struct tty_display_info
 
   int TN_max_colors;            /* "Co" -- number of colors.  */
 
-  /* "pa" -- max. number of color pairs on screen.  Not handled yet.
-     Could be a problem if not equal to TN_max_colors * TN_max_colors.  */
-  int TN_max_pairs;
-
   /* "op" -- SVr4 set default pair to its original value.  */
   const char *TS_orig_pair;
 
@@ -160,6 +156,9 @@ struct tty_display_info
      1 param, the color index.  */
   const char *TS_set_foreground;
   const char *TS_set_background;
+  /* 3 params: red, green and blue. */
+  const char *TS_set_rgb_foreground;
+  const char *TS_set_rgb_background;
 
   int TF_hazeltine;             /* termcap hz flag. */
   int TF_insmode_motion;        /* termcap mi flag: can move while in insert mode. */
diff --git a/src/xfaces.c b/src/xfaces.c
index 0a1315d..e4650fd 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -832,6 +832,45 @@ parse_rgb_list (Lisp_Object rgb_list, XColor *color)
   return true;
 }
 
+static bool
+parse_and_encode_rgb_list (Lisp_Object rgb_list, XColor *color)
+{
+  if (!parse_rgb_list (rgb_list, color))
+    return false;
+
+  color->pixel = FACE_TTY_NONINDEXED_COLOR
+		 | (color->red / 256) << 16
+		 | (color->green / 256) << 8
+		 | (color->blue / 256);
+  return true;
+}
+
+static bool
+tty_supports_rgb (struct frame *f)
+{
+  return f->output_method == output_termcap
+	 && f->output_data.tty->display_info->TS_set_rgb_foreground
+	 && f->output_data.tty->display_info->TS_set_rgb_background;
+}
+
+static bool
+tty_lookup_rgb (Lisp_Object color, XColor *tty_color, XColor *std_color)
+{
+  if (NILP (Ffboundp (Qtty_color_standard_values)))
+    return false;
+
+  if (!NILP (Ffboundp (Qtty_color_canonicalize)))
+    color = call1 (Qtty_color_canonicalize, color);
+
+  color = call1 (Qtty_color_standard_values, color);
+  if (!parse_and_encode_rgb_list (color, tty_color))
+    return false;
+
+  if (std_color)
+    *std_color = *tty_color;
+
+  return true;
+}
 
 /* Lookup on frame F the color described by the lisp string COLOR.
    The resulting tty color is returned in TTY_COLOR; if STD_COLOR is
@@ -844,6 +883,9 @@ tty_lookup_color (struct frame *f, Lisp_Object color, XColor *tty_color,
 {
   Lisp_Object frame, color_desc;
 
+  if (tty_supports_rgb (f))
+    return tty_lookup_rgb (color, tty_color, std_color);
+
   if (!STRINGP (color) || NILP (Ffboundp (Qtty_color_desc)))
     return false;
 
@@ -5707,8 +5749,12 @@ map_tty_color (struct frame *f, struct face *face,
 	  CONSP (def)))
     {
       /* Associations in tty-defined-color-alist are of the form
-	 (NAME INDEX R G B).  We need the INDEX part.  */
-      pixel = XINT (XCAR (XCDR (def)));
+	 (NAME INDEX R G B).  We need the (R G B) or INDEX part.  */
+      XColor tty_color;
+      if (tty_supports_rgb (f) && parse_and_encode_rgb_list (XCDR (XCDR (def)), &tty_color))
+	pixel = tty_color.pixel;
+      else
+	pixel = XINT (XCAR (XCDR (def)));
     }
 
   if (pixel == default_pixel && STRINGP (color))
@@ -6395,6 +6441,7 @@ syms_of_xfaces (void)
   DEFSYM (Qwindow_divider_first_pixel, "window-divider-first-pixel");
   DEFSYM (Qwindow_divider_last_pixel, "window-divider-last-pixel");
   DEFSYM (Qtty_color_desc, "tty-color-desc");
+  DEFSYM (Qtty_color_canonicalize, "tty-color-canonicalize");
   DEFSYM (Qtty_color_standard_values, "tty-color-standard-values");
   DEFSYM (Qtty_color_by_index, "tty-color-by-index");
 
-- 
2.7.4

Re: [PATCH] Support 24-bit terminal colors.

[Daniel Colascione]

On 08/30/2016 01:55 PM, Rami Ylimäki wrote:
> From: Rami Ylimäki <rjy@iki.fi>
>
> Based on previous work by Rüdiger Sonderfeld, Christian Hopps and
> Charles Strahan.
>
> Currently it's not possible to detect whether terminal supports 24-bit
> colors.

Why did we decide to stop using terminfo as intended? Why did we stop 
supporting terminal capability inspection via control codes? We solved 
this problem in the god damn 1980s. Why are we back to specifying 
terminal characteristics manually?

Re: [PATCH] Support 24-bit terminal colors.

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 1255 bytes --]

On 2016-08-30 17:03, Daniel Colascione wrote:
> On 08/30/2016 01:55 PM, Rami Ylimäki wrote:
>> From: Rami Ylimäki <rjy@iki.fi>
>> 
>> Based on previous work by Rüdiger Sonderfeld, Christian Hopps and 
>> Charles Strahan.
>> 
>> Currently it's not possible to detect whether terminal supports
>> 24-bit colors.
> 
> Why did we decide to stop using terminfo as intended? Why did we stop
> supporting terminal capability inspection via control codes? We
> solved this problem in the god damn 1980s. Why are we back to
> specifying terminal characteristics manually?

Hey Daniel,

I don't understand your email. Are you saying that (1) Emacs is already not doing the right thing, and this patch is continuing in this tradition, or that (2) the world is a sad place, because the solution that was found in the 80s wasn't extended to cover 24bit colors, hence the need for this patch, or that (3) this patch is going in the wrong direction, because Emacs is doing things right already and this patch should use the existing facilities instead of whatever it does as submitted, or (4) something else?

I'm not familiar with capability inspection, so apologies if the meaning was clear for people with more background on this.
Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Support 24-bit terminal colors.

[Daniel Colascione]

On 08/30/2016 03:07 PM, Clément Pit--Claudel wrote:
> On 2016-08-30 17:03, Daniel Colascione wrote:
>> On 08/30/2016 01:55 PM, Rami Ylimäki wrote:
>>> From: Rami Ylimäki <rjy@iki.fi>
>>>
>>> Based on previous work by Rüdiger Sonderfeld, Christian Hopps and
>>> Charles Strahan.
>>>
>>> Currently it's not possible to detect whether terminal supports
>>> 24-bit colors.
>>
>> Why did we decide to stop using terminfo as intended? Why did we stop
>> supporting terminal capability inspection via control codes? We
>> solved this problem in the god damn 1980s. Why are we back to
>> specifying terminal characteristics manually?
>
> Hey Daniel,
>
> I don't understand your email. Are you saying that (1) Emacs is already not doing the right thing, and this patch is continuing in this tradition, or that (2) the world is a sad place, because the solution that was found in the 80s wasn't extended to cover 24bit colors, hence the need for this patch, or that (3) this patch is going in the wrong direction, because Emacs is doing things right already and this patch should use the existing facilities instead of whatever it does as submitted, or (4) something else?
>
> I'm not familiar with capability inspection, so apologies if the meaning was clear for people with more background on this.
> Clément.
>

I'm annoyed that there are no common means for detection. I wonder 
whether Emacs can be a forcing function here. If I had it my way,
I'd just tell terminal emulator authors that Emacs will support their
24-bit-color modes as soon as there's a way to detect them and not before.

Re: [PATCH] Support 24-bit terminal colors.

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 1728 bytes --]

On 2016-08-30 18:37, Daniel Colascione wrote:
> On 08/30/2016 03:07 PM, Clément Pit--Claudel wrote:
>> On 2016-08-30 17:03, Daniel Colascione wrote:
>>> On 08/30/2016 01:55 PM, Rami Ylimäki wrote:
>>>> From: Rami Ylimäki <rjy@iki.fi>
>>>>
>>>> Based on previous work by Rüdiger Sonderfeld, Christian Hopps and
>>>> Charles Strahan.
>>>>
>>>> Currently it's not possible to detect whether terminal supports
>>>> 24-bit colors.
>>>
>>> Why did we decide to stop using terminfo as intended? Why did we stop
>>> supporting terminal capability inspection via control codes? We
>>> solved this problem in the god damn 1980s. Why are we back to
>>> specifying terminal characteristics manually?
>>
>> Hey Daniel,
>>
>> I don't understand your email. Are you saying that (1) Emacs is already not doing the right thing, and this patch is continuing in this tradition, or that (2) the world is a sad place, because the solution that was found in the 80s wasn't extended to cover 24bit colors, hence the need for this patch, or that (3) this patch is going in the wrong direction, because Emacs is doing things right already and this patch should use the existing facilities instead of whatever it does as submitted, or (4) something else?
>>
>> I'm not familiar with capability inspection, so apologies if the meaning was clear for people with more background on this.
>> Clément.
>>
> 
> I'm annoyed that there are no common means for detection. I wonder whether Emacs can be a forcing function here. If I had it my way,
> I'd just tell terminal emulator authors that Emacs will support their
> 24-bit-color modes as soon as there's a way to detect them and not before.

Thanks a lot for clarifying.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Support 24-bit terminal colors.

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

Perhaps we should define a way to use termcap or terminfo to
specify whether the terminal supports 24-bit color
and call on terminals to state their capabilities this way.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: [PATCH] Support 24-bit terminal colors.

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Wed, 31 Aug 2016 17:42:39 -0400
> Cc: clement.pit@gmail.com, emacs-devel@gnu.org
> 
> Perhaps we should define a way to use termcap or terminfo to
> specify whether the terminal supports 24-bit color
> and call on terminals to state their capabilities this way.

That was suggested on the ncurses list.

Re: [PATCH] Support 24-bit terminal colors.

[Charles Strahan]

On Wed, Aug 31, 2016, at 10:39 PM, Eli Zaretskii wrote:
> > From: Richard Stallman <rms@gnu.org>
> > Date: Wed, 31 Aug 2016 17:42:39 -0400
> > Cc: clement.pit@gmail.com, emacs-devel@gnu.org
> > 
> > Perhaps we should define a way to use termcap or terminfo to
> > specify whether the terminal supports 24-bit color
> > and call on terminals to state their capabilities this way.
> 
> That was suggested on the ncurses list.
> 

Regrettably, It would appear that the likelihood of that happening is
close to 0%
(https://lists.gnu.org/archive/html/bug-ncurses/2016-08/msg00036.html):

> On Wed, Aug 31, 2016 at 06:40:47PM +0300, Rami Ylimäki wrote:
>> Hi,
>>
>> According to [1], 24-bit terminals have become a de facto standard.
>
> The source is worthless.
>
>> [1] https://gist.github.com/XVilka/8346728
>
>> [2] https://lists.gnu.org/archive/html/bug-ncurses/2013-10/msg00007.html
>
> and the conclusion is unchanged.
>
> end of discussion.
>
> --
> Thomas E. Dickey <address@hidden>
> http://invisible-island.net
> ftp://invisible-island.net

-Charles

Re: [PATCH] Support 24-bit terminal colors.

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > > Perhaps we should define a way to use termcap or terminfo to
  > > > specify whether the terminal supports 24-bit color
  > > > and call on terminals to state their capabilities this way.

  > Regrettably, It would appear that the likelihood of that happening is
  > close to 0%

It isn't easy for me to read an email thread, but your words suggest
you think that this is out of our hands.  That claim surprises me.  We
can establish a convention, and it will catch on because it will be
useful.  That process will take years, but that's ok -- we are not in
a hurry.

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: [PATCH] Support 24-bit terminal colors.

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 1133 bytes --]



On 2016-09-19 18:28, Richard Stallman wrote:
> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> 
>   > > > Perhaps we should define a way to use termcap or terminfo to
>   > > > specify whether the terminal supports 24-bit color
>   > > > and call on terminals to state their capabilities this way.
> 
>   > Regrettably, It would appear that the likelihood of that happening is
>   > close to 0%
> 
> It isn't easy for me to read an email thread, but your words suggest
> you think that this is out of our hands.  That claim surprises me.  We
> can establish a convention, and it will catch on because it will be
> useful.  That process will take years, but that's ok -- we are not in
> a hurry.

Richard, can you elaborate? Would the idea be to define our own standard and use it (noting that it wouldn't do anything at first, since no terminal would advertise itself this way), and wait until people pick that up?

Cheers,
Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Support 24-bit terminal colors.

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Richard, can you elaborate? Would the idea be to define our own
  > standard and use it (noting that it wouldn't do anything at first,
  > since no terminal would advertise itself this way), and wait until
  > people pick that up?

It has worked before.  People who want their terminal programs
to get the best possible handling for Emacs will have a reason
to get them to support it.


-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: [PATCH] Support 24-bit terminal colors.

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 706 bytes --]

On 2016-09-25 13:16, Richard Stallman wrote:
> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> 
>   > Richard, can you elaborate? Would the idea be to define our own
>   > standard and use it (noting that it wouldn't do anything at first,
>   > since no terminal would advertise itself this way), and wait until
>   > people pick that up?
> 
> It has worked before.  People who want their terminal programs
> to get the best possible handling for Emacs will have a reason
> to get them to support it.

Got it; thanks!


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Support 24-bit terminal colors.

[Chad Brown]

> On 25Sep, 2016, at 10:16, Richard Stallman <rms@gnu.org> wrote:
> 
>> Richard, can you elaborate? Would the idea be to define our own
>> standard and use it (noting that it wouldn't do anything at first,
>> since no terminal would advertise itself this way), and wait until
>> people pick that up?
> 
> It has worked before.  People who want their terminal programs
> to get the best possible handling for Emacs will have a reason
> to get them to support it.


This makes sense. It’s probably worthwhile to try to line up with
some other very common terminal programs, to try to avoid splitting
into any more standards. Bash, vim, screen, and tmux come to mind,
but there are probably other good candidates as well.

~Chad

Re: [PATCH] Support 24-bit terminal colors.

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > This makes sense. It’s probably worthwhile to try to line up with
  > some other very common terminal programs, to try to avoid splitting
  > into any more standards. Bash, vim, screen, and tmux come to mind,
  > but there are probably other good candidates as well.

That makes sense; then we could define an extension to terminfo.

Does anyone want to take charge of the project?

-- 
Dr Richard Stallman
President, Free Software Foundation (gnu.org, fsf.org)
Internet Hall-of-Famer (internethalloffame.org)
Skype: No way! See stallman.org/skype.html.

Re: [PATCH] Support 24-bit terminal colors.

[Charles Strahan]

On Mon, Sep 26, 2016, at 12:52 PM, Richard Stallman wrote:
> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
> 
>   > This makes sense. It’s probably worthwhile to try to line up with
>   > some other very common terminal programs, to try to avoid splitting
>   > into any more standards. Bash, vim, screen, and tmux come to mind,
>   > but there are probably other good candidates as well.
> 
> That makes sense; then we could define an extension to terminfo.
> 
> Does anyone want to take charge of the project?
> 
> -- 
> Dr Richard Stallman
> President, Free Software Foundation (gnu.org, fsf.org)
> Internet Hall-of-Famer (internethalloffame.org)
> Skype: No way! See stallman.org/skype.html.


Rami proposed this solution
(https://lists.gnu.org/archive/html/bug-ncurses/2016-08/msg00035.html):

> Hi,
> 
> According to [1], 24-bit terminals have become a de facto standard. However, it seems that 24-bit terminals can't advertise this capability easily.
> 
> I suggest that terminfo database is extended with a generic xterm-24bit entry, which has all the capabilities of xterm-256color and two capabilities for changing non-indexed colors:
> 
> setrgbf=\E[38;2;%p1%d;%p2%d;%p3%dm
> setrgbb=\E[48;2;%p1%d;%p2%d;%p3%dm
> 
> Pretty much all 24-bit terminals listed in [1] support three rgb values separated by semicolons.
> 
> The parameter range could be implicitly 0-255. Alternatively a separate capability could be created to tell how many shades each rgb component supports:
> 
> setrgbn#256
> 
> I know that this has been previously discussed in [2], but I think the biggest problem in that discussion was trying to preserve compability with indexed color modes. I think that the non-indexed capabilities setrgbf, setrgbb and setrgbn should be completely separate from indexed capabilities colors, setab and setaf.


For those more familiar with terminfo, how does that sound? If it seems
reasonable, I could fold that into Rami's patch (unless Rami would like
the honors).

If we do proceed along those lines, would we want to solely rely on that
extension to terminfo, or would we want to (for the time being) also
provide a hardcoded fallback?

Lemme know and I'll see if I can find some time to hack on it.


Thanks,

Charles Strahan

Re: [PATCH] Support 24-bit terminal colors.

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 517 bytes --]

On 2016-08-30 18:37, Daniel Colascione wrote:
> I'm annoyed that there are no common means for detection. I wonder whether Emacs can be a forcing function here. If I had it my way,
> I'd just tell terminal emulator authors that Emacs will support their
> 24-bit-color modes as soon as there's a way to detect them and not before.

Do you know how Microsoft does it? They just announced support for it (https://blogs.msdn.microsoft.com/commandline/2016/09/22/24-bit-color-in-the-windows-console/)

Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] Support 24-bit terminal colors.

[Kalle Olavi Niemitalo]

Clément Pit--Claudel <clement.pit@gmail.com> writes:

> Do you know how Microsoft does it? They just announced support for it
> (https://blogs.msdn.microsoft.com/commandline/2016/09/22/24-bit-color-in-the-windows-console/)

Console Virtual Terminal Sequences
https://msdn.microsoft.com/en-us/library/windows/desktop/mt638032%28v=vs.85%29.aspx
says the Windows console recognizes "38 ; 2 ; <r> ; <g> ; <b>"
(foreground) and "48 ; 2 ; <r> ; <g> ; <b>" (background)
in the Set Graphics Rendition (SGR) control sequence.
However, this Microsoft documentation does not mention how the
feature would be described in termcap or terminfo.

Related bug report: https://github.com/Microsoft/BashOnWindows/issues/52
"Please identify the terminal emulator with TERM, do not pretend to be xterm."

Re: [PATCH] Support 24-bit terminal colors.

[Rami Ylimäki]

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

Unfortunately I'm not aware of any commonly agreed method to detect 24-bit
color support. See https://gist.github.com/XVilka/8346728 and
https://lists.gnu.org/archive/html/bug-ncurses/2013-10/msg00007.html for
some details.


2016-08-31 0:03 GMT+03:00 Daniel Colascione <dancol@dancol.org>:

>
>
> On 08/30/2016 01:55 PM, Rami Ylimäki wrote:
>
>> From: Rami Ylimäki <rjy@iki.fi>
>>
>> Based on previous work by Rüdiger Sonderfeld, Christian Hopps and
>> Charles Strahan.
>>
>> Currently it's not possible to detect whether terminal supports 24-bit
>> colors.
>>
>
> Why did we decide to stop using terminfo as intended? Why did we stop
> supporting terminal capability inspection via control codes? We solved this
> problem in the god damn 1980s. Why are we back to specifying terminal
> characteristics manually?
>

[-- Attachment #2: Type: text/html, Size: 1490 bytes --]

Re: [PATCH] Support 24-bit terminal colors.

[Rami Ylimäki]

[-- Attachment #1: Type: text/plain, Size: 14511 bytes --]

2016-08-31 17:21 GMT+03:00 Eli Zaretskii <eliz@gnu.org>:

> > From: Rami Ylimäki <rjy@iki.fi>
> > Date: Tue, 30 Aug 2016 23:11:46 +0300
> >
> > Based on previous work by Rüdiger Sonderfeld, Christian Hopps and
> > Charles Strahan.
>
> Some of these people don't have copyright assignments on file, so how
> much of the code is actually taken from what they wrote?
>

The code is mostly based on the original patch [1] from Rüdiger.
[1] https://lists.gnu.org/archive/html/emacs-devel/2013-10/msg00461.html


> I also don't find your assignment, but maybe once you adjudicate the
> comments below, the number of changes will be small enough to accept
> without an assignment.  Nevertheless, I encourage you to start your
> legal paperwork rolling, so that we could continue receiving your
> contributions even if this one doesn't need it.
>

Sure, I can assign copyright. I believe you can send me instructions and
necessary documents to sign?


> > Currently it's not possible to detect whether terminal supports 24-bit
> > colors. Therefore following methods can be used to force 24-bit mode on:
> >
> > emacs -nw --color=rgb
> > emacsclient -nw -F "((tty-color-mode . rgb))"
>
> That is a screw.  Is there really no way of detecting true-color
> support?  Not even a way that's specific to each kind of terminal
> emulator that supports this mode?
>

I sent mail to bug-ncurses about this. I think 24-bit terminal support has
become more widespread and de facto standard since Rüdiger contacted them
so that they might be willing to standardize new terminal capabilities
regarding this.


> Asking the user to specify a command-line argument each time is so
> annoying that I'm tempted to say this feature is not yet ready for a
> complex multi-terminal program such as Emacs, and we should wait for
> some kind of standard to emerge before we dump this on our users.
>
> Can we devise some customizable data structure that stores names of
> terminals that support true-color?  Then users could set this list
> from their ~/.emacs init files, and have any frame open on those
> terminals automatically support 24-bit colors.  (If we go this way,
> the data structure will have to be consulted by startup.el at the
> right place, so that the initial frame is already set up correctly.)
>

Yeah, let's see first whether the terminfo guys are willing to add
xterm-24bit entry and related capabilities first.


> In any case, if we do agree to specify this via a command-line
> argument, the same command-line argument should IMO be supported by
> emacsclient; asking users to use Lisp is less user-friendly, IMO.
>

Ok.

> * lisp/server.el (server-process-filter): Pass 'tty-color-mode frame
> >   parameter from command line to server-create-tty-frame.
> >   (server-create-tty-frame): Pass 'tty-color-mode frame parameter to
> >   make-frame.
>
> This part is IMO wrong: the true-color support is a property of the
> terminal, not of a frame.  So we should use terminal-parameters for
> communicating the support, and each frame on that terminal should
> thereafter automatically be set to use 24-bit colors.
>

Got it.

> diff --git a/lisp/term/tty-colors.el b/lisp/term/tty-colors.el
> > index a886950..b9b69cd 100644
> > --- a/lisp/term/tty-colors.el
> > +++ b/lisp/term/tty-colors.el
> > @@ -764,7 +764,8 @@
> >      (auto . 0)
> >      (ansi8 . 8)
> >      (always . 8)
> > -    (yes . 8))
> > +    (yes . 8)
> > +    (rgb . 16777216))
> >    "An alist of supported standard tty color modes and their aliases.")
>
> I think 'rgb' is too general to be useful mnemonically.  How about
> '24bit' or 'true-color' instead?
>

24bit sounds good to me if we can't avoid the entry.


> > +(defun xterm-rgb-support-p ()
> > +  "Check whether 24-bit colors have been forced on."
> > +  (or
> > +   ;; emacs -nw --color=rgb
> > +   (eql (cdr (assoc 'tty-color-mode default-frame-alist)) 'rgb)
> > +   ;; emacsclient -nw -F "((tty-color-mode . rgb))"
> > +   (eql (cdr (assoc 'tty-color-mode (frame-parameters))) 'rgb)))
>
> Not sure why this function is needed.  Why not simply look at the
> number of supported colors, i.e. what display-color-cells returns?


With the changes, xterm-register-default-colors is called when the
auto-detected color count is still in effect. That function is just a dirty
workaround.


> >  (defun xterm-register-default-colors (colors)
> >    "Register the default set of colors for xterm or compatible emulator.
> >
> > @@ -930,6 +938,14 @@ versions of xterm."
> >      ;; are more colors to support, compute them now.
> >      (when (> ncolors 0)
> >        (cond
> > +       ((xterm-rgb-support-p) ; 24-bit xterm
> > +       ;; all named tty colors
> > +       (let ((idx (length xterm-standard-colors)))
> > +         (mapc (lambda (color)
> > +                 (unless (assoc (car color) xterm-standard-colors)
> > +                   (tty-color-define (car color) idx (cdr color))
> > +                   (setq idx (1+ idx))))
> > +               color-name-rgb-alist)))
>
> Why don't you use the RGB spec of the color, encoded in an integer as
> the value of IDX here?  Won't that make your job easier in other
> places, and avoid special-casing the true-color terminal?
>

Good point.


> > +/* Color index bit indicating absence of palette.  */
> > +#define FACE_TTY_NONINDEXED_COLOR ((unsigned long) (1 << 24))
>
> Once again, not sure why this bit is needed, since you have the number
> of colors that can be used for the same purpose.


You are right, it's not needed.


> > +static char *
> > +tparam_color (struct tty_display_info *tty, unsigned long color, bool
> > is_foreground)
> > +{
> > +  const char *ts = is_foreground
> > +                  ? tty->TS_set_foreground
> > +                  : tty->TS_set_background;
> > +  return ts ? tparam (ts, NULL, 0, color, 0, 0, 0) : NULL;
> > +}
> > +
> > +static char *
> > +tparam_rgb (struct tty_display_info *tty, unsigned long color, bool
> > is_foreground)
> > +{
> > +  const char *ts = is_foreground
> > +                  ? tty->TS_set_rgb_foreground
> > +                  : tty->TS_set_rgb_background;
> > +  const int red = (color >> 16) & 0xFF;
> > +  const int green = (color >> 8) & 0xFF;
> > +  const int blue = color & 0xFF;
> > +  return ts ? tparam (ts, NULL, 0, red, green, blue, 0) : NULL;
> > +}
> > +
> > +static void
> > +turn_on_color (struct tty_display_info *tty, unsigned long color, bool
> > is_foreground)
> > +{
> > +  if (face_tty_specified_color (color))
> > +    {
> > +      char *p = color & FACE_TTY_NONINDEXED_COLOR
> > +               ? tparam_rgb (tty, color, is_foreground)
> > +               : tparam_color (tty, color, is_foreground);
> > +      if (p)
> > +       {
> > +         OUTPUT (tty, p);
> > +         xfree (p);
> > +        }
> > +    }
> > +}
> > +
> >  /* Turn appearances of face FACE_ID on tty frame F on.
> >     FACE_ID is a realized face ID number, in the face cache.  */
> >
> > @@ -1930,24 +1967,8 @@ turn_on_face (struct frame *f, int face_id)
> >
> >    if (tty->TN_max_colors > 0)
> >      {
> > -      const char *ts;
> > -      char *p;
> > -
> > -      ts = tty->standout_mode ? tty->TS_set_background :
> > tty->TS_set_foreground;
> > -      if (face_tty_specified_color (fg) && ts)
> > -       {
> > -          p = tparam (ts, NULL, 0, fg, 0, 0, 0);
> > -         OUTPUT (tty, p);
> > -         xfree (p);
> > -       }
> > -
> > -      ts = tty->standout_mode ? tty->TS_set_foreground :
> > tty->TS_set_background;
> > -      if (face_tty_specified_color (bg) && ts)
> > -       {
> > -          p = tparam (ts, NULL, 0, bg, 0, 0, 0);
> > -         OUTPUT (tty, p);
> > -         xfree (p);
> > -       }
> > +      turn_on_color (tty, fg, !tty->standout_mode);
> > +      turn_on_color (tty, bg, tty->standout_mode);
> >      }
> >  }
>
> Is this refactoring really needed?  You now have two extra function
> call levels, in a function that is called _a_lot_ in the inner-most
> loops of the display engine.
>
> AFAIU, the only difference in the true-color case is that the 3 last
> arguments of tparam need to be computed by bitwise operations rather
> than set to zero.  Adding 2 extra levels of indirection just for that
> sounds excessive to me.  I'd keep that inline.
>

Ok.


> > @@ -2067,8 +2089,9 @@ tty_default_color_capabilities (struct
> tty_display_info
> > *tty, bool save)
> >        dupstring (&default_orig_pair, tty->TS_orig_pair);
> >        dupstring (&default_set_foreground, tty->TS_set_foreground);
> >        dupstring (&default_set_background, tty->TS_set_background);
> > +      dupstring (&default_set_rgb_foreground,
> tty->TS_set_rgb_foreground);
> > +      dupstring (&default_set_rgb_background,
> tty->TS_set_rgb_background);
>
> Why do we need 2 new attributes here?  Can't we reuse
> TS_set_foreground and TS_set_background instead?  They seem to be
> unused in the 24-bit color case.
>

Yeah, they aren't needed.


> > -      default_max_pairs = tty->TN_max_pairs;
> [...]
> > @@ -4137,7 +4175,6 @@ use the Bourne shell command 'TERM=...; export
> TERM'
> > (C-shell:\n\
> >         }
>
> >        tty->TN_max_colors = tgetnum ("Co");
> > -      tty->TN_max_pairs = tgetnum ("pa");
> [...]
> > -  /* "pa" -- max. number of color pairs on screen.  Not handled yet.
> > -     Could be a problem if not equal to TN_max_colors * TN_max_colors.
> */
> > -  int TN_max_pairs;
> > -
>
> If deletion of this attribute is not necessary for the patch to work,
> it should be done in a separate commit.
>

It's not needed and is a separate change.


> > +      case 16777216:
> > +       tty->TS_orig_pair = "\033[0m";
> > +       tty->TS_set_foreground = tty->TS_set_background = NULL;
> > +#ifdef TERMINFO
> > +       tty->TS_set_rgb_foreground = "\033[38;2;%p1%d;%p2%d;%p3%dm";
> > +       tty->TS_set_rgb_background = "\033[48;2;%p1%d;%p2%d;%p3%dm";
> > +#else
> > +       tty->TS_set_rgb_foreground = "\033[38;2;%d;%d;%dm";
> > +       tty->TS_set_rgb_background = "\033[48;2;%d;%d;%dm";
> > +#endif
> > +       tty->TN_max_colors = 16777216;
> > +       tty->TN_no_color_video = 0;
> > +        break;
>
> Do all true-color terminals support the same escape sequences for
> 24-bit colors?  Is there any standard documents we could rely upon for
> hard-coding them here?
>

AFAIK, there are no documents. However, it seems [2] that all or almost all
24-bit terminals support those sequences (semicolon separated rgb values in
range 0-255) in practice.
[2] https://gist.github.com/XVilka/8346728


>
> > +static bool
> > +tty_supports_rgb (struct frame *f)
> > +{
> > +  return f->output_method == output_termcap
> > +        && f->output_data.tty->display_info->TS_set_rgb_foreground
> > +        && f->output_data.tty->display_info->TS_set_rgb_background;
> > +}
>
> Once again, why not just look at the number of supported colors?


Yeah.


> > +static bool
> > +parse_and_encode_rgb_list (Lisp_Object rgb_list, XColor *color)
> > +{
> > +  if (!parse_rgb_list (rgb_list, color))
> > +    return false;
> > +
> > +  color->pixel = FACE_TTY_NONINDEXED_COLOR
> > +                | (color->red / 256) << 16
> > +                | (color->green / 256) << 8
> > +                | (color->blue / 256);
> > +  return true;
> > +}
> > +
> > +
> > +static bool
> > +tty_lookup_rgb (Lisp_Object color, XColor *tty_color, XColor *std_color)
> > +{
> > +  if (NILP (Ffboundp (Qtty_color_standard_values)))
> > +    return false;
> > +
> > +  if (!NILP (Ffboundp (Qtty_color_canonicalize)))
> > +    color = call1 (Qtty_color_canonicalize, color);
> > +
> > +  color = call1 (Qtty_color_standard_values, color);
> > +  if (!parse_and_encode_rgb_list (color, tty_color))
> > +    return false;
> > +
> > +  if (std_color)
> > +    *std_color = *tty_color;
> > +
> > +  return true;
> > +}
> >
> >  /* Lookup on frame F the color described by the lisp string COLOR.
> >     The resulting tty color is returned in TTY_COLOR; if STD_COLOR is
> > @@ -844,6 +883,9 @@ tty_lookup_color (struct frame *f, Lisp_Object color,
> > XColor *tty_color,
> >  {
> >    Lisp_Object frame, color_desc;
> >
> > +  if (tty_supports_rgb (f))
> > +    return tty_lookup_rgb (color, tty_color, std_color);
> > +
> >    if (!STRINGP (color) || NILP (Ffboundp (Qtty_color_desc)))
> >      return false;
> >
> > @@ -5707,8 +5749,12 @@ map_tty_color (struct frame *f, struct face *face,
> >           CONSP (def)))
> >      {
> >        /* Associations in tty-defined-color-alist are of the form
> > -        (NAME INDEX R G B).  We need the INDEX part.  */
> > -      pixel = XINT (XCAR (XCDR (def)));
> > +        (NAME INDEX R G B).  We need the (R G B) or INDEX part.  */
> > +      XColor tty_color;
> > +      if (tty_supports_rgb (f) && parse_and_encode_rgb_list (XCDR (XCDR
> > (def)), &tty_color))
> > +       pixel = tty_color.pixel;
> > +      else
> > +       pixel = XINT (XCAR (XCDR (def)));
> >      }
>
> Sorry, I don't understand why this complexity is needed.  AFAIU, a
> true-color terminal can produce color directly from its RGB
> components, without the need to look up that RGB descriptor via
> tty-color-desc.  In addition, support for named colors, such as
> "burlywood1", is still required for these terminals, since most color
> specifications in Emacs are of that variety.  Is my understanding
> correct?
>
> If I'm right, then you could either (a) extend tty-color-desc, such
> that it would cons the (NAME INDEX R G B) list for a true-color
> terminal and hand it back to tty_lookup_color; or (b) if you wanted to
> be more efficient, generate the pixel value directly on the C level by
> parsing the color spec if it is of the form "#RRGGBB", and otherwise
> calling tty-color-desc as usual.  Assuming that tty-color-alist is set
> correctly for the true-color terminal, and stores the RGB values for
> each color, that will do the job, eliminating the need for all these
> support functions: parse_and_encode_rgb_list, tty_supports_rgb, and
> tty_lookup_rgb, and also save you 2 extra calls to Lisp.  Am I missing
> something?
>

Not missing anything. Thanks for detailed review and good advice.


> Thanks.
>

[-- Attachment #2: Type: text/html, Size: 20394 bytes --]

Re: [PATCH] Support 24-bit terminal colors.

[Eli Zaretskii]

> From: Rami Ylimäki <rami.ylimaki@vincit.com>
> Date: Wed, 31 Aug 2016 20:47:10 +0300
> 
> Sure, I can assign copyright. I believe you can send me instructions and necessary documents to sign?

The form was sent off-list.

> Not missing anything. Thanks for detailed review and good advice.

Thanks, looking forward for a revised patch.

[PATCH] Support 24-bit terminal colors.

[Rami Ylimäki]

Based on previous work by Rüdiger Sonderfeld, Christian Hopps and
Charles Strahan.

Currently it's not possible to detect whether terminal supports 24-bit
colors. Therefore following methods can be used to force 24-bit mode on:

emacs -nw --color=rgb
emacsclient -nw -F "((tty-color-mode . rgb))"

* lisp/server.el (server-process-filter): Pass 'tty-color-mode frame
  parameter from command line to server-create-tty-frame.
  (server-create-tty-frame): Pass 'tty-color-mode frame parameter to
  make-frame.

* lisp/term/tty-colors.el (tty-color-mode-alist): Add 'rgb entry to
  support 24-bit terminals. This enables the --color=rgb option as well.

* lisp/term/xterm.el (xterm-register-default-colors): Define all named
  tty colors. Add xterm-rgb-support-b to check whether 24-bit mode has
  been forced on.

* src/dispextern.h: Define bit FACE_TTY_NONINDEXED_COLOR to separate
  indexed and rgb component based tty color encodings. The rgb component
  based color "index" is simply represented by a 24-bit rgb triplet with
  FACE_TTY_NONINDEXED_COLOR set.

* src/termchar.h (tty_display_info): Remove TN_max_pairs field
  describing maximum number of color pairs. The field is unused and
  would also be inconvenient with 24-bit colors. Add new tty control
  functions TS_set_rgb_foreground and TS_set_rgb_background for
  outputting rgb component based tty colors.

* src/term.c (turn_on_face): Output indexed or rgb component based tty
  colors by detecting whether FACE_TTY_NONINDEXED_COLOR has been set.
  (tty_default_color_capabilities): Add TS_set_rgb_foreground and
  TS_set_rgb_background.
  (tty_setup_colors): Initialize tty_display_info for 24-bit terminals.

* src/xfaces.c (tty_lookup_color): Use rgb component based tty color
  encoding on 24-bit terminals.
  (map_tty_color): Use rgb component based tty color encoding for face
  foreground and background on 24-bit terminals.
---
 doc/man/emacs.1.in      |  2 +-
 lisp/server.el          | 14 ++++++--
 lisp/term/tty-colors.el |  3 +-
 lisp/term/xterm.el      | 16 +++++++++
 src/dispextern.h        |  3 ++
 src/term.c              | 89 +++++++++++++++++++++++++++++++++++--------------
 src/termchar.h          |  7 ++--
 src/xfaces.c            | 51 ++++++++++++++++++++++++++--
 8 files changed, 149 insertions(+), 36 deletions(-)

diff --git a/doc/man/emacs.1.in b/doc/man/emacs.1.in
index 3b1566f..79d48fb 100644
--- a/doc/man/emacs.1.in
+++ b/doc/man/emacs.1.in
@@ -252,7 +252,7 @@ Set additional X resources.
 Override color mode for character terminals;
 .I mode
 defaults to "auto", and can also be "never", "auto", "always",
-or a mode name like "ansi8".
+or a mode name like "ansi8". Use "rgb" for 24-bit colors.
 .TP
 .BI \-bw " pixels\fR,\fP " \-\-border\-width " pixels"
 Set the
diff --git a/lisp/server.el b/lisp/server.el
index 5300984..5ecd483 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -792,11 +792,13 @@ This handles splitting the command if it would be bigger than
       (setq prefix "-print-nonl "))
     (server-send-string proc (concat prefix qtext "\n"))))
 
-(defun server-create-tty-frame (tty type proc)
+(defun server-create-tty-frame (tty type color-mode proc)
   (unless tty
     (error "Invalid terminal device"))
   (unless type
     (error "Invalid terminal type"))
+  (unless color-mode
+    (error "Invalid terminal color mode"))
   (add-to-list 'frame-inherited-parameters 'client)
   (let ((frame
          (server-with-environment
@@ -812,6 +814,7 @@ This handles splitting the command if it would be bigger than
            (make-frame `((window-system . nil)
                          (tty . ,tty)
                          (tty-type . ,type)
+                         (tty-color-mode . ,color-mode)
                          ;; Ignore nowait here; we always need to
                          ;; clean up opened ttys when the client dies.
                          (client . ,proc)
@@ -1055,6 +1058,8 @@ The following commands are accepted by the client:
 		(coding-system (and (default-value 'enable-multibyte-characters)
 				    (or file-name-coding-system
 					default-file-name-coding-system)))
+		(tty-color-mode 'default)
+
 		nowait     ; t if emacsclient does not want to wait for us.
 		frame      ; Frame opened for the client (if any).
 		display    ; Open frame on this display.
@@ -1089,7 +1094,10 @@ The following commands are accepted by the client:
                  (let ((alist (pop args-left)))
                    (if coding-system
                        (setq alist (decode-coding-string alist coding-system)))
-                   (setq frame-parameters (car (read-from-string alist)))))
+                   (setq frame-parameters (car (read-from-string alist)))
+                   (let ((color-mode (cdr (assoc 'tty-color-mode frame-parameters))))
+                     (if (assoc color-mode tty-color-mode-alist)
+                         (setq tty-color-mode color-mode)))))
 
                 ;; -display DISPLAY:
                 ;; Open X frames on the given display instead of the default.
@@ -1241,7 +1249,7 @@ The following commands are accepted by the client:
 						       frame-parameters))
 		   ;; When resuming on a tty, tty-name is nil.
 		   (tty-name
-		    (server-create-tty-frame tty-name tty-type proc))))
+		    (server-create-tty-frame tty-name tty-type tty-color-mode proc))))
 
             (process-put
              proc 'continuation
diff --git a/lisp/term/tty-colors.el b/lisp/term/tty-colors.el
index a886950..b9b69cd 100644
--- a/lisp/term/tty-colors.el
+++ b/lisp/term/tty-colors.el
@@ -764,7 +764,8 @@
     (auto . 0)
     (ansi8 . 8)
     (always . 8)
-    (yes . 8))
+    (yes . 8)
+    (rgb . 16777216))
   "An alist of supported standard tty color modes and their aliases.")
 
 (defun tty-color-alist (&optional _frame)
diff --git a/lisp/term/xterm.el b/lisp/term/xterm.el
index 01c0113..e0f3eb5 100644
--- a/lisp/term/xterm.el
+++ b/lisp/term/xterm.el
@@ -905,6 +905,14 @@ hitting screen's max DCS length."
   "Convert an 8-bit primary color value PRIM to a corresponding 16-bit value."
   (logior prim (lsh prim 8)))
 
+(defun xterm-rgb-support-p ()
+  "Check whether 24-bit colors have been forced on."
+  (or
+   ;; emacs -nw --color=rgb
+   (eql (cdr (assoc 'tty-color-mode default-frame-alist)) 'rgb)
+   ;; emacsclient -nw -F "((tty-color-mode . rgb))"
+   (eql (cdr (assoc 'tty-color-mode (frame-parameters))) 'rgb)))
+
 (defun xterm-register-default-colors (colors)
   "Register the default set of colors for xterm or compatible emulator.
 
@@ -930,6 +938,14 @@ versions of xterm."
     ;; are more colors to support, compute them now.
     (when (> ncolors 0)
       (cond
+       ((xterm-rgb-support-p) ; 24-bit xterm
+	;; all named tty colors
+	(let ((idx (length xterm-standard-colors)))
+	  (mapc (lambda (color)
+		  (unless (assoc (car color) xterm-standard-colors)
+		    (tty-color-define (car color) idx (cdr color))
+		    (setq idx (1+ idx))))
+		color-name-rgb-alist)))
        ((= ncolors 240)	; 256-color xterm
 	;; 216 non-gray colors first
 	(let ((r 0) (g 0) (b 0))
diff --git a/src/dispextern.h b/src/dispextern.h
index f2c42de..35d2106 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1751,6 +1751,9 @@ struct face
 
 #define FACE_TTY_DEFAULT_BG_COLOR ((unsigned long) -3)
 
+/* Color index bit indicating absence of palette.  */
+#define FACE_TTY_NONINDEXED_COLOR ((unsigned long) (1 << 24))
+
 /* True if COLOR is a specified (i.e., nondefault) foreground or
    background color for a tty face.  */
 
diff --git a/src/term.c b/src/term.c
index d54ff11..dc6028d 100644
--- a/src/term.c
+++ b/src/term.c
@@ -1891,6 +1891,43 @@ produce_glyphless_glyph (struct it *it, Lisp_Object acronym)
    ? (tty->TN_no_color_video & (ATTR)) == 0             \
    : 1)
 
+static char *
+tparam_color (struct tty_display_info *tty, unsigned long color, bool is_foreground)
+{
+  const char *ts = is_foreground
+		   ? tty->TS_set_foreground
+		   : tty->TS_set_background;
+  return ts ? tparam (ts, NULL, 0, color, 0, 0, 0) : NULL;
+}
+
+static char *
+tparam_rgb (struct tty_display_info *tty, unsigned long color, bool is_foreground)
+{
+  const char *ts = is_foreground
+		   ? tty->TS_set_rgb_foreground
+		   : tty->TS_set_rgb_background;
+  const int red = (color >> 16) & 0xFF;
+  const int green = (color >> 8) & 0xFF;
+  const int blue = color & 0xFF;
+  return ts ? tparam (ts, NULL, 0, red, green, blue, 0) : NULL;
+}
+
+static void
+turn_on_color (struct tty_display_info *tty, unsigned long color, bool is_foreground)
+{
+  if (face_tty_specified_color (color))
+    {
+      char *p = color & FACE_TTY_NONINDEXED_COLOR
+		? tparam_rgb (tty, color, is_foreground)
+		: tparam_color (tty, color, is_foreground);
+      if (p)
+	{
+	  OUTPUT (tty, p);
+	  xfree (p);
+        }
+    }
+}
+
 /* Turn appearances of face FACE_ID on tty frame F on.
    FACE_ID is a realized face ID number, in the face cache.  */
 
@@ -1930,24 +1967,8 @@ turn_on_face (struct frame *f, int face_id)
 
   if (tty->TN_max_colors > 0)
     {
-      const char *ts;
-      char *p;
-
-      ts = tty->standout_mode ? tty->TS_set_background : tty->TS_set_foreground;
-      if (face_tty_specified_color (fg) && ts)
-	{
-          p = tparam (ts, NULL, 0, fg, 0, 0, 0);
-	  OUTPUT (tty, p);
-	  xfree (p);
-	}
-
-      ts = tty->standout_mode ? tty->TS_set_foreground : tty->TS_set_background;
-      if (face_tty_specified_color (bg) && ts)
-	{
-          p = tparam (ts, NULL, 0, bg, 0, 0, 0);
-	  OUTPUT (tty, p);
-	  xfree (p);
-	}
+      turn_on_color (tty, fg, !tty->standout_mode);
+      turn_on_color (tty, bg, tty->standout_mode);
     }
 }
 
@@ -2050,11 +2071,12 @@ TERMINAL does not refer to a text terminal.  */)
    to work around an HPUX compiler bug (?). See
    http://lists.gnu.org/archive/html/emacs-devel/2007-08/msg00410.html  */
 static int default_max_colors;
-static int default_max_pairs;
 static int default_no_color_video;
 static char *default_orig_pair;
 static char *default_set_foreground;
 static char *default_set_background;
+static char *default_set_rgb_foreground;
+static char *default_set_rgb_background;
 
 /* Save or restore the default color-related capabilities of this
    terminal.  */
@@ -2067,8 +2089,9 @@ tty_default_color_capabilities (struct tty_display_info *tty, bool save)
       dupstring (&default_orig_pair, tty->TS_orig_pair);
       dupstring (&default_set_foreground, tty->TS_set_foreground);
       dupstring (&default_set_background, tty->TS_set_background);
+      dupstring (&default_set_rgb_foreground, tty->TS_set_rgb_foreground);
+      dupstring (&default_set_rgb_background, tty->TS_set_rgb_background);
       default_max_colors = tty->TN_max_colors;
-      default_max_pairs = tty->TN_max_pairs;
       default_no_color_video = tty->TN_no_color_video;
     }
   else
@@ -2076,8 +2099,9 @@ tty_default_color_capabilities (struct tty_display_info *tty, bool save)
       tty->TS_orig_pair = default_orig_pair;
       tty->TS_set_foreground = default_set_foreground;
       tty->TS_set_background = default_set_background;
+      tty->TS_set_rgb_foreground = default_set_rgb_foreground;
+      tty->TS_set_rgb_background = default_set_rgb_background;
       tty->TN_max_colors = default_max_colors;
-      tty->TN_max_pairs = default_max_pairs;
       tty->TN_no_color_video = default_no_color_video;
     }
 }
@@ -2097,9 +2121,10 @@ tty_setup_colors (struct tty_display_info *tty, int mode)
     {
       case -1:	 /* no colors at all */
 	tty->TN_max_colors = 0;
-	tty->TN_max_pairs = 0;
 	tty->TN_no_color_video = 0;
-	tty->TS_set_foreground = tty->TS_set_background = tty->TS_orig_pair = NULL;
+	tty->TS_orig_pair = NULL;
+	tty->TS_set_foreground = tty->TS_set_background = NULL;
+	tty->TS_set_rgb_foreground = tty->TS_set_rgb_background = NULL;
 	break;
       case 0:	 /* default colors, if any */
       default:
@@ -2107,6 +2132,7 @@ tty_setup_colors (struct tty_display_info *tty, int mode)
 	break;
       case 8:	/* 8 standard ANSI colors */
 	tty->TS_orig_pair = "\033[0m";
+	tty->TS_set_rgb_foreground = tty->TS_set_rgb_background = NULL;
 #ifdef TERMINFO
 	tty->TS_set_foreground = "\033[3%p1%dm";
 	tty->TS_set_background = "\033[4%p1%dm";
@@ -2115,9 +2141,21 @@ tty_setup_colors (struct tty_display_info *tty, int mode)
 	tty->TS_set_background = "\033[4%dm";
 #endif
 	tty->TN_max_colors = 8;
-	tty->TN_max_pairs = 64;
 	tty->TN_no_color_video = 0;
 	break;
+      case 16777216:
+	tty->TS_orig_pair = "\033[0m";
+	tty->TS_set_foreground = tty->TS_set_background = NULL;
+#ifdef TERMINFO
+	tty->TS_set_rgb_foreground = "\033[38;2;%p1%d;%p2%d;%p3%dm";
+	tty->TS_set_rgb_background = "\033[48;2;%p1%d;%p2%d;%p3%dm";
+#else
+	tty->TS_set_rgb_foreground = "\033[38;2;%d;%d;%dm";
+	tty->TS_set_rgb_background = "\033[48;2;%d;%d;%dm";
+#endif
+	tty->TN_max_colors = 16777216;
+	tty->TN_no_color_video = 0;
+        break;
     }
 }
 
@@ -4137,7 +4175,6 @@ use the Bourne shell command 'TERM=...; export TERM' (C-shell:\n\
 	}
 
       tty->TN_max_colors = tgetnum ("Co");
-      tty->TN_max_pairs = tgetnum ("pa");
 
       tty->TN_no_color_video = tgetnum ("NC");
       if (tty->TN_no_color_video == -1)
@@ -4514,6 +4551,8 @@ bigger, or it may make it blink, or it may do nothing at all.  */);
   default_orig_pair = NULL;
   default_set_foreground = NULL;
   default_set_background = NULL;
+  default_set_rgb_foreground = NULL;
+  default_set_rgb_background = NULL;
 #endif /* !DOS_NT */
 
   encode_terminal_src = NULL;
diff --git a/src/termchar.h b/src/termchar.h
index 35b30fb..81fe96e 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -149,10 +149,6 @@ struct tty_display_info
 
   int TN_max_colors;            /* "Co" -- number of colors.  */
 
-  /* "pa" -- max. number of color pairs on screen.  Not handled yet.
-     Could be a problem if not equal to TN_max_colors * TN_max_colors.  */
-  int TN_max_pairs;
-
   /* "op" -- SVr4 set default pair to its original value.  */
   const char *TS_orig_pair;
 
@@ -160,6 +156,9 @@ struct tty_display_info
      1 param, the color index.  */
   const char *TS_set_foreground;
   const char *TS_set_background;
+  /* 3 params: red, green and blue. */
+  const char *TS_set_rgb_foreground;
+  const char *TS_set_rgb_background;
 
   int TF_hazeltine;             /* termcap hz flag. */
   int TF_insmode_motion;        /* termcap mi flag: can move while in insert mode. */
diff --git a/src/xfaces.c b/src/xfaces.c
index 0a1315d..e4650fd 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -832,6 +832,45 @@ parse_rgb_list (Lisp_Object rgb_list, XColor *color)
   return true;
 }
 
+static bool
+parse_and_encode_rgb_list (Lisp_Object rgb_list, XColor *color)
+{
+  if (!parse_rgb_list (rgb_list, color))
+    return false;
+
+  color->pixel = FACE_TTY_NONINDEXED_COLOR
+		 | (color->red / 256) << 16
+		 | (color->green / 256) << 8
+		 | (color->blue / 256);
+  return true;
+}
+
+static bool
+tty_supports_rgb (struct frame *f)
+{
+  return f->output_method == output_termcap
+	 && f->output_data.tty->display_info->TS_set_rgb_foreground
+	 && f->output_data.tty->display_info->TS_set_rgb_background;
+}
+
+static bool
+tty_lookup_rgb (Lisp_Object color, XColor *tty_color, XColor *std_color)
+{
+  if (NILP (Ffboundp (Qtty_color_standard_values)))
+    return false;
+
+  if (!NILP (Ffboundp (Qtty_color_canonicalize)))
+    color = call1 (Qtty_color_canonicalize, color);
+
+  color = call1 (Qtty_color_standard_values, color);
+  if (!parse_and_encode_rgb_list (color, tty_color))
+    return false;
+
+  if (std_color)
+    *std_color = *tty_color;
+
+  return true;
+}
 
 /* Lookup on frame F the color described by the lisp string COLOR.
    The resulting tty color is returned in TTY_COLOR; if STD_COLOR is
@@ -844,6 +883,9 @@ tty_lookup_color (struct frame *f, Lisp_Object color, XColor *tty_color,
 {
   Lisp_Object frame, color_desc;
 
+  if (tty_supports_rgb (f))
+    return tty_lookup_rgb (color, tty_color, std_color);
+
   if (!STRINGP (color) || NILP (Ffboundp (Qtty_color_desc)))
     return false;
 
@@ -5707,8 +5749,12 @@ map_tty_color (struct frame *f, struct face *face,
 	  CONSP (def)))
     {
       /* Associations in tty-defined-color-alist are of the form
-	 (NAME INDEX R G B).  We need the INDEX part.  */
-      pixel = XINT (XCAR (XCDR (def)));
+	 (NAME INDEX R G B).  We need the (R G B) or INDEX part.  */
+      XColor tty_color;
+      if (tty_supports_rgb (f) && parse_and_encode_rgb_list (XCDR (XCDR (def)), &tty_color))
+	pixel = tty_color.pixel;
+      else
+	pixel = XINT (XCAR (XCDR (def)));
     }
 
   if (pixel == default_pixel && STRINGP (color))
@@ -6395,6 +6441,7 @@ syms_of_xfaces (void)
   DEFSYM (Qwindow_divider_first_pixel, "window-divider-first-pixel");
   DEFSYM (Qwindow_divider_last_pixel, "window-divider-last-pixel");
   DEFSYM (Qtty_color_desc, "tty-color-desc");
+  DEFSYM (Qtty_color_canonicalize, "tty-color-canonicalize");
   DEFSYM (Qtty_color_standard_values, "tty-color-standard-values");
   DEFSYM (Qtty_color_by_index, "tty-color-by-index");
 
-- 
2.7.4

Re: [PATCH] Support 24-bit terminal colors.

[Eli Zaretskii]

> From: Rami Ylimäki <rjy@iki.fi>
> Date: Tue, 30 Aug 2016 23:11:46 +0300
> 
> Based on previous work by Rüdiger Sonderfeld, Christian Hopps and
> Charles Strahan.

Some of these people don't have copyright assignments on file, so how
much of the code is actually taken from what they wrote?

I also don't find your assignment, but maybe once you adjudicate the
comments below, the number of changes will be small enough to accept
without an assignment.  Nevertheless, I encourage you to start your
legal paperwork rolling, so that we could continue receiving your
contributions even if this one doesn't need it.

> Currently it's not possible to detect whether terminal supports 24-bit
> colors. Therefore following methods can be used to force 24-bit mode on:
> 
> emacs -nw --color=rgb
> emacsclient -nw -F "((tty-color-mode . rgb))"

That is a screw.  Is there really no way of detecting true-color
support?  Not even a way that's specific to each kind of terminal
emulator that supports this mode?

Asking the user to specify a command-line argument each time is so
annoying that I'm tempted to say this feature is not yet ready for a
complex multi-terminal program such as Emacs, and we should wait for
some kind of standard to emerge before we dump this on our users.

Can we devise some customizable data structure that stores names of
terminals that support true-color?  Then users could set this list
from their ~/.emacs init files, and have any frame open on those
terminals automatically support 24-bit colors.  (If we go this way,
the data structure will have to be consulted by startup.el at the
right place, so that the initial frame is already set up correctly.)

In any case, if we do agree to specify this via a command-line
argument, the same command-line argument should IMO be supported by
emacsclient; asking users to use Lisp is less user-friendly, IMO.

> * lisp/server.el (server-process-filter): Pass 'tty-color-mode frame
>   parameter from command line to server-create-tty-frame.
>   (server-create-tty-frame): Pass 'tty-color-mode frame parameter to
>   make-frame.

This part is IMO wrong: the true-color support is a property of the
terminal, not of a frame.  So we should use terminal-parameters for
communicating the support, and each frame on that terminal should
thereafter automatically be set to use 24-bit colors.

> diff --git a/lisp/term/tty-colors.el b/lisp/term/tty-colors.el
> index a886950..b9b69cd 100644
> --- a/lisp/term/tty-colors.el
> +++ b/lisp/term/tty-colors.el
> @@ -764,7 +764,8 @@
>      (auto . 0)
>      (ansi8 . 8)
>      (always . 8)
> -    (yes . 8))
> +    (yes . 8)
> +    (rgb . 16777216))
>    "An alist of supported standard tty color modes and their aliases.")

I think 'rgb' is too general to be useful mnemonically.  How about
'24bit' or 'true-color' instead?

> +(defun xterm-rgb-support-p ()
> +  "Check whether 24-bit colors have been forced on."
> +  (or
> +   ;; emacs -nw --color=rgb
> +   (eql (cdr (assoc 'tty-color-mode default-frame-alist)) 'rgb)
> +   ;; emacsclient -nw -F "((tty-color-mode . rgb))"
> +   (eql (cdr (assoc 'tty-color-mode (frame-parameters))) 'rgb)))

Not sure why this function is needed.  Why not simply look at the
number of supported colors, i.e. what display-color-cells returns?

>  (defun xterm-register-default-colors (colors)
>    "Register the default set of colors for xterm or compatible emulator.
>  
> @@ -930,6 +938,14 @@ versions of xterm."
>      ;; are more colors to support, compute them now.
>      (when (> ncolors 0)
>        (cond
> +       ((xterm-rgb-support-p) ; 24-bit xterm
> +       ;; all named tty colors
> +       (let ((idx (length xterm-standard-colors)))
> +         (mapc (lambda (color)
> +                 (unless (assoc (car color) xterm-standard-colors)
> +                   (tty-color-define (car color) idx (cdr color))
> +                   (setq idx (1+ idx))))
> +               color-name-rgb-alist)))

Why don't you use the RGB spec of the color, encoded in an integer as
the value of IDX here?  Won't that make your job easier in other
places, and avoid special-casing the true-color terminal?

> +/* Color index bit indicating absence of palette.  */
> +#define FACE_TTY_NONINDEXED_COLOR ((unsigned long) (1 << 24))

Once again, not sure why this bit is needed, since you have the number
of colors that can be used for the same purpose.

> +static char *
> +tparam_color (struct tty_display_info *tty, unsigned long color, bool 
> is_foreground)
> +{
> +  const char *ts = is_foreground
> +                  ? tty->TS_set_foreground
> +                  : tty->TS_set_background;
> +  return ts ? tparam (ts, NULL, 0, color, 0, 0, 0) : NULL;
> +}
> +
> +static char *
> +tparam_rgb (struct tty_display_info *tty, unsigned long color, bool 
> is_foreground)
> +{
> +  const char *ts = is_foreground
> +                  ? tty->TS_set_rgb_foreground
> +                  : tty->TS_set_rgb_background;
> +  const int red = (color >> 16) & 0xFF;
> +  const int green = (color >> 8) & 0xFF;
> +  const int blue = color & 0xFF;
> +  return ts ? tparam (ts, NULL, 0, red, green, blue, 0) : NULL;
> +}
> +
> +static void
> +turn_on_color (struct tty_display_info *tty, unsigned long color, bool 
> is_foreground)
> +{
> +  if (face_tty_specified_color (color))
> +    {
> +      char *p = color & FACE_TTY_NONINDEXED_COLOR
> +               ? tparam_rgb (tty, color, is_foreground)
> +               : tparam_color (tty, color, is_foreground);
> +      if (p)
> +       {
> +         OUTPUT (tty, p);
> +         xfree (p);
> +        }
> +    }
> +}
> +
>  /* Turn appearances of face FACE_ID on tty frame F on.
>     FACE_ID is a realized face ID number, in the face cache.  */
> 
> @@ -1930,24 +1967,8 @@ turn_on_face (struct frame *f, int face_id)
> 
>    if (tty->TN_max_colors > 0)
>      {
> -      const char *ts;
> -      char *p;
> -
> -      ts = tty->standout_mode ? tty->TS_set_background : 
> tty->TS_set_foreground;
> -      if (face_tty_specified_color (fg) && ts)
> -       {
> -          p = tparam (ts, NULL, 0, fg, 0, 0, 0);
> -         OUTPUT (tty, p);
> -         xfree (p);
> -       }
> -
> -      ts = tty->standout_mode ? tty->TS_set_foreground : 
> tty->TS_set_background;
> -      if (face_tty_specified_color (bg) && ts)
> -       {
> -          p = tparam (ts, NULL, 0, bg, 0, 0, 0);
> -         OUTPUT (tty, p);
> -         xfree (p);
> -       }
> +      turn_on_color (tty, fg, !tty->standout_mode);
> +      turn_on_color (tty, bg, tty->standout_mode);
>      }
>  }

Is this refactoring really needed?  You now have two extra function
call levels, in a function that is called _a_lot_ in the inner-most
loops of the display engine.

AFAIU, the only difference in the true-color case is that the 3 last
arguments of tparam need to be computed by bitwise operations rather
than set to zero.  Adding 2 extra levels of indirection just for that
sounds excessive to me.  I'd keep that inline.

> @@ -2067,8 +2089,9 @@ tty_default_color_capabilities (struct tty_display_info 
> *tty, bool save)
>        dupstring (&default_orig_pair, tty->TS_orig_pair);
>        dupstring (&default_set_foreground, tty->TS_set_foreground);
>        dupstring (&default_set_background, tty->TS_set_background);
> +      dupstring (&default_set_rgb_foreground, tty->TS_set_rgb_foreground);
> +      dupstring (&default_set_rgb_background, tty->TS_set_rgb_background);

Why do we need 2 new attributes here?  Can't we reuse
TS_set_foreground and TS_set_background instead?  They seem to be
unused in the 24-bit color case.

> -      default_max_pairs = tty->TN_max_pairs;
[...]
> @@ -4137,7 +4175,6 @@ use the Bourne shell command 'TERM=...; export TERM' 
> (C-shell:\n\
>         }
 
>        tty->TN_max_colors = tgetnum ("Co");
> -      tty->TN_max_pairs = tgetnum ("pa");
[...] 
> -  /* "pa" -- max. number of color pairs on screen.  Not handled yet.
> -     Could be a problem if not equal to TN_max_colors * TN_max_colors.  */
> -  int TN_max_pairs;
> -

If deletion of this attribute is not necessary for the patch to work,
it should be done in a separate commit.

> +      case 16777216:
> +       tty->TS_orig_pair = "\033[0m";
> +       tty->TS_set_foreground = tty->TS_set_background = NULL;
> +#ifdef TERMINFO
> +       tty->TS_set_rgb_foreground = "\033[38;2;%p1%d;%p2%d;%p3%dm";
> +       tty->TS_set_rgb_background = "\033[48;2;%p1%d;%p2%d;%p3%dm";
> +#else
> +       tty->TS_set_rgb_foreground = "\033[38;2;%d;%d;%dm";
> +       tty->TS_set_rgb_background = "\033[48;2;%d;%d;%dm";
> +#endif
> +       tty->TN_max_colors = 16777216;
> +       tty->TN_no_color_video = 0;
> +        break;

Do all true-color terminals support the same escape sequences for
24-bit colors?  Is there any standard documents we could rely upon for
hard-coding them here?
 
> +static bool
> +tty_supports_rgb (struct frame *f)
> +{
> +  return f->output_method == output_termcap
> +        && f->output_data.tty->display_info->TS_set_rgb_foreground
> +        && f->output_data.tty->display_info->TS_set_rgb_background;
> +}

Once again, why not just look at the number of supported colors?

> +static bool
> +parse_and_encode_rgb_list (Lisp_Object rgb_list, XColor *color)
> +{
> +  if (!parse_rgb_list (rgb_list, color))
> +    return false;
> +
> +  color->pixel = FACE_TTY_NONINDEXED_COLOR
> +                | (color->red / 256) << 16
> +                | (color->green / 256) << 8
> +                | (color->blue / 256);
> +  return true;
> +}
> +
> +
> +static bool
> +tty_lookup_rgb (Lisp_Object color, XColor *tty_color, XColor *std_color)
> +{
> +  if (NILP (Ffboundp (Qtty_color_standard_values)))
> +    return false;
> +
> +  if (!NILP (Ffboundp (Qtty_color_canonicalize)))
> +    color = call1 (Qtty_color_canonicalize, color);
> +
> +  color = call1 (Qtty_color_standard_values, color);
> +  if (!parse_and_encode_rgb_list (color, tty_color))
> +    return false;
> +
> +  if (std_color)
> +    *std_color = *tty_color;
> +
> +  return true;
> +}
> 
>  /* Lookup on frame F the color described by the lisp string COLOR.
>     The resulting tty color is returned in TTY_COLOR; if STD_COLOR is
> @@ -844,6 +883,9 @@ tty_lookup_color (struct frame *f, Lisp_Object color, 
> XColor *tty_color,
>  {
>    Lisp_Object frame, color_desc;
> 
> +  if (tty_supports_rgb (f))
> +    return tty_lookup_rgb (color, tty_color, std_color);
> +
>    if (!STRINGP (color) || NILP (Ffboundp (Qtty_color_desc)))
>      return false;
> 
> @@ -5707,8 +5749,12 @@ map_tty_color (struct frame *f, struct face *face,
>           CONSP (def)))
>      {
>        /* Associations in tty-defined-color-alist are of the form
> -        (NAME INDEX R G B).  We need the INDEX part.  */
> -      pixel = XINT (XCAR (XCDR (def)));
> +        (NAME INDEX R G B).  We need the (R G B) or INDEX part.  */
> +      XColor tty_color;
> +      if (tty_supports_rgb (f) && parse_and_encode_rgb_list (XCDR (XCDR 
> (def)), &tty_color))
> +       pixel = tty_color.pixel;
> +      else
> +       pixel = XINT (XCAR (XCDR (def)));
>      }

Sorry, I don't understand why this complexity is needed.  AFAIU, a
true-color terminal can produce color directly from its RGB
components, without the need to look up that RGB descriptor via
tty-color-desc.  In addition, support for named colors, such as
"burlywood1", is still required for these terminals, since most color
specifications in Emacs are of that variety.  Is my understanding
correct?

If I'm right, then you could either (a) extend tty-color-desc, such
that it would cons the (NAME INDEX R G B) list for a true-color
terminal and hand it back to tty_lookup_color; or (b) if you wanted to
be more efficient, generate the pixel value directly on the C level by
parsing the color spec if it is of the form "#RRGGBB", and otherwise
calling tty-color-desc as usual.  Assuming that tty-color-alist is set
correctly for the true-color terminal, and stores the RGB values for
each color, that will do the job, eliminating the need for all these
support functions: parse_and_encode_rgb_list, tty_supports_rgb, and
tty_lookup_rgb, and also save you 2 extra calls to Lisp.  Am I missing
something?

Thanks.

Re: [PATCH] Support 24-bit terminal colors.

[David Caldwell]

[-- Attachment #1: Type: text/plain, Size: 820 bytes --]

On 8/31/16 7:21 AM, Eli Zaretskii wrote:

> Do all true-color terminals support the same escape sequences for
> 24-bit colors?  Is there any standard documents we could rely upon for
> hard-coding them here?

Yeah, everyone follows ISO/IEC 8613-6 (originally published in 1989 (!!)
but the revised in 1994). The full spec name seems to be called:

> ITU-T Recommendation T.416 (1993) | ISO/IEC 8613-6:1994, Information technology — Open Document Architecture (ODA) and interchange format: Character content architectures.

Anyway, ISO sells it here:
http://www.iso.org/iso/home/store/catalogue_tc/catalogue_detail.htm?csnumber=22943

But ITU has a downloadable pdf:
https://www.itu.int/rec/dologin_pub.asp?lang=e&id=T-REC-T.416-199303-I!!PDF-E&type=items

The relevant section is 13.1.8.

-David


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3819 bytes --]

Re: [PATCH] Support 24-bit terminal colors.

[Yuri Khan]

On Sun, Sep 4, 2016 at 12:45 PM, David Caldwell <david@porkrind.org> wrote:

> Yeah, everyone follows ISO/IEC 8613-6 (originally published in 1989 (!!)
> but the revised in 1994).
> The relevant section is 13.1.8.

“Everyone follows” is a bit too strong.

The spec says parameters for the SGR 38 and 48 codes are delimited by
colons, but many terminals accept semicolons.

Re: [PATCH] Support 24-bit terminal colors.

[Eli Zaretskii]

> From: Yuri Khan <yuri.v.khan@gmail.com>
> Date: Sun, 4 Sep 2016 14:11:22 +0600
> Cc: Eli Zaretskii <eliz@gnu.org>, Rami Ylimäki <rjy@iki.fi>, 
> 	Emacs developers <emacs-devel@gnu.org>
> 
> The spec says parameters for the SGR 38 and 48 codes are delimited by
> colons, but many terminals accept semicolons.

Do all of them also accept colons?  If they do, I think we should use
colons.  (The patch used semicolons.)

Re: [PATCH] Support 24-bit terminal colors.

[Rami Ylimäki]

On 04.09.2016 17:18, Eli Zaretskii wrote:
>> From: Yuri Khan <yuri.v.khan@gmail.com>
>> Date: Sun, 4 Sep 2016 14:11:22 +0600
>> Cc: Eli Zaretskii <eliz@gnu.org>, Rami Ylimäki <rjy@iki.fi>,
>> 	Emacs developers <emacs-devel@gnu.org>
>>
>> The spec says parameters for the SGR 38 and 48 codes are delimited by
>> colons, but many terminals accept semicolons.
> Do all of them also accept colons?  If they do, I think we should use
> colons.  (The patch used semicolons.)
>

According to https://gist.github.com/XVilka/8346728, semicolon separator 
is supported by (almost?) all 24-bit terminals, whereas some 24-bit 
terminals don't support colon. I'd use semicolon separator to support as 
many terminals as possible.

Re: [PATCH] Support 24-bit terminal colors.

[Eli Zaretskii]

> Cc: david@porkrind.org, emacs-devel@gnu.org
> From: Rami Ylimäki <rjy@iki.fi>
> Date: Sun, 4 Sep 2016 19:44:56 +0300
> 
> On 04.09.2016 17:18, Eli Zaretskii wrote:
> >> From: Yuri Khan <yuri.v.khan@gmail.com>
> >> Date: Sun, 4 Sep 2016 14:11:22 +0600
> >> Cc: Eli Zaretskii <eliz@gnu.org>, Rami Ylimäki <rjy@iki.fi>,
> >> 	Emacs developers <emacs-devel@gnu.org>
> >>
> >> The spec says parameters for the SGR 38 and 48 codes are delimited by
> >> colons, but many terminals accept semicolons.
> > Do all of them also accept colons?  If they do, I think we should use
> > colons.  (The patch used semicolons.)
> >
> 
> According to https://gist.github.com/XVilka/8346728, semicolon separator 
> is supported by (almost?) all 24-bit terminals, whereas some 24-bit 
> terminals don't support colon. I'd use semicolon separator to support as 
> many terminals as possible.

It isn't clear to me how authoritative is that URL.

I wonder if we should allow the delimiter character to be controlled
by some option.  Although supporting options that are needed so early
in the startup process could be tricky.