Re: [PATCHv2] gdb/tui: use init_extended_color where possible

Tom Tromey <[email protected]> writes:

>>>>>> "Andrew" == Andrew Burgess <[email protected]> writes:
>
> Andrew> The motivation for using init_extended_color is slightly less than
> Andrew> init_extended_pair.  Assuming the terminal supports it the standard
> Andrew> init_color API supports up to SHRT_MAX (32767) different colors,
> Andrew> switching to init_extended_color removes the SHRT_MAX limit on color
> Andrew> indices, allowing us to support the full range of COLORS.
>
> First, I think the patch is fine.
> Approved-By: Tom Tromey <[email protected]>
>
> However I have a question
>
> Andrew>  	  /* We store RGB as 0..255, but curses wants 0..1000.  */
> Andrew> -	  if (init_color (next, rgb[0] * 1000 / 255, rgb[1] * 1000 / 255,
> Andrew> -			  rgb[2] * 1000 / 255) == ERR)
> Andrew> +	  short r = rgb[0] * 1000 / 255;
> Andrew> +	  short g = rgb[1] * 1000 / 255;
> Andrew> +	  short b = rgb[2] * 1000 / 255;
> Andrew> +
> Andrew> +	  /* If init_extended_pair is not available then we fallback to
> Andrew> +	     using init_pair.  However, init_pair can only handle 'short'
> Andrew> +	     color indices so there is no point using init_extended_color
> Andrew> +	     to allow for the generation of longer 'int' color indices.  */
> Andrew> +#if defined HAVE_INIT_EXTENDED_COLOR && defined HAVE_INIT_EXTENDED_PAIR
> Andrew> +	  if (init_extended_color (next, r, g, b) == ERR)
> Andrew>  	    return false;
> Andrew> +#else
> Andrew> +	  /* NEXT is an int, but is passed as a short.  If COLORS is
> Andrew> +	     more than SHRT_MAX then NEXT will be truncated and end up
> Andrew> +	     redefining a color entry that we don't expect.  */
> Andrew> +	  if (next > SHRT_MAX
> Andrew> +	      || init_color (next, r, g, b) == ERR)
> Andrew> +	    return false;
> Andrew> +#endif
>
> IIUC init_extended_color allows a bigger range for 'next' but also for
> the RGB components.  However despite the text above, I think we don't
> actually use the bigger RGB range.  And, perhaps we don't really care
> to, I don't know.

Great question!  I also wondered about this as I too noticed that
init_extended_color accepted r, g, b as `int`.

The ncurses docs for this are super unclear, at least on my machine.

For init_color I get an explicit paragraph which says:

  "Each of the last three arguments must be a value in the range 0 through 1000."

But for init_extended_color my man page says:

  "Because color_content uses signed shorts for its parameters, that
  limits color-values and their red, green, and blue components to 32767
  on modern hardware.  The extension extended_color_content uses ints
  for the color value and for returning the red, green, and blue
  components, allowing a larger number of colors to be supported."

which seems to suggest that r, g, b can have more range.

However, note that even for init_color, where r, g, b are `short` the
valid range is limited to 0 -> 1000, not the full short range as the
text for init_extended_color seems to suggest.

And the text for start_color, the general function to enable color
support, has some text that talks about the range of the rgb components,
and it too talks about 1000.

None of this is super convincing.  At least, none of it really convinced
me.  So in the end I just went to the sources.

Looking at ncurses-6.6 source, in the file base/lib_color.c we see that
both init_color and init_extended_color just forward their argument
unmodified to _nc_init_color, which uses `int` for all its arguments,
just like init_extended_color.

After some initial checks, none of which check the rgb values, the code
calls:

    if (InitColor
	&& sp->_coloron
	&& (color >= 0 && OkColorHi(color))
	&& (okRGB(r) && okRGB(g) && okRGB(b))) {
      /* This is where r, g, b are actually used.  */
    }

And elsewhere in the file we find:

  #define okRGB(n) ((n) >= 0 && (n) <= 1000)

Which for me is the definitive answer.  The r, g, b components are
always in the range 0 -> 1000 (inclusive).

What this all means is that for both init_color and init_extended_color
there are 1,000,000,000 different rgb color combinations that could be
created, but init_color will only allow you to use 32,767 of these at a
time, while init_extended_color will allow them all to be used.

Is this super useful?  Probably not.  But it doesn't cost much to
support it.

Thanks,
Andrew