[PATCH 1/2] conhost: Copy Character Attributes (colors) to the new screen buffer
CreateConsoleScreenBuffer() copies the Character Attributes (colors) to the new screen buffer when called. Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> --- dlls/kernel32/tests/console.c | 6 +++--- programs/conhost/conhost.c | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 9fa0ce06854..5f9342ffd44 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -1047,7 +1047,7 @@ static void test_new_screen_buffer_color_attributes(HANDLE hConOut) ok(ret, "GetConsoleScreenBufferInfoEx failed: error %u\n", GetLastError()); CloseHandle(hConOut2); - todo_wine ok(csbi2.wAttributes == orig_attr, "Character Attributes should have been copied: " + ok(csbi2.wAttributes == orig_attr, "Character Attributes should have been copied: " "got %#x, expected %#x\n", csbi2.wAttributes, orig_attr); todo_wine ok(csbi2.wPopupAttributes != orig_popup, "Popup Attributes should not match original value\n"); todo_wine ok(csbi2.wPopupAttributes == orig_attr, "Popup Attributes should match Character Attributes\n"); @@ -1068,7 +1068,7 @@ static void test_new_screen_buffer_color_attributes(HANDLE hConOut) ok(ret, "GetConsoleScreenBufferInfoEx failed: error %u\n", GetLastError()); CloseHandle(hConOut2); - todo_wine ok(csbi2.wAttributes == attr, "Character Attributes should have been copied: " + ok(csbi2.wAttributes == attr, "Character Attributes should have been copied: " "got %#x, expected %#x\n", csbi2.wAttributes, attr); todo_wine ok(csbi2.wPopupAttributes != orig_popup, "Popup Attributes should not match original value\n"); todo_wine ok(csbi2.wPopupAttributes == attr, "Popup Attributes should match Character Attributes\n"); @@ -1092,7 +1092,7 @@ static void test_new_screen_buffer_color_attributes(HANDLE hConOut) ok(ret, "GetConsoleScreenBufferInfoEx failed: error %u\n", GetLastError()); CloseHandle(hConOut2); - todo_wine ok(csbi2.wAttributes == orig_attr, "Character Attributes should have been copied: " + ok(csbi2.wAttributes == orig_attr, "Character Attributes should have been copied: " "got %#x, expected %#x\n", csbi2.wAttributes, orig_attr); todo_wine ok(csbi2.wPopupAttributes != orig_popup, "Popup Attributes should not match original value\n"); todo_wine ok(csbi2.wPopupAttributes == orig_attr, "Popup Attributes should match Character Attributes\n"); diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index 7db1261d3be..805df880897 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -85,7 +85,6 @@ static struct screen_buffer *create_screen_buffer( struct console *console, int screen_buffer->cursor_visible = 1; screen_buffer->width = width; screen_buffer->height = height; - screen_buffer->attr = 0x07; screen_buffer->popup_attr = 0xf5; screen_buffer->font.weight = FW_NORMAL; screen_buffer->font.pitch_family = FIXED_PITCH | FF_DONTCARE; @@ -96,6 +95,7 @@ static struct screen_buffer *create_screen_buffer( struct console *console, int screen_buffer->max_height = console->active->max_height; screen_buffer->win.right = console->active->win.right - console->active->win.left; screen_buffer->win.bottom = console->active->win.bottom - console->active->win.top; + screen_buffer->attr = console->active->attr; } else { @@ -103,6 +103,7 @@ static struct screen_buffer *create_screen_buffer( struct console *console, int screen_buffer->max_height = height; screen_buffer->win.right = width - 1; screen_buffer->win.bottom = height - 1; + screen_buffer->attr = FOREGROUND_BLUE|FOREGROUND_GREEN|FOREGROUND_RED; } if (wine_rb_put( &screen_buffer_map, LongToPtr(id), &screen_buffer->entry )) -- 2.32.0
MSDN says the Popup Attributes (colors) are copied to the new screen buffer when CreateConsoleScreenBuffer() is called. Testing shows the new Popup Attributes are actually the existing Character Attributes. Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> --- dlls/kernel32/tests/console.c | 12 ++++++------ programs/conhost/conhost.c | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 5f9342ffd44..6ee00aa49a6 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -1049,8 +1049,8 @@ static void test_new_screen_buffer_color_attributes(HANDLE hConOut) ok(csbi2.wAttributes == orig_attr, "Character Attributes should have been copied: " "got %#x, expected %#x\n", csbi2.wAttributes, orig_attr); - todo_wine ok(csbi2.wPopupAttributes != orig_popup, "Popup Attributes should not match original value\n"); - todo_wine ok(csbi2.wPopupAttributes == orig_attr, "Popup Attributes should match Character Attributes\n"); + ok(csbi2.wPopupAttributes != orig_popup, "Popup Attributes should not match original value\n"); + ok(csbi2.wPopupAttributes == orig_attr, "Popup Attributes should match Character Attributes\n"); /* Test different Character Attributes */ attr = FOREGROUND_BLUE|BACKGROUND_GREEN; @@ -1070,8 +1070,8 @@ static void test_new_screen_buffer_color_attributes(HANDLE hConOut) ok(csbi2.wAttributes == attr, "Character Attributes should have been copied: " "got %#x, expected %#x\n", csbi2.wAttributes, attr); - todo_wine ok(csbi2.wPopupAttributes != orig_popup, "Popup Attributes should not match original value\n"); - todo_wine ok(csbi2.wPopupAttributes == attr, "Popup Attributes should match Character Attributes\n"); + ok(csbi2.wPopupAttributes != orig_popup, "Popup Attributes should not match original value\n"); + ok(csbi2.wPopupAttributes == attr, "Popup Attributes should match Character Attributes\n"); ret = SetConsoleTextAttribute(hConOut, orig_attr); ok(ret, "SetConsoleTextAttribute failed: error %u\n", GetLastError()); @@ -1094,8 +1094,8 @@ static void test_new_screen_buffer_color_attributes(HANDLE hConOut) ok(csbi2.wAttributes == orig_attr, "Character Attributes should have been copied: " "got %#x, expected %#x\n", csbi2.wAttributes, orig_attr); - todo_wine ok(csbi2.wPopupAttributes != orig_popup, "Popup Attributes should not match original value\n"); - todo_wine ok(csbi2.wPopupAttributes == orig_attr, "Popup Attributes should match Character Attributes\n"); + ok(csbi2.wPopupAttributes != orig_popup, "Popup Attributes should not match original value\n"); + ok(csbi2.wPopupAttributes == orig_attr, "Popup Attributes should match Character Attributes\n"); csbi.wPopupAttributes = orig_popup; ret = SetConsoleScreenBufferInfoEx(hConOut, &csbi); diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index 805df880897..93944d02e9c 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -85,7 +85,6 @@ static struct screen_buffer *create_screen_buffer( struct console *console, int screen_buffer->cursor_visible = 1; screen_buffer->width = width; screen_buffer->height = height; - screen_buffer->popup_attr = 0xf5; screen_buffer->font.weight = FW_NORMAL; screen_buffer->font.pitch_family = FIXED_PITCH | FF_DONTCARE; @@ -96,6 +95,7 @@ static struct screen_buffer *create_screen_buffer( struct console *console, int screen_buffer->win.right = console->active->win.right - console->active->win.left; screen_buffer->win.bottom = console->active->win.bottom - console->active->win.top; screen_buffer->attr = console->active->attr; + screen_buffer->popup_attr = console->active->attr; } else { @@ -104,6 +104,7 @@ static struct screen_buffer *create_screen_buffer( struct console *console, int screen_buffer->win.right = width - 1; screen_buffer->win.bottom = height - 1; screen_buffer->attr = FOREGROUND_BLUE|FOREGROUND_GREEN|FOREGROUND_RED; + screen_buffer->popup_attr = screen_buffer->attr; } if (wine_rb_put( &screen_buffer_map, LongToPtr(id), &screen_buffer->entry )) -- 2.32.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=94408 Your paranoid android. === w2008s64 (64 bit report) === kernel32: console.c:3304: Test failed: expected INVALID_FILE_SIZE, got 0x1000
Hi Hugh, On 7/22/21 3:36 PM, Hugh McMaster wrote:
@@ -104,6 +104,7 @@ static struct screen_buffer *create_screen_buffer( struct console *console, int screen_buffer->win.right = width - 1; screen_buffer->win.bottom = height - 1; screen_buffer->attr = FOREGROUND_BLUE|FOREGROUND_GREEN|FOREGROUND_RED; + screen_buffer->popup_attr = screen_buffer->attr;
This part doesn't seem right. For windowed consoles this part is overwritten when we read the config, but it's not the case for pseudo consoles (or when we use Unix console). This breaks the attached test. Thanks, Jacek
Hi Jacek, On Fri, 23 Jul 2021 at 22:24, Jacek Caban wrote:
Hi Hugh,
On 7/22/21 3:36 PM, Hugh McMaster wrote:
@@ -104,6 +104,7 @@ static struct screen_buffer *create_screen_buffer( struct console *console, int screen_buffer->win.right = width - 1; screen_buffer->win.bottom = height - 1; screen_buffer->attr = FOREGROUND_BLUE|FOREGROUND_GREEN|FOREGROUND_RED; + screen_buffer->popup_attr = screen_buffer->attr;
This part doesn't seem right. For windowed consoles this part is overwritten when we read the config, but it's not the case for pseudo consoles (or when we use Unix console). This breaks the attached test.
diff --git a/dlls/kernel32/tests/console.c b/dlls/kernel32/tests/console.c index 6ee00aa49a6..6e53355f6e5 100644 --- a/dlls/kernel32/tests/console.c +++ b/dlls/kernel32/tests/console.c @@ -4352,6 +4352,7 @@ static void test_AllocConsole(void)
static void test_pseudo_console_child(HANDLE input, HANDLE output) { + CONSOLE_SCREEN_BUFFER_INFOEX sb_infoex; CONSOLE_SCREEN_BUFFER_INFO sb_info; CONSOLE_CURSOR_INFO cursor_info; DWORD mode; @@ -4402,6 +4403,13 @@ static void test_pseudo_console_child(HANDLE input, HANDLE output) ok(sb_info.srWindow.Bottom == 29, "srWindow.Bottom = %u\n", sb_info.srWindow.Bottom); ok(sb_info.dwMaximumWindowSize.X == 40, "dwMaximumWindowSize.X = %u\n", sb_info.dwMaximumWindowSize.X); ok(sb_info.dwMaximumWindowSize.Y == 30, "dwMaximumWindowSize.Y = %u\n", sb_info.dwMaximumWindowSize.Y); + ok(sb_info.wAttributes == 0x7, "wAttributes = %#x\n", sb_info.wAttributes); + + sb_infoex.cbSize = sizeof(sb_infoex); + ret = GetConsoleScreenBufferInfoEx(output, &sb_infoex); + ok(ret, "GetConsoleScreenBufferInfo failed: %u\n", GetLastError()); + ok(sb_infoex.wAttributes == 0x7, "wAttributes = %#x\n", sb_infoex.wAttributes); + ok(sb_infoex.wPopupAttributes == 0xf5, "wPopupAttributes = %#x\n", sb_infoex.wPopupAttributes);
ret = GetConsoleCursorInfo(output, &cursor_info); ok(ret, "GetConsoleCursorInfo failed: %u\n", GetLastError());
What is this testing? Currently, it just tells us what the color attributes and popup attributes are. There's nothing unexpected here. I ran a test using the --pseudo-console flag while creating a new screen buffer [1] and found the popup attributes take on the same value as the color attributes, in line with the other tests and patch. How should I be testing this? Hugh [1] https://testbot.winehq.org/JobDetails.pl?Key=94508
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=94407 Your paranoid android. === w2008s64 (64 bit report) === kernel32: console.c:3304: Test failed: expected INVALID_FILE_SIZE, got 0x1000
participants (3)
-
Hugh McMaster -
Jacek Caban -
Marvin