dnkl/foot
41
2.0k
Fork
You've already forked foot
243

WIP: multi-cursor protocol (new kitty RFC) #2169

Draft
dnkl wants to merge 26 commits from multi-cursor into master
pull from: multi-cursor
merge into: dnkl:master
dnkl:master
dnkl:osc-5522
dnkl:sixel-heap-buffer-overflow
dnkl:releases/1.27
dnkl:releases/1.26
dnkl:releases/1.25
dnkl:releases/1.24
dnkl:releases/1.23
dnkl:pixman-16f-2
dnkl:releases/1.22
dnkl:releases/1.21
dnkl:releases/1.20
dnkl:releases/1.19
dnkl:releases/1.18
dnkl:releases/1.17
dnkl:releases/1.16
dnkl:releases/1.15
dnkl:releases/1.14
dnkl:releases/1.13
dnkl:releases/1.12
dnkl:releases/1.11
dnkl:releases/1.10
dnkl:releases/1.9
dnkl:releases/1.8
dnkl:releases/1.7
dnkl:releases/1.6
dnkl:releases/1.5
dnkl:releases/1.4
dnkl:releases/1.3
dnkl:releases/1.2
dnkl:releases/1.1
dnkl:releases/1.0
Owner
Copy link

Ref:

This implements the multi-cursor protocol, with all features as they exist today (they are subject to change):

  • All shapes (copy primary, block, beam, underline)
  • Custom colors (all variants, but see limitations below)
  • All queries ("global" query for the feature itself, query current "extra" cursors' positions, query "extra" cursors' color)

Implementation details

Existing grid/row/cell data types are not used, hence virtually no additional runtime memory usage when no extra cursors are active. When at least one extra cursor is enabled, we allocate a flattened 2-dimensional array, mapping to the grid. This array tracks each cell's extra cursor type/shape (no cursor/disabled is one such type/shape).

As per the specification, custom colors can be assigned to all extra cursors, but not to each one individually. Hence there's no need to track per-cell cursor color.

While the flattened array does track enabled/disabled state of the extra cursors, we also track this in a pixman region. This is to a) be able to quickly determine if no cursors are active at all, or at least one (by checking if the region is empty), and b) to be able to iterate the enabled cursors without having to scan the entire array (TODO: some use cases might be faster to scan the array - need to benchmark).

