starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-09-24 16:05:21 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

ConPTY: Optimize SGR sequences by combining them into a single sequence #4208

New Issue
Closed
opened 2026-01-30 23:41:04 +00:00 by claunia · 6 comments
claunia commented 2026-01-30 23:41:04 +00:00
Owner
Copy Link

Originally created by @zadjii-msft on GitHub (Oct 1, 2019).

I wonder if there's an optimization we can take later that'll let us compress SGRs together when we know we're writing a bunch. \x1b[40m+\x1b[31m+\x1b[4m+\x1b[3m => \x1b[40;31;4;3m?

Originally posted by @DHowett-MSFT in https://github.com/microsoft/terminal/pull/2917

Originally created by @zadjii-msft on GitHub (Oct 1, 2019). I wonder if there's an optimization we can take later that'll let us compress SGRs together when we know we're writing a bunch. `\x1b[40m`+`\x1b[31m`+`\x1b[4m`+`\x1b[3m` => `\x1b[40;31;4;3m`? _Originally posted by @DHowett-MSFT in https://github.com/microsoft/terminal/pull/2917_
claunia commented 2026-01-30 23:41:05 +00:00
Author
Owner
Copy Link

@DHowett-MSFT commented on GitHub (Oct 1, 2019):

One complication: we need to know the cancel-out combos (4, 24) and not emit crazy things that turn the state off and on in the same sequence.

@DHowett-MSFT commented on GitHub (Oct 1, 2019): One complication: we need to know the cancel-out combos (4, 24) and not emit crazy things that turn the state off and on in the same sequence.
claunia commented 2026-01-30 23:41:05 +00:00
Author
Owner
Copy Link

@egmontkob commented on GitHub (Oct 4, 2019):

Apologies again that I'm commenting without fully understanding the context. What's the source of the data that you want to optimize this way?

we need to know the cancel-out combos (4, 24) and not emit crazy things that turn the state off and on in the same sequence

If the source is some in-memory array of TextAttributes or so then this cannot happen, can it? If a cell isn't underlined but the next one is, you emit SGR 4, if it's the other way around, you emit SGR 24, but you never want to emit both of them next to each other.

If the source is a sequence of SGRs, then {

Would this particular case with cancel-out combos be a problem? I don't think so. \e[4;24m must also be interpreted from left to right, subsequent ones overriding the preceding ones, i.e. equivalent to \e[4m\e[24m or simply \e[24m.

However, it's easy to shoot yourself in the foot when you enter the territory of often misinterpreted and/or not (yet) supported escape sequences. E.g. \e[38m\e[5;3m is an invalid sequence (to be ignored) followed by enabling blinking and italics. But \e[38;5;3m is a commonly understood sequence for switching to the 3rd color of the 256-color palette (although according to the standard the subparameters should be separated by : rather than ;, and it's unclear whether that should be the case between 38 and 5 too, but it's another topic – in practice usually ; is used).

when we know we're writing a bunch.

I'm afraid this sounds like the behavior would start depending on the timing of incoming data, and/or some heuristics, making it much harder to reproduce and debug potential issues.

Also comes with additional code complexity, and along with it possibly additional runtime cost (detecting such situations), just to shave off 2 bytes on a local communication channel (if I understand the architecture correctly), and maybe (but not necessarily) make subsequent parsing of this data a tiny bit faster. Depending on many circumstances unbeknownst to me, it might result in an overall performance penalty, but even if that's not the case, it might easily come with pointless code complexity and engineering efforts not well spent – let alone the aforementioned problem of combining invalid (or just simply unknown, unrecognized) sequences into a valid one.

}

I hope my comment contained at least some usable parts :)

