Distortion when rowstride is not a multiple of 4

[jstkdng]

I'm trying to implement chafa support on ueberzugpp but sometimes the images appear moved and/or discolored. Like this.

both moved and discolored

just moved

The borders are from lf.

The main code I'm using is this.

ChafaCanvas::ChafaCanvas()
{
    gchar **envp = g_get_environ();
    term_info = chafa_term_db_detect(chafa_term_db_get_default(), envp);
    g_strfreev(envp);
}

ChafaCanvas::~ChafaCanvas()
{
    chafa_canvas_unref(canvas);
    chafa_canvas_config_unref(config);
    chafa_symbol_map_unref(symbol_map);
    chafa_term_info_unref(term_info);
}

void ChafaCanvas::init(const Dimensions& dimensions, std::unique_ptr<Image> new_image)
{
    image = std::move(new_image);
    x = dimensions.x + 1;
    y = dimensions.y + 1;
    horizontal_cells = std::ceil(static_cast<double>(image->width()) / dimensions.terminal.font_width);
    vertical_cells = std::ceil(static_cast<double>(image->height()) / dimensions.terminal.font_height);

    symbol_map = chafa_symbol_map_new();
    config = chafa_canvas_config_new();
    chafa_symbol_map_add_by_tags(symbol_map, CHAFA_SYMBOL_TAG_ALL);
    chafa_canvas_config_set_symbol_map(config, symbol_map);
    chafa_canvas_config_set_pixel_mode(config, CHAFA_PIXEL_MODE_SYMBOLS);
    chafa_canvas_config_set_geometry(config, horizontal_cells, vertical_cells);
    canvas = chafa_canvas_new(config);

    auto pixel_type = CHAFA_PIXEL_RGB8;
    if (image->channels() == 4) {
        pixel_type = CHAFA_PIXEL_RGBA8_PREMULTIPLIED;
    }
    chafa_canvas_draw_all_pixels(canvas,
            pixel_type,
            image->data(),
            image->width(),
            image->height(),
            image->width() * image->channels());
}

void ChafaCanvas::draw()
{
    const auto result = std::unique_ptr<GString, gstring_deleter> {
        chafa_canvas_print(canvas, term_info)
    };
    auto ycoord = y;
    const auto lines = util::str_split(result->str, "\n");
    util::save_cursor_position();
    std::ranges::for_each(std::as_const(lines), [&] (const auto& line) {
        util::move_cursor(ycoord++, x);
        std::cout << line;
    });
    std::cout << std::flush;
    util::restore_cursor_position();
}

I've tested in multiple terminals and in all of them show some form of this issue, not necessarily on the same image.

[jstkdng]

On some images it works without issues.

I forgot to add that with the chafa command the images are displayed correctly.

[hpjansson]

Hi! So, my first thought is that this could be caused by a bad rowstride parameter. I see you're using image->width() * image->channels(), but is it possible that the image loader adds some padding to each row?

Second, you're passing in the pixels as premultiplied alpha, but I think it's more common for image loaders to give you unassociated alpha. I don't know the details of how the image was loaded, though. This probably doesn't matter unless your image is partially transparent.

[jstkdng]

I see you're using image->width() * image->channels()

yeah, it's in your examples

is it possible that the image loader adds some padding to each row

I don't think so, this issue only happens with chafa, all the other output methods I support work fine. (x11, sixel, kitty, iterm2)

Second, you're passing in the pixels as premultiplied alpha

ah, I was just testing values to see if anything changed, doesn't work still with unassociated

I don't know the details of how the image was loaded, though

I'm using libvips and opencv for image loading. The issue kinda depends on the terminal though, in alacritty it shows bad but on mate terminal it displays correctly that image.

[hpjansson]

This helped when I tried it here:

CHAFA_SYMBOL_TAG_ALL includes symbols that may not be available in all terminals/fonts. I suggest using CHAFA_SYMBOL_TAG_BLOCK or (ChafaSymbolTags) (CHAFA_SYMBOL_TAG_BLOCK | CHAFA_SYMBOL_TAG_BORDER). These tend to have good support.

[hpjansson]

If you have an example picture that shows discoloration, I'd be very interested to have a copy to test with.

[hpjansson]

Managed to repro it with uberzeugpp and one of my test images just now. Figuring out what's going on.

[jstkdng]

CHAFA_SYMBOL_TAG_ALL includes symbols that may not be available in all terminals/fonts.

is there a way to discover which symbols are supported by a terminal at runtime?

