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

Terminal does not render true color #484

Closed
Tyriar opened this issue Jan 16, 2017 · 20 comments
Closed

Terminal does not render true color #484

Tyriar opened this issue Jan 16, 2017 · 20 comments
Assignees
Labels
type/enhancement Features or improvements to existing features
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jan 16, 2017

See microsoft/vscode#18544

@cnsumner
Copy link

Can't wait for this. I've been hacking away at hterm trying to get truecolor support but I feel like this is a way better option.

@Tyriar
Copy link
Member Author

Tyriar commented Jan 31, 2017

Implementing this will need to be done here https://github.com/sourcelair/xterm.js/blob/365a9f949cc1acfb6c4649ee1d85da3bbc60f928/src/InputHandler.ts#L1276

The tricky part is determining how we store the colors against the character(s) and then how to render it (probably inline style attributes). Something like #450 would probably simplify the adding of the attributes.

@cnsumner
Copy link

cnsumner commented Feb 1, 2017

I may take a look at it when I get some time, but I can't say I'm too familiar with control sequences.

I took a look at http://invisible-island.net/xterm/ctlseqs/ctlseqs.html but couldn't seem to find anything about true color or 24-bit color. Am I missing something, or is there some other resource that covers this?

@jerch
Copy link
Member

jerch commented Feb 7, 2017

@cnsumner xterm currently cant display true color but has support for one of the proposed escape sequences. You can read more about this here: https://gist.github.com/XVilka/8346728
BTW this is partly specified in the ECMA-48 standard.

Tyriar added a commit to Tyriar/xterm.js that referenced this issue Jul 2, 2017
@Tyriar
Copy link
Member Author

Tyriar commented Jul 2, 2017

Put together a messy proof of concept to get a better idea of what we need to do here https://github.com/Tyriar/xterm.js/tree/484_truecolor

image

Things that need to be done:

  • The branch currently adds another number to each character representing it's 'truecolor' in the format 0x1RRGGBB (fg) or 0x0RRGGBB (bg), which fits in a 32 bit integer

  • It needs to support both fg and bg colors, and be able to mix truecolor and regular color

  • curTrueColor is super ugly

  • Support inverse

  • Color flags need to be cleared when cells are reused

  • It might be time to rethink how characters are encoded, the current format is:

    [attribute, character, width]
    
    • attribute: A 32-bit integer that holds ascii fg, ascii bg and flags (bold, underline, blink, inverse, invisible), not all bits are used
    • char: Either the empty string (for 0 width chars that pad wide chars) or a single character unicode string
    • width: The width of the char, currently can be 0, 1 or 2 but this could include more to support tabs properly Support rendering of tab characters in terminal #734
  • The eventual solution should be compact for memory and also quick to access, sticking with an array, binary flags and bit shifting for data is probably the best idea.

@jerch
Copy link
Member

jerch commented Jul 3, 2017

@Tyriar
Yeah memory usage will explode if it is not optimized for low consumption. Maybe it can be packed together with width, this field is unlikely to get any bigger than 16 consuming only 4 bytes (tab defaults to 4 or 8 normally). This leaves room for one RGB definition* (assuming width is a 32bit integer). Not sure how many bits are free in attribute, maybe there can go the second RGB definition*. The 256 color definitions could end up in one of those since they can't be defined simultaneously with RGB on one cell.
(*) A cell will need room for two RGB definitions for foreground and background.

@Tyriar
Copy link
Member Author

Tyriar commented Jul 3, 2017

It looks like attributes currently consumed 9 bytes for bg, 9 bytes for bg and 5 bytes for flags, leaving plenty to squeeze width in at the end. The RGB definitions need 24 bytes (8*3) each.

Here's the (slightly complicated) space saving approach I think you're suggesting:

[char, attr1, attr2]

Where attr1 is (29 bits):

  • width: 4 bits
  • is 256 color or truecolor flag: 1 bit
  • bg color: 24 bits

And attr2 is (30 bits):

  • flags: 5 bits
  • is 256 color or truecolor flag: 1 bit
  • fg color: 24 bits

A simpler approach would use a number specifically for attributes:

[char, attr, truecolorBg, truecolorFg]

Where attr is:

  • width: 4 bytes
  • flags: 5 bytes
  • 256 bg color: 9 bytes
  • 256 fg color: 9 bytes

I want so badly to encapsulate these details as they really complicate things, I wonder if typescript will let us put getters on an array somehow. Using an object was 20x slower than an array based on a micro benchmark I did on jsperf, I know they're not always reliable but this is kind of what I expected as overhead to create an object based on a prototype.

Whatever we do, the format needs to be documented very well. Right now you have to figure it out yourself by looking at InputHandler.charAttributes.

@Tyriar
Copy link
Member Author

Tyriar commented Jul 4, 2017

Some more thoughts on memory saving:

  • If a character has a width of > 1, we don't really need to store any data at all for the following characters. The "empty characters" are there as a workaround and would probably work fine being null.
  • Normally a set of characters is styled, meaning attributes could be shared between a sequence of characters.
  • Attributes could be cached and each char uses a pointer to one of the cached values,

@jerch
Copy link
Member

jerch commented Jul 4, 2017

Yes I think on most JS engines arrays with index access are still faster than objects and their property resolving. V8 got some extra optimizations for objects though. Construction and GC wise an array is faster due to the much simpler memory layout (this disadvantage can be omitted by reutilizing existing objects).

I have no clue about the right layout here, problem is that any higly packed stuff will raise runtime penalty due to the needed transformations. (This is one reason not to use bitfields in C if memory is no concern - the needed shift operations will pollute the instruction cache leading to cache misses - the code runs much slower).
Imho only tests will show what works best here.

