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

GetConsoleScreenBufferInfoEx doesn't return the actual palette #10639

Open
alabuzhev opened this issue Jul 12, 2021 · 17 comments
Open

GetConsoleScreenBufferInfoEx doesn't return the actual palette #10639

alabuzhev opened this issue Jul 12, 2021 · 17 comments
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Conpty For console issues specifically related to conpty
Milestone

Comments

@alabuzhev
Copy link
Contributor

Windows Terminal version (or Windows build number)

1.9.1523.0

Other Software

No response

Steps to reproduce

  1. Compile the following code:
#include <iomanip>
#include <iostream>

#include <windows.h>


int main()
{
	CONSOLE_SCREEN_BUFFER_INFOEX Info{ sizeof(Info) };
	if (!GetConsoleScreenBufferInfoEx(GetStdHandle(STD_OUTPUT_HANDLE), &Info))
		return 1;

	for (const auto& i: Info.ColorTable)
	{
		std::cout << std::hex << std::setw(8) << std::setfill('0') << i << '\n';
	}

	std::cout.flush();
}
  1. Set WT palette to "Vintage" in settings
  2. Run the compiled code

Expected Behavior

The same colour codes as in the corresponding section of C:\Program Files\WindowsApps\Microsoft.WindowsTerminalPreview_1.9.1523.0_x64__8wekyb3d8bbwe\defaults.json:

000000
800000
008000
808000
000080
800080
008080
C0C0C0
808080
FF0000
00FF00
FFFF00
0000FF
FF00FF
00FFFF
FFFFFF

Actual Behavior

It looks like the default Windows 10 palette is returned instead:

000c0c0c
00da3700
000ea113
00dd963a
001f0fc5
00981788
00009cc1
00cccccc
00767676
00ff783b
000cc616
00d6d661
005648e7
009e00b4
00a5f1f9
00f2f2f2
@zadjii-msft zadjii-msft added Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Conpty For console issues specifically related to conpty labels Oct 5, 2021
@zadjii-msft zadjii-msft added this to the Console Backlog milestone Oct 5, 2021
@zadjii-msft zadjii-msft modified the milestones: Console Backlog, Backlog Jan 4, 2022
@chrisant996
Copy link

@zadjii-msft This is problematic because it makes it impossible for a console mode application to detect whether the current terminal theme is Light or Dark. For example, clink cannot automatically choose colors with correct contrast ratio to be visible. Instead I have to force users to manually configure their colors. Users are unhappy about this.

It would be much appreciated if this API could get fixed to return the actual current terminal theme colors.