Managed to repro it with uberzeugpp and one of my test images just now. Figuring out what's going on.

thanks for the help

[hpjansson]

There's no reliable way to discover symbol support that I know of, unfortunately. But blocks and borders are pretty reliable (i.e. works on all terminals I've tested, even Windows DOS/PowerShell and various MacOS ones).

[jstkdng]

But blocks and borders are pretty reliable

Is that what the chafa binary uses?

also, this image is both discolored and moved under alacritty but it does have color under kitty.

[hpjansson]

Fixed. Thanks for reporting this.

It may take some time for fixed packages to make their way into updates on the various distros. If you're already pre-scaling the images, scale them to a multiple of 4 pixels wide if you want to work around this bug in the meantime. Sorry for the inconvenience.

chafa adds CHAFA_SYMBOL_TAG_BLOCK | CHAFA_SYMBOL_TAG_BORDER | CHAFA_SYMBOL_TAG_SPACE and then removes CHAFA_SYMBOL_TAG_WIDE by default. See:

chafa/tools/chafa/chafa.c

Lines 1452 to 1462 in 3843ac7

	#ifdef G_OS_WIN32
	chafa_symbol_map_add_by_tags (options.symbol_map, CHAFA_SYMBOL_TAG_HALF);
	chafa_symbol_map_add_by_tags (options.symbol_map, CHAFA_SYMBOL_TAG_BORDER);
	chafa_symbol_map_add_by_tags (options.symbol_map, CHAFA_SYMBOL_TAG_SPACE);
	chafa_symbol_map_add_by_tags (options.symbol_map, CHAFA_SYMBOL_TAG_SOLID);
	#else
	chafa_symbol_map_add_by_tags (options.symbol_map, CHAFA_SYMBOL_TAG_BLOCK);
	chafa_symbol_map_add_by_tags (options.symbol_map, CHAFA_SYMBOL_TAG_BORDER);
	chafa_symbol_map_add_by_tags (options.symbol_map, CHAFA_SYMBOL_TAG_SPACE);
	#endif
	chafa_symbol_map_remove_by_tags (options.symbol_map, CHAFA_SYMBOL_TAG_WIDE);

Note the selection is more conservative on Windows due to historically more limited symbol support there.

[jstkdng]

It may take some time for fixed packages to make their way into updates on the various distros.

Are you making a new release with this fixes? Sadly can't test them since there's no chafa-git on the AUR. A real shame though, only rolling release distros will have this issue fixed. Unless I modify my resizing algorithm.... I'll see what I can do.

chafa adds CHAFA_SYMBOL_TAG_BLOCK | CHAFA_SYMBOL_TAG_BORDER | CHAFA_SYMBOL_TAG_SPACE and then removes CHAFA_SYMBOL_TAG_WIDE by default. See:

thanks for the example.

[hpjansson]

Are you making a new release with this fixes? Sadly can't test them since there's no chafa-git on the AUR. A real shame though, only rolling release distros will have this issue fixed. Unless I modify my resizing algorithm.... I'll see what I can do.

Another option is to ensure it gets 4-channel data in, as the issue only affects 3-channel input where (width % 4 != 0). Stupid, but at least it's fixed now.

I'll make a point release this weekend.

thanks for the example.

You're welcome. If there's anything else you need, don't hesitate to ask.

[jstkdng]

Another option is to ensure it gets 4-channel data in

hmm, that reduces performance but it works at least.

If there's anything else you need, don't hesitate to ask.

hmm, if I could ask you something, could you add a variant of chafa_canvas_print that returns an array of GStrings? otherwise I have to split the resulting string in order to move it and print it.

but is it possible that the image loader adds some padding to each row

so it was chafa adding the padding all along...

Thanks for the help.

[hpjansson]

Sure, I can do that. It would be for the 1.14.0 feature release, which should happen in not too long. Do you prefer GStrings or would a strv work as well? I used GString before because then you don't need to call strlen() again.

[jstkdng]

yeah, GString should be better, it's already hard enough managing c-strings (and in turn glib strings) in C++, if you can return how many items are in the array even better :)

Edit: maybe return a struct? even easier wrapping that with smart pointers

Edit2: I guess gchar** would be best, as glib already has g_strfreev

[hpjansson]

yeah, GString should be better, it's already hard enough managing c-strings (and in turn glib strings) in C++, if you can return how many items are in the array even better :)

Commit 803c3be adds the functions; both flavors :-)

[jstkdng]

awesome, too bad chafa is still on 12.4 in arch.