All extra cursors are deleted on:

  • Terminal reset
  • Window resize (we could/should reflow the extra cursors, but we expect the application to redraw anyway, so let's skip that for now)
  • Alt/normal screen switches
  • ED (param 2 and 3 - erase screen, and erase screen and scrollback)

When rendering, the same function is used to render both the extra cursors and the primary cursor. Whether a cell has an extra cursor or not is passed as a new parameter to render_cell(). If a cell has both the primary cursor, and an extra cursor, the primary cursor will be rendered, and the extra cursor will not.

Limitations

The specification says the extra cursors should use the exact same colors as the primary cursor (when set to mirror the primary cursor's colors, and not using custom colors). This includes using the exact same colors as the primary cursor also when the primary cursor has no set colors, but reverses the cell colors. This is currently not happening; when the primary cursor is set to use reversed colors, all extra cursors will also reverse the colors, but of their cells, not the primary cursor's cell.

Should this be merged?

I'm not sure. There's a lot of code for what I consider a small feature. Let's wait a bit, and see if any editors start using this feature.

Ref: * https://github.com/kovidgoyal/kitty/issues/8927 * https://github.com/kovidgoyal/kitty/blob/master/docs/multiple-cursors-protocol.rst This implements the multi-cursor protocol, with all features as they exist today (they are subject to change): * All shapes (copy primary, block, beam, underline) * Custom colors (all variants, but see limitations below) * All queries ("global" query for the feature itself, query current "extra" cursors' positions, query "extra" cursors' color) ## Implementation details Existing grid/row/cell data types are not used, hence virtually no additional runtime memory usage when no extra cursors are active. When at least one extra cursor is enabled, we allocate a flattened 2-dimensional array, mapping to the grid. This array tracks each cell's extra cursor type/shape (no cursor/disabled is one such type/shape). As per the specification, custom colors can be assigned to all extra cursors, but not to each one individually. Hence there's no need to track per-cell cursor color. While the flattened array does track enabled/disabled state of the extra cursors, we _also_ track this in a pixman region. This is to **a)** be able to quickly determine if no cursors are active at all, or at least one (by checking if the region is empty), and **b)** to be able to iterate the enabled cursors without having to scan the entire array (TODO: some use cases might be faster to scan the array - need to benchmark). All extra cursors are deleted on: * Terminal reset * Window resize (we could/should reflow the extra cursors, but we expect the application to redraw anyway, so let's skip that for now) * Alt/normal screen switches * `ED` (param 2 and 3 - erase screen, and erase screen and scrollback) When rendering, the same function is used to render both the extra cursors and the primary cursor. Whether a cell has an extra cursor or not is passed as a new parameter to `render_cell()`. If a cell has both the primary cursor, and an extra cursor, the primary cursor will be rendered, and the extra cursor will not. ## Limitations The specification says the extra cursors should use the exact same colors as the primary cursor (when set to mirror the primary cursor's colors, and not using custom colors). This includes using the **exact** same colors as the primary cursor also when the primary cursor has no set colors, but reverses the cell colors. This is currently not happening; when the primary cursor is set to use reversed colors, all extra cursors will also reverse the colors, but of _their_ cells, _not_ the primary cursor's cell. ## Should this be merged? I'm not sure. There's a lot of code for what I consider a small feature. Let's wait a bit, and see if any editors start using this feature.
csi: wip: parse the multi-cursor escape (CS > SPACE q)
Some checks reported errors
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job failed
5e0524bf99
 
We do nothing with it yet
multi-cursor: wip: start tracking which cells has secondary cursors
Some checks reported errors
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job failed
dccbdb97cd
multi-cursor: wip: track cursor shapes and colors
Some checks reported errors
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job failed
beb4556dc0
 
And erase all extra cursors on reset, resize and clear
screen/scrollback (ED 2/3).
multi-cursor: wip: render extra cursors
Some checks reported errors
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job failed
d5e99862af
 
Currently ignores custom shapes and colors (i.e. all extra cursors are
rendered exactly as the primary cursor).
multi-cursor: render: wip
Some checks reported errors
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job failed
44bc720755
 
Shapes and most color options implemented
render: multi-cell: decrement 'extra-cursor' pointer instead of indexing
Some checks reported errors
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job failed
dcabe14392
render: multi-cursor: wip: colors
Some checks reported errors
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job failed
6319ed8b29
csi: multi-cursor: implement cursor and color queries
Some checks reported errors
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job failed
e4fcb0cc33
csi: multi-cursor: convert debug logging to actual debug logs
Some checks reported errors
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job failed
ci/woodpecker/pr/woodpecker Pipeline failed
410b2c246d
csi: disable debug logging
Some checks failed
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pr/woodpecker Pipeline failed
bab4a2158b
csi: multi-cursor: ensure 'len' is initialized
Some checks reported errors
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job failed
ci/woodpecker/pr/woodpecker Pipeline failed
acf62bdda5
csi: disable debug logging (again)
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pr/woodpecker Pipeline was successful
0dba15c648
render: fix hollow cursor style for "extra" cursors
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pr/woodpecker Pipeline was successful
fba9bb6853
 
When the cursor is "block style", i.e. either when the window is
focused and the cursor is block, or when unfocused and we're supposed
to draw a hollow cursor, then we need to draw it *before* compositing
the glyph.
In all other cases (i.e. beam and underline cursors), it should be
rendered after the glyph.
With the addition of "extra" cursors, the logic for determining which
type it is has become much more complex, and the single if-statement
we had was wrong (it didn't handle "extra" cursors correctly).
Fix by splitting up all the checks into multiple if-statements.
term_remove_all_multi_cursors(): early return if no extra cursors are active
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pr/woodpecker Pipeline was successful
1055d32e9f
First-time contributor
Copy link

I'm implementing this for Kakoune (not yet sure if I want it, maybe it's better to let the application draw something like U+258F LEFT ONE EIGHTH BLOCK on top of existing text)

Anyway, i noticed two issues using 1055d32e (term_remove_all_multi_cursors(): early return if no extra cursors are active, 2025年08月29日).

  1. Running

    clear; printf '033円[?25l'; printf '033円[>29;2:2:2 q'; sleep inf

shows the second cursor even though it's probably supposed to be hidden
by '033円[?25l';. Same if I add another '033円[?25l'; after adding the
second cursor.

It's easy to work around in the application, of course.

  1. Sometimes foot segfaults (when launching the above command, typing a few random characters and enter).
    I haven't figured out how to reproduce reliably,
    but it usually happens after a few attempts.

    │ 2336 /* Set shape, and dirty affected cells */ │
    │ 2337 for (int j = 0; j < rect_count; j++) { │
    │ 2338 const pixman_box32_t *box = &boxes[j]; │
    │ 2339 for (int r = box->y1; r < box->y2; r++) { │
    │ 2340 struct row *row = term->grid->rows[r]; │
    │ > 2341 row->dirty = true;

    (gdb) p rect_count
    14ドル = 1
    (gdb) p r
    10ドル = 1
    (gdb) p box.y1
    11ドル = 1
    (gdb) p box.y2
    12ドル = 2
    (gdb) p row
    13ドル = (struct row *) 0x0

I'm implementing this for Kakoune (not yet sure if I want it, maybe it's better to let the application draw something like U+258F LEFT ONE EIGHTH BLOCK on top of existing text) Anyway, i noticed two issues using 1055d32e (term_remove_all_multi_cursors(): early return if no extra cursors are active, 2025年08月29日). 1. Running clear; printf '033円[?25l'; printf '033円[>29;2:2:2 q'; sleep inf shows the second cursor even though it's probably supposed to be hidden by `'033円[?25l';`. Same if I add another `'033円[?25l';` after adding the second cursor. It's easy to work around in the application, of course. 2. Sometimes foot segfaults (when launching the above command, typing a few random characters and enter). I haven't figured out how to reproduce reliably, but it usually happens after a few attempts. │ 2336 /* Set shape, and dirty affected cells */ │ │ 2337 for (int j = 0; j < rect_count; j++) { │ │ 2338 const pixman_box32_t *box = &boxes[j]; │ │ 2339 for (int r = box->y1; r < box->y2; r++) { │ │ 2340 struct row *row = term->grid->rows[r]; │ │ > 2341 row->dirty = true; (gdb) p rect_count 14ドル = 1 (gdb) p r 10ドル = 1 (gdb) p box.y1 11ドル = 1 (gdb) p box.y2 12ドル = 2 (gdb) p row 13ドル = (struct row *) 0x0
Author
Owner
Copy link

For 1:

https://github.com/kovidgoyal/kitty/blob/master/docs/multiple-cursors-protocol.rst#interaction-with-other-terminal-controls-and-state

The extra cursors must all have the same color and opacity and blink state as the main cursor. The main cursor's visibility must not affect the visibility of the extra cursors. Their visibility and shape are controlled only by this protocol.

For 2; I haven't seen any issues, but please let me know if you find a deterministic reproduction case.

For 1: https://github.com/kovidgoyal/kitty/blob/master/docs/multiple-cursors-protocol.rst#interaction-with-other-terminal-controls-and-state > _The extra cursors must all have the same color and opacity and blink state as the main cursor. **The main cursor's visibility must not affect the visibility of the extra cursors**. Their visibility and shape are controlled only by this protocol._ For 2; I haven't seen any issues, but please let me know if you find a deterministic reproduction case.
multi-cursor: bug: use grid_row() when accessing rows
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pr/woodpecker Pipeline was successful
8bfbbc89fd
Author
Owner
Copy link

For 2; I haven't seen any issues, but please let me know if you find a deterministic reproduction case.

Hopefully fixed now.

> For 2; I haven't seen any issues, but please let me know if you find a deterministic reproduction case. Hopefully fixed now.
Author
Owner
Copy link

There are redraw issues when scrolling back and forth in the scrollback. Haven't investigated exactly what's happening yet.

There are redraw issues when scrolling back and forth in the scrollback. Haven't investigated exactly what's happening yet.
This way, we can safely index with any correctly wrapped row number
Otherwise we'll lookup the wrong multi-cursor shape.
render: multi-cursor: dirty all multi-cursors before rendering
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pr/woodpecker Pipeline was successful
4907c5d08a
 
In most cases, this isn't necessary, since the cursors don't move (and
when the application adds/deletes multi-cursors, we dirty the grid
_then_).
However, when we scroll the viewport, we're basically memmov:ing grid
contents without re-rendering, this can cause multi-cursors to be
"overwritten", and thus we need to explicitly re-render them.
Note: there's still an issue where scrolling content *with* a
multi-cursor doesn't correctly erase the scrolled multi-cursor.
render: dirty all multi-cursor cells *after* rendering a frame
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pr/woodpecker Pipeline was successful
4f89c461b1
 
This ensures they're re-rendered the next frame, and fixes an issue
were multi-cursors weren't erased when scrolled. That is, they
scrolled with the content, *and* was re-drawn at the correct (fixed)
location they were scrolled away from.
Since multi-cursors shouldn't scroll with the content, they need to be
erased when the content is scrolled.
Author
Owner
Copy link

Scrolling issues should be fixed now, although perhaps not in the most efficient way (all multi-cursors are re-rendered every frame).

Scrolling issues should be fixed now, although perhaps not in the most efficient way (all multi-cursors are re-rendered every frame).
multi-cursor: refactor: add function for dirtying/damaging all multi-cursors
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pr/woodpecker Pipeline was successful
265c93c4c4
csi: multi-cursor: refactor: loop boxes directly, without indexing
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pr/woodpecker Pipeline was successful
7c4ac3f348
Author
Owner
Copy link

maybe it's better to let the application draw something like U+258F LEFT ONE EIGHTH BLOCK on top of existing text)

The problem is you can't draw it on top of existing text, you can only have it replace an existing character.

> maybe it's better to let the application draw something like U+258F LEFT ONE EIGHTH BLOCK on top of existing text) The problem is you can't draw it on top of existing text, you can only have it _replace_ an existing character.
First-time contributor
Copy link

The problem is you can't draw it on top of existing text, you can only have it replace an existing character.

Ok thanks. I still wonder why cursors are different from other text decoration like underlines, which would seem less hacky for the Kakoune use case (which I still need to test for a week at least).
I left a brain dump at https://github.com/mawww/kakoune/issues/5377#issuecomment-3240003958

> The problem is you can't draw it on top of existing text, you can only have it replace an existing character. Ok thanks. I still wonder why cursors are different from other text decoration like underlines, which would seem less hacky for the Kakoune use case (which I still need to test for a week at least). I left a brain dump at https://github.com/mawww/kakoune/issues/5377#issuecomment-3240003958
First-time contributor
Copy link

https://github.com/neurocyte/flow is an editor that supports the protocol.

https://github.com/neurocyte/flow is an editor that supports the protocol.
All checks were successful
ci/woodpecker/push/woodpecker Pipeline was successful
builds.sr.ht/freebsd-x64 Job completed
ci/woodpecker/pr/woodpecker Pipeline was successful
This pull request is marked as a work in progress.
This branch is out-of-date with the base branch
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin multi-cursor:multi-cursor
git switch multi-cursor

Merge

Merge the changes and update on Forgejo.
git switch master
git merge --no-ff multi-cursor
git switch multi-cursor
git rebase master
git switch master
git merge --ff-only multi-cursor
git switch multi-cursor
git rebase master
git switch master
git merge --no-ff multi-cursor
git switch master
git merge --squash multi-cursor
git switch master
git merge --ff-only multi-cursor
git switch master
git merge multi-cursor
git push origin master
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
dnkl/foot!2169
Reference in a new issue
dnkl/foot
No description provided.
Delete branch "multi-cursor"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?