Skip to content

attach markers and decorations to LogicalLine#5853

Open
PerBothner wants to merge 19 commits into
xtermjs:masterfrom
PerBothner:markdecor
Open

attach markers and decorations to LogicalLine#5853
PerBothner wants to merge 19 commits into
xtermjs:masterfrom
PerBothner:markdecor

Conversation

@PerBothner
Copy link
Copy Markdown
Contributor

@PerBothner PerBothner commented Apr 28, 2026

This is a moderately large change as it combines a number of separate changes that depend on each other. I'm sorry about that; the net code increase is modest as lots of code has also been removed and simplified (and some comments have been added). Parts have been proposed as smaller changes; OTOH it might be more useful to see how it all comes together..

High-level overview:

  • The text and attributes from BufferLine are moved to a new class LogicalLine. The latter represents a group of wrapped lines: The data in the LogicalLine is invariant when buffer width is changed; each BufferLine is basically a sub-range of the parent LogicalLine. (See PR move _data from BufferLine to new LogicalLine class #5797.)
  • Buffer reflow is much faster and simpler. There is no moving data around; instead we just recalculate how to split a LogicalLine into multiple BufferLine parts. There are hooks for lazy reflow if that appears worthwhile.
  • Each Marker now belongs to a LogicalLine for the corresponding line. The marker does not have an explicit line number that needs to be updated when the buffer changes. (You can calculate the line number of a marker, but that requires a linear scan through the buffer, so should only be used for tests and debugging.)
  • Each Marker has a new payload field. This has multiple uses; for now it is mainly used to map from LogicalLine to Marker to Decoration. This allows us to replace the slow forEachDecorationAtCell by the much faster forEachDecorationAtCellLine method. Specifically, this speeds up search tremendously. (See issue Search is too slow #5176.)
  • ExtendedAttrs is likewise extended by a payload field. This allows a cleanup of the ImageStorage logic, including getting rid of the ExtendedAttrsImage class. (See PR Change image storage to use new ExtendedAttrs payload field #5879.)

Planned follow-up work:

Old text

This is a work-in-progress / proof-of-concept of the idea of attaching Markers to the corresponding LogicalLine, and attaching Decorations to Markers. It is a follow-up to this idea, but this update does not use sub-classing of Markers through sub-classin.

This builds on PR #5797 - i.e. a LogicalLine contains the cell contents of a line assuming infinite screen width; a visible BufferLine becomes a sub-range of a LogicalLine.

Each LogicalLine points to a list (array or linked list) of Markers associated with the LogicalLine.

The Marker does not have a line number. Therefore, it does not need to be updated on a window resize. Getting the line number of a Marker becomes possible but expensive.

A Marker has an assocated column number. This is relative to the start of the LogicalLine (i.e. assuming an infinitely-wide terminal). Therefore the column position does not need to be updated on terminal resize.

There is no global list of Markers associated with a buffer. If you really need to iterate over all Markers, you iterate over all lines, then each Marker associated with the line.

The Marker object has an extra payload field. This can be used to quickly access application-specific data, such as the associated Decoration, image data, OSC url, shell integration prompt data, etc. I previously assumed one would extend the Marker class with sub-classes (and that is still possible), but that would be too big an API change.

We might consider getting rid of ExtendedAttrs and just using markers.

A Decoration is attached to a Marker, and can be accessed by the latter's payload field.

Instead of the expensive forEachDecorationAtCell metod we use the much cheaper new forEachDecorationAtCellLine (which should be renamed). I only updated the dom renderer, but the webgl renderer should be handled similarly.

This provides a solution to issue #5176.

To do:

  • API cleanups.
  • There are almost certainly some problems in lifetime (dispose) handling.
  • Fix some test regressions.
  • Maybe get rid of forEachDecorationAtCell.
  • Speed up getDecorationsAtCell similarly.
  • Update webgl renderer.
  • Maybe get rid of the global list of decorations: Neither rendering or serialization need it - they just need to find the markers associated with the LogicalLines they are working on. When a LogicalLine is disposed (perhaps because it is scrolled away), we dispose of the associated markers.
  • Perhaps update link handler and image addon to use Marker payload field.
  • Decide on how to handle decorations and other payloads that span multiple logical lines.
  • Some fixes in how we handle wrapped lines.

Instead to change isWrapped state use a Buffer.setWrapped method.
This allows for potential flexibility in how BufferLine
and line-wrapping are implemented.
A BufferLine is now the sub-range of a LogicalLine for a specific visible line,
while LogicalLine is independent of window width.
@PerBothner
Copy link
Copy Markdown
Contributor Author

It is probably possible to make a variation of this idea, but attach the Marker list to BufferLine, without the PR #5797 LogicalLine changes. However, that would complicate some other things, most notably buffer resize. You would need to maintain the invariant that for a marker m that m belongs to buffer.lines.get(m.line). There are also complications if a decoration (or something like a URL link) spans across a line wrap: you would need to split the decoration - and then merge them if that changes after resize. This can be a bit tricky, and complicates something (resize) that is already quite complicated. (As opposed to the LogicalLine changes, which simplify and speed up resize.) It might be possible to handle the needed updates inside the existing event handlers, which would help a bit.

You probably do want a Marker to have a column number, either way.

PerBothner added 3 commits May 1, 2026 21:24
Main thing is changing soe the CircularList's onTrim emitter
gets called while lines are in the list, which means clearMarkers
can be used. This complicates the slice function a bit.
@PerBothner
Copy link
Copy Markdown
Contributor Author

Merged from upstream/master.
Only 3 testsuite failures, related to reflow, which I'll tackle next.
There are also problems with the image addon.

@PerBothner
Copy link
Copy Markdown
Contributor Author

Merged in commit from PR #5879 - it is needed to avoid acsessing no-longer available BufferLine internals.

All tests now pass.

@PerBothner PerBothner changed the title [WIP] attach markers and decorations to LogicalLine attach markers and decorations to LogicalLine May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant