Implement Kitty graphics protocol as a follow-up to #2973, not a replacement for Sixel - #5238
Open
kotamat wants to merge 16 commits into
Open
Implement Kitty graphics protocol as a follow-up to #2973, not a replacement for Sixel#5238kotamat wants to merge 16 commits into
kotamat wants to merge 16 commits into
Conversation
…n` only on Android 5 as later function is not available on it and will cause a runtime crash The change was done in 6c00f1f via termux#2997
… text received for setting to clipboard from terminal and fix error format typo
… for OSC/DCS/APC in `TerminalEmulator`, add support for fast path for OSC/DCS terminal commands, and properly clear buffer after commands Rename `ESC_P` to `ESC_DCS`, `ESC_APC_ESCAPE` to `ESC_APC__ESC`, `doDeviceControl()` to `doDcs()`, `doOsc()` to `receiveOsc()`, `doOscEsc()` to `receiveOscEsc()`, `doApc()` to `receiveApc()`, `doApcEscape()` to `receiveApcEsc()`, `doOscSetTextParameters()` to `doOsc()`. `mOSCOrDeviceControlArgs` `StringBuilder` buffer has been renamed to `mTerminalControlArgs` as it stores args for all 3 OSC/DCS/APC commands. Additionally, for `mTerminalControlArgs`, only `setLength(0)` was called at end of commands to clear it, which will only update internal length marker and not reduce internal array capacity, and to deallocate extra memory `trimToSize()` needs to be called, which creates another smaller array. So if an OSC/APC command was ever received with `MAX_OSC_STRING_LENGTH`, then the buffer size would never be brought back down to a lower capacity (default 16bytes), which would have slightly affected performance/memory for later smaller commands. That is fixed now by `clearTerminalControlArgs()` with a newer logic for freeing memory, which is required to free memory for long commands like sixels or iterm image to be added later. `ensureTerminalControlArgsCapacity()` has also been added to allow control commands to increase buffer capacity to reduce/prevent reallocation and copying of memory as more data is received. `collectTerminalControlArgs()` has been updated with better error message, catches OOM in case caused when buffer capacity is increased when appending, and returns `false` if an error occurred like an overflow/OOM so that callers can terminate processing. This was required because `receiveApcEsc()`/`receiveOscEsc()` functions appended `ESC`/`27` and then appended `b` without checking if `collectOSCArgs()` had already failed when collecting `ESC`, triggering a second error. `collectTerminalControlArgs()` is now also used for `DCS` commands properly handle errors/exceptions. `MAX_OSC_STRING_LENGTH` has been renamed to `TERMINAL_CONTROL_ARGS__MAX_LENGTH` and the length has been increased from `8192` to `16384` to receive larger sixel data and iTerm image data. With `16384`, around 12KB images can be received with legacy `File=` command, otherwise with `8192`, any image over 6KB would have triggered an overflow. Sixel commands strings can also be split into larger parts in future with the new limit. The `TerminalEmulator.processCodePoint()` processes bytes received by the terminal, however there are around ~30 case comparisons done before `ESC_OSC` or `ESC_DCS` is processed for each byte received. While it may be necessary for general commands, some commands like sixel or iterm image data parse their own input byte, which may be random data, so a fast path has been added at start of `processCodePoint()` to call `doOsc` or `doDcs()` if enabled by the currently running command. Fast path can be enabled for OSC commands via `setOscTypeVariables()` and for `DCS` commands via `doDcs()`. OSC commands can also prevent CR/LF characters being printed by enabling `mIgnoreCrLfForOsc` via `setOscTypeVariables()`. Escape states for Terminal APC commands fast path added in f35063d to `TerminalEmulator.processCodePoint()` has also been fixed.
- In TerminalEmulator, interpret sixel sequences, and send them to TerminalBuffer for constructing a bitmap. - Sixel sequences may be longer than 8192 characters, so break them in natural places ($,-,#), rather than collecting all in the buffer. - The bitmap is sliced to character cell sized slices, and each the the style attribute is used to store which bitmap slice is displayed in place of this character. - In TerminalRenderer the style is interpreted, and drawn using drawBitmap, instead of drawText. Support iTerm inline image protocol (OSC 1337): - Using the same bitmap display infrastructure introduced for sixels. - Collects the image data outside of the OSC buffer. - Ignoring some parameters. Small emulator changes: - Also eat APC sequences, not echoing to screen. - Fix `CSI 14 t` to give actual size - Add `CSI 16 t` - Add `4` (sixel) to device attributes
…age (`OSC 1337`) Support for displaying bitmaps inside the terminal has been added via `TerminalBitmap` which can be created from image `byte[]` or sixel bitmap. The bitmaps are sliced to character cell sized slices. The `TerminalBuffer` stores a map for bitmap number to the `TerminalBitmap` loaded in the terminal. The bitmap number and coordinates are encoded in the `long` `TerminalRow.mStyle` for the `TerminalRow` character of a column by `TerminalBitmap#buildOrThrow()` by getting encoded value from `TextStyle.encodeTerminalBitmap()`. The `TerminalRenderer.render()` then checks during rendering terminal output whether a character at a row/coloumn index is a bitmap instead of text by calling `TextStyle.isTerminalBitmap()`, then draws it using `Canvas.drawBitmap()` instead of `Canvas.drawText()`. Sixel images can be created with Sixel Device Control String command sent via `DCS q s..s ST` or `DCS P1; P2; P3; q s..s ST`. - The `TerminalEmulator` interprets sixel sequences, and sends them to `TerminalBuffer` for constructing a `TerminalSixel`. Once the sixel command has been completely processed, a `TerminalBitmap` is created from the `TerminalSixel`. If an error occurred during processing (like OOM), then remaining sixel command is completely read, but is ignored and no sixel is drawn (done by setting `mTerminalSixel` to `null` so that `TerminalBuffer.sixelReadData()` ignores further commands). - Since a sixel sequence can be very long to render a full image and can have length greater than `TERMINAL_CONTROL_ARGS__MAX_LENGTH` (`16384`), the entire sequence is not stored in the `mTerminalControlArgs` buffer before being processed as it will result in an overflow error, instead as soon as length crosses `TERMINAL_CONTROL_ARGS__MAX_LENGTH / 2` and a complete sixel sub command (`#`, `!`, or `"`) has been received, it is immediately processed, and then further commands are read after emptying buffer. - If "rough" horizontal and vertical size of image is received at start of sixel data string with a `Raster Attributes` command, like done by `img2sixel` command, then sixel commands args buffer capacity (`mTerminalControlArgs`) is increased and sixel bitmap in `TerminalSixel` is resized at start, instead of having to keep resizing buffer/bitmap as more sixel data is received, which has a performance hit due to memory reallocations and copying. - The `4` (sixel) value has been added to `CSI` `Primary Device Attributes` terminal response. The `img2sixel` command can be used to display sixel images after installing with `libsixel` package with `pkg install libsixel`, like with `img2sixel --width=1000px image.jpg`. To manually send an escape sequence, check the `digiater.nl` link below, but it is too cumbersome to create images large enough to be easy viewable in the terminal. See Also: - https://vt100.net/docs/vt3xx-gp/chapter14.html - https://en.wikipedia.org/wiki/Sixel - https://www.digiater.nl/openvms/decus/vax90b1/krypton-nasa/all-about-sixels.text iTerm images can be created with `1337` Operating System Control command. - Both `File=` and `MultipartFile=` (chunk based) protocols are supported. The `inline` parameter should be `1` to display inline images in the terminal. Downloading images to Downloads folder with the value `0` will be ignored as that is not supported. - The escape sequences/image data cannot be greater than `TERMINAL_CONTROL_ARGS__MAX_LENGTH` (`16384`) bytes if sent via (`File=`) protocol, otherwise it will be ignored with an overflow error. For larger images, send images via `MultipartFile=` protocol in chunks with `FilePart=`, the `imgcat` utility uses that with 200-byte chunks if `--legacy` flag is not passed. - The `TerminalEmulator` interprets iTerm images sequences and creates an `ITermImage` to process parameters and store the base64 encoded image sent. Once all the data has been received, which can be over multiple `OSC` commands for `MultipartFile=` protocol, the encoded image is decoded to a `byte[]`, which is then passed to `TerminalBuffer`, which creates a `TerminalBitmap` for the image. The `imgcat` utility can be used for sending images, like with `imgcat --width 1000px image.jpg` (`MultipartFile=`) or `imgcat --width 1000px --legacy image.jpg` (`File=`). To manually send an escape sequence, run `echo -en '\e]1337;File=inline=1;keepAspectRatio=0;width=1000px;:' ; base64 -w 0 ./image.jpg ; echo -e '\e\\'` (`File=`). See Also: - https://iterm2.com/documentation-images.html - https://iterm2.com/utilities/imgcat - https://github.com/gnachman/iTerm2-shell-integration/blob/d1d4012068c3c6761d5676c28ed73e0e2df2b715/utilities/imgcat
The `TerminalBitmap.MAX_BITMAP_SIZE` defines the max size of a Terminal bitmap for its pixels. Each pixel is stored on 4 bytes for a `Bitmap.Config.ARGB_8888` bitmap color config. The value should normally be between `100-200MB` depending on device and Android version. Check the variable docs for more info. The sixel image size cannot be greater than `TerminalBitmap.MAX_BITMAP_SIZE`. The repeat value for sixel Graphics Repeat Introducer command cannot be greater than `TerminalSixel.SIXEL__MAX_REPEAT` (`8192`). The iTerm image data sent for `File=` and `MultipartFile=` protocols cannot be greater than `TerminalBitmap.MAX_BITMAP_SIZE` bytes.
…olon and allow not passing all optional parameters
…ds could not be decoded as it hangs the terminal `TerminalBitmap.build()` first decodes only the bounds of the image data received for an iTerm image (`OSC 1337`) by calling `BitmapFactory.decodeByteArray()` with `BitmapFactory.Options.inJustDecodeBounds` set to `true`, so that an image that is too large to be drawn is not decoded at all. If `BitmapFactory` fails to decode the bounds, then `BitmapFactory.Options.outWidth` and `outHeight` are set to `0` instead of the image dimensions, which was not checked before the values were used. A `0` value for `imageWidth` or `imageHeight` results in a division by `0` while calculating the `wFactor` and `hFactor` scaling factors for an image whose aspect ratio is to be preserved, and in the `scaleFactor` loop after it never terminating, since the `imageHeight >= 2 * newHeight * scaleFactor` and `imageWidth >= 2 * newWidth * scaleFactor` conditions stay `true` for `0 >= 0` for any value of `scaleFactor`, until `scaleFactor` overflows to `0` after `201` iterations and then stays `0` forever. The terminal is hung on the thread that processes the terminal output and does not recover. The hang is triggered by sending an iTerm image command with a `width` parameter and image data that cannot be decoded, like `OSC 1337;File=inline=1;width=10:<invalid base64 image data> ST`.
…t before an image on the same line is not lost `TerminalRenderer.render()` accumulates consecutive columns of a line that have the same style into a text run, which is drawn with a single `drawTextRun()` call once the style changes or the end of the line is reached. The branch for a cell whose style is a terminal bitmap did not draw the run that was still pending before it and only reset the run state, so any text before an image on the same line was discarded and never drawn at all. The start of the run after the bitmap was also wrong. The `lastRunStartColumn` was set to `column + 1` after `column` had already been incremented past the cell of the bitmap, so the next run started one column too late and its text was drawn shifted by one cell. The `lastRunStartIndex` was set before `currentCharIndex` was incremented past the cell of the bitmap, so it pointed at the character of the bitmap cell instead of at the first character of the next run. Both issues affect sixel (`DCS q`) and iTerm (`OSC 1337`) images equally and are reproduced by printing text before an image escape sequence without a newline between them.
…he column after it is the last column of the screen After a terminal bitmap has been added to the screen for an image, the cursor is moved to the column after the image on the last row the image covers if that position is still on the screen, and is otherwise wrapped to the first column of the row after the image. The condition used to check this was `col < mColumns - 1`, where `col` is the `0` based index of the column after the last column the image covers, which is off by one. The position is on the screen as long as `col` is a valid column index, so the condition must be `col < mColumns`. With the old condition, an image whose following column is the last column of the screen was treated as if it did not fit, and so one row more than the image covers was consumed and the cursor was left at the first column of that row. Note that this changes the behaviour for sixel (`DCS q`) and iTerm (`OSC 1337`) images, since this is the shared code path for the cursor position after any terminal bitmap. On a `56` column screen with the cursor at column `54`, printing an iTerm image that is `2` cells wide and `1` cell high covers the columns `54` and `55` and the cursor position reported by `CSI 6 n` changes from row `7` column `1` to row `6` column `56`.
…_G`) Support for displaying images sent with the kitty graphics protocol has been added on top of the existing terminal bitmap infrastructure that is used for sixel (`DCS q`) and iTerm (`OSC 1337`) images. A command is in the format `APC _ G <control data> [; <payload>] ST`, where `control data` is a comma separated list of `<key>=<value>` pairs with single character keys and `payload` is the `base64` encoded image data. Only a minimal subset of the protocol is supported, which is enough to transmit an image and display it. The `CSI` `Primary Device Attributes` response is not changed, since clients detect kitty graphics protocol support by sending an `a=q` query command and checking the response for it. The following is supported: - The `a=t` (transmit), `a=T` (transmit and display), `a=p` (display an image that was already transmitted), `a=q` (query) and `a=d` (delete) actions. - The `t=d` (direct) transmission medium, where the image data is sent in the command payload. - The `f=100` (`PNG`), `f=24` (raw `RGB`) and `f=32` (raw `RGBA`) image formats, with the pixel dimensions of the raw formats passed with the `s` and `v` keys. - The `m=1`/`m=0` chunked transmission of the image data of a single image with multiple commands. - The `i=<image id>` and `p=<placement id>` keys, and the `d=a`, `d=A`, `d=i` and `d=I` delete modes. - The `c=<columns>` and `r=<rows>` keys for the number of cells to display the image in. - The `x=`, `y=`, `w=` and `h=` keys for displaying only a source rectangle of the transmitted image. - The `C=1` key for not moving the cursor after displaying an image. - The `q=<quiet level>` key for suppressing success and error responses. The following is **not** supported and results in an `ENOTSUP` error response: - The `t=f`, `t=t` and `t=s` transmission mediums, where the image data is read from a file or shared memory. Reading arbitrary files that the terminal has access to on behalf of a client is a security concern, so this is intentionally not implemented. - The `o=z` (`zlib`) compressed image data. - The `a=f` (animation frame), `a=a` (animate) and `a=c` (compose) actions. The keys for unsupported features that a client is expected to be able to send for images this terminal can display are ignored instead of resulting in an error response. These are the `z` (z-index), `X` and `Y` (cell pixel offset) and `U` (unicode placeholder) keys. Not honouring `X` and `Y` means an image is aligned to the cell grid instead of being offset by up to one cell width and height. Implementation: - `KittyImage` parses the control data of a command, decodes the `base64` image data as it is received and holds the state of an image that is transmitted with multiple commands. The `base64` image data is not collected in `TerminalEmulator.mTerminalControlArgs` first, so a client may send an entire image with a single command instead of splitting it into the chunks of at most `4096` bytes the protocol recommends, and the total image data size is only bound by `KittyImage.IMAGE_DATA__MAX_LENGTH` (`TerminalBitmap.MAX_BITMAP_SIZE`). Only the control data is stored in the args buffer, which is limited to `4KB` by `TerminalEmulator.KITTY_GRAPHICS_CONTROL_DATA__MAX_LENGTH`, and a command with longer control data is discarded instead of being printed on the terminal as text. - `TerminalEmulator` gained an APC command type, set by `setApcTypeVariables()` from the first code point after the `APC` escape sequence, so that a kitty graphics command takes the fast path in `processCodePoint()` while all other APC commands keep being parsed and ignored like `xterm` does. The command is dispatched by `doApcKittyGraphics()` and responded to with `APC _Gi=<image id>;OK ST` or `APC _Gi=<image id>;<error code>:<error message> ST` as per the `q` key. - The image data transmitted with the `a=t` and `a=T` actions is stored for its image id in `TerminalEmulator.mKittyImages` so that it can be displayed again with an `a=p` command without being transmitted again. The map is in insertion order and the oldest images are evicted first once `KITTY_IMAGES__MAX_COUNT` (`32`) or `KITTY_IMAGES__MAX_TOTAL_SIZE` (`8MB`) is exceeded, and it is cleared on a terminal reset. - `TerminalBitmap.buildForKittyImage()` creates the bitmap for a placement. It cannot use the iTerm image path since the source rectangle of the `x=`, `y=`, `w=` and `h=` keys must be cropped before the image is scaled to the cells it is displayed in, otherwise the wrong part of the image would be scaled, and the raw `RGB`/`RGBA` formats are not decoded by `BitmapFactory`. The scaling calculation that both paths share has been extracted into `getScaledImageSize()`. A `PNG` is only subsampled while decoding if the entire image is displayed, since the source rectangle coordinates are in the coordinate space of the full size image, and its size is checked against `MAX_BITMAP_SIZE` before decoding instead. - `TerminalBuffer.addTerminalBitmapForKittyImage()` adds the bitmap to the screen, and `deleteKittyImagePlacements()` and `deleteAllKittyImagePlacements()` remove the bitmaps of the placements deleted by an `a=d` command and clear the cells of the screen and the transcript they were rendered in. The image id and placement id are stored in the `TerminalBitmap` itself instead of a separate registry so that they do not need to be kept in sync when bitmaps are removed and their bitmap numbers are reused. - `TerminalBuffer.releaseUnreferencedTerminalBitmaps()` releases the bitmaps of the cells that a new bitmap overwrote if they are no longer referenced by any cell. This does not rely on `doTerminalBitmapsGC()`, which only runs once per `timeDelta`, since an `a=p` command can display an already transmitted image with around 30 bytes, so a client that displays a new image at the same position for every frame would otherwise keep every one of them in memory. `KittyGraphicsTest` has been added with 74 tests covering the control data parsing, the supported and unsupported actions, keys and formats, the response and quiet levels, chunked transmission, the cursor position after an image, deletion and the image data limits. The comment of `ApcTest.testApcConsumed()` has been updated, since the `yazi` capability query it sends is now answered on stdin instead of being consumed silently. - https://sw.kovidgoyal.net/kitty/graphics-protocol/
Member
Thank you, the bug fixes for the PR #2973 that you have made might be helpful to apply on top of it before it is merged. It is fine to leave this PR open so that people are aware of it and can review it also. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements the Kitty graphics protocol (issue #5068) by wiring up the APC (
ESC _ G ... ESC \) parsing path that currently exists but is inert onmasterand on #2973 —doApc()is a no-op and the call fromreceiveApcEsc()is commented out ("Eat APC sequences silently for now").This is not a standalone feature. It is built entirely on top of the image infrastructure introduced in #2973 ("Add graphics in terminal support: - Sixel and iTerm2 protocols", by @MatanZ and @agnostic-apollo). This PR does not modify #2973's Sixel or iTerm2 code paths; it adds a second front-end that shares the same backend (image registry, bitmap storage, rendering). It only makes sense as a PR to be applied after #2973, and I've written it and tested it that way (rebased on top of #2973's branch).
Relationship to #2973 and to the maintainer's stated direction
In #5068, @robertkirkman noted that Sixel should be prioritized for merge, and that Kitty support is a possible future addition rather than something that should land first, since Termux isn't in the position kitty is in and shouldn't drop Sixel. I agree with that ordering and I'm not asking for this to be merged ahead of, or instead of, #2973.
I'm opening this now, ahead of #2973 actually merging, mainly so the work is visible and reviewable in parallel, and so it doesn't get lost. I'd expect this to sit until #2973 lands, and I'm fine with this being revisited, rebased, or reconsidered at that point — including the possibility that the maintainers decide not to take it. If it's more useful for me to close this and resubmit once #2973 merges, I'm happy to do that instead; just let me know your preference.
What's implemented
a=t(transmit)a=T(transmit + display)a=p(put / placement)a=q(capability query)a=d(delete: all,d=a/d=A,d=i/d=I)t=d(direct, base64)f=100(PNG)f=24(RGB raw)f=32(RGBA raw)m=1/m=0i=(full 32-bit unsigned range)p=(image_id, placement_id)c=/r=s=/v=x=/y=/w=/h=q=(0/1/2)OK,<CODE>:<msg>)C=1(don't move cursor after placement)t=f/t=t/t=s(file, temp file, shared memory)ENOTSUPo=z(zlib)ENOTSUPa=f/a=a/a=cENOTSUPU=1)z=(z-index)X=/Y=(sub-cell pixel offset)On
U=1: some clients (e.g. yazi) can use the Unicode placeholder path, but since Termux advertises Sixel support in DA1, yazi selects Sixel instead of Kitty's placeholder mechanism in that case, so this gap does not affect yazi in practice. It would matter for a client that requires Unicode placeholders specifically.Reuse of #2973's infrastructure
This PR does not introduce a parallel image system. It reuses, unmodified in behavior:
TextStyleTerminalBuffer's image registry and scroll-driven garbage collectionTerminalBitmap's cell-splitting and size-limiting logicTerminalRenderer's existing bitmap draw branchThe Sixel and iTerm2 code paths (
build(byte[]...),build(TerminalSixel...),addTerminalBitmapForImage()) are structurally unchanged by this PR — Kitty parsing produces the same intermediate bitmap representation that Sixel/iTerm2 already produce, and goes through the same storage/rendering code from there.Tests
masterand on Add graphics in terminal support: - Sixel and iTerm2 protocols #2973's head, unmodified../gradlew :terminal-emulator:testreports 219 tests / 0 failures / 0 errors for bothtestDebugUnitTestandtestReleaseUnitTest.len % 4 == 1, excess padding, data after padding, padding in a non-final chunk), unknown actions/keys, interruption mid-chunk-transfer, sequences left open withm=1and never closed, oversized dimensions, deletion of nonexistent ids, out-of-scope keys for a given action, very largei=values, and response suppression behavior for eachq=level.java.util.Base64as an independent reference implementation, across a range of chunk split points, to confirm the incremental decoder produces byte-identical output to decoding the same payload in one piece.Verification on emulator
All of the following was verified by running the build on an Android emulator (API 35, AOSP system image, arm64-v8a, headless, software rendering) and inspecting the result — not just by reading the code. Verification consisted of pixel measurements on captured screenshots,
dumpsys meminforeadings, andadb logcat -b crashchecks, unless noted otherwise:f=24), raw RGBA (f=32), and PNG (f=100), at sizes from 1×1 up to 64×64.a=p: one transmit followed by five separate put commands, all five renders confirmed present.c=/r=cell-based scaling is pixel-accurate against a 19×38px cell size: both dimensions specified matches exactly, one dimension specified preserves aspect ratio, neither specified renders at native size.C=1leaves the cursor position unchanged, confirmed via DSR cursor-position queries before and after placement.Verification on physical device
The above was also verified on a physical device: Samsung SM-F966Q (Galaxy Z Fold7-generation), Android 16 / SDK 36, arm64-v8a, 10.85GB RAM. Measured terminal calibration: origin at (7,119), cell size 19×38px, 56 columns × 56 rows. A 1-cell image renders as exactly 19×38 = 722px of solid color with zero bleed into neighboring cells; this was reproduced twice.
Verified items (each backed by a pixel measurement or a DSR-based numeric check):
L2_A/ image /L2_B/ image /L2_C), three adjacent segments (text in columns 0–3, a 6-cell-wide bitmap in columns 4–9, text in columns 10–13), and a case where styled text spans a renderer flush boundary —L7_RED_rendered in (205,0,0) before an image,L7_GRNrendered in (0,205,0) after it — confirming that the flush correctly carrieslastRunStyleacross the boundary.f=24,f=32, andf=100, with histograms matching exactly across formats (no channel-order drift).OutOfMemoryErroror ANR occurred.cmd device_state state 3was used to force a column-count change (56×56 → 102×48), exercising the terminal's reflow path. No crash. iTerm2, sixel, and Kitty images all produce identical reflow artifacts — see the corresponding entry in Known Limitations below.Memory under sustained placement load
Measured under a streaming workload (one
a=ttransmit followed by 200a=pre-placements of the same image), sampled 1,738 times:Memory returns to baseline+13MB within roughly 10 seconds and stays flat from there; the residual is consistent with the single displayed 1064×760×4 = 3.2MB bitmap. This was checked against a false-pass: the on-screen marker
=== HEAVY DONE 200 frames ===confirmed the full run completed, the rendered image area measured 1064×760 with each of the four quadrants at exactly 200,870px, and there were noKittyImage/TerminalBitmaperror log lines during the run.Why this matters:
a=pre-places an already-cached image with a command of roughly 30 bytes, so placements can happen at a much higher rate than Sixel/iTerm2 allow, since those protocols require re-sending the full image on every frame and are bandwidth-bound as a result. A naive implementation could accumulate orphaned bitmaps under that kind of load. This implementation performs targeted collection at placement time: it collects thebitmapNums referenced by the cells being overwritten, scans only the rows withmHasTerminalBitmapset, and exits early once a reference to a given bitmap is found elsewhere. It does not callrecycle()— references are simply dropped and left to the GC, since the renderer may hold a reference from a different thread. The existing throttleddoTerminalBitmapsGCfrom #2973 is unmodified.Included bug fixes (independent of Kitty, cherry-pickable to #2973)
While building on #2973's image pipeline, I found and fixed three pre-existing issues unrelated to the Kitty protocol itself. Each is isolated into its own commit so it can be reviewed, applied, or cherry-picked to #2973 independently of the rest of this PR. All three commits apply cleanly against a pristine #2973 tree — I confirmed
git apply --checksucceeds for each one individually and for all three together, without any of the Kitty commits.Infinite loop in
TerminalBitmap.build(). WhenBitmapFactoryfails to decode bounds for the input,outWidth/outHeightcome back as 0, and the aspect-ratio-preservingscaleFactorloop condition0 >= 0never terminates. In testing,scaleFactoroverflows and pins at 0 after 201 iterations, so the loop spins indefinitely rather than overflowing into a crash. On current Add graphics in terminal support: - Sixel and iTerm2 protocols #2973, this is reachable fromOSC 1337;File=inline=1;width=10:<garbage>, i.e. a malformed iTerm2 image sequence can hang the JVM.Text preceding an image on the same row is dropped. In
TerminalRenderer.java(lines 106–121), the bitmap-drawing branch discards a pending text run instead of callingdrawTextRunfor it, and separately has an off-by-one inlastRunStartColumn(column + 1). This reproduces identically with Add graphics in terminal support: - Sixel and iTerm2 protocols #2973's existing iTerm2 (OSC 1337) images, so it isn't Kitty-specific — it's a pre-existing rendering issue in the shared renderer path that this PR's tests happened to surface.Cursor position when an image ends exactly in the last column. The bounds check used
col < mColumns - 1, which treats an image that ends exactly at the last column as "didn't fit," consuming one more row than the image actually covers and leaving the cursor at the start of the following row.col < mColumnsis the correct condition.The evidence for this one comes specifically from the existing iTerm2 (
OSC 1337) path, not from a Kitty-vs-Kitty comparison — on a pristine Add graphics in terminal support: - Sixel and iTerm2 protocols #2973 tree, Kitty's APC sequences are still parsed and discarded, so no Kitty image is ever displayed and there is nothing to compare against there. To isolate the bug from anything Kitty-related, I built a pristine Add graphics in terminal support: - Sixel and iTerm2 protocols #2973 tree in a separate git worktree and compared it directly against this PR's tree using an iTerm2 image, both built and run the same way:[7;1(wrapped to the next row, column 1).[6;56(the image's last row, column 56 — the next empty cell after the image, so subsequent characters do not overlap it).Because this changes existing Sixel/iTerm2 cursor behavior, not just Kitty, it's kept as its own commit so it can be evaluated, merged, or dropped independently of the rest of this PR. If upstream considers the current behavior intentional, this commit can be dropped without affecting the Kitty support — the only consequence would be that a Kitty image ending exactly in the last column would keep consuming one extra row, same as iTerm2/Sixel currently do. Three unit tests are included (image ending exactly in the last column, image that genuinely doesn't fit, and the normal case).
Known limitations / honest caveats
s=/v=values are used as the primary input to source-rectangle validation. The decoded bitmap dimensions remain the ground truth for rendering, so this is safe in the sense that it can't produce out-of-bounds reads, but a client that declares dimensions inconsistent with the actual PNG could get a legitimate crop request rejected withEINVAL.d=a/d=Adeletes all Kitty placements regardless of current on-screen visibility; the spec describes this action as scoped to placements "visible on screen."TextStyleencodesbitmapNumin 16 bits whileTERMINAL_BITMAP__NUM_ENDis defined asInteger.MAX_VALUE. This mismatch predates this PR (introduced in Add graphics in terminal support: - Sixel and iTerm2 protocols #2973) and is not touched here.TerminalBuffer.resize()does not remap bitmap-bearing cells. This is existing behavior from Add graphics in terminal support: - Sixel and iTerm2 protocols #2973, not changed by this PR.Emulator vs. physical device: notable differences
What I'd like reviewers to look at
U=1/ Unicode-placeholder gap — whether that's an acceptable scope cut for a first pass, or a blocker.ENOTSUPpaths (file/shared-memory transmission, zlib, animation), in case there's a preferred convention already used elsewhere in the codebase.Note on where to review this: the same four commits are also open against #2973's own branch as MatanZ/termux-app#2, where the diff is limited to this PR's own changes (9 files) instead of also including #2973's 15 commits. Either one is fine to review — whichever is more convenient. I'll keep them in sync.