[vim/vim] Refactor popup image feature (PR #21234)

Foxe Chen

unread,

Sep 5, 2026, 10:27:46 PMSep 5

to vim/vim, Subscribed

Refactor the popup image feature to fix various issues. I've separated the image logic from the popup window logic, which is more maintainable in my opinion. This should also allow for other possible uses of images (graphical icons in the statusbar?)

I plan on this to be fully backwards compatible, except now instead of automatically detecting the image protocol to use (for terminal), to use imageprotocol option instead (yet to be implemented), similar to keyprotocol.
Additionally, images can be added with the image_add() function (to be implemented), which can then be used by multiple popup windows/sources. Images are separated from their placements (visible sources of the image), which decreases memory usage when needing to use the same image multiple times.
Sixel (terminal)
GDI (MS-Windows GUI)
Cairo (GTK2/3)
GDK (GTK4)
Kitty (terminal)

Other unrelated bugs I've noticed:

clipwindow does not seem to work correctly with nowrap in general

You can view, comment on, or merge this pull request online at:

https://github.com/vim/vim/pull/21234

Commit Summary

25692cd initial commit
835e428 f_image_add implemented
f329e3f basic mage rendering
f033c30 Revert "basic mage rendering"
f54d892 Revert "f_image_add implemented"
25ef1fa refactor

File Changes

(40 files)

Patch Links:

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.

Yamagi

unread,

Sep 6, 2026, 6:59:10 AMSep 6

to vim/vim, Subscribed

Yamagi left a comment (vim/vim#21234)

I plan on this to be fully backwards compatible, except now instead of automatically detecting the image protocol to use (for terminal), to use imageprotocol option instead (yet to be implemented), similar to keyprotocol.

A suggestion for improvement:

The SIXEL protocol is the most inferior protocol supported by Vim right now since it's limited to 255 colors. For good results any non trivial image must be properly quantized to 255 colors. While it would be nice to have quantization in Vim, I understand that quantization is a non-trivial problem and not in the scope of the project. After all Vim isn't an image viewer.

I've experimented with quantization through pngquant (properly the best quantization implementation out there) and was able to yield good results.

This image is just 'resize by ImageMagick -> convert to RGBA -> display by Vim'. You can see the artifacts caused by missing quantization:

Image: Bildschirmfoto_20260906_124756 (view on web)

This one was rendered by 'resize by ImageMagick -> quantize with pngquant -o $OUTPUT --posterize 2 255 -> convert to RGBA -> display by Vim'. The quality is drastically better:

Image: quant (view on web)

Source code of my highly experimental plugin implementing this can be found here: https://codeberg.org/yamagi/vim-imagepopup

Long story short: It would be very appreciated if Vim provided a way to detect the color depth supported by the active image backend. Or at least what image backend is active. That way a plugin could determine automatically how the image should be processed before it's displayed by Vim.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 6, 2026, 10:30:32 AMSep 6

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

I plan on this to be fully backwards compatible, except now instead of automatically detecting the image protocol to use (for terminal), to use imageprotocol option instead (yet to be implemented), similar to keyprotocol.

A suggestion for improvement:

The SIXEL protocol is the most inferior protocol supported by Vim right now since it's limited to 255 colors. For good results any non trivial image must be properly quantized to 255 colors. While it would be nice to have quantization in Vim, I understand that quantization is a non-trivial problem and not in the scope of the project. After all Vim isn't an image viewer.

I've experimented with quantization through pngquant (properly the best quantization implementation out there) and was able to yield good results.

This image is just 'resize by ImageMagick -> convert to RGBA -> display by Vim'. You can see the artifacts caused by missing quantization:

This one was rendered by 'resize by ImageMagick -> quantize with `pngquant -o $OUTPUT --posterize 2 255` -> convert to RGBA -> display by Vim'. The quality is drastically better: Source code of my highly experimental plugin implementing this can be found here: https://codeberg.org/yamagi/vim-imagepopup

Long story short: It would be very appreciated if Vim provided a way to detect the color depth supported by the active image backend. Or at least what image backend is active. That way a plugin could determine automatically how the image should be processed before it's displayed by Vim.

I suppose there can be a v: variable for this. However I'm not sure how you would detect the color depth from the image backend? Shouldn't this be possible by just knowing what image backend is being used?

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Yamagi

unread,

Sep 6, 2026, 12:34:51 PMSep 6

to vim/vim, Subscribed

Yamagi left a comment (vim/vim#21234)

I suppose there can be a v: variable for this. However I'm not sure how you would detect the color depth from the image backend? Shouldn't this be possible by just knowing what image backend is being used?

Good point. Thinking about it the only real advantage of a colordepth variable would be the addition of new backends existing plugins don't know about. But I would suspect that any new backend, be it in GUI or some new terminal protocol, would likely support 24 bit colors. So knowing what backend is used would be enough for plugin authors to special case SIXELs.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

dezza

unread,

Sep 6, 2026, 4:19:18 PMSep 6

to vim/vim, Subscribed

dezza left a comment (vim/vim#21234)

I like both of your ideas of a more generic image_add and embedding images inside documents.

I'd suggest keep the idea/impl open to the idea of animated images and fixing to to lines with the tallest image setting the lineheight.

Animated images/shaders inside a markdown document templeOS style;
Image: image (view on web)

I have conceal on all my markdown formatting and animated images unconcealing to file-refs on cursor over would look so cool, especially if blog writing from within vim.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 6, 2026, 10:06:54 PMSep 6

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

I've decided to switch to using the libsixel library for the sixel support. Sixel is honestly really complex and has a bunch of edge cases that does not seem right for a text editor to have to implement. Additionally libsixel seems to be very well known and well distributed, and supported on all relevant platforms. This makes maintainance in the future easier

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 7, 2026, 2:46:14 AMSep 7

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

I'm going to put off creating the tests in this PR. I've thought of several solutions, but I think the most suitable one would to literally implement sixel in the builtin terminal (libvterm). Then there could be a term_dumpwrite() like function that can dump a pixel buffer view of the terminal, which can be compared. This should be maybe feasible? (with the help of the Cairo library and maybe Pango to render text? (probably unecessary for testing)). This is a nice feature in its own right as well.

The initial sixel support can be very rudimentary, possibly without scrollback support or sixel scrolling (DECSDM).

As for the kitty graphics protocol, that would be very complicated to implement (theres no libsixel equivalent for this), but GTK4 (GDK image backend) is similar enough that we can cover it using the GTK4 testing infastructure.

This all sounds a bit overkill for just a test suite, but it looks like a fun challenge for me 😄

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 8, 2026, 11:27:11 PM (13 days ago) Sep 8

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

I have done a big refactor, now pixman is a dependency for the image feature. This is because I need to handle having some parts of the image being under some text (e.g. border). For sixel this is simply a matter of ordering how you draw everything to the screen. However for the kitty graphics protocol and GTK4, images are actual "objects", not just pixels blit to the screen, meaning there has to be a way to figure out what rectangles in an image are visible. Pixman does that, its very fast (SIMD optimized) and well tested, and has no dependencies. Additionally pixman has some image manipulation features that can we possibly expose to the user (e.g. cropping, scaling).

Before:

https://github.com/user-attachments/assets/b2b77ef6-3c52-46cf-b677-412dbf25ad14

After:

https://github.com/user-attachments/assets/11fcc47d-cba6-45b0-8411-05c52afb6281

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 11, 2026, 6:58:05 PM (10 days ago) Sep 11

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

Including the pixman region API (not the entire library) in the source tree might be better. Because pixman has no binary releases for Windows, I had to get it through vcpkg. I'm not sure if this is a blocker for the vim-win32-installer repo to enable image support.

As for libsixel, it seems that its distribution on Windows has issues, I could not find a binary release for it all. I suppose the solution would be to write a custom sixel encoder (which could be more optimized for our specific use case).

@chrisbra is this okay with you? pixman is MIT licensed, so I think its OK to include it in the source tree (I'm not a license expert).

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Sep 12, 2026, 12:24:57 PM (9 days ago) Sep 12

to vim/vim, Subscribed

chrisbra left a comment (vim/vim#21234)

If pixman is MIT licensed, it should be fine, we will have to do some changes for the vim-win32-installer to include it, but is should be okay, if vcpkg works I guess.
libvterm is also MIT licensed, so I don't see an issue with this.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 12, 2026, 12:40:53 PM (9 days ago) Sep 12

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

If pixman is MIT licensed, it should be fine, we will have to do some changes for the vim-win32-installer to include it, but is should be okay, if vcpkg works I guess. libvterm is also MIT licensed, so I don't see an issue with this.

Okay I will include part of the pixman library and the libsixel library in the source tree, unless I misunderstood your statement.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

h_east

unread,

Sep 13, 2026, 1:12:58 PM (8 days ago) Sep 13

to vim/vim, Subscribed

h-east left a comment (vim/vim#21234)

@64-bitman
The branch removes every Test_popup_image_* test from test_popupwin.vim, 385
lines against its base 9.2.1080. Please put them back before this leaves
draft. Test_popup_image_kitty_leave_tabpage and
Test_popup_image_sixel_leave_tabpage (9.2.1074), and
Test_popup_image_kitty_covered from #21298 once that is in, run Vim on a pty
and check what it writes for the kitty and sixel protocols, so they do not
depend on how the drawing is organized; they cover what #21273 and #21298
fixed. The others check the option handling of popup_create() and
popup_setoptions(), which this PR keeps backwards compatible.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 15, 2026, 12:47:57 PM (6 days ago) Sep 15

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

Not sure why this test_smoothscroll_zero_width() test is failing...

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 17, 2026, 3:46:24 PM (4 days ago) Sep 17

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

I think this is ready for review. I'm not sure why these unrelated tests are failing however...

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Sep 17, 2026, 5:15:57 PM (4 days ago) Sep 17

to vim/vim, Subscribed

chrisbra left a comment (vim/vim#21234)

thanks for working on that. Let me re-trigger CI first

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 18, 2026, 10:15:23 AM (3 days ago) Sep 18

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

Nevermind I can reproduce the test failures, just had to not use debug flags

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

h_east

unread,

Sep 18, 2026, 3:04:06 PM (3 days ago) Sep 18

to vim/vim, Subscribed

h-east left a comment (vim/vim#21234)

The kitty and sixel tests are still gone

The branch still deletes Test_popup_image_kitty_leave_tabpage,
Test_popup_image_kitty_covered and
Test_popup_image_sixel_leave_tabpage from test_popupwin.vim, together
with the s:PtyOutput(), s:StartVimWithImageOnPty() and
s:WaitForPtyOutput() helpers, and nothing in test_popupwin.vim
replaces them.

test_image.vim is not the same thing. It checks the arguments of
image_add(), image_discard() and image_info() and that
'imageprotocol' switches v:imagebackend, which is all at the Vim
script level. The tests that went away run Vim on a pty and look at the
bytes it writes: that the placement is deleted when you leave the tab
page and placed again without transmitting the pixels when you come
back, and that a popup with a higher zindex leaves four placements
around it. Those are what 9.2.1074 and 9.2.1109 fixed. With this rewrite
of the drawing code they are exactly the kind of thing that breaks
quietly, and CI stays green.

I reminded you about this, didn't I? And you reacted with a thumbs-up emoji. #21234 (comment)
Please bring them back.

Documentation

Mistakes I noticed while reading:

builtin.txt: "a blob containg RGB or RGBA data" (containing); "it
simply loads the image data so that can be reference elsewhere" (so
that it can be referenced); "the popup window still display the image"
(displays); the method example of image_discard() is
id->image_discard without the parentheses.
builtin.txt: the list says image_info({id}) and Dict, the entry
says image_info([{id}]) and Return type: list<dict<any>>. The
argument is optional and a List comes back, so the list is wrong on
both counts.
popup.txt: "based on the image dimensions nad terminal/GUI cell size"
(and); "The dimensions of a popup containing an image is automatically
calculated" (dimensions are).
usr_41.txt: "Get information about an image" starts with a capital
while the two lines above it do not.
Two spaces after a period, one is used in popup.txt "supported on all
platforms. The protocol" and in options.txt "the image backend
changes. This includes".
E1584, E1585 and E1586 are not in any help file. E1587 is.

Comments in the code

image.h has two empty comment blocks:

/*
 *
 */
typedef struct image_S image_T;

The one above image_placement_S is empty as well. Either say what the
struct is for or drop them.

This one says the opposite of what the flag does:

#define IMAGEF_FORCE 4      // If image backend should use a cache

image_sixel.c uses it the other way around, if (!(place->flags & IMAGEF_FORCE) && ...) takes the cached sequences, so the flag forces
the cache to be skipped.

image_ref(), image_unref() and image_free() have no comment. Who
takes a reference and who drops it is the thing a reader needs here,
more than for the functions around them that do have one.

ga_concat_int() in alloc.c has none either, while every other function
in that file does. And since ga_concat_len() is right there, a word on
why the new one exists would save the next reader the trip: it reports
OOM and it does not stop at a NUL byte.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

h_east

unread,

Sep 18, 2026, 3:20:49 PM (3 days ago) Sep 18

to vim/vim, Subscribed

h-east left a comment (vim/vim#21234)

The GTK4 image removal is replaced by work in screen_char()

9.2.1079 dropped the images that a fill or a scroll writes over, inside
the draw area:

    vim_draw_area_remove_images_in(self, row1, col1, row2, col2);

This branch deletes DrawImage, vim_draw_area_remove_image(),
vim_draw_area_remove_images_in() and gui_gtk4_remove_image(), and
screen.c tells image.c about the cell instead:

    mark_dirty_region_for_images(row, col, 1, 1);

The placement is marked dirty and drawn again rather than dropped, which
should come out the same on the screen. What I am unsure about is the
cost: that call walks all placements and runs
pixman_region32_contains_rectangle() on each, for every cell that is
written. With no image on the screen it returns at once, with one image
a full redraw does it for every cell. Did you measure it? screen_char()
does not have much room to spare.

It also calls redraw_all_later(UPD_VALID) from there while
"updating_screen" is not set. I could not tell by reading whether that
can bounce.

I have no test to point at for this one, 9.2.1079 came without one.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 18, 2026, 4:32:35 PM (3 days ago) Sep 18

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

I do not think the performance hit of screen_char is an issue. It only queues a screen redraw if the drawn cell intersects an image. Though I may be wrong, I'll perform a benchmark when I get back to my laptop to verify.

As for the removed tests, sorry 😅 . I'm guessing I just removed them because they were failing then just forgot about it.

Thank you for pointing out the many small issues though, I will fix them.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

h_east

unread,

Sep 18, 2026, 5:56:39 PM (3 days ago) Sep 18

to vim/vim, Subscribed

h-east left a comment (vim/vim#21234)

I meant the check itself, not the redraw. For every cell that is
written, mark_dirty_region_for_images() goes through all placements
and calls pixman_region32_contains_rectangle() on each one. With no
image n_placements is 0 and it returns, so this only shows up when an
image is on the screen. Thanks for running a benchmark. One image up and
a full redraw of a big file, with and without it, should tell.

Sorry, "bounce" was a poor word. I meant it might loop.
mark_dirty_region_for_images() calls redraw_all_later(UPD_VALID)
when "updating_screen" is not set. That redraw writes the cells again,
each write calls it again, and that can ask for another redraw, and so
on. Maybe "updating_screen" is always set there and it stops after one
round. I could not tell from reading.

The pty tests are the ones I want back. They cover 9.2.1074 and 9.2.1109
and do not care how the drawing works. 👍

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 18, 2026, 8:07:07 PM (3 days ago) Sep 18

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

@h-east I have performed some benchmarks (with some unpushed optimizations). I ran with a 100 image placements of a 512x512 rgba image, which increased the redraw time by ~6 ms. However if i put an early return in mark_dirty_region_for_images(), it only increases by ~5 ms compared to no images. This means the main bottleneck is the popup rendering code itself. Though maybe 1 ms is a big deal? Not sure.

Tested with -O2 (default makefile), foot terminal with sixel backend, and just held Ctrl-L to test in a syntax heavy file. profile_start() was called before the main drawing logic in update_screen(), and profile_end() was called just before draw_image_placements().

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 18, 2026, 11:07:50 PM (3 days ago) Sep 18

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

Tried adding the pty tests back but they are failing (despite manual testing being fine), so I'll work on that later.

However I think these pty tests seem awfully hacky and fragile. Once we get testing infrastructure for the image feature then I think we can remove those (and reimplement them of course).

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 19, 2026, 10:30:58 PM (2 days ago) Sep 19

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

I have only added back one pty test Test_popup_image_kitty_leave_tabpage() because still works with this refactor. Test_popup_image_kitty_covered() I could not get it work, and what it covers was thoroughly manually tested by me. Test_popup_image_sixel_leave_tabpage() does not seem to be needed anymore because the screen is not cleared now when leaving tabpage.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

h_east

unread,

Sep 20, 2026, 1:46:19 AM (yesterday) Sep 20

to vim/vim, Subscribed

h-east left a comment (vim/vim#21234)

I have only added back one pty test Test_popup_image_kitty_leave_tabpage() because still works with this refactor. Test_popup_image_kitty_covered() I could not get it work, and what it covers was thoroughly manually tested by me. Test_popup_image_sixel_leave_tabpage() does not seem to be needed anymore because the screen is not cleared now when leaving tabpage.

Hmm,
Test_popup_image_kitty_covered() covers what 9.2.1109 fixed, an image
drawn over a popup with a higher zindex. That it no longer works is
itself something this refactor changed, which is a reason to keep the
test, not to drop it.

Testing by hand does not carry over to the next person who touches this
code.

Test_popup_image_sixel_leave_tabpage() checks that leaving the tab
page clears the screen, which is what 9.2.1074 fixed. If the screen is
no longer cleared, the image still has to go somehow, and that is what
the test should check instead. With nothing in its place, the original
bug coming back would not show up.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

h_east

unread,

Sep 20, 2026, 2:56:19 AM (yesterday) Sep 20

to vim/vim, Subscribed

h-east left a comment (vim/vim#21234)

I put the two tests back, adapted to how this PR works. Both pass
here, and so does the rest of test_image.vim.

Test_popup_image_kitty_covered() is the old one with the z=50 part
of each placement dropped. The split into p=1..4 is what keeps the
image out of the covered cells, and kitty treats any z of 0 or more as
"above the text", so the value itself does not carry the popup's
zindex. In kitty two overlapping image popups still come out in
zindex order.

Test_popup_image_sixel_leave_tabpage() no longer waits for the screen
to be cleared. It checks that the image is not drawn again while the
other tab page is up, and is drawn again on coming back.

diff --git a/src/testdir/test_image.vim b/src/testdir/test_image.vim
index 62b455403..40c5bb855 100644
--- a/src/testdir/test_image.vim
+++ b/src/testdir/test_image.vim
@@ -198,6 +198,11 @@ let s:kitty_place = "\<Esc>_Ga=p,"
 " a=d,d=i: delete the placement of an image, the terminal keeps the pixels.
 let s:kitty_delete = "\<Esc>_Ga=d,d=i,"
 
+" The start of a sixel image, a DCS with the "q" command.
+let s:sixel_start_pat = "\<Esc>P[0-9;]*q"
+" The tab line of the second tab page, drawn after ":tabedit".
+let s:tabline = "[No Name] "
+
 " The script for the Vim on the pty: create a popup with an image.
 let s:image_popup_script =<< trim END
   set imageprotocol=.*:kitty
@@ -206,6 +211,14 @@ let s:image_popup_script =<< trim END
   redraw
 END
 
+" The same with the sixel backend.
+let s:image_popup_script_sixel =<< trim END
+  set imageprotocol=.*:sixel
+  let img = repeat([0xff, 0, 0], 16 * 32)->list2blob()
+  call popup_create('', #{image: #{data: img, width: 16, height: 32}})
+  redraw
+END
+
 " Collect what the Vim on the pty writes in s:pty_out.
 func s:PtyOutput(job, msg)
   let s:pty_out ..= a:msg
@@ -255,4 +268,74 @@ func Test_popup_image_kitty_leave_tabpage()
   endtry
 endfunc
 
+func Test_popup_image_kitty_covered()
+  CheckUnix
+  CheckFeature job
+  CheckFeature image_popup
+
+  let lines =<< trim END
+    set imageprotocol=.*:kitty
+    let img = repeat([0xff, 0, 0], 64 * 64)->list2blob()
+    call popup_create('', #{image: #{data: img, width: 64, height: 64},
+          \ line: 2, col: 2, zindex: 50})
+    let g:cover = popup_create(['xx', 'xx'], #{line: 3, col: 6, zindex: 100})
+    redraw
+  END
+  call writefile(lines, 'XpopupImageTab', 'D')
+  let job = s:StartVimWithImageOnPty()
+  try
+    call s:WaitForPtyOutput(',p=4,', 0)
+    " The row above the cover, the parts left and right of it, the row below.
+    for seq in [',p=1,x=0,y=0,w=64,h=16,', ',p=2,x=0,y=16,w=32,h=32,',
+          \ ',p=3,x=48,y=16,w=16,h=32,', ',p=4,x=0,y=48,w=64,h=16,']
+      call assert_notequal(-1, stridx(s:pty_out, seq), seq)
+    endfor
+
+    " Without the cover one placement is enough, the others are deleted.  The
+    " screen may be drawn once more before that, with the four placements
+    " again: wait for the deletion of the last one.
+    let start = len(s:pty_out)
+    call ch_sendraw(job, ":call popup_close(g:cover)\<CR>")
+    call WaitForAssert({-> assert_match(s:kitty_delete .. 'i=\d\+,p=4,',
+          \ s:pty_out[start :])})
+    call assert_notequal(-1, stridx(s:pty_out, ',p=1,x=0,y=0,w=64,h=64,',
+          \ start))
+    for p in [2, 3, 4]
+      call assert_match(s:kitty_delete .. 'i=\d\+,p=' .. p .. ',',
+            \ s:pty_out[start :])
+    endfor
+  finally
+    call job_stop(job, 'kill')
+    call WaitForAssert({-> assert_equal('dead', job_status(job))})
+  endtry
+endfunc
+
+func Test_popup_image_sixel_leave_tabpage()
+  CheckUnix
+  CheckFeature job
+  CheckFeature image_popup
+
+  call writefile(s:image_popup_script_sixel, 'XpopupImageTab', 'D')
+  let job = s:StartVimWithImageOnPty()
+  try
+    call WaitForAssert({-> assert_match(s:sixel_start_pat, s:pty_out)})
+
+    " Sixel pixels cannot be deleted, the cells they cover are drawn over
+    " instead.  Leaving the tab page does not draw the image again.
+    let start = len(s:pty_out)
+    call ch_sendraw(job, ":tabedit\<CR>")
+    call s:WaitForPtyOutput(s:tabline, start)
+    let mid = len(s:pty_out)
+    call assert_equal(-1, match(s:pty_out[start : mid], s:sixel_start_pat))
+
+    " Entering the tab page draws it again.
+    call ch_sendraw(job, ":tabnext\<CR>")
+    call WaitForAssert({-> assert_notequal(-1,
+          \ match(s:pty_out[mid :], s:sixel_start_pat))})
+  finally
+    call job_stop(job, 'kill')
+    call WaitForAssert({-> assert_equal('dead', job_status(job))})
+  endtry
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

out_str((char_u *)"\033[?80l") at term.c:5450 goes out whenever Vim
is built with FEAT_IMAGE_SIXEL, before a backend has been chosen. A
terminal without sixel answers it with an error: kitty logs
Unsupported screen mode: 80 (private) for every Vim it runs. Nothing
like it comes from master.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

Sep 20, 2026, 8:48:43 PM (24 hours ago) Sep 20

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

Thank you I have applied your patches. I've also made it so that DECSDM is only sent when sixel backend is being used.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

h_east

unread,

12:08 AM (20 hours ago) 12:08 AM

to vim/vim, Subscribed

h-east left a comment (vim/vim#21234)

Check the following CI errors.

Linux / linux (normal, gcc, vimtags, proto, preproc_indent, encoding, codestyle)
https://github.com/vim/vim/actions/runs/35548797649/job/106179560917

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

h_east

unread,

2:29 AM (18 hours ago) 2:29 AM

to vim/vim, Subscribed

h-east left a comment (vim/vim#21234)

When displaying a popup image in GUI Vim (GTK4) and changing the window size, a SEGV or ASAN error occurs.
(I avoided adding the check to screen_draw_rectangle() because it is a core function that assumes other callers (such as redraw_block()) always pass valid values.)

diff --git a/src/image.c b/src/image.c
index 2e70c5b14..2247a455f 100644
--- a/src/image.c
+++ b/src/image.c
@@ -648,10 +648,16 @@ redraw_region(pixman_region32_t *region, bool now, bool restore)
 
 	if (now)
 	{
+	    // The region may be from before a resize, when the screen was
+	    // bigger.
+	    int height = MIN(rect.y2, screen_Rows) - rect.y1;
+	    int width = MIN(rect.x2, screen_Columns) - rect.x1;
+
 	    // Tip: if you want to debug stale pixels appearing, set "inverse"
 	    // of screen_draw_rectangle() to TRUE.
-	    screen_draw_rectangle(rect.y1, rect.x1,
-		    rect.y2 - rect.y1, rect.x2 - rect.x1, FALSE, TRUE);
+	    if (height > 0 && width > 0)
+		screen_draw_rectangle(rect.y1, rect.x1, height, width,
+			FALSE, TRUE);
 	    continue;
 	}
 
diff --git a/src/testdir/test_image.vim b/src/testdir/test_image.vim
index 40c5bb855..1feaaec88 100644
--- a/src/testdir/test_image.vim
+++ b/src/testdir/test_image.vim
@@ -338,4 +338,34 @@ func Test_popup_image_sixel_leave_tabpage()
   endtry
 endfunc
 
+func Test_popup_image_sixel_screen_shrinks()

+  CheckUnix
+  CheckFeature job
+  CheckFeature image_popup
+

+  " The image covers cells that are outside a smaller screen.

+  let lines =<< trim END

+    set imageprotocol=.*:sixel
+    let img = repeat([0xff, 0, 0], 400 * 400)->list2blob()
+    call popup_create('', #{image: #{data: img, width: 400, height: 400},
+          \ line: 3, col: 20})

+    redraw
+  END
+  call writefile(lines, 'XpopupImageTab', 'D')
+  let job = s:StartVimWithImageOnPty()
+  try

+    call WaitForAssert({-> assert_match(s:sixel_start_pat, s:pty_out)})
+

+    " Clearing the smaller screen must not look at the cells the image had
+    " on the bigger one.

+    let start = len(s:pty_out)

+    call ch_sendraw(job, ":set columns=40 lines=10\<CR>")
+    call WaitForAssert({-> assert_match(s:sixel_start_pat, s:pty_out[start :])})
+    call assert_equal('run', job_status(job))

+  finally
+    call job_stop(job, 'kill')
+    call WaitForAssert({-> assert_equal('dead', job_status(job))})
+  endtry
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

—

Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Foxe Chen

unread,

2:13 PM (6 hours ago) 2:13 PM

to vim/vim, Subscribed

64-bitman left a comment (vim/vim#21234)

I have applied your patch, thanks

—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!

You are receiving this because you are subscribed to this thread.

Reply all

Reply to author

Forward