-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Add support for Kitty/VTE extended underlines (4:x) [SGR] #7228
Comments
I added this to xterm.js last week, currently the sequences get ignored by conpty, we need them to pass through. xtermjs/xterm.js#3921 |
I suspect even the premptive flushing of #13462 wouldn't fix this for conpty consumers either. We'd need to actually specifically note this category of sequence and then return false (or just, handle ourselves). Though, handling ourselves might be fairly engineering expensive (parsing, storing, re-rendering to conpty, rendering in dx/atlas), so maybe it does make sense to include the no-op passthrough for a smaller conpty package update. |
FYI, passthrough for sequences like this is not workable, even with flushing. Testing with a simple To understand why, have a look at the debug tab while scrolling through a file in vim. Notice how it appears to redraw the entire screen every time you scroll down the page? That's not vim doing the redrawing - that's conpty. Vim is just using something like a linefeed combined with scroll margins. So what happens when vim decides to output some wavy underlines? They may appear to work the first time the screen is rendered, but as soon as you scroll, it'll be conpty that does the redrawing. And since conpty knows nothing of those attributes, it's just going to obliterate them. Bottom line: Unless conpty stores and forwards the extended attributes, they're not going to work. |
Yep, these need to be stored in conpty's model, as opposed to OSC 133 and the like which just need pass-through and don't need to be emitted again on redraw |
Technically OSC 133 can break in a similar manner - it's just less likely in typical usage. The only way we can guarantee perfect fidelity with passthrough is if we're doing it for everything, with no conpty buffering at all (i.e. the passthrough mode of #1173). So our choices are either buffer everything and then rerender to conpty, or buffer nothing and pass everything through directly. Mixing the two is always going to be flaky. |
Any status on this one? |
Nope, but you can "Subscribe" for updates by clicking the "Subscribe" button on the right side of the screen! That will also help avoid sending e-mail to everyone who already has subscribed 😄 |
Adds support for colon `:` separated sub parameters in parser. Technically, after this PR, nothing should change except, now sub parameters are parsed, stored safely and we don't invalidate the whole sequence when a `:` is received within a parameter substring. In this PR: - If sub parameters are detected with a parameter, but the usage is unrecognised, we simply *skip* the parameter in `adaptDispatch`. - A separate store for sub parameters is used to avoid too many changes to the codebase. - We currently allow up to `6` sub parameters for each parameter, extra sub parameters are *ignored*. - Introduced `VTSubParameters` for easy access to underlying sub parameters. > **Info**: We don't use sub parameters for any feature yet, this is just the core implementation to support newer usecases. ## Validation Steps Performed - [x] Use of sub parameters must not have any effect on the output. - [x] Skip parameters with unexpected set of sub parameters. - [x] Skip sequences with unexpected set of sub parameters. References #4321 References #7228 References #15599 References xtermjs/xterm.js#2751 Closes #4321
Underline color sequence _SGR 58_ (unlike *SGR 38*, *SGR 48*) only works with sub parameters, eg. `\e[58:5:<n>m` or `\e[58:2::<r>:<g>:<b>m` will work, but something like `\e[58;5;<n>m` won't work. This is a requirement for the implementation to avoid problems with VT clients that don't support sub parameters. ## Detailed Description - Added `underlineColor` to `TextAttribute`, and `UnderlineStyle` into `CharacterAttributes`. - Added two new entries in `GraphicOptions` namely, `UnderlineColor` (58) and `UnderlineColorDefault` (59). - _SGR 58_ renders a sequence with sub parameters in the VT renderer. - _SGR 4:x_ renders a sequence with sub parameters in the VT renderer, except for single, double, and no-underline, which still use backward-compatible _SGR 4_, _SGR 21_ and _SGR 24_. - `XtermEngine` will send `\e[4m` without any styling information. This means all underline style (except NoUnderline) will be rendered as single underline. ## Reference issues - #7228 ### PR Checklist - [x] update DECRARA, DECCARA to respect underline color and style. - [x] update DECRQSS to send underline color and style in the query response. - [x] update DECRQPSR/DECRSPS/DECCIR - [x] Tests added
Add support for underline style and color in the renderer > [!IMPORTANT] > The PR adds underline style and color feature to AtlasEngine (WT) and GDIRenderer (Conhost) only. After the underline style and color feature addition to Conpty, this PR takes it further and add support for rendering them to the screen! Out of five underline styles, we already supported rendering for 3 of those types (Singly, Doubly, Dotted) in some form in our (Atlas) renderer. The PR adds the remaining types, namely, Dashed and Curly underlines support to the renderer. - All renderer engines now receive both gridline and underline color, and the latter is used for drawing the underlines. **When no underline color is set, we use the foreground color.** - Curly underline is rendered using `sin()` within the pixel shader. - To draw underlines for DECDWL and DECDHL, we send the line rendition scale within `QuadInstance`'s texcoord attribute. - In GDI renderer, dashed and dotted underline is drawn using `HPEN` with a desired style. Curly line is a cubic Bezier that draws one wave per cell. ## PR Checklist - ✅ Set the underline color to underlines only, without affecting the gridline color. - ❌ Port to DX renderer. (Not planned as DX renderer soon to be replaced by **AtlasEngine**) - ✅ Port underline coloring and style to GDI renderer (Conhost). - ✅ Wide/Tall `CurlyUnderline` variant for `DECDWL`/`DECDHL`. Closes #7228
Add support for underline style and color in the renderer > [!IMPORTANT] > The PR adds underline style and color feature to AtlasEngine (WT) and GDIRenderer (Conhost) only. After the underline style and color feature addition to Conpty, this PR takes it further and add support for rendering them to the screen! Out of five underline styles, we already supported rendering for 3 of those types (Singly, Doubly, Dotted) in some form in our (Atlas) renderer. The PR adds the remaining types, namely, Dashed and Curly underlines support to the renderer. - All renderer engines now receive both gridline and underline color, and the latter is used for drawing the underlines. **When no underline color is set, we use the foreground color.** - Curly underline is rendered using `sin()` within the pixel shader. - To draw underlines for DECDWL and DECDHL, we send the line rendition scale within `QuadInstance`'s texcoord attribute. - In GDI renderer, dashed and dotted underline is drawn using `HPEN` with a desired style. Curly line is a cubic Bezier that draws one wave per cell. ## PR Checklist - ✅ Set the underline color to underlines only, without affecting the gridline color. - ❌ Port to DX renderer. (Not planned as DX renderer soon to be replaced by **AtlasEngine**) - ✅ Port underline coloring and style to GDI renderer (Conhost). - ✅ Wide/Tall `CurlyUnderline` variant for `DECDWL`/`DECDHL`. Closes #7228 (cherry picked from commit e268c1c) Service-Card-Id: 91349180 Service-Version: 1.19
depends on #4321
The text was updated successfully, but these errors were encountered: