I did a bunch of testing of various ansi escape codes (including
italics) with all three char atlas implementations (none, static,
dynamic), with and without `drawBoldTextInBrightColors`, and with and
without `enableBold` to test all of the edge cases that I could.
Drawing dim colors sets the globalAlpha to 0.5, so we need to
save/restore the ctx state. Otherwise, every time we draw some dim
content, our display will get progressively dimmer.
I tested this by running `echo '\u001b[2mfoo'` a few times in zsh.
Fixes#1424
It turns out that `glyph.char.charCodeAt(0)` isn't equivalent to the
character's actual code because of utf-16 garbage, so this adds
`glyph.code` to the `glyph` object so that we can use the `code` value,
and don't need to regenerate it.
This limits the amount of damage a pathological program can cause in
thrashing the LRU cache.
I ran the script described here:
https://github.com/xtermjs/xterm.js/pull/1327#issuecomment-374003960
Without this patch, the entire output would get drawn in one frame that
took about 140 ms to draw. With this patch, it takes about 60 ms. It's
still nowhere as good as the static or none atlas implementations, but
it's not terrible, like it was.
This should have minimal impact on real-world applications: it'll just
take a little longer for the atlas to warm up.
The implementation is a little hacky, since it involves throwing some
code into TextRenderLayer that violates the separation of concerns, but
we can clean this up later.
The API defined by BaseCharAtlas will let us support multiple char atlas
implementations at once.
I tested this by running the demo with Chrome's profiler, which shows
that calls are going to StaticCharAtlas/drawImage instead of
_drawUncachedChar. Applications using 256 colors also still work fine,
via _drawUncachedChar.
This mostly worked before, but clearColor made the assumption that the
colors were always in #RRBBGG format. See the discussion here:
https://github.com/xtermjs/xterm.js/pull/1327#issuecomment-374812842
This changes the internal representation of a color so that we hold onto
into an object containing the original css string along with an RGBA
value encoded as a 32-bit uint. clearColor can then use that RGBA
representation.
Additionally, this deduplicates some code between ColorManager and
Terminal. Terminal was generating the 256 ansi colors the same way
ColorManager was, so this just makes Terminal use the same constant.
Two parts were commented out:
- _writeStopped: This is because _xoffSentToCatchUp references this variable
and the feature is sort of half implemented/disabled.
- _clearChar: This is because some comments still reference this, it's part
of the work needed to do better line redrawing.
- Ensures that everything is only defined in one place, and everything
imports from that place.
- Moves CHAR_ATLAS_CELL_SPACING into a subdirectory of shared/ so that
CharAtlasGenerator can pull from it.
This addresses the comments on
https://github.com/xtermjs/xterm.js/pull/1307/files/454fd4bfb1ab5c9ece
The only thing `didCharSizeChange` was used for was to figure out if we
need to refresh the character atlas or not. However, `acquireCharAtlas`
already inspects the atlas owned by this terminal, and if it matches, it
avoids generating a new atlas.
So if `didCharSizeChange` is true and it's not needed, it quickly bails
out as a no-op. But if `didCharSizeChange` is false, and it was needed
(possible given the complexity of testing all of these edge cases), it
could introduce a bug.
I think it makes sense to just get rid of `didCharSizeChange`, and just
always call `acquireCharAtlas`.
This provides an option for fonts to be more spaced out. This is particularly useful
for fonts that can seem crammed due to not allowing floating point numbers when
drawing, such as Monoid or Operator Mono.
Viraj Sinha
Benjamin Woodruff
Linus Unnebäck
Daniel Imms
Brandon Bayer
Daniel Imms
CHaBou
Bruno Ribeito
Thomas Zilz