Hi Marc,
On Mon, Jun 20, 2016 at 09:35:10AM +0200, Marc Lehmann wrote:
> 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.
Glad that you like this approach! I'll further improve the patch and
documentation.
> Anyways, what follows is a cursory look over your patch, maybe you have some
> comments?
Sure.
> > + * 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.
Good point. Will do that in v2.
> > +#define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
>
> You should use ecb_array_length instead of ARRAY_SIZE.
Ah didn't know ecb_array_length. Will use it of cause!
>
> > + 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?
Sorry in the other email I'm talking about possibilies for v2.
The "hidden" colour cube is obviously nice to have in v2 -- if you
agree with the general direction of this RFC patch.
> > + // update_fade_color (idx, false); // who cares?
>
> Well, the people who use shading will certainly care, and almost certainly
> complain, so fading should work :)
OK, I'll add back that code. :)
> > +// vim:et:ts=2:sw=2
>
> These kind of editor meta comments shouldn't be in the patch.
OK.
> 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.
Great to hear that! I'll go that direction.
Regards,
Fengguang
_______________________________________________
rxvt-unicode mailing list
[email protected]
http://lists.schmorp.de/mailman/listinfo/rxvt-unicode
