Skip to content

Make sixel images grid resident - #5463

Open
mgrant0 wants to merge 36 commits into
masterfrom
image-grid-sixel
Open

Make sixel images grid resident#5463
mgrant0 wants to merge 36 commits into
masterfrom
image-grid-sixel

Conversation

@mgrant0

@mgrant0 mgrant0 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Replace screen-coordinate image placements with immutable pixel images referenced by ordinary grid cells. Draw visible marker runs through the redraw scene using per-client sixel or text backends so images follow history, copy mode, clipping and overwrites.

mgrant0 added 4 commits August 3, 2026 11:56
Replace screen-coordinate image placements with immutable pixel images referenced by ordinary grid cells. Draw visible marker runs through the redraw scene using per-client sixel or text backends so images follow history, copy mode, clipping and overwrites.
Use the DEC hue origin and channel order, and round percentage and HLS conversions to the nearest byte value.
Retain the padded canonical pixel canvas separately from the image content. Scale only the populated portion of partial edge cells so an image keeps its exact raster size on the originating terminal while retaining its cell footprint.
Build an adaptive 256-colour palette with median cut instead of using a fixed colour cube. Apply Floyd-Steinberg error diffusion while mapping pixels to reduce banding and preserve image detail.
@nicm

nicm commented Aug 3, 2026

Copy link
Copy Markdown
Member

Here are a few quick comments:

  • It hardly seems like any of image_x or image_y need to be 32 bits, maybe image_id also?

  • An image extended cell can't usefully have attr, fg, bg, data, right? So I think there could be an (anonymous) union in there?

  • IMAGE_SIZE_LIMIT is unused?

  • struct image_cell and image_sample can be private to image.c? If possible I would like struct image and image_rectangle also to be private and accessed with accessors.

  • sixel_draw_rectangle is in the wrong place in tmux.h.

  • We should have ENABLE_SIXEL or ENABLE_IMAGES but not both. SIXEL should always be on when images are enabled and off when not. The old SIXEL code needs to go away if it is unused.

  • I would get rid of all the phrase "protocol-neutral", what else is there? Just say "image support" or "image".

  • In image-sixel.c struct definitions need to go at the head of the file not mixed in with function definitions.

  • sixel_box should not have multiple declarations on one line.

  • A lot of functions are missing header comments. They should all have them.

  • sixel_from_image is clearly a disaster and needs to be tidied up. Could probably do with some comments.

  • Some of the naming could be better image_rect would be much neater than image_rectangle. Maybe sixel_hgram not sixel_histogram.

  • Instead of image_tty_is_graphical and image_tty_scrolls I would move the flags into tmux.h and add image_backend_flags.

  • image_tty_geometry_changed to just call another public function is silly.

  • image_cmp/RB_GENERATE_STATIC should be at the head of the file.

  • There is some crazy casting going on here eg in image_size_in_cells.

  • SIXEL should definitely not imply Sync.

  • tty_draw_line should not know so much about images. The ENABLE_IMAGES bit in tty-draw.c should call a image helper to get the cell.

@nicm

nicm commented Aug 3, 2026

Copy link
Copy Markdown
Member

image_get_text_cell could just go into image.c and image-ascii.c could be removed.

@nicm nicm moved this from Not Started to In Progress in Open Issues & PRs Aug 5, 2026
mgrant0 added 4 commits August 5, 2026 13:55
…77. Kitty images now use the adaptive 256-colour median-cut palette with direct nearest-colour mapping.
…n the image, the image was overflowing lines and banding.
…xel.c:1077. Kitty images now use the adaptive 256-colour median-cut palette with direct nearest-colour mapping."

This reverts commit 00ffe74.
@mgrant0

mgrant0 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

image_get_text_cell could just go into image.c and image-ascii.c could be removed.

I've done this. There are 3 branches now:
image-grid-sixel - lays the foundation for the kitty support by moving images into the grid
4902-image-support - the kitty support
image-fallback-rendering - improved fallback rendering which depends on the image-grid-sixel foundation but is not dependent on the kitty support.

@mgrant0

