On Tue, Jun 28, 2016 at 12:18:28AM +0300, Emanuele Giaquinta wrote: > Hi, > > On Thu, Jun 23, 2016 at 08:28:46PM +0800, Fengguang Wu wrote: > > When the color cube slot is found to be already occupied by a similar > > 24-bit color, search through the -1, 0, +1 R/G/B indices trying to find > > an empty slot, or the oldest used one (which hopefully is no longer in > > active use). > > > > This effectively reduces random collisions, hence make it pretty hard to > > hit a vim GUI color scheme that cannot be correctly showed in urxvt. > > thanks for the patch! The general idea looks sensible. I have two > preliminary comments, general principle being simplicity over speed: > > - Is the speed gain given by hashing significant enough to justify the > additional space of 'rgb24_index'?
There will no user noticeable performance change, I guess. Feel free to reduce RGB24_HASH_BITS from 4 to 3, which will reduce the space from 4KB to 512B, the latter should still maintain low hash key conflict rate. Or allocate that array on demand -- only when users start using the 24-bit color feature. Or remove the rgb24_index[] related code altogether. > - IIUC, the new code, when all the slots in the neighbourhood are used, > reuses the oldest mapped slot rather than the slot onto which the > color is initially mapped. Does this provide any significant > improvement to justify the additional code and space of 'rgb24_seqno'? Without the rgb24_seqno[] logic, colors shown once tend to occupy the slots forever. As users switch among different vim color schemes or different 24-bit color aware applications, map_rgb24_color() could eventually degrade into the situation before this patch. Users may find a color scheme that works fine in fresh urxvt no longer looks right after some operations. Thanks, Fengguang _______________________________________________ rxvt-unicode mailing list [email protected] http://lists.schmorp.de/mailman/listinfo/rxvt-unicode
