[PATCH v2 0/4] MR7183: gdi32: Fix 64 bit EMREXTCREATEPEN.
-- v2: gdi32/tests: Add an EMREXTCREATEPEN test. gdi32: EMREXTCREATEPEN contains a 32 bit EXTLOGPEN. gdi32: Set EMREXTCREATEPEN offBmi and offBits. gdi32: Windows adds an extra 4 bytes to EMREXTCREATEPEN. https://gitlab.winehq.org/wine/wine/-/merge_requests/7183
From: Stefan Dösinger <stefan(a)codeweavers.com> The application in question doesn't care since it is at the end of a struct, but the behavior is consistent and adding it is easier than making our tests check for it. --- dlls/gdi32/emfdc.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index e48da848a96..2fd4f72f9cc 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -666,8 +666,11 @@ static DWORD emfdc_ext_create_pen( struct emf *emf, HPEN pen ) if (!(size = GetObjectW( pen, 0, NULL ))) return 0; - emr_size = sizeof(*emr) - sizeof(emr->elp) + size; - emr = HeapAlloc( GetProcessHeap(), 0, emr_size ); + + /* Native adds an extra 4 bytes, presumably because someone wasn't careful about the + * dynamic array [1] at the end of EXTLOGPEN. */ + emr_size = sizeof(*emr) - sizeof(emr->elp) + sizeof(emr->elp.elpStyleEntry) + size; + emr = calloc( 1, emr_size ); if (!emr) return 0; GetObjectW( pen, size, &emr->elp ); @@ -677,16 +680,15 @@ static DWORD emfdc_ext_create_pen( struct emf *emf, HPEN pen ) emr->elp.elpBrushStyle == BS_PATTERN) { FIXME( "elpBrushStyle = %d\n", emr->elp.elpBrushStyle ); - HeapFree( GetProcessHeap(), 0, emr ); + free( emr ); return 0; } - emr->offBmi = emr->cbBmi = emr->offBits = emr->cbBits = 0; emr->emr.iType = EMR_EXTCREATEPEN; emr->emr.nSize = emr_size; emr->ihPen = ret = emfdc_add_handle( emf, pen ); ret = emfdc_record( emf, &emr->emr ) ? ret : 0; - HeapFree( GetProcessHeap(), 0, emr ); + free( emr ); return ret; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7183
From: Stefan Dösinger <stefan(a)codeweavers.com> Again to match what the tests get on native. --- dlls/gdi32/emfdc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index 2fd4f72f9cc..66a9ef7ed0f 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -683,6 +683,8 @@ static DWORD emfdc_ext_create_pen( struct emf *emf, HPEN pen ) free( emr ); return 0; } + /* Native sets these even if there is no bitmap or brush BITMAPINFO. */ + emr->offBmi = emr->offBits = emr_size; emr->emr.iType = EMR_EXTCREATEPEN; emr->emr.nSize = emr_size; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7183
From: Stefan Dösinger <stefan(a)codeweavers.com> --- This is what actually mattered. The extra offset moved the style count and broke creation of the stored pens. --- dlls/gdi32/emfdc.c | 43 ++++++++++++++++++++++++++---------- dlls/gdi32/tests/generated.c | 16 ++++++-------- dlls/wow64win/gdi.c | 11 --------- include/wingdi.h | 13 ++++++++++- 4 files changed, 50 insertions(+), 33 deletions(-) diff --git a/dlls/gdi32/emfdc.c b/dlls/gdi32/emfdc.c index 66a9ef7ed0f..098caff0c0b 100644 --- a/dlls/gdi32/emfdc.c +++ b/dlls/gdi32/emfdc.c @@ -660,37 +660,56 @@ static BOOL emfdc_select_font( DC_ATTR *dc_attr, HFONT font ) static DWORD emfdc_ext_create_pen( struct emf *emf, HPEN pen ) { - EMREXTCREATEPEN *emr; + EMREXTCREATEPEN *emr = NULL; + EXTLOGPEN *elp = NULL; int size, emr_size; + unsigned int i; DWORD ret = 0; if (!(size = GetObjectW( pen, 0, NULL ))) return 0; + elp = malloc( size ); + if (!elp) + return 0; + /* Native adds an extra 4 bytes, presumably because someone wasn't careful about the - * dynamic array [1] at the end of EXTLOGPEN. */ - emr_size = sizeof(*emr) - sizeof(emr->elp) + sizeof(emr->elp.elpStyleEntry) + size; + * dynamic array [1] at the end of EXTLOGPEN. Also note that GetObject returns an + * EXTLOGPEN with a pointer-sized elpHatch, whereas a metafile always has a 32 bit + * sized emr->elp.elpHatch .*/ + emr_size = sizeof(*emr) - sizeof(*elp) + sizeof(emr->elp.elpStyleEntry) + size; emr = calloc( 1, emr_size ); if (!emr) - return 0; - GetObjectW( pen, size, &emr->elp ); + goto out; + GetObjectW( pen, size, elp ); - if (emr->elp.elpBrushStyle == BS_DIBPATTERN || - emr->elp.elpBrushStyle == BS_DIBPATTERNPT || - emr->elp.elpBrushStyle == BS_PATTERN) + if (elp->elpBrushStyle == BS_DIBPATTERN || + elp->elpBrushStyle == BS_DIBPATTERNPT || + elp->elpBrushStyle == BS_PATTERN) { - FIXME( "elpBrushStyle = %d\n", emr->elp.elpBrushStyle ); - free( emr ); - return 0; + FIXME( "elpBrushStyle = %d\n", elp->elpBrushStyle ); + goto out; } - /* Native sets these even if there is no bitmap or brush BITMAPINFO. */ emr->offBmi = emr->offBits = emr_size; + emr->elp.elpPenStyle = elp->elpPenStyle; + emr->elp.elpWidth = elp->elpWidth; + emr->elp.elpBrushStyle = elp->elpBrushStyle; + emr->elp.elpColor = elp->elpColor; + emr->elp.elpHatch = elp->elpHatch; + emr->elp.elpNumEntries = elp->elpNumEntries; + + for (i = 0; i < elp->elpNumEntries; ++i) + emr->elp.elpStyleEntry[i] = elp->elpStyleEntry[i]; + emr->emr.iType = EMR_EXTCREATEPEN; emr->emr.nSize = emr_size; emr->ihPen = ret = emfdc_add_handle( emf, pen ); ret = emfdc_record( emf, &emr->emr ) ? ret : 0; + +out: free( emr ); + free( elp ); return ret; } diff --git a/dlls/gdi32/tests/generated.c b/dlls/gdi32/tests/generated.c index 02898227b2b..43467e75a04 100644 --- a/dlls/gdi32/tests/generated.c +++ b/dlls/gdi32/tests/generated.c @@ -5,8 +5,6 @@ * Unit tests for data structure packing */ -#define WINE_NOWINSOCK - #include "windows.h" #include "wine/test.h" @@ -1082,8 +1080,8 @@ static void test_pack_EMREXTCREATEFONTINDIRECTW(void) static void test_pack_EMREXTCREATEPEN(void) { /* EMREXTCREATEPEN */ - TEST_TYPE_SIZE (EMREXTCREATEPEN, 64) - TEST_TYPE_ALIGN (EMREXTCREATEPEN, 8) + TEST_TYPE_SIZE (EMREXTCREATEPEN, 56) + TEST_TYPE_ALIGN (EMREXTCREATEPEN, 4) TEST_FIELD_SIZE (EMREXTCREATEPEN, emr, 8) TEST_FIELD_ALIGN (EMREXTCREATEPEN, emr, 4) TEST_FIELD_OFFSET(EMREXTCREATEPEN, emr, 0) @@ -1102,9 +1100,9 @@ static void test_pack_EMREXTCREATEPEN(void) TEST_FIELD_SIZE (EMREXTCREATEPEN, cbBits, 4) TEST_FIELD_ALIGN (EMREXTCREATEPEN, cbBits, 4) TEST_FIELD_OFFSET(EMREXTCREATEPEN, cbBits, 24) - TEST_FIELD_SIZE (EMREXTCREATEPEN, elp, 32) - TEST_FIELD_ALIGN (EMREXTCREATEPEN, elp, 8) - TEST_FIELD_OFFSET(EMREXTCREATEPEN, elp, 32) + TEST_FIELD_SIZE (EMREXTCREATEPEN, elp, 28) + TEST_FIELD_ALIGN (EMREXTCREATEPEN, elp, 4) + TEST_FIELD_OFFSET(EMREXTCREATEPEN, elp, 28) } static void test_pack_EMREXTFLOODFILL(void) @@ -5091,8 +5089,8 @@ static void test_pack_PEMREXTCREATEPEN(void) /* PEMREXTCREATEPEN */ TEST_TYPE_SIZE (PEMREXTCREATEPEN, 8) TEST_TYPE_ALIGN (PEMREXTCREATEPEN, 8) - TEST_TARGET_SIZE (PEMREXTCREATEPEN, 64) - TEST_TARGET_ALIGN(PEMREXTCREATEPEN, 8) + TEST_TARGET_SIZE (PEMREXTCREATEPEN, 56) + TEST_TARGET_ALIGN(PEMREXTCREATEPEN, 4) } static void test_pack_PEMREXTFLOODFILL(void) diff --git a/dlls/wow64win/gdi.c b/dlls/wow64win/gdi.c index bcd430be4c9..61e719fba5d 100644 --- a/dlls/wow64win/gdi.c +++ b/dlls/wow64win/gdi.c @@ -39,17 +39,6 @@ typedef struct ULONG bmBits; } BITMAP32; -typedef struct -{ - DWORD elpPenStyle; - DWORD elpWidth; - UINT elpBrushStyle; - COLORREF elpColor; - ULONG elpHatch; - DWORD elpNumEntries; - DWORD elpStyleEntry[1]; -} EXTLOGPEN32; - typedef struct { UINT otmSize; diff --git a/include/wingdi.h b/include/wingdi.h index 039436fbbf1..be3d724cb49 100644 --- a/include/wingdi.h +++ b/include/wingdi.h @@ -1568,6 +1568,17 @@ typedef struct tagEXTLOGPEN DWORD elpStyleEntry[1]; } EXTLOGPEN, *PEXTLOGPEN, *NPEXTLOGPEN, *LPEXTLOGPEN; +typedef struct tagEXTLOGPEN32 +{ + DWORD elpPenStyle; + DWORD elpWidth; + UINT elpBrushStyle; + COLORREF elpColor; + ULONG elpHatch; + DWORD elpNumEntries; + DWORD elpStyleEntry[1]; +} EXTLOGPEN32, *PEXTLOGPEN32, *NPEXTLOGPEN32, *LPEXTLOGPEN32; + #define PS_SOLID 0x00000000 #define PS_DASH 0x00000001 #define PS_DOT 0x00000002 @@ -2338,7 +2349,7 @@ typedef struct { DWORD cbBmi; DWORD offBits; DWORD cbBits; - EXTLOGPEN elp; + EXTLOGPEN32 elp; } EMREXTCREATEPEN, *PEMREXTCREATEPEN; typedef struct tagEMREXTESCAPE -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7183
From: Stefan Dösinger <stefan(a)codeweavers.com> --- dlls/gdi32/tests/metafile.c | 106 ++++++++++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/dlls/gdi32/tests/metafile.c b/dlls/gdi32/tests/metafile.c index ab7882f9641..a1c8ede04dc 100644 --- a/dlls/gdi32/tests/metafile.c +++ b/dlls/gdi32/tests/metafile.c @@ -4556,6 +4556,111 @@ static void test_mf_select(void) ok(ret, "DeleteMetaFile(%p) error %ld\n", hmf, GetLastError()); } +static void test_emf_extpen(void) +{ + static const DWORD user_style[3] = { 0xabc, 0xdef, 0x888 }; + HENHMETAFILE hemf; + HPEN pen, pen2; + LOGBRUSH brush; + HGDIOBJ obj; + DWORD size; + BOOL ret; + HDC hdc; + + /* EMREXTCREATEPEN has two pitfalls: + * + * 1) EXTLOGPEN contains a pointer-sized integer. Metafiles always use the + * 32 bit version of this structure. + * + * 2) Native adds an extra 4 bytes to the EXTLOGPEN32 structure, presumably + * because someone forgot to account for the elpStyleEntry[1] dummy array. + * This is filled with zeroes and at the end of the struct, so it only + * matters when you make assumptions about the metafile or entry size. */ + static const unsigned char extpen_bits[] = + { + 0x01, 0x00, 0x00, 0x00, 0x6c, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0xe9, 0xff, 0xff, 0xff, 0xe9, 0xff, 0xff, 0xff, + 0x20, 0x45, 0x4d, 0x46, 0x00, 0x00, 0x01, 0x00, + 0x38, 0x01, 0x00, 0x00, 0x09, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x78, 0x05, 0x00, 0x00, 0x1a, 0x04, 0x00, 0x00, + 0x40, 0x01, 0x00, 0x00, 0xf0, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0xe2, 0x04, 0x00, + 0x80, 0xa9, 0x03, 0x00, 0x5f, 0x00, 0x00, 0x00, + 0x38, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x38, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x45, 0x67, 0x89, 0x00, + 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x25, 0x00, 0x00, 0x00, + 0x0c, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x5f, 0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, + 0x02, 0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x44, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x07, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x12, 0x34, 0x56, 0x00, 0x04, 0x00, 0x00, 0x00, + 0x03, 0x00, 0x00, 0x00, 0xbc, 0x0a, 0x00, 0x00, + 0xef, 0x0d, 0x00, 0x00, 0x88, 0x08, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x25, 0x00, 0x00, 0x00, + 0x0c, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x25, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, + 0x07, 0x00, 0x00, 0x80, 0x28, 0x00, 0x00, 0x00, + 0x0c, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, + 0x28, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, + 0x01, 0x00, 0x00, 0x00, 0x0e, 0x00, 0x00, 0x00, + 0x14, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x10, 0x00, 0x00, 0x00, 0x14, 0x00, 0x00, 0x00 + }; + + hdc = CreateEnhMetaFileA(NULL, NULL, NULL, NULL); + ok(hdc != 0, "CreateEnhMetaFileA failed\n"); + + brush.lbStyle = BS_SOLID; + brush.lbColor = RGB(0x45,0x67,0x89); + brush.lbHatch = HS_CROSS; + pen = ExtCreatePen(PS_SOLID, 1, &brush, 0, NULL); + ok(!!pen, "ExtCreatePen failed: %lu\n", GetLastError()); + size = GetObjectW(pen, 0, NULL); + ok(size == offsetof(EXTLOGPEN, elpStyleEntry[0] ), "Got unexpected size %#lx\n", size); + + obj = SelectObject(hdc, pen); + ok(obj == GetStockObject(BLACK_PEN), "pen is not a stock BLACK_PEN: %p\n", obj); + + brush.lbColor = RGB(0x12,0x34,0x56); + pen2 = ExtCreatePen(PS_USERSTYLE, 1, &brush, 3, user_style); + ok(!!pen2, "ExtCreatePen failed: %lu\n", GetLastError()); + size = GetObjectW(pen2, 0, NULL); /* cf pen.c, test_logpen() */ + ok(size == offsetof(EXTLOGPEN, elpStyleEntry[3]), "Got unexpected size %#lx\n", size); + + obj = SelectObject(hdc, pen2); + ok(obj == pen, "Expected pen %p, got %p.\n", obj, pen); + + obj = SelectObject(hdc, GetStockObject(BLACK_PEN)); + ok(obj == pen2, "Expected pen %p, got %p.\n", obj, pen2); + + DeleteObject(pen2); + DeleteObject(pen); + hemf = CloseEnhMetaFile(hdc); + ok(hemf != 0, "CloseEnhMetaFile failed\n"); + + if (compare_emf_bits(hemf, extpen_bits, sizeof(extpen_bits), "emf_extpen", FALSE)) + { + dump_emf_bits(hemf, "emf_extpen"); + dump_emf_records(hemf, "emf_extpen"); + } + + ret = DeleteEnhMetaFile(hemf); + ok(ret, "DeleteEnhMetaFile(%p) error %ld\n", hemf, GetLastError()); +} + + static void test_emf_select(void) { HENHMETAFILE hemf; @@ -10831,6 +10936,7 @@ START_TEST(metafile) test_emf_StretchDIBits(); test_emf_SetDIBitsToDevice(); test_emf_palette(); + test_emf_extpen(); /* For win-format metafiles (mfdrv) */ test_mf_SaveDC(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7183
No, these files need to be regenerated by hand, and checked against a Visual Studio build.
I've included the updated gdi32/test/generated.c in patch 3 now. The winapi_test script removed "#define WINE_NOWINSOCK". I've left it as the tool generated, let me know if you want that changed. Otherwise only the EMREXTCREATEPEN entries changed. Should I add the new EXTLOGPEN32 to tools/winapi/tests.dat? With a bit of manual work I managed to run generated.c through Visual Studio 2022 and the 10.0.26100.0 SDK. It previously failed in 64 bit build and worked in 32 bit. Now it succeeds in both: ``` -------------------- before Regenerate --------------------------- Die Erstellung wurde um 13:14 gestartet... 1>------ Erstellen gestartet: Projekt: gdi32_tests, Konfiguration: Debug x64 ------ 1>generated.c 1>C:\Users\stefan\source\repos\wine\dlls\gdi32\tests\generated.c(1085,5): error C2118: Negativer Index 1>C:\Users\stefan\source\repos\wine\dlls\gdi32\tests\generated.c(1086,5): error C2118: Negativer Index 1>C:\Users\stefan\source\repos\wine\dlls\gdi32\tests\generated.c(1105,5): error C2118: Negativer Index 1>C:\Users\stefan\source\repos\wine\dlls\gdi32\tests\generated.c(1107,5): error C2118: Negativer Index 1>C:\Users\stefan\source\repos\wine\dlls\gdi32\tests\generated.c(5094,5): error C2118: Negativer Index 1>Die Erstellung des Projekts "gdi32_tests.vcxproj" ist abgeschlossen -- FEHLER. ========== Build: 0 erfolgreich, 1 Fehler, 0 aktuell, 0 übersprungen ========== ========== Erstellen abgeschlossen um 13:14 und dauerte 00,704 Sekunden ========== Die Erstellung wurde um 13:19 gestartet... 1>------ Erstellen gestartet: Projekt: gdi32_tests, Konfiguration: Debug Win32 ------ 1>generated.c ========== Build: 1 erfolgreich, 0 Fehler, 0 aktuell, 0 übersprungen ========== ========== Erstellen abgeschlossen um 13:19 und dauerte 01,586 Sekunden ========== -------------------- after Regenerate --------------------------- Die Erstellung wurde um 13:46 gestartet... 1>------ Erstellen gestartet: Projekt: gdi32_tests, Konfiguration: Debug x64 ------ 1>generated.c ========== Build: 1 erfolgreich, 0 Fehler, 0 aktuell, 0 übersprungen ========== ========== Erstellen abgeschlossen um 13:46 und dauerte 00,672 Sekunden ========== Die Erstellung wurde um 13:46 gestartet... 1>------ Erstellen gestartet: Projekt: gdi32_tests, Konfiguration: Debug Win32 ------ 1>generated.c ========== Build: 1 erfolgreich, 0 Fehler, 0 aktuell, 0 übersprungen ========== ========== Erstellen abgeschlossen um 13:46 und dauerte 02,247 Sekunden ========== ``` And no, I have no idea how to tell my msvc to write english output without reinstalling it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7183#note_92710
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7183
participants (2)
-
Huw Davies (＠huw) -
Stefan Dösinger