mgrant0 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Here are a few quick comments:

  • It hardly seems like any of image_x or image_y need to be 32 bits, maybe image_id also?

I looked into this. Unfortunately not really. Kitty uses a 24-bit image-id so internally it becomes 32-bits. Kitty also handles things like the x and y and other things as uint so reducing the x and y to 16 bits would introduce a lot more checking and could potentially cause an issue with large images. The tradeoff to save a few bytes here is not going to be worth it.

  • An image extended cell can't usefully have attr, fg, bg, data, right? So I think there could be an (anonymous) union in there?

I would not add a union there.

Although graphical backends replace the character with a space, the other fields still matter:

  • fg, bg, and attr are used when Kitty draws the placeholder cells and when fallback rendering converts the image to
    characters.

  • The cell’s style must remain available for redraws and background handling.

  • data is still needed when the extended cell is later reused or converted back to a normal text cell.

  • Grid copying, comparison, compaction, and reconstruction currently preserve all fields unconditionally.

In particular, an image flag is not immutable: later grid operations can clear or replace it while retaining the rest of
the cell state. Overlaying the image coordinates on top of the text/style fields would lose that state.

A separate image-specific extended-cell representation could save memory, but it would require explicit conversion logic
and more invariants. Given that extended cells already exist for many non-image reasons, I think keeping the structure
uniform is safer and clearer.

  • IMAGE_SIZE_LIMIT is unused?

I've propagated the checks using this in the 4902-image-support branch to the foundation sixel branch now.

  • struct image_cell and image_sample can be private to image.c? If possible I would like struct image and image_rectangle also to be private and accessed with accessors.

Done. image_sample, image_cell, image, and image_rectangle are now private to image.c; tmux.h contains only forward declarations and the accessor interface. The SIXEL, Kitty, and fallback renderers now use those accessors rather than reaching into the image structures directly.

  • sixel_draw_rectangle is in the wrong place in tmux.h.

fixed.

  • We should have ENABLE_SIXEL or ENABLE_IMAGES but not both. SIXEL should always be on when images are enabled and off when not. The old SIXEL code needs to go away if it is unused.
  • Removed ENABLE_SIXEL; ENABLE_IMAGES is now the sole compile-time switch.
  • Image support always includes SIXEL parsing/rendering.
  • Replaced all former SIXEL guards in input.c, screen-write.c, and tmux.h.
  • Kept --enable-sixel only as a deprecated configure alias for --with-image-support;
  • I would get rid of all the phrase "protocol-neutral", what else is there? Just say "image support" or "image".

Done.

  • In image-sixel.c struct definitions need to go at the head of the file not mixed in with function definitions.

should be fixed now.

  • sixel_box should not have multiple declarations on one line.

Done.

@nicm

nicm commented Aug 6, 2026

Copy link
Copy Markdown
Member

An extra 12 bytes is adding almost a third to the size of grid_extd_entry and it is heavily used now.

I think we should ignore images more than 64k wide or 64k high and reduce the X and Y to u_short. Even with an enormous 64 x 64 pixels per cell, that would be an image of 1000 x 1000 cells.

Or, if you could store X and Y as cells instead of pixels you could have the image as 64k cells wide or high, which would be impossible to reach.

For the ID, it is silly to store the same 32 bit ID in every cell which references the image and waste four bytes in cells which have no image. Nobody will have 4 billion images. So I would either:

  • use u_short for the ID as an index into a global array of images (so 64k images per tmux server);
  • or use u_char for the ID as an index into a per-pane array of images (so 256 images per pane).

@nicm

nicm commented Aug 6, 2026

Copy link
Copy Markdown
Member

You don't need to store the data in the same format in grid_extd_entry and grid_cell - the latter can be any size and the image_id can just be looked up in grid_get_cell1. Everything outside grid.c would see it as 32 bits.

@nicm nicm mentioned this pull request Aug 6, 2026
17 tasks
@mgrant0 mgrant0 moved this from In Progress to For Review in Open Issues & PRs Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: For Review

Development

Successfully merging this pull request may close these issues.

2 participants