(I understand that GCSBIE() doesn't have any way to return the default foreground or default background colors. At least for my usage cases, that's easily worked around by printing ESC [ m and then calling GCSBIE() and checking the .wAttributes value.)

@j4james
Copy link
Collaborator

j4james commented Jul 13, 2023

The reason we can't easily do this is because conhost (which handles GCSBIE) doesn't know what the terminal's colors are. To find that out, it would need to send an escape sequence to the connected conpty client, and then block while it waits for a response, which isn't great for performance.

This is problematic, because some applications call GCSBIE a lot. Imagine a number that you think is a ridiculous amount, and it's likely more than that. And to further complicate the matter, there's no guarantee that the connected conpty client even supports palette queries.

I did play around with querying the palette once at startup, but that still introduces the complexity of having to wait for a response which may never arrive. An easier version of that would be to have the client pass in the palette via the initial command line. But both of those solutions fall apart if a user updates their palette settings at run-time.

So while I would love to get this working somehow, I just don't see how. But if anyone has any bright ideas, feel free to share them here.

@alabuzhev
Copy link
Contributor Author

alabuzhev commented Jul 13, 2023

To find that out, it would need to send an escape sequence to the connected conpty client, and then block while it waits for a response

Why? I have a very basic idea of how all this is organised, but this sounds unnecessarily inverted.
Why can't conhost return its internal local array, exactly as it does currently, but update that array whenever the connected conpty client updates its palette?
There should be some notification mechanism.
After all, you somehow do know client's window and buffer dimensions, and I don't think they are queried every time in a blocking way.


Our usage of this API is mostly related to colours mixing, e.g. transparency and shadows. To properly mix index colours we have to know to which exactly RGB values they are mapped.

@chrisant996
Copy link

@j4james @zadjii-msft Here are some brainstorming thoughts. Maybe some of them will be viable, or maybe some of them with spark further ideas.

conhost (which handles GCSBIE) doesn't know what the terminal's colors are. To find that out, it would need to send an escape sequence to the connected conpty client, and then block while it waits for a response, which isn't great for performance.

What if conhost didn't need to wait?

  • What if the conpty client could notify conhost when the colors are set? (Not a requirement for conpty clients, but an option for them.)
    • Maybe a notification could provide the specific colors, so GCSBIE doesn't need to query the colors.
    • Maybe a notification could simply indicate that the colors changed, so the next GCSBIE knows to query the colors.
    • What if a named event were the notification mechanism? Then it's optional and simple.
  • What if conhost could make async queries to update its knowledge?
    • A given GCSBIE call wouldn't necessarily return perfectly up-to-date color state, but that can be considered as a race condition -- it's always possible a user or app could change the colors right after a GCSBIE call anyway.

This is problematic, because some applications call GCSBIE a lot. Imagine a number that you think is a ridiculous amount, and it's likely more than that.

Yes, GCSBIE can get called a ton. What if the work weren't done every time?

  • What if GCSBIE only queried colors from conpty at most once per N seconds?
  • What if GCSBIE cached the results of a previous query, and used the cached results until the next time it makes a successful query?
  • What if the query mechanism could be asynchronous such that GCSBIE initiated a query but continued without waiting for the result? The result could get updated asynchronously, and would become available soon for a subsequent GCSBIE.

And to further complicate the matter, there's no guarantee that the connected conpty client even supports palette queries.

Do you mean that there is a single two-way channel of communication, i.e. all data communication must be through input and output streams, so the two sides can only communicate by passing escape codes to each other? And so if one side doesn't support responding to a query sequence then the other side can hang?

  • What if there were a way to know in advance whether the other side supported a particular query sequence?
    • What if there were a handshake? What if conpty clients could optionally send conhost their capabilities when they first connect? Maybe as a payload of some kind in an escape code.
  • What if another communication channel could be used optionally? Maybe not as a strict requirement on all conpty clients, but as an option.

I did play around with querying the palette once at startup, but that still introduces the complexity of having to wait for a response which may never arrive. An easier version of that would be to have the client pass in the palette via the initial command line. But both of those solutions fall apart if a user updates their palette settings at run-time.

I agree that supporting palette changes at runtime should be considered a requirement for any solution.
Making runtime palette changes instantly be noticed is not necessarily a requirement, though.

So while I would love to get this working somehow, I just don't see how. But if anyone has any bright ideas, feel free to share them here.

I'm not suggesting it would be trivial to implement, nor that it should be perfect with no caveats. But this is currently a pretty significant regression from the perspective of console mode programs.

@j4james
Copy link
Collaborator

j4james commented Jul 13, 2023

My understanding is that conpty is designed to work in a similar fashion to the Linux pty system, so a Linux terminal could theoretically be ported to Windows without too much effort. I don't know all the details, but I think there's an input stream, an output stream, and a signal stream, and it's the latter that is responsible for notifying conhost of window size changes, similar to the SIGWINCH signal on Linux

There isn't an equivalent signal for palette changes (as far as I'm aware), so that would be something we'd have to invent. Whereas conhost querying the palette from the conpty client just relies on standard escape sequences that a lot of terminals should already support out of the box. That's why I was considering a query as my initial approach to the problem.

But as you're both proposing, having the conpty client notify conhost of palette changes does seem like a much better idea. And if some don't support it, they'd be no worse off than they are now. I'm not very familiar with this part of the code, so there may be technical reasons why this isn't feasible, but if the core devs don't raise any objections, I'd be willing to give it a try some time.

@alabuzhev
Copy link
Contributor Author

conhost querying the palette from the conpty client just relies on standard escape sequences

By the way, are these standard escape sequences already supported by WT?

@chrisant996
Copy link

Btw, not all terminals support the same escape sequences. So I don't think it's safe for conhost to make assumptions about which codes it can send internally. If the conpty client indicated its capabilities, then it could be considered safe.

@j4james
Copy link
Collaborator

j4james commented Jul 14, 2023

