[PATCH v5 0/7] MR9973: Draft: conhost: add basic VT control sequence
This is a minimal approach to running MC in wineconsole and addresses https://bugs.winehq.org/show_bug.cgi?id=49780. Wineconsole thus supports basic CSI control sequences for: - SGR (Select Graphic Rendition) and - CUP (Cursor Position) {width=900 height=507} **TODO:** - [x] check for ENABLE_VIRTUAL_TERMINAL_PROCESSING - [x] bug states that SetConsoleMode does not return error code and therefore applications fail to handle it correctly - [ ] rewrite, if that is the way to go i will do that, to use newer API. This would allow more colors than ANSI colors only - [x] test shared: several programs attached to the same console
- tried to test it with mc and w64devkit in wineconsole both works as far control sequences are implemented. - tested following seq: - write esc seq to change current color in red, - don't emit esc seq to go back to default color - use old API to get screen buffer color and check the result (GetConsoleScreenBufferInfo)
-> GetConsoleScreenBufferInfo matches the color which was set via CSI/SGR seq.
-- v5: conhost: add basic CUP color handling kernel32/tests: add console test for CUP conhost: improve basic SGR color handling conhost: add basic SGR color handling kernel32/tests: add console tests for basic SGR conhost: add basic VT control sequence parsing kernel32/tests: add console tests for CSI https://gitlab.winehq.org/wine/wine/-/merge_requests/9973
From: Thomas Csovcsity <thc.fr13nd@gmail.com> based on suggestions by @epo --- dlls/kernel32/tests/console.c | 43 +++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 3c3538041f4..c3fa141ee44 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5717,6 +5717,48 @@ static void test_FreeConsoleStd(void) } } +static void test_ANSI_escape_sequences(void) +{ + CONSOLE_SCREEN_BUFFER_INFO sb_info; + HANDLE hConOut; + BOOL ret; + DWORD mode, dw; + COORD c = {}; + + FreeConsole(); + AllocConsole(); + hConOut = CreateFileA("CONOUT$", GENERIC_READ|GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, 0); + ret = SetConsoleMode(hConOut, ENABLE_PROCESSED_OUTPUT | ENABLE_VIRTUAL_TERMINAL_PROCESSING); + ok(ret, "SetConsoleMode failed\n"); + ret = GetConsoleMode(hConOut, &mode); + ok(ret, "GetConsoleMode failed\n"); + ok(mode == (ENABLE_PROCESSED_OUTPUT | ENABLE_VIRTUAL_TERMINAL_PROCESSING), "Unexpected mode\n"); + ret = SetConsoleCursorPosition(hConOut, c); + ok(ret, "SetConsoleCursorPosition failed\n"); + ret = SetConsoleTextAttribute(hConOut, FOREGROUND_GREEN | FOREGROUND_INTENSITY); + ok(ret, "SetConsoleTextAttribute failed\n"); + ret = WriteConsoleW(hConOut, L"GREEN\x1b[91mRED", 5+5+3, &dw, NULL); + ok(dw == 5+5+3, "Wrong count\n"); + ok(ret, "WriteConsole failed\n"); + ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); + ok(ret, "GetConsoleScreenBufferInfo failed\n"); + ok(sb_info.dwCursorPosition.X == 5 + 3, "Incorrect X cursor position\n"); + ok(sb_info.dwCursorPosition.Y == 0, "Incorrect X cursor position\n"); + ok(sb_info.wAttributes == (FOREGROUND_RED | FOREGROUND_INTENSITY), "Unexpected attributes got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | FOREGROUND_INTENSITY); + ret = SetConsoleTextAttribute(hConOut, FOREGROUND_BLUE | FOREGROUND_INTENSITY); + ret = WriteConsoleW(hConOut, L"BLUE\x1b[m", 4+3, &dw, NULL); + ok(dw == 4+3, "Wrong count\n"); + ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); + ok(sb_info.dwCursorPosition.X == 5 + 3 + 4, "Incorrect X cursor position\n"); + ok(sb_info.dwCursorPosition.Y == 0, "Incorrect X cursor position\n"); + ok(sb_info.wAttributes == (FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN); + /* TODO should call ReadConsoleOutput to check that the colors are correct (they are visually) */ + /* could test more sequences */ + CloseHandle(hConOut); + + FreeConsole(); +} + START_TEST(console) { HANDLE hConIn, hConOut, revert_output = NULL, unbound_output; @@ -6021,6 +6063,7 @@ START_TEST(console) test_condrv_server_as_root_directory(); test_CreateProcessCUI(); test_CtrlHandlerSubsystem(); + test_ANSI_escape_sequences(); } else if (revert_output) SetConsoleActiveScreenBuffer(revert_output); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9973
From: Thomas Csovcsity <thc.fr13nd@gmail.com> Remove Control Sequence Introducer(CSI) commands avoids console scrambling --- dlls/kernel32/tests/console.c | 6 ++-- programs/conhost/conhost.c | 56 +++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index c3fa141ee44..8ced7f1ad7e 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5744,16 +5744,14 @@ static void test_ANSI_escape_sequences(void) ok(ret, "GetConsoleScreenBufferInfo failed\n"); ok(sb_info.dwCursorPosition.X == 5 + 3, "Incorrect X cursor position\n"); ok(sb_info.dwCursorPosition.Y == 0, "Incorrect X cursor position\n"); - ok(sb_info.wAttributes == (FOREGROUND_RED | FOREGROUND_INTENSITY), "Unexpected attributes got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | FOREGROUND_INTENSITY); + todo_wine ok(sb_info.wAttributes == (FOREGROUND_RED | FOREGROUND_INTENSITY), "Unexpected attributes got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | FOREGROUND_INTENSITY); ret = SetConsoleTextAttribute(hConOut, FOREGROUND_BLUE | FOREGROUND_INTENSITY); ret = WriteConsoleW(hConOut, L"BLUE\x1b[m", 4+3, &dw, NULL); ok(dw == 4+3, "Wrong count\n"); ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); ok(sb_info.dwCursorPosition.X == 5 + 3 + 4, "Incorrect X cursor position\n"); ok(sb_info.dwCursorPosition.Y == 0, "Incorrect X cursor position\n"); - ok(sb_info.wAttributes == (FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN); - /* TODO should call ReadConsoleOutput to check that the colors are correct (they are visually) */ - /* could test more sequences */ + todo_wine ok(sb_info.wAttributes == (FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN); CloseHandle(hConOut); FreeConsole(); diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index 8298743553f..c472fff9c29 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -2107,6 +2107,9 @@ static NTSTATUS write_console( struct screen_buffer *screen_buffer, const WCHAR { RECT update_rect; size_t i, j; + enum { + ST_START, ST_PARAM, ST_INTER, ST_FINAL + } state; TRACE( "%s\n", debugstr_wn(buffer, len) ); @@ -2143,6 +2146,59 @@ static NTSTATUS write_console( struct screen_buffer *screen_buffer, const WCHAR case '\r': screen_buffer->cursor_x = 0; continue; + case '\e': + if ((screen_buffer->mode & ENABLE_VIRTUAL_TERMINAL_PROCESSING)) + { + if (buffer[i+1] == '[') + { + FIXME( "CSI sequences not supported fully yet, only skipping control sequences!\n" ); + state = ST_PARAM; + i+=2; + } + else + { + ERR("Invalid CSI start sequence\n"); + break; + } + /* intermediate bytes 0x30 - 0x3f (0-?) */ + for (; i<len && (state == ST_PARAM); i++) + { + if (buffer[i] >= 0x30 && buffer[i] <= 0x3b) + continue; + else if (buffer[i] >= 0x3c && buffer[i] <= 0x3f) + { + WARN( "This is a private -terminal manufacturer only- CSI sequence\n" ); + continue; + } + else + { + state = ST_INTER; + break; + } + } + /* intermediate bytes 0x20 - 0x2f*/ + for (; i<len && (state == ST_INTER || state == ST_PARAM); i++) + { + if (buffer[i] >= 0x20 && buffer[i] <= 0x2f) + continue; + else + { + state = ST_FINAL; + break; + } + } + if (state == ST_START || state == ST_FINAL) + { + if (buffer[i] >= 0x70 && buffer[i] <= 0x7e) + WARN("This is a private -terminal manufacturer only- CSI sequence\n"); + else if (!(buffer[i] >= 0x40 && buffer[i] <= 0x6f)) + ERR("This was no valid CSI sequence\n"); + } + } + else + WARN( "ENABLE_VIRTUAL_TERMINAL_PROCESSING is disabled\n" ); + + continue; } } if (screen_buffer->cursor_x == screen_buffer->width && !(screen_buffer->mode & ENABLE_WRAP_AT_EOL_OUTPUT)) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9973
From: Thomas Csovcsity <thc.fr13nd@gmail.com> --- dlls/kernel32/tests/console.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 8ced7f1ad7e..b7d00e6b306 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5737,21 +5737,34 @@ static void test_ANSI_escape_sequences(void) ok(ret, "SetConsoleCursorPosition failed\n"); ret = SetConsoleTextAttribute(hConOut, FOREGROUND_GREEN | FOREGROUND_INTENSITY); ok(ret, "SetConsoleTextAttribute failed\n"); - ret = WriteConsoleW(hConOut, L"GREEN\x1b[91mRED", 5+5+3, &dw, NULL); - ok(dw == 5+5+3, "Wrong count\n"); + ret = WriteConsoleW(hConOut, L"GREEN\x1b[91mBRIGHT RED", 5+5+10, &dw, NULL); + ok(dw == 5+5+10, "Wrong count\n"); ok(ret, "WriteConsole failed\n"); ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); ok(ret, "GetConsoleScreenBufferInfo failed\n"); - ok(sb_info.dwCursorPosition.X == 5 + 3, "Incorrect X cursor position\n"); - ok(sb_info.dwCursorPosition.Y == 0, "Incorrect X cursor position\n"); + ok(sb_info.dwCursorPosition.X == 5 + 10, "Incorrect X cursor position\n"); + ok(sb_info.dwCursorPosition.Y == 0, "Incorrect Y cursor position\n"); todo_wine ok(sb_info.wAttributes == (FOREGROUND_RED | FOREGROUND_INTENSITY), "Unexpected attributes got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | FOREGROUND_INTENSITY); ret = SetConsoleTextAttribute(hConOut, FOREGROUND_BLUE | FOREGROUND_INTENSITY); ret = WriteConsoleW(hConOut, L"BLUE\x1b[m", 4+3, &dw, NULL); ok(dw == 4+3, "Wrong count\n"); ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); - ok(sb_info.dwCursorPosition.X == 5 + 3 + 4, "Incorrect X cursor position\n"); - ok(sb_info.dwCursorPosition.Y == 0, "Incorrect X cursor position\n"); + ok(sb_info.dwCursorPosition.X == 5 + 10 + 4, "Incorrect X cursor position\n"); + ok(sb_info.dwCursorPosition.Y == 0, "Incorrect Y cursor position\n"); todo_wine ok(sb_info.wAttributes == (FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN); + ret = WriteConsoleW(hConOut, L"\n\x1b[31mRED", 1+5+3, &dw, NULL); + ok(dw == 1+5+3, "Wrong count\n"); + ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); + ok(sb_info.dwCursorPosition.X == 3, "Incorrect X cursor position: got %d, expected %d\n", sb_info.dwCursorPosition.X, 3); + ok(sb_info.dwCursorPosition.Y == 1, "Incorrect Y cursor position\n"); + todo_wine ok(sb_info.wAttributes == (FOREGROUND_RED), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED); + ret = WriteConsoleW(hConOut, L"\x1b[31;44mRED on BLUE", 5+14, &dw, NULL); + ok(dw == 5+14, "Wrong count\n"); + ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); + ok(sb_info.dwCursorPosition.X == 3+11, "Incorrect X cursor position: got %d, expected %d\n", sb_info.dwCursorPosition.X, 3+11); + ok(sb_info.dwCursorPosition.Y == 1, "Incorrect Y cursor position\n"); + todo_wine ok(sb_info.wAttributes == (FOREGROUND_RED | BACKGROUND_BLUE), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | BACKGROUND_BLUE); + CloseHandle(hConOut); FreeConsole(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9973
From: Thomas Csovcsity <thc.fr13nd@gmail.com> --- dlls/kernel32/tests/console.c | 8 +- programs/conhost/conhost.c | 204 +++++++++++++++++++++++++++++++++- 2 files changed, 205 insertions(+), 7 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index b7d00e6b306..e16fffe34f4 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5744,26 +5744,26 @@ static void test_ANSI_escape_sequences(void) ok(ret, "GetConsoleScreenBufferInfo failed\n"); ok(sb_info.dwCursorPosition.X == 5 + 10, "Incorrect X cursor position\n"); ok(sb_info.dwCursorPosition.Y == 0, "Incorrect Y cursor position\n"); - todo_wine ok(sb_info.wAttributes == (FOREGROUND_RED | FOREGROUND_INTENSITY), "Unexpected attributes got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | FOREGROUND_INTENSITY); + ok(sb_info.wAttributes == (FOREGROUND_RED | FOREGROUND_INTENSITY), "Unexpected attributes got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | FOREGROUND_INTENSITY); ret = SetConsoleTextAttribute(hConOut, FOREGROUND_BLUE | FOREGROUND_INTENSITY); ret = WriteConsoleW(hConOut, L"BLUE\x1b[m", 4+3, &dw, NULL); ok(dw == 4+3, "Wrong count\n"); ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); ok(sb_info.dwCursorPosition.X == 5 + 10 + 4, "Incorrect X cursor position\n"); ok(sb_info.dwCursorPosition.Y == 0, "Incorrect Y cursor position\n"); - todo_wine ok(sb_info.wAttributes == (FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN); + ok(sb_info.wAttributes == (FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN); ret = WriteConsoleW(hConOut, L"\n\x1b[31mRED", 1+5+3, &dw, NULL); ok(dw == 1+5+3, "Wrong count\n"); ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); ok(sb_info.dwCursorPosition.X == 3, "Incorrect X cursor position: got %d, expected %d\n", sb_info.dwCursorPosition.X, 3); ok(sb_info.dwCursorPosition.Y == 1, "Incorrect Y cursor position\n"); - todo_wine ok(sb_info.wAttributes == (FOREGROUND_RED), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED); + ok(sb_info.wAttributes == (FOREGROUND_RED), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED); ret = WriteConsoleW(hConOut, L"\x1b[31;44mRED on BLUE", 5+14, &dw, NULL); ok(dw == 5+14, "Wrong count\n"); ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); ok(sb_info.dwCursorPosition.X == 3+11, "Incorrect X cursor position: got %d, expected %d\n", sb_info.dwCursorPosition.X, 3+11); ok(sb_info.dwCursorPosition.Y == 1, "Incorrect Y cursor position\n"); - todo_wine ok(sb_info.wAttributes == (FOREGROUND_RED | BACKGROUND_BLUE), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | BACKGROUND_BLUE); + ok(sb_info.wAttributes == (FOREGROUND_RED | BACKGROUND_BLUE), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | BACKGROUND_BLUE); CloseHandle(hConOut); diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index c472fff9c29..3b8c9aca5fb 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -2103,13 +2103,202 @@ static NTSTATUS set_output_info( struct screen_buffer *screen_buffer, return STATUS_SUCCESS; } +#define BLACK 0x0000 +#define BLUE 0x0001 +#define GREEN 0x0002 +#define CYAN BLUE | GREEN +#define RED 0x0004 +#define MAGENTA BLUE | RED +#define YELLOW GREEN | RED +#define WHITE BLUE | GREEN | RED + +static unsigned int next_csi_sgr_argument( const WCHAR *seq, unsigned int len, unsigned int *arg ) +{ + unsigned int i=0, value = 0; + TRACE( "next SGR arg with %d length and [%s] as sequence! seq_p %p\n",len, debugstr_wn( seq, len ), seq ); + for (i=0; i<len; i++) + { + if ((seq[i] == ';') || (seq[i]=='m')) + { + *arg = value; + return i+1; + } + value = 10 * value + seq[i]-'0'; + } + return 0; +} + +static void process_csi_sequence_console( struct screen_buffer *screen_buffer, const WCHAR *seq, size_t len ) +{ + unsigned int colors[] = {BLACK, RED, GREEN, YELLOW, BLUE, MAGENTA, CYAN, WHITE}; + unsigned int value = 0, *arg = &value; + size_t seq_len = 0; + unsigned int r = 0, g = 0, b = 0; + struct condrv_output_info_params info_params; + BOOL is_bright = FALSE; + + switch (seq[len-1]) + { + case 'm': /* Select Graphic Rendition (SGR) */ + while (len > 0) + { + TRACE( "This is an SGR seq with %Iu length and [%s] as payload! seq_p %p\n",len, debugstr_wn( seq, len ), seq ); + seq_len = next_csi_sgr_argument(seq, len, arg); + if ((value>=90 && value<=97) || (value>=100 && value<=107)) + { + is_bright = TRUE; + value -= 60; + } + switch (value) + { + case 30: + case 31: + case 32: + case 33: + case 34: + case 35: + case 36: + case 37: + { + len -= seq_len; + seq += seq_len; + info_params.info.attr = ( info_params.info.attr & 0xf0 ) | colors[*arg-30]; + if (is_bright) + { + info_params.info.attr |= FOREGROUND_INTENSITY; + is_bright = FALSE; + } + break; + } + case 38: + { + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument( seq, len, arg ); + if (*arg == 2) + { + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument(seq, len, &r); + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument(seq, len, &g); + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument(seq, len, &b); + len -= seq_len; + seq += seq_len; + FIXME( "24bit rgb is not supported yet for foreground r[%d]g[%d]b[%d]\n", r, g, b ); + info_params.info.attr = ( BLACK < 8 ) | ( WHITE ); /* Fixme set correct color */ + } + if (*arg == 5) + { + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument( seq, len, arg ); + len -= seq_len; + seq += seq_len; + info_params.info.attr = ( info_params.info.attr & 0xf0 ) | colors[*arg]; + } + break; + } + case 39: + { + len -= seq_len; + seq += seq_len; + info_params.info.attr = ( info_params.info.attr & 0xf0 ) | ( WHITE ); /* default white fg */ + break; + } + case 40: + case 41: + case 42: + case 43: + case 44: + case 45: + case 46: + case 47: + { + len -= seq_len; + seq += seq_len; + info_params.info.attr = ( info_params.info.attr & 0x0f ) | ( colors[*arg-40] << 4 ); + if (is_bright) + { + info_params.info.attr |= BACKGROUND_INTENSITY; + is_bright = FALSE; + } + break; + } + case 48: + { + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument( seq, len, arg ); + if (*arg == 2) + { + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument( seq, len, &r ); + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument( seq, len, &g ); + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument( seq, len, &b ); + len -= seq_len; + seq += seq_len; + FIXME( "24bit rgb is not supported yet for background r[%d]g[%d]b[%d]\n", r, g, b ); + info_params.info.attr = ( BLACK < 4 ) | ( WHITE ); /* Fixme set correct color */ + } + if (*arg == 5) + { + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument( seq, len, arg ); + len -= seq_len; + seq += seq_len; + info_params.info.attr = ( info_params.info.attr & 0xf0 ) | colors[*arg]; + } + break; + } + case 49: + { + len -= seq_len; + seq += seq_len; + info_params.info.attr = ( info_params.info.attr & 0x000f ) | ( BLACK < 4 ); /* default black bg */ + break; + } + case 0: + { + len -= seq_len; + seq += seq_len; + info_params.info.attr = ( BLACK < 4 ) | ( WHITE ); /* white on black */ + break; + } + default: + FIXME( "unhandled sgr sequence %s len[%Iu]\n", debugstr_wn( seq, len ), len ); + info_params.info.attr = ( BLACK < 4 ) | ( WHITE ); /* white on black */ + len = 0; + break; + + } + info_params.mask = SET_CONSOLE_OUTPUT_INFO_ATTR; + set_output_info( screen_buffer, &info_params, sizeof(info_params) ); + } + break; + default: + FIXME( "unhandled sequence %s switch char [%s] len[%Iu]\n", debugstr_wn( seq, len ), debugstr_wn( &seq[len], 1 ), len ); + break; + } +} + static NTSTATUS write_console( struct screen_buffer *screen_buffer, const WCHAR *buffer, size_t len ) { RECT update_rect; - size_t i, j; + size_t csi_len = 0, i, j; enum { ST_START, ST_PARAM, ST_INTER, ST_FINAL } state; + const WCHAR *csi_start; TRACE( "%s\n", debugstr_wn(buffer, len) ); @@ -2151,18 +2340,21 @@ static NTSTATUS write_console( struct screen_buffer *screen_buffer, const WCHAR { if (buffer[i+1] == '[') { - FIXME( "CSI sequences not supported fully yet, only skipping control sequences!\n" ); state = ST_PARAM; + csi_start = &buffer[i+2]; + csi_len = 0; i+=2; } else { - ERR("Invalid CSI start sequence\n"); + ERR( "Invalid CSI start sequence\n" ); + ERR( "%s\n", debugstr_wn(buffer, len) ); break; } /* intermediate bytes 0x30 - 0x3f (0-?) */ for (; i<len && (state == ST_PARAM); i++) { + csi_len++; if (buffer[i] >= 0x30 && buffer[i] <= 0x3b) continue; else if (buffer[i] >= 0x3c && buffer[i] <= 0x3f) @@ -2173,17 +2365,20 @@ static NTSTATUS write_console( struct screen_buffer *screen_buffer, const WCHAR else { state = ST_INTER; + csi_len--; break; } } /* intermediate bytes 0x20 - 0x2f*/ for (; i<len && (state == ST_INTER || state == ST_PARAM); i++) { + csi_len++; if (buffer[i] >= 0x20 && buffer[i] <= 0x2f) continue; else { state = ST_FINAL; + csi_len--; break; } } @@ -2193,6 +2388,9 @@ static NTSTATUS write_console( struct screen_buffer *screen_buffer, const WCHAR WARN("This is a private -terminal manufacturer only- CSI sequence\n"); else if (!(buffer[i] >= 0x40 && buffer[i] <= 0x6f)) ERR("This was no valid CSI sequence\n"); + + csi_len++; + process_csi_sequence_console(screen_buffer, csi_start, csi_len); } } else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9973
From: Thomas Csovcsity <thc.fr13nd@gmail.com> add simple RGB24 to 3bit RGB --- programs/conhost/conhost.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index 3b8c9aca5fb..04252459a80 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -2177,6 +2177,7 @@ static void process_csi_sequence_console( struct screen_buffer *screen_buffer, c seq_len = next_csi_sgr_argument( seq, len, arg ); if (*arg == 2) { + int color = BLACK; len -= seq_len; seq += seq_len; seq_len = next_csi_sgr_argument(seq, len, &r); @@ -2188,10 +2189,16 @@ static void process_csi_sequence_console( struct screen_buffer *screen_buffer, c seq_len = next_csi_sgr_argument(seq, len, &b); len -= seq_len; seq += seq_len; - FIXME( "24bit rgb is not supported yet for foreground r[%d]g[%d]b[%d]\n", r, g, b ); - info_params.info.attr = ( BLACK < 8 ) | ( WHITE ); /* Fixme set correct color */ + /* a simple color mapping for basic support */ + if ( r > 127 ) + color |= RED; + if ( g > 127 ) + color |= GREEN; + if ( b > 127 ) + color |= BLUE; + info_params.info.attr = ( info_params.info.attr & 0xf0 ) | color; } - if (*arg == 5) + else if (*arg == 5) { len -= seq_len; seq += seq_len; @@ -2235,6 +2242,7 @@ static void process_csi_sequence_console( struct screen_buffer *screen_buffer, c seq_len = next_csi_sgr_argument( seq, len, arg ); if (*arg == 2) { + int color = BLACK; len -= seq_len; seq += seq_len; seq_len = next_csi_sgr_argument( seq, len, &r ); @@ -2246,10 +2254,17 @@ static void process_csi_sequence_console( struct screen_buffer *screen_buffer, c seq_len = next_csi_sgr_argument( seq, len, &b ); len -= seq_len; seq += seq_len; - FIXME( "24bit rgb is not supported yet for background r[%d]g[%d]b[%d]\n", r, g, b ); - info_params.info.attr = ( BLACK < 4 ) | ( WHITE ); /* Fixme set correct color */ + /* a simple color mapping for basic support */ + if ( r > 127 ) + color |= RED; + if ( g > 127 ) + color |= GREEN; + if ( b > 127 ) + color |= BLUE; + info_params.info.attr = ( info_params.info.attr & 0x0f ) | ( color << 4 ); + } - if (*arg == 5) + else if (*arg == 5) { len -= seq_len; seq += seq_len; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9973
From: Thomas Csovcsity <thc.fr13nd@gmail.com> --- dlls/kernel32/tests/console.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index e16fffe34f4..8c35d5786b1 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5765,6 +5765,13 @@ static void test_ANSI_escape_sequences(void) ok(sb_info.dwCursorPosition.Y == 1, "Incorrect Y cursor position\n"); ok(sb_info.wAttributes == (FOREGROUND_RED | BACKGROUND_BLUE), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | BACKGROUND_BLUE); + ret = WriteConsoleW(hConOut, L"\x1b[1;1H", 6, &dw, NULL); + ok(dw == 6, "Wrong count\n"); + ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); + todo_wine ok(sb_info.dwCursorPosition.X == 0, "Incorrect X cursor position: got %d, expected %d\n", sb_info.dwCursorPosition.X, 0); + todo_wine ok(sb_info.dwCursorPosition.Y == 0, "Incorrect Y cursor position\n: got %d, expected %d\n", sb_info.dwCursorPosition.Y, 0); + ok(sb_info.wAttributes == (FOREGROUND_RED | BACKGROUND_BLUE), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | BACKGROUND_BLUE); + CloseHandle(hConOut); FreeConsole(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9973
From: Thomas Csovcsity <thc.fr13nd@gmail.com> --- dlls/kernel32/tests/console.c | 4 ++-- programs/conhost/conhost.c | 40 +++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 8c35d5786b1..6f1bb6a0474 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -5768,8 +5768,8 @@ static void test_ANSI_escape_sequences(void) ret = WriteConsoleW(hConOut, L"\x1b[1;1H", 6, &dw, NULL); ok(dw == 6, "Wrong count\n"); ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); - todo_wine ok(sb_info.dwCursorPosition.X == 0, "Incorrect X cursor position: got %d, expected %d\n", sb_info.dwCursorPosition.X, 0); - todo_wine ok(sb_info.dwCursorPosition.Y == 0, "Incorrect Y cursor position\n: got %d, expected %d\n", sb_info.dwCursorPosition.Y, 0); + ok(sb_info.dwCursorPosition.X == 0, "Incorrect X cursor position: got %d, expected %d\n", sb_info.dwCursorPosition.X, 0); + ok(sb_info.dwCursorPosition.Y == 0, "Incorrect Y cursor position\n: got %d, expected %d\n", sb_info.dwCursorPosition.Y, 0); ok(sb_info.wAttributes == (FOREGROUND_RED | BACKGROUND_BLUE), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | BACKGROUND_BLUE); CloseHandle(hConOut); diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index 04252459a80..0f8d0bb058f 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -2300,6 +2300,46 @@ static void process_csi_sequence_console( struct screen_buffer *screen_buffer, c set_output_info( screen_buffer, &info_params, sizeof(info_params) ); } break; + case 'H': + unsigned int x = 0, y = 0, value = 0, i; + TRACE( "This is an CUP seq with %Iu length and [%s] as payload! seq_p %p\n",len, debugstr_wn( seq, len ), seq ); + + for (i = 0; i < len; i++) + { + if (seq[i] != ';' && seq[i] != 'H') + value = 10 * value + seq[i] - '0'; + else if (seq[i] == ';') + { + if (value != 0) + { + y = value; + value = 0; + } + else + y = 1; + } + else if (seq[i] == 'H') + { + if (y == 0 && value == 0) + { + x = 1; + y = 1; + } else if (y == 0 && value != 0) + { + y = value; + x = 1; + } else /* (y != 0 && value != 0) */ + x = value; + } + else + { + ERR( "CSI CUP parsing failed, this should not happen!\n" ); + break; + } + } + screen_buffer->cursor_x = x-1; + screen_buffer->cursor_y = y-1; + break; default: FIXME( "unhandled sequence %s switch char [%s] len[%Iu]\n", debugstr_wn( seq, len ), debugstr_wn( &seq[len], 1 ), len ); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9973
On Fri Mar 13 22:26:24 2026 +0000, eric pouech wrote:
as the test detaches from current console (and creates a new one), you'd probably need to redirect stdout and stderr when running the tests also, setting env variable WINETEST_COLOR=1 should insert ANSI colors for the tests results Thanks! Tests and implementation done to support MC.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9973#note_132247
eric pouech (@epo) commented about dlls/kernel32/tests/console.c:
+ ok(ret, "GetConsoleMode failed\n"); + ok(mode == (ENABLE_PROCESSED_OUTPUT | ENABLE_VIRTUAL_TERMINAL_PROCESSING), "Unexpected mode\n"); + ret = SetConsoleCursorPosition(hConOut, c); + ok(ret, "SetConsoleCursorPosition failed\n"); + ret = SetConsoleTextAttribute(hConOut, FOREGROUND_GREEN | FOREGROUND_INTENSITY); + ok(ret, "SetConsoleTextAttribute failed\n"); + ret = WriteConsoleW(hConOut, L"GREEN\x1b[91mRED", 5+5+3, &dw, NULL); + ok(dw == 5+5+3, "Wrong count\n"); + ok(ret, "WriteConsole failed\n"); + ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); + ok(ret, "GetConsoleScreenBufferInfo failed\n"); + ok(sb_info.dwCursorPosition.X == 5 + 3, "Incorrect X cursor position\n"); + ok(sb_info.dwCursorPosition.Y == 0, "Incorrect X cursor position\n"); + ok(sb_info.wAttributes == (FOREGROUND_RED | FOREGROUND_INTENSITY), "Unexpected attributes got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | FOREGROUND_INTENSITY); + ret = SetConsoleTextAttribute(hConOut, FOREGROUND_BLUE | FOREGROUND_INTENSITY); + ret = WriteConsoleW(hConOut, L"BLUE\x1b[m", 4+3, &dw, NULL); should be 4+4
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9973#note_132999
eric pouech (@epo) commented about dlls/kernel32/tests/console.c:
test_condrv_server_as_root_directory(); test_CreateProcessCUI(); test_CtrlHandlerSubsystem(); + test_ANSI_escape_sequences();
it looks like there are conflicts with previous tests, this line has to be moved just after test_FreeConsole(); line and this will trigger errors when run under wine that should marked with todo_wine (each single commit shall not introduce failures in the CI) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9973#note_133000
globally, the first and third patch should be squashed together (as first patch) second patch is pure dead code, so it should be squashed with fourth IMO the global design of the parser has to be be reviewed: * it mostly assumes that the whole sequence will be passed in the buffer; this is not always the case (esp if emittter has unbuffered output) * so it should be ready to handle an incomplete sequence, stash the incomplete sequence somewhere, and try to get a complete sequence when new data is available * error handling: the strategy when dealing with an "invalid" sequence shall be defined; note that since you can have several emitters, pushing characters one by one, that you must be prepare for incorrect sequence so basically, the parser shall look like: * if valid complete sequence, remove it from input buffer, and process it * if valid incomplete sequence, stash incomplete sequence and retry to complete incomplete seq with new incoming data * if invalid sequence.... not 100% clear to me what would be the best behavior... this MR drops the \\x1b and moves on to the next characters... invalid here covers both syntactically incorrect char stream and recognized but unsupported esc sequence... maybe we would need to separate the two (in the MR for an unsupported sequence, current code drop the \\1xb and emits the rest of the string... either we should drop the whole sequence, or keep it untouched - on one hand the unix console could make use of it, but on the other hand that means two different behavior between the two backends... which we currently have) I leave that open for now, feedback welcome :-) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9973#note_133064
eric pouech (@epo) commented about programs/conhost/conhost.c:
+ { + is_bright = TRUE; + value -= 60; + } + switch (value) + { + case 30: + case 31: + case 32: + case 33: + case 34: + case 35: + case 36: + case 37: + { + len -= seq_len; since you mostly decrease len and increase seq at the same time, it looks to me you can get rid of one of the two variables
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9973#note_133065
eric pouech (@epo) commented about programs/conhost/conhost.c:
+ seq += seq_len; + seq_len = next_csi_sgr_argument( seq, len, arg ); + if (*arg == 2) + { + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument(seq, len, &r); + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument(seq, len, &g); + len -= seq_len; + seq += seq_len; + seq_len = next_csi_sgr_argument(seq, len, &b); + len -= seq_len; + seq += seq_len; + FIXME( "24bit rgb is not supported yet for foreground r[%d]g[%d]b[%d]\n", r, g, b ); this shall rather be WARN (ditto for the others)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9973#note_133066
eric pouech (@epo) commented about dlls/kernel32/tests/console.c:
ok(ret, "GetConsoleScreenBufferInfo failed\n"); ok(sb_info.dwCursorPosition.X == 5 + 3, "Incorrect X cursor position\n"); ok(sb_info.dwCursorPosition.Y == 0, "Incorrect X cursor position\n"); - ok(sb_info.wAttributes == (FOREGROUND_RED | FOREGROUND_INTENSITY), "Unexpected attributes got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | FOREGROUND_INTENSITY); + todo_wine ok(sb_info.wAttributes == (FOREGROUND_RED | FOREGROUND_INTENSITY), "Unexpected attributes got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_RED | FOREGROUND_INTENSITY); ret = SetConsoleTextAttribute(hConOut, FOREGROUND_BLUE | FOREGROUND_INTENSITY); ret = WriteConsoleW(hConOut, L"BLUE\x1b[m", 4+3, &dw, NULL); ok(dw == 4+3, "Wrong count\n"); ret = GetConsoleScreenBufferInfo(hConOut, &sb_info); ok(sb_info.dwCursorPosition.X == 5 + 3 + 4, "Incorrect X cursor position\n"); ok(sb_info.dwCursorPosition.Y == 0, "Incorrect X cursor position\n"); - ok(sb_info.wAttributes == (FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN), "Unexpected attributes: got %x, expected %x\n", sb_info.wAttributes, FOREGROUND_BLUE | FOREGROUND_RED | FOREGROUND_GREEN); - /* TODO should call ReadConsoleOutput to check that the colors are correct (they are visually) */
comment was not meant to be removed comment was meant to be implemented -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9973#note_133067
@thc13 thank you for laying the groundwork here. I've picked up this work in !11582, building on your SGR and CUP implementation with a redesigned parser that handles split sequences across WriteConsole calls, plus ReadConsoleOutput verification in the tests. Credited you in both commits. Would be great to get your thoughts. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9973#note_147909
participants (4)
-
eric pouech (＠epo) -
Lokesh Poovaragan (＠lokeshpoovaragan)
-
Thomas Csovcsity
-
Thomas Csovcsity (＠thc13)