Sixel graphics rendering issue #151

Closed
opened 1 year ago by zar · 12 comments
zar commented 1 year ago

I've added sixel-based image preview in ranger to use it with foot.
However, it works OK in mlterm, xterm, mintty and iTerm2, but only in foot there is an issue with rendering:

image

I don't really know how to reproduce it properly (it happens in random moments, not depends on font size changing) and I'm not really sure that this bug is related to foot, but I've found very reliable way to get this bug quickly:

  1. Launch foot -c /dev/null

  2. Install and run my modified ranger:

git clone https://github.com/3ap/ranger/ /tmp/ranger
wget http://tmp.nazaryev.com/miku-90.jpg -O /tmp/ranger/miku-90.jpg
cd /tmp/ranger && ./ranger.py --cmd="set preview_images true" --cmd="set preview_images_method sixel" miku-90.jpg
  1. Press Ctrl++ to increase font size approx. 5 times and you'll see that image is splitted. You can press Up and Down to refresh image preview, but image still will be splitted. Restarting ranger doesn't help too.

However, if you close ranger, apply the patch below and launch ranger again, foot will render everything properly:

sed -i /tmp/ranger/ranger/ext/img_display.py -e 's/rows_per_iteration = fit_image_height_in_rows/rows_per_iteration = 1/g'

BONUS: I've also found the way to crash foot with sixel:

  1. Launch foot -c /dev/null
  2. Run tput cup 0 30; convert /tmp/ranger/miku-90.jpg -geometry 600x1200 sixel:-
  3. Press and hold Ctrl++ for 2-3 seconds and run previous command again
  4. SEGFAULT :)
I've added [sixel-based image preview](https://github.com/ranger/ranger/issues/723#issuecomment-701025904) in `ranger` to use it with `foot`. However, it works OK in `mlterm`, `xterm`, `mintty` and `iTerm2`, but only in `foot` there is an issue with rendering: ![image](/attachments/08ac1366-d28d-4f94-8426-8e956acca0d5) I don't really know how to reproduce it properly (it happens in random moments, not depends on font size changing) and I'm not really sure that this bug is related to `foot`, but I've found very reliable way to get this bug quickly: 0. Launch `foot -c /dev/null` 1. Install and run my modified `ranger`: ``` git clone https://github.com/3ap/ranger/ /tmp/ranger wget http://tmp.nazaryev.com/miku-90.jpg -O /tmp/ranger/miku-90.jpg cd /tmp/ranger && ./ranger.py --cmd="set preview_images true" --cmd="set preview_images_method sixel" miku-90.jpg ``` 2. Press **Ctrl**+**+** to increase font size approx. 5 times and you'll see that image is splitted. You can press **Up** and **Down** to refresh image preview, but image still will be splitted. Restarting `ranger` doesn't help too. However, if you close `ranger`, apply the patch below and launch `ranger` again, `foot` will render everything properly: ``` sed -i /tmp/ranger/ranger/ext/img_display.py -e 's/rows_per_iteration = fit_image_height_in_rows/rows_per_iteration = 1/g' ``` **BONUS:** I've also found the way to crash `foot` with sixel: 0. Launch `foot -c /dev/null` 1. Run `tput cup 0 30; convert /tmp/ranger/miku-90.jpg -geometry 600x1200 sixel:-` 2. Press and hold **Ctrl**+**+** for 2-3 seconds and run previous command again 3. SEGFAULT :)
297 KiB
dnkl commented 1 year ago
Owner

I’ve added sixel-based image preview in ranger

This is awesome! I'm really glad to see more applications using sixel :)

