On Sun, Jun 19, 2016 at 06:13:49PM +0800, Fengguang Wu <[email protected]>
wrote:
> Support directly setting RGB fg/bg colors via ISO-8613-3 24-bit
> ANSI color escapes:
Hi, this looks quite good. We had an implementation in mind that would look
for the nearest available colour, but if I understand your mails correctly (I
haven't looked very closely at your patch), what you do is allocate another
static colour cube that is outside the normal colour range, so that the
(user-accessible) palette exists in parallel to the 24 bit colours.
That would be quite an interesting approach. I also like that you add
documentation.
Anyways, what follows is a cursory look over your patch, maybe you have some
comments?
> + * Rule of thumb boundaries for segmenting the nonlinear RGB color space.
> +#if USE_256_COLORS
> +static unsigned int colorcube_map[5] = { 0x42, 0x77, 0x99, 0xc6, 0xec };
Deep magic constants, it would be nice if these were computed so one can see
where the numbers come from.
> +#define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
You should use ecb_array_length instead of ARRAY_SIZE.
> + idx = minTermCOLOR + colorcube_index (r) * COLORCUBE_SIZE * COLORCUBE_SIZE
> +
> + colorcube_index (g) * COLORCUBE_SIZE +
> + colorcube_index (b);
This seems to index into existing colours, which seems to contradict your
other explanations of using a "hidden" colour cube - do I miss something?
> + // update_fade_color (idx, false); // who cares?
Well, the people who use shading will certainly care, and almost certainly
complain, so fading should work :)
> +// vim:et:ts=2:sw=2
These kind of editor meta comments shouldn't be in the patch.
Otherwise, the principal idea of using unused colour space in the existing
bitfield and interpolate colours into it is something we would welcome quite
a lot, and is probably even better than to try and find a good match in the
existing palette.
--
The choice of a Deliantra, the free code+content MORPG
-----==- _GNU_ http://www.deliantra.net
----==-- _ generation
---==---(_)__ __ ____ __ Marc Lehmann
--==---/ / _ \/ // /\ \/ / [email protected]
-=====/_/_//_/\_,_/ /_/\_\
_______________________________________________
rxvt-unicode mailing list
[email protected]
http://lists.schmorp.de/mailman/listinfo/rxvt-unicode
