chafa icon indicating copy to clipboard operation
chafa copied to clipboard

improve compatibility with older ncurses

Open jose1711 opened this issue 5 years ago • 15 comments

the goal is to use chafa as an image viewer inside vifm while preview mode is active (the other panel is still visible). as per comment by vifm's author it looks like there's a compatibility issue with limitation of color pair coming with ncurses. to cite:

If the image looks better in the same terminal outside vifm, then this is probably a
result of limited number of color pairs. curses library (except for some recent versions,
which do it in backwards incompatible way, I think) limits how many foreground-background
pairs it can handle for historical reasons. And vifm can't work around it except for supporting
newer curses API, which might lift this limitation on systems where it's available. Currently
that API isn't supported.

t4jlwzb The whole discussion is at https://q2a.vifm.info/407/configure-chafa-image-viewer-images-within-vifm-image-defects. I wonder if it's possible for add something like a "legacy ncurses" mode which would make the images render better in such configuration.

jose1711 avatar Feb 02 '19 13:02 jose1711

Thanks for filing the issue.

I tested this with a master build of vifm, and despite being built with NCurses 6.1 and having COLOR_PAIRS defined to 65536, it presents the same issue. What's more, subsequent images viewed in the same session will have worse symptoms.

It looks like the color pairs are exhausted too early for some reason, and the allocations are not reset between images. This could be due to one or more bugs in either vifm or NCurses.

I'll keep this open until I've figured out exactly what's going on.

hpjansson avatar Feb 02 '19 17:02 hpjansson

What @xaizek said is correct -- vifm is using obsolete NCurses API and it needs to be fixed there. I sent a pull request with a minimal fix: https://github.com/vifm/vifm/pull/401/commits/b55de5d28252c784d4695e3c155f95fbc206a12a

hpjansson avatar Feb 02 '19 19:02 hpjansson

As for limiting the number of color pairs Chafa makes use of (which is what this issue is really about), I'll have to think about it for a bit. It'll limit the color palette severely, and maybe it'll affect the symbols too (e.g. there are inverse versions of some symbols meaning we can invert the color in some cells "for free" without changing attributes). But quantizing for color pairs is an interesting challenge.

This has commonalities with a quality setting I've been thinking about that would reduce the number of colors/color changes in order to generate more compact output with fewer escape codes.

hpjansson avatar Feb 02 '19 19:02 hpjansson

It looks like the color pairs are exhausted too early for some reason, and the allocations are not reset between images. This could be due to one or more bugs in either vifm or NCurses.

I suspect this is a side effect of how unused color pairs get collected. All dynamically allocated ones are dropped once the limit is hit. Better bookkeeping is needed to prevent that.

Thanks for the pull request, I'll update other places which use COLOR_PAIR macro. I discovered --enable-ext-colors flag of ncurses only today, which explained why I was always getting only 256 pairs.

xaizek avatar Feb 02 '19 21:02 xaizek

@hpjansson, just in case you already know, do I get it right that new API doesn't provide a way to set background color pair with value >= 256? I don't see new versions of functions like wbkgdset().

xaizek avatar Feb 03 '19 10:02 xaizek

@hpjansson, just in case you already know, do I get it right that new API doesn't provide a way to set background color pair with value >= 256? I don't see new versions of functions like wbkgdset().

Take a look at wbkgrndset(). It takes a pointer to cchar_t, which is a struct that looks like this (from ncurses.h):

/*
 * cchar_t stores an array of CCHARW_MAX wide characters.  The first is
 * normally a spacing character.  The others are non-spacing.  If those
 * (spacing and nonspacing) do not fill the array, a null L'\0' follows.
 * Otherwise, a null is assumed to follow when extracting via getcchar().
 */
#define CCHARW_MAX	5
typedef struct
{
    attr_t	attr;
    wchar_t	chars[CCHARW_MAX];
    int		ext_color; /* color pair, must be more than 16-bits */
}
cchar_t;

Wide chars go in chars[] which is null-terminated if less than CCHARW_MAX (with a cleared array, you should be fine just dropping a char in chars[0]). ext_color is the pair.

hpjansson avatar Feb 03 '19 13:02 hpjansson

I don't think that such an implementation detail should be used in client code, however you pointing it out explained why there is setcchar() function which looked weird until now. It's purpose is to create cchar_t to be used by those other functions which accept it. Thank you, I was too hung up on NCURSES_PAIRS_T type and ignored other functions.

xaizek avatar Feb 03 '19 15:02 xaizek

Oh, yeah, that's even better :smiley:

hpjansson avatar Feb 03 '19 15:02 hpjansson

Attention please. I developed software for curses on BSD, in the 1980s, before ncurses. We used hardware serial terminals.

curses was created as an abstraction layer so that programs could control terminals with incompatible control codes (e.g. wyse, digital).

now...

since we all use virtual terminals, not 80s hardware terminals by wyse etc, all virtual terminals use ANSI control codes.

as a result, ncurses makes no sense, since over 20 years.

software developers: please stop imagining that to control a virtual terminal you need to use ncurses. you can issue Vt-100/ANSI control codes directly, and set colors directly.

thanks for reading.

clort81 avatar Oct 15 '21 08:10 clort81

as a result, ncurses makes no sense, since over 20 years.

If you were right, output of infocmp between two random $TERM values would be empty. And it's not.

xaizek avatar Oct 15 '21 09:10 xaizek

To make the case that the ncurses abstraction of terminal capability is needed and relevant today, you'd need to show real world cases of software for which ncurses is mapping differently for different virtual terminals (beyond the trivial case for number of colors supported).

I'm aware that most terminal emulators are only terminal emulator approximators. invisible-island.net has shown xterm emulates >2x feature set compared other term emulators.

It is a problem that in 2023, most programmers still imagine that they need curses (ncurses) to do the basic terminal control like cursor movement, saving screen, clearing screen, erasing to end-of-line, setting color, bold, inverse, restoring screen etc.

They're flat out wrong. They're binding themselves to a hamstrung and outdated abstraction layer.

For all practical purposes in virtual terminals, ANSI is all you need, since over 20 years.

People are not using hardware serial terminals that do not understand the DEC VT-100/ANSI control codes and they haven't been for decades.

clort81 avatar Feb 02 '23 18:02 clort81