By the way, are these standard escape sequences already supported by WT?

@alabuzhev No, for the same reason we don't return the palette in GCSBIE. That's actually the main reason I want to fix this. I don't particular care about GCSBIE, but I would like to get the VT palette queries working (and a number of other VT queries that we can't currently handle).

Btw, not all terminals support the same escape sequences.

@chrisant996 Yeah, but we only care about terminals that are using conpty, and it also doesn't really matter if they don't support the sequence - it should just be ignored. But that's not something I'm planning to do now anyway. I much prefer the approach that you guys were suggesting, i.e. the conpty client pushing the palette to conhost.

chrisant996 added a commit to chrisant996/clink that referenced this issue Nov 16, 2023
Terminal and the new conhost do not support GetConsoleScreenBufferInfoEx
returning the actual color table.  If/when that issues gets fixed, then
Clink will be able to return the color table.  (For example, so scripts
can infer "light theme" versus "dark theme".)

microsoft/terminal#10639
chrisant996 added a commit to chrisant996/clink that referenced this issue Nov 17, 2023
Can't work in Windows Terminal until Terminal#10639 is fixed.
microsoft/terminal#10639
@alabuzhev
Copy link
Contributor Author

But as you're both proposing, having the conpty client notify conhost of palette changes does seem like a much better idea

Thinking about it again, do we even need a separate notification here?
Can the frontend just build an OSC 4 string and push it to the stream a) on startup b) whenever the user changes the palette?

@DHowett
Copy link
Member

DHowett commented Mar 1, 2024

Well, yes, that would rather constitute a notification.

@chrisant996

This comment was marked as off-topic.

@alabuzhev
Copy link
Contributor Author

@DHowett I mean that in a more conventional sense a "notification" is rather something like:
Frontend to backend: "hey we have a change here, do something"
Backend: extracts the palette from the frontend using some internal (and not yet existing) magic

Whereas "build and shove OSC 4 to the stream" is more like a direct request.
Notably it should not require any extra changes in the backend, it can do it already.

@DHowett
Copy link
Member

DHowett commented Mar 1, 2024

Byte swapping discussion (hidden as off-topic)

The implementations of GetConsoleScreenBufferInfoEx and SetConsoleScreenBufferInfoEx are swapping Red and Blue bytes in the COLORREF.

This is the first we've heard of GCSBIEx returning invalid data in the color palette! Do you have a test app that exhibits this? It sounds (sorry, based on "... detect that the API is malfunctioning") like it isn't consistent.

(EDIT: Oh, I see you added one. Thanks!)

it can do it already

Alas, it still will. ConPTY's VT input state machine is vastly different from the output one.

@chrisant996

This comment was marked as off-topic.

@DHowett
Copy link
Member

DHowett commented Mar 1, 2024

(For thread tending purposes, I moved the byte swapping discussion over to #16795)

@j4james
Copy link
Collaborator

j4james commented Mar 1, 2024

FYI, I came across a discussion recently on the issue tracker of another terminal (most likely VTE), where somebody was asking for similar functionality. The use case was for multiplexers like tmux and zellij, which are in a similar situation to us with conpty, i.e. needing to know the palette of the connected terminal.

I think one of the proposals was to trigger SIGWINCH whenever the color changes, with the assumption that the app could then request the current palette whenever that happens (personally I don't think that's a great idea though).

The other suggestion was something like a mode that the app would set letting the terminal know that it wanted to receive automatic updates. This seemed a more promising approach to me, but it would need to cover more than just palette changes in my opinion (I made some notes on this subject in #16672 (comment)).

Anyway, the main reason I wanted to bring this up was because it would be good if we could agree with other terminals on a standard for this if they are going to be doing the same sort of thing.

@bash
Copy link

bash commented Mar 2, 2024

I think one of the proposals was to trigger SIGWINCH whenever the color changes [...]

That was my proposal / patch you came across :)

Here's the link for reference:
https://gitlab.gnome.org/GNOME/vte/-/issues/2740

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Server Down in the muck of API call servicing, interprocess communication, eventing, etc. Issue-Task It's a feature request, but it doesn't really need a major design. Priority-3 A description (P3) Product-Conpty For console issues specifically related to conpty
Projects
None yet
Development

No branches or pull requests

6 participants