@Tyriar
Copy link
Member Author

Tyriar commented Jul 4, 2017

@jerch interesting, I have 4 number version for simplicity right now in #756.

So memory here is expected to be around: 5 * (1000 scrollback) * (8 bytes on 64-bit machine) * 80 = 3.2mb (before overhead of js array). The actual width of a typical VS Code terminal would see that double if we keep the current format. Seems pretty steep now that I look at it, especially since users will often have higher than 1000 scrollback and multiple active terminals. The trailing whitespace which really doesn't add much at all and could just as easily be null which would probably be a pretty huge win.

The attribute/color set cache idea also seems promising, as long as there's a good way for fast access. A lightweight splay tree-like structure would be ideal (fast access for attributes uses most frequently).

It might also be worth looking into reusing line data arrays when they're trimmed instead of just discarding them (eg. here).

@jerch
Copy link
Member

jerch commented Jul 4, 2017

Yeah and with the engine overhead in mind it grows even bigger - everything in an array or object is stored as a reference, basically a pointer to the content. Dense js-arrays have a slight advantage here, they are formed as a C like array somewhere down the path (well actually ArrayLists). Sparse arrays and others objects are build with some tree magic to solve the property resolution.
So for 64bit the reference thing adds 5 * 8 more bytes per cell (it basically doubles your calculation above - 6.4mb). The content can add even more bytes beside the "real content", but this depends on the actual implementation of the Integer and String type.

Maybe a TypedArray can solve this. There the integer stuff can reside directly at the index position in memory without an additional indirection. asm.js basically operates only on those.

@jerch
Copy link
Member

jerch commented Jul 8, 2017

Here is a quick micro benchmark to compare TypedArray and js-Array for creation and data insertion: http://jsbench.github.io/#af3ea1056a70df2aa6775699d174df0d

The TypedArray runs 10 times faster with FF and Chrome on my machine. This might be due to the preallocation, have not tested this with js-Array since it is discouraged. Not sure if the JITs spot the fact, that the value just increases by 1 every 8 bytes and even optimize this out. Then the benchmark is useless at all.

The access is slightly worse for a TypedArray (maybe due to Number conversion), memory consumption is much better with a TypedArray.

Downside of a TypedArray is the static block of memory when it comes to spare data (tricks to avoid data at all wont work here, the memory is "lost" anyways) or resizing with content wrapping. It is hard to get it right and likely to involve copying big chunks of data over and over.

@szmarczak
Copy link

Hello. Any updates on this? Can't wait to get this working 😃

@vastbinderj
Copy link

vastbinderj commented May 27, 2018

Is anyone actively working this issue? Is it being discussed anywhere else? @chabou @Tyriar? Is Microsoft helping given the usage in VS Code?

This is the last lagging issue for using hyper in WSL on Windows 10 to provide a decent dev CLI workflow supporting tmux and vim/neovim. Other terminal emulators available to use with WSL just aren't pleasant to look at, wsltty is close, but tab support and plugin support is missing...

@rfgamaral
Copy link

@vastbinderj Why the missing true color support doesn't give you a decent dev CLI workflow? Honest question.

@jerch
Copy link
Member

jerch commented May 28, 2018

@szmarczak and @vastbinderj
This issue is still on the agenda, but kinda blocked due to the high memory usage it will introduce with the current terminal buffer design. It will get more attention when #791 has brought us a more efficient buffer layout.

@vastbinderj
Copy link

@rfgamaral simple - I work across many many different unix/linux servers a day on very large enterprise systems with many microservices and not having decent color support in the main host is disruptive. To deal with the colors not being the same whether I'm on rhel or centos or debian, is painful. Also, sitting down to help a jr or peer developer on windows means I have to context switch if things don't work uniformly. However, I am super impressed how far wsl has come so quickly with improvements released on an almost monthly basis and the same with VS Code, it is outstanding.

@jerch Thanks for the update, will wait patiently for #791 to be worked out...

@jcjolley
Copy link

Greatly looking forward to this as well.

@PerBothner
Copy link
Contributor

It is worth remembering that JavaScript numbers can represent exact integers up to 53 bits. So you have 21 bits in addition to the 32 "convenient" bits. You can "or" in a bit by simple addition, if it is not set already. To read the high-order bits, just divide by a suitable power-of-two integer, and then use the regular bit operations.

Of course there is a little more overhead working with the higher-order bits, but on modern CPUs division by power-or-two is relatively cheap - cheaper than memory accesses.

I suggest using a single number with 50 bits: 25 bits each for foreground and background. One bit if set indicates we're using a 24-bit "true" color; if not set we're using "logical" (themeable) or 8-bit colors. The indicator bits should be the 2 lower-order bits; if unset the logical/8-bit colors could be in the next 30 bits, so we only use bits 32-50 if we're actually using true-color.

@jerch
Copy link
Member

jerch commented Nov 20, 2018

@PerBothner We decided to go with a Uint32Array for the new buffer, and for convenience to put the colors into that array as well. This is not done yet as we want to sanitize the new impl first before supporting truecolor.

Also I did some tests/calculations about possible layouts (see https://gist.github.com/jerch/27c2cf91ad313a25415873e4047b2900). Long story short - the ideas are about the tradeoff mem saving vs. runtime penalty. The transition to typed array already gave huge mem saving (currently implements the second idea from above, though the true color values are not applied yet).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Features or improvements to existing features
Projects
None yet
Development

No branches or pull requests

9 participants