Allocate fewer color pairs (reuse the existing one when only the attributes differ)

[egmontkob]

Is there an existing issue for this?

• I have searched the existing issues

Midnight Commander version and build configuration

4.8.33

Operating system

.

Is this issue reproducible using the latest version of Midnight Commander?

• I confirm the issue is still reproducible with the latest version of Midnight Commander

How to reproduce

Forking from #5018, in particular #5018 (comment):

This is relevant for terminals that only support 8 colors, and their terminfo describes 64 or so color pairs (tput pairs), e.g TERM=linux or TERM=xterm.

Probably only relevant for ncurses.

---

We allocate a new ncurses color pair whenever we encounter a (fg, bg, attrs) combo that we haven't seen before. This is wasteful, and with only 64 colors pairs available we should be more frugal.

We should only allocate a new color pair whenever the (fg, bg) combo is a new one, and move the attributes out of this game. ncurses only insists on the palette-based approach for the (fg, bg) color pair, and allows to set the attributes independently in attr_set() and friends.

So there should be a two-step lookup. Internals of mc identify each different (fg, bg, attrs) combo with an index, I wouldn't change this at this point. This would point to a potentially different ncurses color pair id, as well as the attributes.

In 8-color mode, where we map bright colors to the bold attribute, this mapping should be performed first, and then check if we need a new ncurses color pair. Currently these two steps are performed in the opposite order: first color.c tty_try_alloc_color_pair()) checks if we've seen the combo, then color-ncurses.c tty_color_try_alloc_lib_pair() does the bright->bold mapping.

For example, seeing a red;black would allocate a color pair. Then seeing a brightred;black would reuse the same ncurses color pair (since it's mapped to the same red;black), and apply the bold attribute.

---

I think we should see an improvement in the rendering of the test case of #5018 in 8-color terminals, more text should be displayed in its desired colors.

We'd still run out of the available color pairs eventually and start to use wrong ones, since as explained in #5018 (comment) the number of color pairs supported by ncurses should really be 81 instead of 64, but we'd start to err later, meaning that real life syntax highlighting scenarios will more likely work correctly.

Expected behavior

.

Actual behavior

.

Additional context

No response

[egmontkob]

ncurses and the number of color pairs...

For terminals like xterm or linux, terminfo describes to support 64 color pairs (as shown by tput pairs or by printing the value of COLOR_PAIRS from C code).

(Mind you, I can't get my head around why an arbitrary and pointless limitation of the screen drawing library, not a limitation of the terminal, would reside in terminfo; that's not what it's for. Anyway...)

The manual of init_pair() says it'll return with an error for "color pairs identifiers outside the range 0-COLOR_PAIRS-1 inclusive."

As stated in the opening post, even for 8-color terminals like xterm this isn't enough, it should support 81 color pairs. The only related webpage I've found is this.

---

Now, guess what the actual behavior of ncurses is...

It supports exactly 81 color pairs (assuming you didn't build it with --disable-ext-funcs, and that you call use_default_colors()). Color pair 0 is hardcoded for the defaults, indices 1..80 can be assigned, 81 and above cannot.

It seems to me that this undocumented extension was added in version 5.8, more precisely 20090124.

---

I guess it could have been implemented by simply creating 81 slots which are freely assignable (except slot 0). The number being computed as COLOR_PAIRS + 2 * COLORS + 1. But it's more complicated.

The custom pairs that do involve a default color are counted separately. The cap for the color pairs is not 81 but 64 + however many color pairs involving a default color are already defined.

Quoting from ncurses-6.6/ncurses/base/lib_color.c:

        /*
         * Keep track of the number of entries in the color pair table which
         * used a default color.
         */

        /*
         * As an extension, ncurses allows the pair number to exceed the
         * terminal's color_pairs value for pairs using a default color.
         *
         * Note that updating a pair which used a default color with one
         * that does not will decrement the count - and possibly interfere
         * with sequentially adding new pairs.
         */

At this point it's unclear to me if re-assigning all the slots in a different order (when changing skin) can break it or not.

I would think that adding a reset_color_pairs() beforehand should be the safe and robust way to start over, but of course its implementation forgets to reset the _default_pairs counter (the number of custom pairs involving at least one default color) and at this point I can't tell whether things can go wrong.

Anyway, if we manage to make mc use all the 81 colors pairs on 8-bit terminals, with the caveat that a runtime skin change can break, that's already a nice improvement.

[egmontkob]

First draft: mc-5020-ncurses-color-pairs-draft1.patch

It pretty much fixes the look with TERM=xterm. The before state is even more broken than the first screenshot of #5018. The after state is almost perfect, only the last color is broken.

That's because I'm yet to add the special handling of index 0.

The code itself is highly draft, don't look at it :)

[egmontkob]

mc-5020-ncurses-color-pairs-draft2.patch

Added the special handling of index 0, thereby fixing all the entries.

Fixed shadow color. It's a temporary kludge, even uglier code than the rest, still don't look at :)

---

Seems to me that we never de-allocate or re-assign ncurses color pairs. So we should be fine, even when switching between 8-color skins. (Problems may arise if the user tries so many different 256-color/truecolor skins that we run out of the 65536 color pairs ncurses uses for modern terminals.)