sixel image with specific height and terminal font height cause broken image display

[blcc]

Hi, For example, terminal font height 18px then the line height will be set as 22px. When display a figure with 482px height image, the image height will be set as 484px (22 lines). Which is at line 481 of sixel.c But when sixel codec be process to last line, it will be treat as wrong image size at line 349. Then the image will be resize to a mess.

My solution is insert this in line 482 of sixel.c: sy = sy + (6-sy%6)%6; //but follow one line be covered. or if (sy%6 > 0) sy = sy+st->grid_height; //but too many space below image or simply remove the '+ 6' after st->pos_y in line 349.

Any idea?

[veltza]

I can reproduce this. Interestingly, this doesn't happen on my st fork, so I'll have a look and come back tomorrow.

[veltza]

Here is my analysis of this issue.

When the parsed sixel image is larger than the image buffer, the width and height of the buffer are doubled. That is fine, because the buffer size is resized back to the final image size when sixel_parser_finalize() is called. But the problem is that the width and height of the image are read from the sixel engine before the function is called, so the pixel buffer is larger than the final image:

    new_image->width = sixel_st.image.width;
    new_image->height = sixel_st.image.height;
    new_image->pixels = malloc(new_image->width * new_image->height * 4);
    if (sixel_parser_finalize(&sixel_st, new_image->pixels) != 0) {
        perror("sixel_parser_finalize() failed");
        sixel_parser_deinit(&sixel_st);
        return;
    }

So when the image buffer is resized back to the final image size in sixel_parser_finalize(), the conversion loop thinks that the image buffer and the pixel buffer are the same size and messes up the image:

    for (y = 0; y < st->image.height; ++y) {
        for (x = 0; x < st->image.width; ++x) {
            color = st->image.palette[*src++];
            *dst++ = color >> 16 & 0xff;   /* b */
            *dst++ = color >> 8 & 0xff;    /* g */
            *dst++ = color >> 0 & 0xff;    /* r */
            *dst++ = color >> 24 & 0xff;   /* a */
        }
        /* fill right padding with bgcolor */
        for (; x < st->image.width; ++x) {
            color = st->image.palette[0];  /* bgcolor */
            *dst++ = color >> 16 & 0xff;   /* b */
            *dst++ = color >> 8 & 0xff;    /* g */
            *dst++ = color >> 0 & 0xff;    /* r */
            *dst++ = color >> 24 & 0xff;   /* a */
        }
    }

So you get something like this:

Instead of this:

Interestingly, my unmerged PR accidentally fixes this issue as well when it removes the black bars from the sixel images. So I need to create a new PR based on the unmerged PR to fix this issue. But I also need to fix edge cases that cause unnecessary image buffer resizing when image buffer height is not divisible by 6 (sixel height).

[veltza]

I also found a bug that could cause a segmentation fault if the height of the image is greater than DECSIXEL_HEIGHT_MAX:

if (st->repeat_count > 0 && st->pos_y - 5 < image->height) {

It should be + 5 not - 5. I'll fix that too.

[veltza]

@blcc I created a new PR to fix this issue. Would you like to test it before the merge?

[blcc]

Tested. Work perfect! Thank you very much.