[PATCH v2 0/4] 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. -- v2: 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. 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 a25f291a2cf..b4be4843b35 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 | 156 ++++++++++++++++++++++++++++------- 1 file changed, 125 insertions(+), 31 deletions(-) diff --git a/dlls/opengl32/tests/opengl.c b/dlls/opengl32/tests/opengl.c index 55125722297..8caca0d6cc5 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 % 4; + int y = i / 4; + + 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; @@ -1913,7 +1946,15 @@ static void test_16bit_bitmap_rendering(void) 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 +1962,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 +2010,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 8caca0d6cc5..acc7829b909 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) { @@ -1934,14 +1934,14 @@ static void test_16bit_bitmap_rendering(void) /* 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); + 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); /* 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); + 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); /* This next test might fail, depending on your drivers. */ ok(pfd.cBlueShift == 0, "Wrong blue shift (got %d, expected 0)\n", pfd.cBlueShift); 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 38b14f57e3b..7e860dbbf99 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 57e0aa0e27c..19949b1cab0 100644 --- a/dlls/win32u/opengl.c +++ b/dlls/win32u/opengl.c @@ -1233,6 +1233,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; @@ -1253,9 +1292,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 ); } @@ -1264,12 +1364,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
@rbernon I just saw your post on the wine-devel mailing list on LLM-assisted code. I'm not sure if you were referring to my MR, but in case you were, I do want to clarify that I don't use LLMs except as a StackOverflow replacement. The em-dashes are written the old-fashioned way (compose + dash + dash + dash with XCompose). The comments in the code are probably also fairly excessive, but are also written by hand. Just to substantiate that, I'd like to note that I've got articles on my blog site from 2022 with em-dashes and excessive code comments. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116219
I checked 24-bit bitmap rendering today and it has the same set of issues as 16-bit bitmap rendering. The main problem (garbled rendering) is addressed by !8999. Besides that, direct bitmap memory edits are lost on glFinish/glFlush just like they were for 16-bit rendering, although the program in question (Moyo Go Studio) didn't use memory edits so it hasn't been an actual problem thus far. To fix it I think it might be better to use the workaround for 24-bit bitmaps as well. I'm not sure if the workaround would also work for 32-bit bitmaps, since the alpha channel isn't necessarily free for those, although it likely might work. Ideal solution would of course be a compliant software renderer, but that's a lot of work. I have a test case for 24-bit bitmap rendering on https://gitlab.winehq.org/zowie/wine/-/commit/acc6d8dc9a8fd4f8e2efee4ef1248e... — but that's still only on a draft branch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8969#note_116324
participants (2)
-
Zowie van Dillen -
Zowie van Dillen (＠zowie)