@egmontkob commented on GitHub (Oct 4, 2019): Apologies again that I'm commenting without fully understanding the context. What's the _source_ of the data that you want to optimize this way? > we need to know the cancel-out combos (4, 24) and not emit crazy things that turn the state off and on in the same sequence If the _source_ is some in-memory array of `TextAttribute`s or so then this cannot happen, can it? If a cell isn't underlined but the next one is, you emit SGR 4, if it's the other way around, you emit SGR 24, but you never want to emit both of them next to each other. If the _source_ is a sequence of SGRs, then { Would this particular case with cancel-out combos be a problem? I don't think so. `\e[4;24m` must also be interpreted from left to right, subsequent ones overriding the preceding ones, i.e. equivalent to `\e[4m\e[24m` or simply `\e[24m`. However, it's easy to shoot yourself in the foot when you enter the territory of often misinterpreted and/or not (yet) supported escape sequences. E.g. `\e[38m\e[5;3m` is an invalid sequence (to be ignored) followed by enabling blinking and italics. But `\e[38;5;3m` is a commonly understood sequence for switching to the 3rd color of the 256-color palette (although according to the standard the subparameters should be separated by `:` rather than `;`, and it's unclear whether that should be the case between `38` and `5` too, but [it's another topic](https://gist.github.com/XVilka/8346728#gistcomment-2774523) – in practice usually `;` is used). > when we know we're writing a bunch. I'm afraid this sounds like the behavior would start depending on the timing of incoming data, and/or some heuristics, making it much harder to reproduce and debug potential issues. Also comes with additional code complexity, and along with it possibly additional runtime cost (detecting such situations), just to shave off 2 bytes on a local communication channel (if I understand the architecture correctly), and maybe (but not necessarily) make subsequent parsing of this data a tiny bit faster. Depending on many circumstances unbeknownst to me, it might result in an overall performance penalty, but even if that's not the case, it might easily come with pointless code complexity and engineering efforts not well spent – let alone the aforementioned problem of combining invalid (or just simply unknown, unrecognized) sequences into a valid one. } I hope my comment contained at least some usable parts :)
claunia commented 2026-01-30 23:41:06 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Oct 4, 2019):

@egmontkob This kinda gets at the core difference between ConPTY and a true pty. With a true pty it's more like a dumb pipe, the pty isn't generating any sequences of its own, it's just forwarding them along from client to terminal.

With conpty however, that's not really the case. While yes, some applications (like WSL) are using VT sequences to interact with the "terminal", the majority of client apps are using the win32 Console API to interact with the "terminal". Conpty is then doing its own work to make sure that the state changed by the API is also kept in sync with the terminal, who's only reading a stream of VT sequences generated by conpty itself.

In this particular case, as we're "rendering" a string of text (and colors) to the terminal (from conpty), we might decide that we need to both change the foreground to red and enable the underline. Currently, we'll add a \x1[31m and a \x1b[4m to the "frame" to change this state. What we're suggesting doing here is that when we add multiple SGR sequences in a row like this, that we combine all the sequences into a single sequence like \x1b[32;4m.

Since conpty is fully in charge of the sequences generated here, I think we can pretty safely assume that we won't be accidentally combining a \x1b[38m, \x1b[5m and \x1b[3m into a \x1b[38;5;3m, simply because we know we;ll never be sending a \x1b[38m all by itself 😄

This would also be something that wouldn't happen in passthrough mode (#1173) at all, because in that scenario we would be acting as a dumb pipe.

@zadjii-msft commented on GitHub (Oct 4, 2019): @egmontkob This kinda gets at the core difference between ConPTY and a true pty. With a true pty it's more like a dumb pipe, the pty isn't generating any sequences of its own, it's just forwarding them along from client to terminal. With conpty however, that's not really the case. While yes, some applications (like WSL) are using VT sequences to interact with the "terminal", the majority of client apps are using the win32 Console API to interact with the "terminal". Conpty is then doing its own work to make sure that the state changed by the API is also kept in sync with the terminal, who's only reading a stream of VT sequences generated by conpty itself. In this particular case, as we're "rendering" a string of text (and colors) to the terminal (from conpty), we might decide that we need to both change the foreground to red and enable the underline. Currently, we'll add a `\x1[31m` and a `\x1b[4m` to the "frame" to change this state. What we're suggesting doing here is that when we add multiple SGR sequences in a row like this, that we combine all the sequences into a single sequence like `\x1b[32;4m`. Since conpty is fully in charge of the sequences generated here, I think we can pretty safely assume that we won't be accidentally combining a `\x1b[38m`, `\x1b[5m` and `\x1b[3m` into a `\x1b[38;5;3m`, simply because we know we;ll never be sending a `\x1b[38m` all by itself 😄 This would also be something that _wouldn't_ happen in passthrough mode (#1173) at all, because in that scenario we would be acting as a dumb pipe.
claunia commented 2026-01-30 23:41:06 +00:00
Author
Owner
Copy Link

@egmontkob commented on GitHub (Oct 4, 2019):

Thanks a lot for the clarification! I've just rewatched the relevant part of the WT announcement video to make sure I have the right architecture in mind.

So the first "if" branch of my previous comment was the relevant one, i.e. I'm not sure how cancel-out combos like (4, 24) were feared to appear at all in the first place. Anyway, it's definitely something to remember during developing this feature, just in case.

If optimizing matters, you might also do some guesswork whether sending the actual diff (turning off certain modes only) or building up from scratch after a reset is shorter; e.g. from bold+underlined+italic to italic should you do 22;24 (turn off bold and underlined) or 0;3 (turn off everything and then re-enable italic)? The latter approach could become pretty costly with 256-color or truecolor entries when it's not the color that changes. However, when reverting to the default attributes, it's probably better to just reset them all as \e[m rather than turning them off one by one.

@egmontkob commented on GitHub (Oct 4, 2019): Thanks a lot for the clarification! I've just rewatched the relevant part of the [WT announcement video](https://youtu.be/KMudkRcwjCw?t=1275) to make sure I have the right architecture in mind. So the first "if" branch of my previous comment was the relevant one, i.e. I'm not sure how cancel-out combos like (4, 24) were feared to appear at all in the first place. Anyway, it's definitely something to remember during developing this feature, just in case. If optimizing matters, you might also do some guesswork whether sending the actual diff (turning off certain modes only) or building up from scratch after a reset is shorter; e.g. from bold+underlined+italic to italic should you do `22;24` (turn off bold and underlined) or `0;3` (turn off everything and then re-enable italic)? The latter approach could become pretty costly with 256-color or truecolor entries when it's not the color that changes. However, when reverting to the default attributes, it's probably better to just reset them all as `\e[m` rather than turning them off one by one.
claunia commented 2026-01-30 23:41:06 +00:00
Author
Owner
Copy Link

@DHowett-MSFT commented on GitHub (Oct 4, 2019):

@egmontkob oh, you're totally right. I forgot to consider that if we're rendering from the buffer, we'll never have conflicting states stored. 😄

@DHowett-MSFT commented on GitHub (Oct 4, 2019): @egmontkob oh, you're totally right. I forgot to consider that if we're rendering from the buffer, we'll never have conflicting states stored. :smile:
claunia commented 2026-01-30 23:41:06 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Oct 4, 2019):

Other notes from the PR:

@miniksa:

Nnnn... what happens in the scenario where:

* We're attempting to set Italics, Blink, and Invisible.

* Italics succeeds

* Blink fails

* Invisible would have succeeded, but we early returned...

Are we OK with that happening or should we have a provision to try the others and aggregate the result code later?

I don't think there's really a way for us to make this atomic where they all work or none of them work... or is there?

EDIT: I'm worried that the potentially torn state will result in a weird debugging situation in the future...

@zadjii-msft

You're right that's it's possible that we'd end in a bad state if one of these fails. Looking at the implementation of these, this is the only spot that can actually fail:

    try
    {
        _buffer.append(str);
        return S_OK;
    }
    CATCH_RETURN();

(where str is the VT sequence we're writing). IMO, if that fails, then we're probably going to end up in a much worse torn state than just this one sequence being lost.

The idea to do them all atomically would probably match well with #3016.

@zadjii-msft commented on GitHub (Oct 4, 2019): Other notes from the PR: @miniksa: > > Nnnn... what happens in the scenario where: > > * We're attempting to set Italics, Blink, and Invisible. > > * Italics succeeds > > * Blink fails > > * Invisible would have succeeded, but we early returned... > > > Are we OK with that happening or should we have a provision to try the others and aggregate the result code later? > > I don't think there's really a way for us to make this atomic where they all work or none of them work... or is there? > > EDIT: I'm worried that the potentially torn state will result in a weird debugging situation in the future... @zadjii-msft > > > You're right that's it's possible that we'd end in a bad state if one of these fails. Looking at the implementation of these, this is the only spot that can actually fail: > > ```c++ > try > { > _buffer.append(str); > return S_OK; > } > CATCH_RETURN(); > ``` > > (where `str` is the VT sequence we're writing). IMO, if that fails, then we're probably going to end up in a much worse torn state than just this one sequence being lost. > > The idea to do them all atomically would probably match well with #3016.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Area-Rendering Help Wanted In-PR Issue-Task Needs-Tag-Fix Product-Conpty
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#4208
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?