[PATCH v4 0/5] MR8969: opengl32, win32u: Fix 16-bit PFD_DRAW_TO_BITMAP rendering.
This MR fixes 16-bit PFD_DRAW_TO_BITMAP rendering, as tested with SimGolf and Civilization 3. The fix isn't super performant since it involves a lot of back-and-forth between CPU and GPU, which is mostly noticeable when moving staff around in SimGolf — it slows to around two or three fps —, but on the plus side it does work, which is a straight upgrade over the current version of Wine. Couple of notes: * The third commit touches several different subsystems because I had to add a constant that's visible from both opengl32 and win32u. I wasn't sure how to write this down in the commit message. * There's a bug in LLVMpipe where a 32-bit pbuffer will cause it to crash. I opened an issue for it here: https://gitlab.freedesktop.org/mesa/mesa/-/issues/13890. I needed a 32-bit pbuffer to make this work, so I added a special check that effectively disables 16-bit bitmap rendering for LLVMpipe. * I also had to use a workaround to make blitting & OpenGL rendering work together. I wrote about it pretty extensively below. ## Combining blitting & OpenGL rendering Something that's difficult about bitmap rendering is that it allows programs to combine OpenGL drawing with direct blitting onto bitmap memory. That's difficult to imitate with the pbuffer-based implementation because there's no way to replicate memory edits on the bitmap to the GPU-based pbuffer (since there's no API calls involved in the memory edit). This had to work to make the games actually work, so I used this workaround: Each time after overwriting the bitmap from the pbuffer, we clear the pbuffer with transparent. Also instead of "overwriting" the bitmap, we instead blend the pbuffer's pixels onto the bitmap. That way we're effectively overlaying the most recent OpenGL drawings on top of the bitmap. I'm pretty confident that that approach would also help with 32-bit and 24-bit bitmap rendering, but I don't know of any programs that use 32-bit or 24-bit bitmap rendering, so I wasn't able to check. I've just left 32-bit and 24-bit bitmaps as they were. While the workaround works really well for the games, I did have to edit another test because it used glReadPixels on 16-bit bitmaps. glReadPixels will grab pixels from the transparent-background pbuffer instead of the bitmap, which gives the wrong result. I'm not under the impression that any consumer programs use bitmap rendering together with glReadPixels (it doesn't make as much sense when you could just directly read the pixels from the bitmap), but it's possible to add a wrap_glReadPixels that grabs pixels from the bitmap in case of a 16-bit memory DC. It's not important to read but I wanted to write it down for the record: I also tried another workaround to make combining blitting & OpenGL work. The thought was to overwrite the pbuffer with the bitmap just before OpenGL draws to the pbuffer, to keep them in sync. It probably would have worked out if it was possible to add something to the front of the OpenGL command buffer in a glFinish/glFlush call — then I could have inserted glDrawPixels in the front, before all of the program's rendering operations —, but it's only possible to add things to the end of the command buffer unfortunately. I tried adding a wrapper to glBegin and glClear, which checks if they were the first render operation after a glFlush or glFinish. Unfortunately that didn't work at all. I'm guessing that Civ 3 and SimGolf still do direct bitmap edits after the first call to glBegin, so by the time the program gets to glFinish/glFlush the pixels that were uploaded in the glDrawPixels command are already stale (although I didn't really check). Anyhow, that approach was pretty fruitless. -- v4: win32u: Fix garbled 24-bit bitmap rendering win32u: Add support for 16-bit bitmaps to set_dc_pixel_format and flush_memory_dc. opengl32: Return a fake pixel format when requesting 16-bit bitmap rendering. opengl32/tests: Extend 16-bit bitmap rendering test. winex11: Refine PFD_DRAW_TO_BITMAP requirements. https://gitlab.winehq.org/wine/wine/-/merge_requests/8969
From: Zowie van Dillen <zowie+wine(a)vandillen.io> Previously all pixel formats whose bpp was not 24 were rejected for bitmap capability, because 32-bit pbuffers lead to a crash in LLVMpipe. I need a 32-bit pbuffer for fake 16-bit bitmap rendering, and therefore I've narrowed the 24-bpp requirement down to applying specifically to LLVMpipe. --- dlls/winex11.drv/opengl.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c index b810689ff3d..3f90dad4a73 100644 --- a/dlls/winex11.drv/opengl.c +++ b/dlls/winex11.drv/opengl.c @@ -184,6 +184,7 @@ static const char *glxExtensions; static char wglExtensions[4096]; static int glxVersion[2]; static int glx_opcode; +static BOOL is_llvmpipe = FALSE; struct glx_pixel_format { @@ -408,6 +409,7 @@ static BOOL X11DRV_WineGL_InitOpenglInfo(void) glxExtensions = pglXQueryExtensionsString(gdi_display, screen); glx_direct = pglXIsDirect(gdi_display, ctx); + is_llvmpipe = (strstr(gl_renderer, "llvmpipe") != NULL); TRACE("GL version : %s.\n", gl_version); TRACE("GL renderer : %s.\n", gl_renderer); @@ -735,18 +737,31 @@ static int get_render_type_from_fbconfig(Display *display, GLXFBConfig fbconfig) /* Check whether a fbconfig is suitable for Windows-style bitmap rendering */ static BOOL check_fbconfig_bitmap_capability( GLXFBConfig fbconfig, const XVisualInfo *vis ) { - int dbuf, value; - - pglXGetFBConfigAttrib( gdi_display, fbconfig, GLX_BUFFER_SIZE, &value ); - if (vis && value != vis->depth) return FALSE; - + int dbuf, drawable, bpp; + pglXGetFBConfigAttrib( gdi_display, fbconfig, GLX_BUFFER_SIZE, &bpp ); + + /* Besides 24-bit and 32-bit, WGL supports 4-bit, 8-bit and 16-bit + * DRAW_TO_BITMAP rendering. Out of those three, Wine only supports 16-bit. + * I'm not sure if the other two are actually used by consumer software. + * Our 16-bit bitmap support is faked and uses 32-bit pbuffers under + * the hood (because WGL uses r5_g5_b5 with the highest bit always being + * zero, which is not a format that many GPU drivers support). Because we + * don't support using real 16-bit pixel formats for bitmap rendering, we + * only let 24-bit and 32-bit pixel formats through. */ + if (bpp < 24) return FALSE; + + /* In llvmpipe, there's currently a bug where pbuffers that aren't 24 bpp + * will cause a crash upon glFinish/glFlush. Once it's fixed, consider + * adding an llvmpipe version check to this code. Here's the bug report: + * https://gitlab.freedesktop.org/mesa/mesa/-/issues/13890 */ + if (is_llvmpipe && vis && bpp != vis->depth) return FALSE; + + /* Windows only supports bitmap rendering on single buffered formats. The + * fbconfig also needs to support pbuffers, because Wine's implementation + * of bitmap rendering uses pbuffers. */ pglXGetFBConfigAttrib( gdi_display, fbconfig, GLX_DOUBLEBUFFER, &dbuf ); - pglXGetFBConfigAttrib(gdi_display, fbconfig, GLX_DRAWABLE_TYPE, &value); - - /* Windows only supports bitmap rendering on single buffered formats. The fbconfig also needs to - * have the GLX_PBUFFER_BIT set, because Wine's implementation of bitmap rendering uses - * pbuffers. */ - return !dbuf && (value & GLX_PBUFFER_BIT); + pglXGetFBConfigAttrib(gdi_display, fbconfig, GLX_DRAWABLE_TYPE, &drawable); + return !dbuf && (drawable & GLX_PBUFFER_BIT); } static UINT x11drv_init_pixel_formats( UINT *onscreen_count ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8969
From: Zowie van Dillen <zowie+wine(a)vandillen.io> --- dlls/opengl32/tests/opengl.c | 162 +++++++++++++++++++++++++++-------- 1 file changed, 125 insertions(+), 37 deletions(-) diff --git a/dlls/opengl32/tests/opengl.c b/dlls/opengl32/tests/opengl.c index 55125722297..8c3077d7e8e 100644 --- a/dlls/opengl32/tests/opengl.c +++ b/dlls/opengl32/tests/opengl.c @@ -1560,7 +1560,9 @@ static void test_bitmap_rendering( BOOL use_dib ) bmi.bmiHeader.biWidth = 12; bmi.bmiHeader.biHeight = -12; - bmi.bmiHeader.biBitCount = 16; + /* biBitCount used to be 16 for the second bitmap, but our fake 16-bit bitmap + * rendering approach is a little bit hacky and doesn't work well with glReadPixels. */ + bmi.bmiHeader.biBitCount = 32; bmp2 = CreateDIBSection( 0, &bmi, DIB_RGB_COLORS, (void **)&pixels2, NULL, 0 ); memset( (void *)pixels2, 0xdc, sizeof(*pixels2) * 12 * 12 ); } @@ -1787,8 +1789,7 @@ static void test_bitmap_rendering( BOOL use_dib ) if (pixels == buffer) read_bitmap_pixels( hdc, bmp, pixels, 4, 4, bpp ); if (pixels2 == buffer2) read_bitmap_pixels( hdc, bmp2, pixels2, 12, 12, bpp ); ok( (pixels[0] & 0xffffff) == 0x223344, "got %#x\n", pixels[0] ); - if (use_dib) todo_wine ok( (pixels2[0] & 0xffffff) == 0x03148, "got %#x\n", pixels2[0] ); - else ok( (pixels2[0] & 0xffffff) == 0x665544, "got %#x\n", pixels2[0] ); + ok( (pixels2[0] & 0xffffff) == 0x665544, "got %#x\n", pixels2[0] ); ret = wglMakeCurrent( hdc, hglrc ); ok( ret, "wglMakeCurrent failed, error %lu\n", GetLastError() ); @@ -1823,8 +1824,7 @@ static void test_bitmap_rendering( BOOL use_dib ) if (pixels == buffer) read_bitmap_pixels( hdc, bmp, pixels, 4, 4, bpp ); if (pixels2 == buffer2) read_bitmap_pixels( hdc, bmp2, pixels2, 12, 12, bpp ); - if (use_dib) todo_wine ok( (pixels[0] & 0xffffff) == 0x45cc, "got %#x\n", pixels[0] ); - else ok( (pixels[0] & 0xffffff) == 0x887766, "got %#x\n", pixels[0] ); + ok( (pixels[0] & 0xffffff) == 0x887766, "got %#x\n", pixels[0] ); ok( (pixels2[0] & 0xffffff) == 0x667788, "got %#x\n", pixels2[0] ); wglDeleteContext( hglrc2 ); @@ -1838,6 +1838,39 @@ static void test_bitmap_rendering( BOOL use_dib ) winetest_pop_context(); } +static void ensure_16bit_bitmap_matches(const USHORT actual[], const USHORT expected[], int width, int height) +{ + /* The WGL software renderer only implements r5_g5_b5 for 16-bit bitmaps, + * with the highest bit always being zero. Because the highest bit is always + * zero, I'm reusing that bit to store whether or not a pixel is allowed to + * vary depending on the graphics driver. Between different GPUs, it's very + * inconsistent which exact pixels will be drawn, so we need to be lax. */ + USHORT may_differ_on_unix_bit = 0x8000; + int size = width * height; + + for (int i = 0; i < size; i++) + { + USHORT expected_pixel = expected[i] & ~may_differ_on_unix_bit; + BOOL may_differ = (expected[i] & may_differ_on_unix_bit) != 0; + BOOL matches = (actual[i] == expected_pixel); + BOOL highest_bit_wrongly_set = (actual[i] & may_differ_on_unix_bit) != 0; + int x = i % width; + int y = i / height; + + if (!matches && may_differ && winetest_platform_is_wine + && !highest_bit_wrongly_set) + { + skip("Pixel (%d,%d) is different from Windows, but it's allowed to " + "vary. Got %#x, expected %#x\n", + x, y, actual[i], expected_pixel); + continue; + } + + ok(matches, "Wrong color at (%d,%d). Got %#x, expected %#x\n", + x, y, actual[i], expected_pixel); + } +} + static void test_16bit_bitmap_rendering(void) { PIXELFORMATDESCRIPTOR pfd; @@ -1898,22 +1931,24 @@ static void test_16bit_bitmap_rendering(void) * the program does (DRAW_TO_BITMAP is normally used in combination with blitting). */ success = DescribePixelFormat(hdc, pixel_format, sizeof(pfd), &pfd); ok(success != 0, "Failed to DescribePixelFormat (error: %lu)\n", GetLastError()); - /* Likely MSDN inaccuracy: According to the PIXELFORMATDESCRIPTOR docs, alpha bits are excluded - * from cColorBits. It doesn't seem like that's true. */ ok(pfd.cColorBits == 16, "Wrong amount of color bits (got %d, expected 16)\n", pfd.cColorBits); todo_wine ok(pfd.cRedBits == 5, "Wrong amount of red bits (got %d, expected 5)\n", pfd.cRedBits); todo_wine ok(pfd.cGreenBits == 5, "Wrong amount of green bits (got %d, expected 5)\n", pfd.cGreenBits); todo_wine ok(pfd.cBlueBits == 5, "Wrong amount of blue bits (got %d, expected 5)\n", pfd.cBlueBits); - /* Quirky: It seems that there's an alpha bit, but it somehow doesn't count as one for - * DescribePixelFormat. On Windows cAlphaBits is zero. - * ok(pfd.cAlphaBits == 1, "Wrong amount of alpha bits (got %d, expected 1)\n", pfd.cAlphaBits); */ todo_wine ok(pfd.cRedShift == 10, "Wrong red shift (got %d, expected 10)\n", pfd.cRedShift); todo_wine ok(pfd.cGreenShift == 5, "Wrong green shift (got %d, expected 5)\n", pfd.cGreenShift); - /* This next test might fail, depending on your drivers. */ ok(pfd.cBlueShift == 0, "Wrong blue shift (got %d, expected 0)\n", pfd.cBlueShift); success = SetPixelFormat(hdc, pixel_format, &pixel_format_args); - ok(success, "Failed to SetPixelFormat (error: %lu)\n", GetLastError()); + + if (!success) + { + /* This skip is here for LLVMpipe, which currently does not support + * 16-bit or 32-bit pbuffers correctly. Here's the bug report: + * https://gitlab.freedesktop.org/mesa/mesa/-/issues/13890 */ + skip("Skipping 16-bit bitmap tests because SetPixelFormat failed.\n"); + return; + } /* Create an OpenGL context. */ gl = wglCreateContext(hdc); @@ -1921,12 +1956,40 @@ static void test_16bit_bitmap_rendering(void) success = wglMakeCurrent(hdc, gl); ok(success, "Failed to wglMakeCurrent (error: %lu)\n", GetLastError()); - /* Try setting the bitmap to white. */ - glClearColor(1.0f, 1.0f, 1.0f, 1.0f); + /* Try setting the bitmap to pure blue. */ + glClearColor(0.0f, 0.0f, 1.0f, 1.0f); glClear(GL_COLOR_BUFFER_BIT); glFinish(); - todo_wine ok(pixels[0] == 0x7fff, "Wrong color after glClear at (0, 0): %#x\n", pixels[0]); - todo_wine ok(pixels[1] == 0x7fff, "Wrong color after glClear at (1, 0): %#x\n", pixels[1]); + for (int i = 0; i < 16; i++) + { + int x = i % 4; + int y = i / 4; + ok(pixels[i] == 0x001f, "Wrong color at (%d,%d). Got %#x, expected %#x\n", + x, y, pixels[i], 0x001f); + } + + /* Try setting the bitmap to a color that uses all color channels. + * I've noticed that clearing the canvas with a color like (0.2, 0.4, 0.8) + * gives inconsistent results on Windows. About half of the pixels are + * 0x1999 as they should be, and others are 0x1998, or 0x19b9, or 0x1db9. + * So the lowest bit of each color channel varies. Maybe it's dithering? */ + glClearColor(0.2f, 0.4f, 0.8f, 1.0f); + glClear(GL_COLOR_BUFFER_BIT); + glFinish(); + + { + const USHORT mask = 0xfbde; /* Mask out the lowest bit of each channel */ + const USHORT expected = 0x1999 & mask; + + for (int i = 0; i < 16; i++) + { + int x = i % 4; + int y = i / 4; + ok((pixels[i] & mask) == expected, + "Wrong color at (%d,%d). Got %#x, expected %#x\n", + x, y, pixels[i], 0x1999); + } + } /* Try setting the bitmap to black with a white line. */ glMatrixMode(GL_PROJECTION); @@ -1941,34 +2004,59 @@ static void test_16bit_bitmap_rendering(void) glColor3f(1.0f, 1.0f, 1.0f); glLineWidth(1.0f); glBegin(GL_LINES); - glVertex2i(1, 1); - glVertex2i(1, 3); + glVertex2f(1.5f, 1.0f); + glVertex2f(1.5f, 3.0f); glEnd(); glFinish(); { - /* Note that the line stops at (1,2) on Windows despite the second vertex being (1,3). - * I'm not sure if that's an implementation quirk or expected OpenGL behaviour. */ USHORT X = 0x7fff, _ = 0x0; - USHORT expected[16] = { - _,_,_,_, - _,X,_,_, - _,X,_,_, - _,_,_,_ - }; + USHORT x = 0xffff, o = 0x8000; /* Optional match outside of Windows */ + winetest_push_context( "Line bitmap" ); + ensure_16bit_bitmap_matches( + pixels, (USHORT[]) { + _,_,_,_, + _,x,_,_, + _,X,_,_, + _,o,_,_ + }, 4, 4 + ); + winetest_pop_context(); + } - for (int i = 0; i < 16; i++) - { - BOOL matches = (pixels[i] == expected[i]); - int x = i % 4; - int y = i / 4; - /* I'm using a loop so that I can put the expected image in an easy-to-understand array. - * Unfortunately this way of working doesn't work great with `todo_wine` since only half - * of the elements are a mismatch. I'm using `todo_wine_if` as a workaround. */ - todo_wine_if(!matches) ok(matches, "Wrong color at (%d,%d). Got %#x, expected %#x\n", - x, y, pixels[i], expected[i]); - } + /* Without clearing, edit the bitmap directly and draw another line. */ + pixels[0] = 0x001f; /* Set a pixel to red. Note that the bitmap is BGR. */ + + glBegin(GL_LINES); + glVertex2f(2.5f, -10.0f); + /* Another quirk, or more like a bug in Windows: If you draw a line that + * goes through the top side of clip space, it looks like the line gets + * clipped one pixel too soon, leaving one pixel undrawn. That's not + * something we can likely replicate, so I've left it out. (I haven't + * tested that hypothesis, so the bug might be something else, but one pixel + * wasn't drawn.) + * glVertex2f(2.5f, 10.0f); */ + glVertex2f(2.5f, 3.0f); + glEnd(); + + pixels[15] = 0x7c00; /* Set a pixel to blue. */ + + glFinish(); + + { + USHORT X = 0x7fff, _ = 0x0, R = 0x001f, B = 0x7c00; + USHORT x = 0xffff, o = 0x8000; /* Optional match outside of Windows */ + winetest_push_context( "Blit+OpenGL bitmap" ); + ensure_16bit_bitmap_matches( + pixels, (USHORT[]) { + R,_,_,_, + _,x,x,_, + _,X,X,_, + _,o,X,B + }, 4, 4 + ); + winetest_pop_context(); } /* Clean up. */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8969
From: Zowie van Dillen <zowie+wine(a)vandillen.io> This commit does not yet actually implement the fake pixel format: wglSetPixelFormat will still fail. --- dlls/opengl32/tests/opengl.c | 12 ++++++------ dlls/opengl32/wgl.c | 33 +++++++++++++++++++++++++++++++++ include/wine/opengl_driver.h | 6 ++++++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/dlls/opengl32/tests/opengl.c b/dlls/opengl32/tests/opengl.c index 8c3077d7e8e..82e8c1cfd9f 100644 --- a/dlls/opengl32/tests/opengl.c +++ b/dlls/opengl32/tests/opengl.c @@ -1911,7 +1911,7 @@ static void test_16bit_bitmap_rendering(void) /* Choose a pixel format. */ pixel_format = ChoosePixelFormat(hdc, &pixel_format_args); - todo_wine ok(pixel_format != 0, "Failed to get a 16 bit pixel format with the DRAW_TO_BITMAP flag.\n"); + ok(pixel_format != 0, "Failed to get a 16 bit pixel format with the DRAW_TO_BITMAP flag.\n"); if (pixel_format == 0) { @@ -1932,11 +1932,11 @@ static void test_16bit_bitmap_rendering(void) success = DescribePixelFormat(hdc, pixel_format, sizeof(pfd), &pfd); ok(success != 0, "Failed to DescribePixelFormat (error: %lu)\n", GetLastError()); ok(pfd.cColorBits == 16, "Wrong amount of color bits (got %d, expected 16)\n", pfd.cColorBits); - todo_wine ok(pfd.cRedBits == 5, "Wrong amount of red bits (got %d, expected 5)\n", pfd.cRedBits); - todo_wine ok(pfd.cGreenBits == 5, "Wrong amount of green bits (got %d, expected 5)\n", pfd.cGreenBits); - todo_wine ok(pfd.cBlueBits == 5, "Wrong amount of blue bits (got %d, expected 5)\n", pfd.cBlueBits); - todo_wine ok(pfd.cRedShift == 10, "Wrong red shift (got %d, expected 10)\n", pfd.cRedShift); - todo_wine ok(pfd.cGreenShift == 5, "Wrong green shift (got %d, expected 5)\n", pfd.cGreenShift); + ok(pfd.cRedBits == 5, "Wrong amount of red bits (got %d, expected 5)\n", pfd.cRedBits); + ok(pfd.cGreenBits == 5, "Wrong amount of green bits (got %d, expected 5)\n", pfd.cGreenBits); + ok(pfd.cBlueBits == 5, "Wrong amount of blue bits (got %d, expected 5)\n", pfd.cBlueBits); + ok(pfd.cRedShift == 10, "Wrong red shift (got %d, expected 10)\n", pfd.cRedShift); + ok(pfd.cGreenShift == 5, "Wrong green shift (got %d, expected 5)\n", pfd.cGreenShift); ok(pfd.cBlueShift == 0, "Wrong blue shift (got %d, expected 0)\n", pfd.cBlueShift); success = SetPixelFormat(hdc, pixel_format, &pixel_format_args); diff --git a/dlls/opengl32/wgl.c b/dlls/opengl32/wgl.c index 6125d9a4574..54a9ea5f1c4 100644 --- a/dlls/opengl32/wgl.c +++ b/dlls/opengl32/wgl.c @@ -138,6 +138,16 @@ INT WINAPI wglChoosePixelFormat(HDC hdc, const PIXELFORMATDESCRIPTOR* ppfd) if (!NtGdiGetDCDword( hdc, NtGdiIsMemDC, &is_memdc )) is_memdc = 0; + if (is_memdc && ppfd->cColorBits == 16) + { + /* 16-bit-per-pixel memory DCs are a special case. Windows uses a software renderer with a + * quirky pixel format for this. That pixel format is only rarely supported on actual GPUs. + * We return a special value, which we'll later check for in SelectPixelFormat. + * Check `test_16bit_bitmap_rendering` for more info. */ + TRACE( "Returning special 16-bit pixel format %d\n", FAKE_16BIT_MEMDC_PIXEL_FORMAT ); + return FAKE_16BIT_MEMDC_PIXEL_FORMAT; + } + best_format = 0; best.dwFlags = 0; best.cAlphaBits = -1; @@ -820,6 +830,29 @@ INT WINAPI wglDescribePixelFormat( HDC hdc, int index, UINT size, PIXELFORMATDES if (!(formats = get_pixel_formats( hdc, &num_formats, &num_onscreen_formats ))) return 0; if (!ppfd) return num_onscreen_formats; + + if (index == FAKE_16BIT_MEMDC_PIXEL_FORMAT) + { + /* 16-bit memory DCs are a special case where we need to fake the pixel format. + * Check `test_16bit_bitmap_rendering` for more info. */ + *ppfd = (PIXELFORMATDESCRIPTOR) { + .nSize = sizeof(PIXELFORMATDESCRIPTOR), + .nVersion = 1, + .dwFlags = PFD_DRAW_TO_BITMAP | PFD_SUPPORT_OPENGL, + .iPixelType = PFD_TYPE_RGBA, + .iLayerType = PFD_MAIN_PLANE, + .cColorBits = 16, + .cRedBits = 5, + .cGreenBits = 5, + .cBlueBits = 5, + .cAlphaBits = 0, + .cRedShift = 10, + .cGreenShift = 5, + .cBlueShift = 0, + }; + return num_onscreen_formats; + } + if (size < sizeof(*ppfd)) return 0; if (index <= 0 || index > num_onscreen_formats) return 0; diff --git a/include/wine/opengl_driver.h b/include/wine/opengl_driver.h index 296b030021d..bdc87fdd83c 100644 --- a/include/wine/opengl_driver.h +++ b/include/wine/opengl_driver.h @@ -59,6 +59,12 @@ struct wgl_pixel_format int float_components; }; + +/* A special pixel format number, which is given by wglChoosePixelFormat when + * you ask for a 16 bpp format with PFD_DRAW_TO_BITMAP. */ +#define FAKE_16BIT_MEMDC_PIXEL_FORMAT 65000 + + #ifdef WINE_UNIX_LIB #include "wine/gdi_driver.h" -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8969
From: Zowie van Dillen <zowie+wine(a)vandillen.io> --- dlls/win32u/opengl.c | 151 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 148 insertions(+), 3 deletions(-) diff --git a/dlls/win32u/opengl.c b/dlls/win32u/opengl.c index fae9a71c988..760873ef824 100644 --- a/dlls/win32u/opengl.c +++ b/dlls/win32u/opengl.c @@ -1254,6 +1254,45 @@ static BOOL create_memory_pbuffer( HDC hdc ) return ret; } +/* Convert an r5_g5_b5 pixel to r8_g8_b8_a8, or a b5_g5_r5 pixel to b8_g8_r8_a8. + * The highest bit of `pixel` is ignored. The alpha bits of the result will be + * zero, and the lowest three bits of each color are the same as the highest + * three bits (that's important, because if we get 0b11111 as input (i.e. color + * fully on) we should also give 0b11111111 as output, rather than 0b11111000). + * For reference, the bits are converted like this: + * ?rrrrrgggggbbbbb -> 00000000rrrrrrrrggggggggbbbbbbbb + * |12 |8 |4 |0 |28 |24 |20 |16 |12 |8 |4 |0 */ +static UINT convert_555_pixel_to_888(USHORT pixel) +{ + UINT red = (pixel & 0x7c00) << (16 + 3 - 10) + | (pixel & 0x7000) << (16 - 2 - 10); + UINT green = (pixel & 0x03e0) << (8 + 3 - 5 ) + | (pixel & 0x0380) << (8 - 2 - 5 ); + UINT blue = (pixel & 0x001f) << (0 + 3 - 0 ) + | (pixel & 0x001c) >> ( 2 ); + return red | green | blue; +} + +/* Blends an r8_g8_b8_a8 pixel onto an r5_b5_g5 pixel, and gives the result as + * an r5_b5_g5 pixel. */ +static USHORT blend_8888_pixel_onto_555(UINT pixel, USHORT old_pixel) { + float alpha = ((pixel >> 24) & 0xff) / 256.0f; + float red = ((pixel >> 16) & 0xff) / 256.0f; + float green = ((pixel >> 8 ) & 0xff) / 256.0f; + float blue = ((pixel >> 0 ) & 0xff) / 256.0f; + float old_red = ((old_pixel >> 10) & 0x1f) / 32.0f; + float old_green = ((old_pixel >> 5 ) & 0x1f) / 32.0f; + float old_blue = ((old_pixel >> 0 ) & 0x1f) / 32.0f; + + red = red * alpha + old_red * (1 - alpha); + green = green * alpha + old_green * (1 - alpha); + blue = blue * alpha + old_blue * (1 - alpha); + + return ((int)(red * 32.0f) & 0x1f) << 10 + | ((int)(green * 32.0f) & 0x1f) << 5 + | ((int)(blue * 32.0f) & 0x1f); +} + static BOOL flush_memory_dc( struct wgl_context *context, HDC hdc, BOOL write, void (*flush)(void) ) { const struct opengl_funcs *funcs = &display_funcs; @@ -1274,9 +1313,70 @@ static BOOL flush_memory_dc( struct wgl_context *context, HDC hdc, BOOL write, v if (!get_image_from_bitmap( bmp, info, &bits, &src )) { - int width = info->bmiHeader.biWidth, height = info->bmiHeader.biSizeImage / 4 / width; - if (write) funcs->p_glDrawPixels( width, height, GL_BGRA, GL_UNSIGNED_BYTE, bits.ptr ); - else funcs->p_glReadPixels( 0, 0, width, height, GL_BGRA, GL_UNSIGNED_BYTE, bits.ptr ); + /* Depending on the `write` parameter, either overwrite the bitmap from the + * GL canvas or vice versa. + * Note: biHeight is negative if the image origin is the top-left. */ + int width = info->bmiHeader.biWidth, height = abs( info->bmiHeader.biHeight ); + int pixel_count = width * height; + + if (info->bmiHeader.biBitCount == 16) + { + /* Special case: 16 bpp bitmap. + * The GDI Generic software renderer only implements r5g5b5, with the most + * significant bit being zero. Most OpenGL drivers do not implement this pixel + * format, so for portability, we render to a 32 bpp OpenGL context and then convert + * it to r5g5b5 on the CPU. + * Note: Doing this malloc each flush isn't exactly efficient, but (according to my + * intuition) its performance impact is dwarfed by the cost of glReadPixels + * later on. A matter of maybe microseconds versus tens of milliseconds. */ + UINT *temp_image = malloc( 4 * pixel_count ); + USHORT *bitmap_pixels = bits.ptr; + + if (temp_image && write) + { + for (int i = 0; i < pixel_count; i++) + temp_image[i] = convert_555_pixel_to_888( bitmap_pixels[i] ); + funcs->p_glDrawPixels( width, height, GL_BGRA, GL_UNSIGNED_BYTE, temp_image ); + } + else if (temp_image) + { + float clear_color[4]; + + /* Note: Despite asking for BGRA we actually get ABGR, with + * the alpha bits being the eight highest bits. */ + funcs->p_glReadPixels( 0, 0, width, height, GL_BGRA, GL_UNSIGNED_BYTE, temp_image ); + for (int i = 0; i < pixel_count; i++) + bitmap_pixels[i] = blend_8888_pixel_onto_555( temp_image[i], bitmap_pixels[i] ); + + /* Something difficult to fake about bitmap rendering is that a program can use + * a mix of blitting and OpenGL operations. Civilization 3 and SimGolf depend on + * this. There's no easy way to replicate the direct memory edits on the + * GPU-based pbuffer (since there's no API call), so instead I'm using a + * workaround: Each time after we draw the pbuffer onto the bitmap, we clear the + * pbuffer with transparent. In the end, this gives you the same bitmap as a + * result (unless you call glReadPixels, which will give you pixels from the + * transparent-background pbuffer). */ + + /* Remember the old clear color (rather inefficient). */ + funcs->p_glClear( GL_COLOR_BUFFER_BIT ); + funcs->p_glReadPixels( 0, 0, 1, 1, GL_RGBA, GL_FLOAT, clear_color ); + + /* Set the color buffer to transparent. */ + funcs->p_glClearColor( 0.0f, 0.0f, 0.0f, 0.0f ); + funcs->p_glClear( GL_COLOR_BUFFER_BIT ); + + /* Restore the old clear color. */ + funcs->p_glClearColor( clear_color[0], clear_color[1], clear_color[2], clear_color[3] ); + } + + free( temp_image ); + } + else + { + /* Normal case: 24 bpp or 32 bpp bitmap. */ + if (write) funcs->p_glDrawPixels( width, height, GL_BGRA, GL_UNSIGNED_BYTE, bits.ptr ); + else funcs->p_glReadPixels( 0, 0, width, height, GL_BGRA, GL_UNSIGNED_BYTE, bits.ptr ); + } } GDI_ReleaseObj( dc->hBitmap ); } @@ -1285,12 +1385,57 @@ static BOOL flush_memory_dc( struct wgl_context *context, HDC hdc, BOOL write, v return ret; } +static int find_bitmap_compatible_r8g8b8a8_pixel_format( void ) +{ + const struct opengl_funcs *funcs = &display_funcs; + struct wgl_pixel_format* formats; + UINT num_formats, num_onscreen; + + funcs->p_get_pixel_formats( NULL, 0, &num_formats, &num_onscreen ); + if (!(formats = calloc( num_formats, sizeof(*formats) ))) goto error; + funcs->p_get_pixel_formats( formats, num_formats, &num_formats, &num_onscreen ); + + for (int i = 1; i <= num_formats; i++) + { + const PIXELFORMATDESCRIPTOR pfd = formats[i].pfd; + + /* The pixel format must have eight bits for each color. We don't need to check the bitshift + * because it's always RGBA, and also because the Wine X11 driver doesn't actually check the + * bitshift, it just makes up some numbers, so even if it wasn't RGBA, we wouldn't be + * able to tell. */ + if (!(pfd.dwFlags & PFD_DRAW_TO_BITMAP) + || pfd.cRedBits != 8 || pfd.cGreenBits != 8 || pfd.cBlueBits != 8 || pfd.cAlphaBits != 8) + continue; + + free( formats ); + return i; + } + + error:; + WARN( "Unable to get bitmap-compatible r8g8b8a8 pixel format. " + "The program will likely not be able to initialize OpenGL.\n" ); + free( formats ); + return 0; +} + static BOOL set_dc_pixel_format( HDC hdc, int new_format, BOOL internal ) { const struct opengl_funcs *funcs = &display_funcs; UINT total, onscreen; HWND hwnd; + if (new_format == FAKE_16BIT_MEMDC_PIXEL_FORMAT) + { + /* 16-bit memory DCs are a special case where we need to fake the pixel format. + * Check `test_16bit_bitmap_rendering` for more info. */ + int rgba_format; + + TRACE( "Setting pixel format to fake format for 16-bit bitmaps. (hdc: %p)\n", hdc ); + if (!(rgba_format = find_bitmap_compatible_r8g8b8a8_pixel_format())) return FALSE; + + return NtGdiSetPixelFormat( hdc, rgba_format ); + } + funcs->p_get_pixel_formats( NULL, 0, &total, &onscreen ); if (new_format <= 0 || new_format > total) return FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8969
From: Zowie van Dillen <zowie+wine(a)vandillen.io> --- dlls/win32u/opengl.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/dlls/win32u/opengl.c b/dlls/win32u/opengl.c index 760873ef824..15bd98fb763 100644 --- a/dlls/win32u/opengl.c +++ b/dlls/win32u/opengl.c @@ -1371,11 +1371,16 @@ static BOOL flush_memory_dc( struct wgl_context *context, HDC hdc, BOOL write, v free( temp_image ); } - else + else if (info->bmiHeader.biBitCount == 24 || info->bmiHeader.biBitCount == 32) { /* Normal case: 24 bpp or 32 bpp bitmap. */ - if (write) funcs->p_glDrawPixels( width, height, GL_BGRA, GL_UNSIGNED_BYTE, bits.ptr ); - else funcs->p_glReadPixels( 0, 0, width, height, GL_BGRA, GL_UNSIGNED_BYTE, bits.ptr ); + UINT format = (info->bmiHeader.biBitCount == 24 ? GL_BGR : GL_BGRA); + if (write) funcs->p_glDrawPixels( width, height, format, GL_UNSIGNED_BYTE, bits.ptr ); + else funcs->p_glReadPixels( 0, 0, width, height, format, GL_UNSIGNED_BYTE, bits.ptr ); + } + else + { + WARN( "Unsupported bpp: %d", info->bmiHeader.biBitCount ); } } GDI_ReleaseObj( dc->hBitmap ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8969
v2: Integrated the 24-bit bitmap fix (otherwise you'd get an out-of-bounds write with a 24-bit bitmap), tiny changes in opengl32/tests (removed some old comments and fixed one small oversight that makes no practical difference) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116499
Rémi Bernon (@rbernon) commented about dlls/opengl32/wgl.c:
if (!(formats = get_pixel_formats( hdc, &num_formats, &num_onscreen_formats ))) return 0; if (!ppfd) return num_onscreen_formats; + + if (index == FAKE_16BIT_MEMDC_PIXEL_FORMAT)
I think we should fixup the 16bpp pixel formats in the list (for instance replacing RGB565 with RGBA5551), or extend, or hardcode the list, for memory DCs, rather than returning a fake index from only one of the functions. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116619
Rémi Bernon (@rbernon) commented about dlls/win32u/opengl.c:
+ * ?rrrrrgggggbbbbb -> 00000000rrrrrrrrggggggggbbbbbbbb + * |12 |8 |4 |0 |28 |24 |20 |16 |12 |8 |4 |0 */ +static UINT convert_555_pixel_to_888(USHORT pixel) +{ + UINT red = (pixel & 0x7c00) << (16 + 3 - 10) + | (pixel & 0x7000) << (16 - 2 - 10); + UINT green = (pixel & 0x03e0) << (8 + 3 - 5 ) + | (pixel & 0x0380) << (8 - 2 - 5 ); + UINT blue = (pixel & 0x001f) << (0 + 3 - 0 ) + | (pixel & 0x001c) >> ( 2 ); + return red | green | blue; +} + +/* Blends an r8_g8_b8_a8 pixel onto an r5_b5_g5 pixel, and gives the result as + * an r5_b5_g5 pixel. */ +static USHORT blend_8888_pixel_onto_555(UINT pixel, USHORT old_pixel) { Wouldn't it be easier to convert between RGB565 <-> RGB555 instead?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116620
Rémi Bernon (@rbernon) commented about dlls/win32u/opengl.c:
+ funcs->p_glReadPixels( 0, 0, width, height, GL_BGRA, GL_UNSIGNED_BYTE, temp_image ); + for (int i = 0; i < pixel_count; i++) + bitmap_pixels[i] = blend_8888_pixel_onto_555( temp_image[i], bitmap_pixels[i] ); + + /* Something difficult to fake about bitmap rendering is that a program can use + * a mix of blitting and OpenGL operations. Civilization 3 and SimGolf depend on + * this. There's no easy way to replicate the direct memory edits on the + * GPU-based pbuffer (since there's no API call), so instead I'm using a + * workaround: Each time after we draw the pbuffer onto the bitmap, we clear the + * pbuffer with transparent. In the end, this gives you the same bitmap as a + * result (unless you call glReadPixels, which will give you pixels from the + * transparent-background pbuffer). */ + + /* Remember the old clear color (rather inefficient). */ + funcs->p_glClear( GL_COLOR_BUFFER_BIT ); + funcs->p_glReadPixels( 0, 0, 1, 1, GL_RGBA, GL_FLOAT, clear_color ); I think this could be `funcs->p_glGetFloatv( GL_COLOR_CLEAR_VALUE, clear_color )`?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116621
Rémi Bernon (@rbernon) commented about dlls/win32u/opengl.c:
+ float clear_color[4]; + + /* Note: Despite asking for BGRA we actually get ABGR, with + * the alpha bits being the eight highest bits. */ + funcs->p_glReadPixels( 0, 0, width, height, GL_BGRA, GL_UNSIGNED_BYTE, temp_image ); + for (int i = 0; i < pixel_count; i++) + bitmap_pixels[i] = blend_8888_pixel_onto_555( temp_image[i], bitmap_pixels[i] ); + + /* Something difficult to fake about bitmap rendering is that a program can use + * a mix of blitting and OpenGL operations. Civilization 3 and SimGolf depend on + * this. There's no easy way to replicate the direct memory edits on the + * GPU-based pbuffer (since there's no API call), so instead I'm using a + * workaround: Each time after we draw the pbuffer onto the bitmap, we clear the + * pbuffer with transparent. In the end, this gives you the same bitmap as a + * result (unless you call glReadPixels, which will give you pixels from the + * transparent-background pbuffer). */ Hmm, so maybe RGB565 would make the trick with alpha bit more difficult. Another option I was thinking about would be to use the FBO-based surface instead of a pbuffer, now that we have it, with a GL_RGB5_A1 renderbuffer.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116622
On Mon Sep 22 09:29:28 2025 +0000, Rémi Bernon wrote:
> Hmm, so maybe RGB565 would make the trick with alpha bit more difficult.
> Another option I was thinking about would be to use the FBO-based
> surface instead of a pbuffer, now that we have it, with a GL_RGB5_A1 renderbuffer.
There's two good reasons to use an RGBA8888 secret buffer (that's what I'm going to be calling the GPU-based buffer from now on):
- Support for 16-bit pixel formats is pretty spotty. AMD's open source driver supports rgba5551 and rgb565, Nvidia supports only rgb565 and LLVMpipe does not support any 16-bit pixel formats (only rgba8888 it seems).
- Civ 3 actually relies on transparency, although it's subtle. In the screenshot below it's using OpenGL to draw a semi-transparent outline around the settler. Based on that I'm guessing that GDI OpenGL blends its drawings onto the bitmap with full floating point transparency. To support semi-transparent rendering we'll need to preserve the transparency until the secret buffer is drawn onto the bitmap.
<details>
<summary>Civilization 3 screenshot</summary>
{width=800 height=600}
</details>
I'll be sure to add a test with transparent rendering to make sure that's actually how GDI OpenGL works.
I'm not sure what you mean by using an FBO-based surface, since we're gonna need a drawable of some kind to bind the OpenGL context (although the drawable could be a 1x1 pbuffer I suppose). In any case GL_RGB5_A1 is not going to work with the semi-transparent line rendering.
I also made a pixel format dumper while working on this. Probably useful to share the print-outs:
- [Linux desktop, LLVMpipe, GLX](/uploads/11763ff3ed0d9f48022d5a1b1700a4e7/pf_desktop_llvmpipe) (by far the most restrictive)
- [Windows 10 VM, GDI generic, WGL](/uploads/127210e12fea60594423b76b051a4db9/pf_windows_gdigeneric_vm.txt)
- [Linux laptop, AMD open source, GLX](/uploads/8de9328ab29f9e0c77e2f8146f4ef6c5/pf_linux_amd_laptop.txt)
- [Linux desktop, NVidia, GLX](/uploads/eb7dd6d9ac6febfe78307db966be2181/pf_linux_nvidia.txt)
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116736
On Mon Sep 22 09:29:28 2025 +0000, Rémi Bernon wrote:
I think this could be `funcs->p_glGetFloatv( GL_COLOR_CLEAR_VALUE, clear_color )`? I feel a bit silly for not finding the easy way here. I'll have it fixed next revision.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116737
On Mon Sep 22 09:29:27 2025 +0000, Rémi Bernon wrote:
I think we should fixup the 16bpp pixel formats in the list (for instance replacing RGB565 with RGBA5551), or extend, or hardcode the list, for memory DCs, rather than returning a fake index from only one of the functions. Extending or hardcoding the list for memory DCs is probably the best option. Right now I've done it in a very ugly but easy way. I also looked at adding a fake pixel format to the list of pixel formats in winex11, but the list there had a whole bunch of GLX information for each format, which I couldn't really fill in. The PIXELFORMATDESCRIPTOR was derived from the GLX info, so I'd also need to add a whole bunch of special cases to make that work — and I might have needed to edit the other Wine drivers as well. Hence the ugly solution.
Hardcoding the PFD_DRAW_TO_BITMAP DCs seems less ugly, and GDI's software OpenGL only supports 28 different RGBA formats in total (see the attachment on the last thread). I don't think we need full coverage, so it shouldn't be too much work. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116740
On Tue Sep 23 07:35:00 2025 +0000, Zowie van Dillen wrote:
There's two good reasons to use an RGBA8888 secret buffer (that's what I'm going to be calling the GPU-based buffer from now on): - Support for 16-bit pixel formats is pretty spotty. AMD's open source driver supports rgba5551 and rgb565, Nvidia supports only rgb565 and LLVMpipe does not support any 16-bit pixel formats (only rgba8888 it seems). - Civ 3 actually relies on transparency, although it's subtle. In the screenshot below it's using OpenGL to draw a semi-transparent outline around the settler. Based on that I'm guessing that GDI OpenGL blends its drawings onto the bitmap with full floating point transparency. To support semi-transparent rendering we'll need to preserve the transparency until the secret buffer is drawn onto the bitmap. <details> <summary>Civilization 3 screenshot</summary> {width=800 height=600} </details> I'll be sure to add a test with transparent rendering to make sure that's actually how GDI OpenGL works. I'm not sure what you mean by using an FBO-based surface, since we're gonna need a drawable of some kind to bind the OpenGL context (although the drawable could be a 1x1 pbuffer I suppose). In any case GL_RGB5_A1 is not going to work with the semi-transparent line rendering. I also made a pixel format dumper while working on this. Probably useful to share the print-outs: - [Linux desktop, LLVMpipe, GLX](/uploads/11763ff3ed0d9f48022d5a1b1700a4e7/pf_desktop_llvmpipe) (by far the most restrictive) - [Windows 10 VM, GDI generic, WGL](/uploads/127210e12fea60594423b76b051a4db9/pf_windows_gdigeneric_vm.txt) - [Linux laptop, AMD open source, GLX](/uploads/8de9328ab29f9e0c77e2f8146f4ef6c5/pf_linux_amd_laptop.txt) - [Linux desktop, NVidia, GLX](/uploads/eb7dd6d9ac6febfe78307db966be2181/pf_linux_nvidia.txt) Correcting myself: I've done a test now and it appears that Windows does _not_ blend newly drawn elements onto the bitmap. Drawing (0, 0.5, 0, 0.5) onto (1, 1, 1), you get (0, 0.5, 0), so there's no color blending at all.
When picking colors, Civ 3 does this: ``` 011c:trace:opengl:glColor4f red 0.564706, green 0.407843, blue 0.000000, alpha 1.000000 011c:trace:opengl:glLineWidth width 2.000000 011c:trace:opengl:glColor4f red 0.564706, green 0.407843, blue 0.000000, alpha 0.501961 ``` I'm guessing the second glColor4f call is "in case it's supported", but the GDI software renderer in any case does not support semi-transparent rendering. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116761
participants (3)
-
Rémi Bernon -
Zowie van Dillen
-
Zowie van Dillen (＠zowie)