Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BufferLine rewrite [WIP] #4928

Open
wants to merge 62 commits into
base: master
Choose a base branch
from

Conversation

PerBothner
Copy link
Contributor

@PerBothner PerBothner commented Dec 29, 2023

This PR implements #4800. It also sort-of implements #4232 - it does reflow lazily, though it doesn't specifically use the "idle callbacks". Motivation is described in #4800; however, the implementation is different (though the same basic idea).

The NewBufferLine implementation is enabled if the newBufferLine option is true; otherwise the old implementation (renamed to OldBufferLine) is used. The goal is to merge the PR with newBufferLine defaulting to false to avoid breaking things, but allowing adventurous users and developers to try newBufferLine set to true. (Some more testing will be needed even for the newBufferLine false case; I believe there are regressions.)

Note that NewBufferLine is abstract and has two implementation classes:

  • LogicalBufferLine represents an unwrapped line, but it also contains the data for following wrapped lines.
  • WrappedBufferLine represents a wrapped line (duh) and basically just indirects to the previous LogicalBufferLine.

Operations that are O(1) in OldBufferLine may require linear scanning in NewBufferLine. However, LogicalBufferLine has a cache (mapping column index to _data array index) to avoid linear scanning for common operations. There is some needless overhead in the interest of minimizing API changes.

Later, the newBufferLine default will change to true. Later still, we will remove the support for OldBufferLine and NewBufferLine will be renamed to plain BufferLine.

The AbstractBufferLine class is intended as a parent for both [New]BufferLine and future classes for "other" content. Specifically, general HTML "paragraphs" (<div> elements) are planned.

Tasks that must be done before merger:

  • Fix any oldBufferLine regressions. (Minor barely-measurable performance regressions may be OK.)
  • Update or replace any commented-out testcases.
  • Fix remaining lint regressions. Note this issue.
  • ...

Tasks that must be done before defaulting to newBufferLine true:

  • Fix all testsuite regressions (at least all that may be in real usage).
  • IExtendedAttrs handling is incomplete (though basic data structure support is there).
  • The image addon doesn't work. It uses custom classes that textually copy the old classes.
  • Fix at least noticeable performance regressions, if any.
  • Evaluate any FIXME comments in the code.
  • ...

The new data structure is more flexible and more compact.
However, mapping column number to cell information is no longer O(1).
So we use a cursor as much as possible to reduce the need for that.

This is an early check-in to have a backup and baseline.
A few basic things work but it is NOT READY FOR USE.
Fix some bugs. Add simple grapheme support.
- Store character values in _data array. Get rid of _text string. (In progress.)
  This is likely to be faster.
- Add a cache in BufferLine for the current position. (In progress.)
- Re-implement loadCell to use above cache.
- Revert DOM renderer to use laodCell. (webgl and canvas renderer s still to do.)
Replace fg, bg, and eAttrs parameters by single IAttributeData parameter.
In addition to making for a simpler/cleaner API, it also increases flexibility
for future CellData or BufferLine changes.
(Rename function to reduce risk of using accidentally using old function.)

Also removed redundant eraseAttr parameter in deleteCells, insertCells, and replaceCells.
Using the per-BufferLine cache in loadCell should make this reasonably performant.
InputHandler.print just calls insertText, without a loop. (WIP)
Use it in insertText and deleteCells.
Get rid of some old cursor stuff in favor of buffer-local cache.
Controlled by USE_NewBufferLine variable
(Still a bug with extra lines sometimes being added.)
New classes LogicalBufferLine and WrappedBufferLine.
This reflows the lines and calculates WrapperBufferLine offsets.
It does *not* yet handle updating y, ybase, ydist,
calling onDeleteEmitter handler, and similar tasks.
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.

None yet

1 participant