I don’t really know how to reproduce it properly (it happens in random moments,...

It happens when you happen to print the sixel on foot's scrollback's wrap-around; the scrollback buffer is a circular buffer, and the wrap-around is where the 'end' is stitched to the 'start'. The logic required to handle sixel images crossing this boundary is very difficult to get right. So foot cheats and splits up sixels that would cross the boundary, but failed to position the second piece of the sixel correctly.

BONUS: I’ve also found the way to crash foot with sixel:

Nice find! And thanks for digging into this too.

Both issues should be solved by #152. Care to take it for a test ride?

> I’ve added sixel-based image preview in ranger This is awesome! I'm really glad to see more applications using sixel :) > I don’t really know how to reproduce it properly (it happens in random moments,... It happens when you happen to print the sixel on foot's scrollback's wrap-around; the scrollback buffer is a circular buffer, and the wrap-around is where the 'end' is stitched to the 'start'. The logic required to handle sixel images crossing this boundary is very difficult to get right. So foot cheats and splits up sixels that _would_ cross the boundary, but failed to position the second piece of the sixel correctly. > BONUS: I’ve also found the way to crash foot with sixel: Nice find! And thanks for digging into this too. Both issues should be solved by https://codeberg.org/dnkl/foot/pulls/152. Care to take it for a test ride?
dnkl added the
bug
label 1 year ago
dnkl self-assigned this 1 year ago
zar commented 1 year ago
Poster
foot: sixel.c:125: verify_sixel_list_order: Assertion `row <= prev_row' failed.
Aborted (core dumped)

Unfortunately, it crashes now when I change font size.
Please check this screen recording: http://tmp.nazaryev.com/bugfoot.mp4

Sometimes it fails with another error:

foot: sixel.c:133: verify_sixel_list_order: Assertion `col + col_count <= prev_col || prev_col + prev_col_count 
Aborted (core dumped)                                                     
``` foot: sixel.c:125: verify_sixel_list_order: Assertion `row <= prev_row' failed. Aborted (core dumped) ``` Unfortunately, it crashes now when I change font size. Please check this screen recording: http://tmp.nazaryev.com/bugfoot.mp4 Sometimes it fails with another error: ``` foot: sixel.c:133: verify_sixel_list_order: Assertion `col + col_count <= prev_col || prev_col + prev_col_count Aborted (core dumped) ```
dnkl commented 1 year ago
Owner

The two assertions are related, so it's very likely the same bug. Is it enough to start foot, start ranger, preview an image and then change the font size?

I thought I did that test before pushing the PR, and didn't see this crash...

The two assertions are related, so it's very likely the same bug. Is it enough to start foot, start ranger, preview an image and then change the font size? I thought I did that test before pushing the PR, and didn't see this crash...
zar commented 1 year ago
Poster

The two assertions are related, so it's very likely the same bug. Is it enough to start foot, start ranger, preview an image and then change the font size?

I thought I did that test before pushing the PR, and didn't see this crash...

All the required actions you can find in my video above, I didn't do anything else. In my experience, it's necessary to start foot in full screen with default configuration (with tiny font size). After that you should start ranger, preview an image and just hold Ctrl++ until it crashes.

> The two assertions are related, so it's very likely the same bug. Is it enough to start foot, start ranger, preview an image and then change the font size? > > I thought I did that test before pushing the PR, and didn't see this crash... All the required actions you can find in my video above, I didn't do anything else. In my experience, it's necessary to start `foot` in full screen with default configuration (with tiny font size). After that you should start `ranger`, preview an image and just **hold** Ctrl+**+** until it crashes.
dnkl commented 1 year ago
Owner

Alright, I'll give it a try.

It would also be very helpful if you could upload your build of foot (the main binary only) and the core dump file. Assuming you're comfortable with that, of course :)

Alright, I'll give it a try. It would also be very helpful if you could upload your build of foot (the main binary only) and the core dump file. Assuming you're comfortable with that, of course :)
dnkl commented 1 year ago
Owner

It would also be very helpful if you could upload your build of foot (the main binary only) and the core dump file.

This isn't necessary anymore. I've managed to reproduce the crash.

It appears to be related to reflowing of the sixel images when either the window is resized, or the font size changes.

Right now I'm not sure this is fixable. Meaning I may have to simply remove all sixel images when the window is resized (or the font size changed).

For alternate screen applications, like ranger, this shouldn't be a problem since they need to redraw themselves anyway.

But it does mean e.g. lsix output, or img2sixel output on stdout will disappear. Which is something I'd like to avoid.

> It would also be very helpful if you could upload your build of foot (the main binary only) and the core dump file. This isn't necessary anymore. I've managed to reproduce the crash. It appears to be related to reflowing of the sixel images when either the window is resized, or the font size changes. Right now I'm not sure this is fixable. Meaning I may have to simply remove all sixel images when the window is resized (or the font size changed). For alternate screen applications, like ranger, this shouldn't be a problem since they need to redraw themselves anyway. But it does mean e.g. `lsix` output, or `img2sixel` output on stdout will disappear. Which is something I'd like to avoid.
dnkl commented 1 year ago
Owner

I've pushed a couple of fixes that improves stability. But, though harder, it is still possible to crash foot. Still working on it though.

I've pushed a couple of fixes that improves stability. But, though harder, it is still possible to crash foot. Still working on it though.
dnkl commented 1 year ago
Owner

Many hours and commits later, I am no longer able to crash foot :)

Would appreciate if you could test the PR once more.

Many hours and commits later, I am no longer able to crash foot :) Would appreciate if you could test the PR once more.
dnkl closed this issue 1 year ago
dnkl commented 1 year ago
Owner

I've been testing the PR pretty exensively under a period of time, and felt confident enough to merge it.

@zar please re-open, or open a new issue if you're still seeing glitches or crashes.

I've been testing the PR pretty exensively under a period of time, and felt confident enough to merge it. @zar please re-open, or open a new issue if you're still seeing glitches or crashes.

Hi, I've been poking around the terminals with sixel support again. I run X11+KDE normally, is there a way to test foot in that environment?

I am curious if foot can handle the mixed-mode text and sixel my library produces here . If so, I would like to add it to this list of terminals .

Hi, I've been poking around the terminals with sixel support again. I run X11+KDE normally, is there a way to test foot in that environment? I am curious if foot can handle the mixed-mode text and sixel [my library produces here](https://jexer.sourceforge.io/sixel.html) . If so, I would like to add it to this [list of terminals](https://gitlab.com/klamonte/jexer/-/wikis/terminals) .
Owner

@lamonte

I run X11+KDE normally, is there a way to test foot in that environment?

Unfortunately not. Foot does not run under X11, only Wayland.

If you would like to test foot without setting up a full Wayland desktop, try Weston. It can run either natively, or nested inside an X11 session.

Create the file ~/.config/weston.ini, with the content:

[launcher]
icon=terminal
path=/usr/bin/foot

Then run weston, either from a TTY, or nested inside KDE. You should see a status bar with a single icon (probably a cross since the icon is invalid). Note that "tap" isn't enabled by default, if you're using a touchpad, and it'll default to a US keyboard layout.

I am curious if foot can handle the mixed-mode text and sixel

It should, yes. In fact, jexer (the demo) was one of the best real world torture tests I put foot through. I'm very grateful for it, thanks!

(I resized the terminal window after cat:ing the lady, hence the weird colors)

@lamonte > I run X11+KDE normally, is there a way to test foot in that environment? Unfortunately not. Foot does not run under X11, only Wayland. If you would like to test foot without setting up a full Wayland desktop, try Weston. It can run either natively, or nested inside an X11 session. Create the file `~/.config/weston.ini`, with the content: ``` [launcher] icon=terminal path=/usr/bin/foot ``` Then run `weston`, either from a TTY, or nested inside KDE. You should see a status bar with a single icon (probably a cross since the `icon` is invalid). Note that "tap" isn't enabled by default, if you're using a touchpad, and it'll default to a US keyboard layout. > I am curious if foot can handle the mixed-mode text and sixel It should, yes. In fact, jexer (the demo) was one of the best real world torture tests I put foot through. I'm very grateful for it, thanks! (I resized the terminal window after cat:ing the lady, hence the weird colors)

@dnkl That's awesome! Glad to have been of help. :) foot does indeed work, woohoo. I've added it to the list.

@dnkl That's awesome! Glad to have been of help. :) foot does indeed work, woohoo. I've added it to the list.
Sign in to join this conversation.
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.