[PATCH v2 0/2] MR7318: gdiplus: Use correct format guid for .ico files.
Adapted from https://github.com/reactos/reactos/pull/7714 -- v2: gdiplus: Use correct format guid for .ico files. https://gitlab.winehq.org/wine/wine/-/merge_requests/7318
From: Fabian Maurer <dark.shadow4(a)web.de> --- dlls/gdiplus/tests/image.c | 44 +++++++++++++++++++++++++++++++ dlls/gdiplus/tests/resource.rc | 3 +++ dlls/gdiplus/tests/wine_test.ico | Bin 0 -> 4375 bytes 3 files changed, 47 insertions(+) create mode 100644 dlls/gdiplus/tests/wine_test.ico diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index df26f699e86..256aa360d47 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -486,11 +486,39 @@ static void test_GdipImageGetFrameDimensionsCount(void) GdipDisposeImage((GpImage*)bm); } +static void _load_resource(int line, const WCHAR *filename, BYTE **data, DWORD *size) +{ + HRSRC resource = FindResourceW(NULL, filename, (const WCHAR *)RT_RCDATA); + ok_(__FILE__, line)(!!resource, "FindResourceW failed, error %lu\n", GetLastError()); + *data = LockResource(LoadResource(GetModuleHandleW(NULL), resource)); + ok_(__FILE__, line)(!!*data, "LockResource failed, error %lu\n", GetLastError()); + *size = SizeofResource(GetModuleHandleW(NULL), resource); + ok_(__FILE__, line)(*size > 0, "SizeofResource failed, error %lu\n", GetLastError()); +} + +static void create_test_resource(const WCHAR *filename, int resource) +{ + DWORD written, length; + HANDLE file; + void *ptr; + + file = CreateFileW(filename, GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, 0); + ok(file != INVALID_HANDLE_VALUE, "file creation failed, at %s, error %ld\n", wine_dbgstr_w(filename), GetLastError()); + + _load_resource(__LINE__, MAKEINTRESOURCEW(resource), (BYTE **)&ptr, &length); + WriteFile(file, ptr, length, &written, NULL); + ok(written == length, "couldn't write resource\n"); + CloseHandle(file); +} + static void test_LoadingImages(void) { + static const GUID format_ico = { 0xb96b3cb5U, 0x0728U, 0x11d3U, {0x9d, 0x7b, 0x00, 0x00, 0xf8, 0x1e, 0xf3, 0x2e} }; + static const WCHAR filename_ico[] = L"a.ico"; GpStatus stat; GpBitmap *bm; GpImage *img; + GUID format; stat = GdipCreateBitmapFromFile(0, 0); expect(InvalidParameter, stat); @@ -530,6 +558,22 @@ static void test_LoadingImages(void) stat = GdipLoadImageFromFileICM(L"nonexistent", &img); todo_wine expect(OutOfMemory, stat); ok(!img, "returned %p\n", img); + + create_test_resource(filename_ico, 5); + + bm = NULL; + stat = GdipLoadImageFromFile(filename_ico, (GpImage**)&bm); + expect(Ok, stat); + if (stat != Ok) goto cleanup; + + stat = GdipGetImageRawFormat((GpImage*)bm, &format); + expect(Ok, stat); + expect_guid(&format_ico, &format, __LINE__, TRUE); + +cleanup: + if (bm) + GdipDisposeImage((GpImage*)bm); + ok(DeleteFileW(filename_ico), "Delete failed.\n"); } static void test_SavingImages(void) diff --git a/dlls/gdiplus/tests/resource.rc b/dlls/gdiplus/tests/resource.rc index 4af61fef65d..b16b933fb43 100644 --- a/dlls/gdiplus/tests/resource.rc +++ b/dlls/gdiplus/tests/resource.rc @@ -31,3 +31,6 @@ /* Generated with: fonttools ttx wine_unicode_mac.ttx */ /* @makedep: wine_unicode_mac.ttf */ 4 RCDATA wine_unicode_mac.ttf + +/* Copied from shell32/blank.ico */ +5 RCDATA wine_test.ico diff --git a/dlls/gdiplus/tests/wine_test.ico b/dlls/gdiplus/tests/wine_test.ico new file mode 100644 index 0000000000000000000000000000000000000000..63b7cdda93f03885fe39c067411eb2a021f492ba GIT binary patch literal 4375 zcmZQzU}WH800Bk(a)1qMe(28Lxowt)c?kk80qz|6q#94Lm&7iUG~yK^uwC(a)4VH1J#yu zqw<&Ypz^QtGB5}TK-6yl+QrU~%HNL0PZwZd=nU|4=jD<DIh5Da!zBphS`Y(a)23?-aX zPXj5IbVpxD28NCO+<y{*PGnF>_6YK2V5m}KU}$JzVE6?TYIwoGP-?)y(a)G60U!D<Er zgLuK5U4b(a)038s=DzhDN2vdL#HK)fVxcNYf6yIsOdfIRjRPhVH|M;y|;;@XYZQ$GWR z1d<c|%7E%BfNlV~jsX|~3>^Aw%R+#(yr+v}NXEUl2N{9>WH_|oyMHd9`2rvl4EFIV z0`=5O&RGuRs+PD$l%yn<q*^5xr2;7iBLhPVT>}eUBl8eLLn|X=D-&aF10yQ~1Ag;Y zswf(A^HVa(a)DsgM5%L<wave47j&t;ucLX!rN4fHre0uV!kjiCV?dO!gnV1iJi<RA%w z|6sttFiL|$fd1hynqLM<Mj+A|qxocD@(C>5NAnAj33ZStgT>n(a)JuvwOlwTBp*a3*) z?J*!51_oPu43bU;jvNgy(!zc;yhsl(P)|+)h++BB0my~{YUf8tn-<irB(a)ct*chtSG M*ukX94Of^x0AQP$MF0Q* literal 0 HcmV?d00001 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7318
From: Fabian Maurer <dark.shadow4(a)web.de> Adapted from https://github.com/reactos/reactos/pull/7714 --- dlls/gdiplus/image.c | 16 ++++++++-------- dlls/gdiplus/tests/image.c | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index 6e55113dbce..89d1ab99c4f 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -5069,7 +5069,7 @@ static const struct image_codec codecs[NUM_CODECS] = { { { /* BMP */ /* Clsid */ { 0x557cf400, 0x1a04, 0x11d3, { 0x9a, 0x73, 0x0, 0x0, 0xf8, 0x1e, 0xf3, 0x2e } }, - /* FormatID */ { 0xb96b3cabU, 0x0728U, 0x11d3U, {0x9d, 0x7b, 0x00, 0x00, 0xf8, 0x1e, 0xf3, 0x2e} }, + /* FormatID */ ImageFormatBMP, /* CodecName */ bmp_codecname, /* DllName */ NULL, /* FormatDescription */ bmp_format, @@ -5089,7 +5089,7 @@ static const struct image_codec codecs[NUM_CODECS] = { { { /* JPEG */ /* Clsid */ { 0x557cf401, 0x1a04, 0x11d3, { 0x9a, 0x73, 0x0, 0x0, 0xf8, 0x1e, 0xf3, 0x2e } }, - /* FormatID */ { 0xb96b3caeU, 0x0728U, 0x11d3U, {0x9d, 0x7b, 0x00, 0x00, 0xf8, 0x1e, 0xf3, 0x2e} }, + /* FormatID */ ImageFormatJPEG, /* CodecName */ jpeg_codecname, /* DllName */ NULL, /* FormatDescription */ jpeg_format, @@ -5109,7 +5109,7 @@ static const struct image_codec codecs[NUM_CODECS] = { { { /* GIF */ /* Clsid */ { 0x557cf402, 0x1a04, 0x11d3, { 0x9a, 0x73, 0x0, 0x0, 0xf8, 0x1e, 0xf3, 0x2e } }, - /* FormatID */ { 0xb96b3cb0U, 0x0728U, 0x11d3U, {0x9d, 0x7b, 0x00, 0x00, 0xf8, 0x1e, 0xf3, 0x2e} }, + /* FormatID */ ImageFormatGIF, /* CodecName */ gif_codecname, /* DllName */ NULL, /* FormatDescription */ gif_format, @@ -5129,7 +5129,7 @@ static const struct image_codec codecs[NUM_CODECS] = { { { /* TIFF */ /* Clsid */ { 0x557cf405, 0x1a04, 0x11d3, { 0x9a, 0x73, 0x0, 0x0, 0xf8, 0x1e, 0xf3, 0x2e } }, - /* FormatID */ { 0xb96b3cb1U, 0x0728U, 0x11d3U, {0x9d, 0x7b, 0x00, 0x00, 0xf8, 0x1e, 0xf3, 0x2e} }, + /* FormatID */ ImageFormatTIFF, /* CodecName */ tiff_codecname, /* DllName */ NULL, /* FormatDescription */ tiff_format, @@ -5149,7 +5149,7 @@ static const struct image_codec codecs[NUM_CODECS] = { { { /* EMF */ /* Clsid */ { 0x557cf403, 0x1a04, 0x11d3, { 0x9a, 0x73, 0x0, 0x0, 0xf8, 0x1e, 0xf3, 0x2e } }, - /* FormatID */ { 0xb96b3cacU, 0x0728U, 0x11d3U, {0x9d, 0x7b, 0x00, 0x00, 0xf8, 0x1e, 0xf3, 0x2e} }, + /* FormatID */ ImageFormatEMF, /* CodecName */ emf_codecname, /* DllName */ NULL, /* FormatDescription */ emf_format, @@ -5169,7 +5169,7 @@ static const struct image_codec codecs[NUM_CODECS] = { { { /* WMF */ /* Clsid */ { 0x557cf404, 0x1a04, 0x11d3, { 0x9a, 0x73, 0x0, 0x0, 0xf8, 0x1e, 0xf3, 0x2e } }, - /* FormatID */ { 0xb96b3cadU, 0x0728U, 0x11d3U, {0x9d, 0x7b, 0x00, 0x00, 0xf8, 0x1e, 0xf3, 0x2e} }, + /* FormatID */ ImageFormatWMF, /* CodecName */ wmf_codecname, /* DllName */ NULL, /* FormatDescription */ wmf_format, @@ -5189,7 +5189,7 @@ static const struct image_codec codecs[NUM_CODECS] = { { { /* PNG */ /* Clsid */ { 0x557cf406, 0x1a04, 0x11d3, { 0x9a, 0x73, 0x0, 0x0, 0xf8, 0x1e, 0xf3, 0x2e } }, - /* FormatID */ { 0xb96b3cafU, 0x0728U, 0x11d3U, {0x9d, 0x7b, 0x00, 0x00, 0xf8, 0x1e, 0xf3, 0x2e} }, + /* FormatID */ ImageFormatPNG, /* CodecName */ png_codecname, /* DllName */ NULL, /* FormatDescription */ png_format, @@ -5209,7 +5209,7 @@ static const struct image_codec codecs[NUM_CODECS] = { { { /* ICO */ /* Clsid */ { 0x557cf407, 0x1a04, 0x11d3, { 0x9a, 0x73, 0x0, 0x0, 0xf8, 0x1e, 0xf3, 0x2e } }, - /* FormatID */ { 0xb96b3cabU, 0x0728U, 0x11d3U, {0x9d, 0x7b, 0x00, 0x00, 0xf8, 0x1e, 0xf3, 0x2e} }, + /* FormatID */ ImageFormatIcon, /* CodecName */ ico_codecname, /* DllName */ NULL, /* FormatDescription */ ico_format, diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 256aa360d47..63b559b5cee 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -568,7 +568,7 @@ static void test_LoadingImages(void) stat = GdipGetImageRawFormat((GpImage*)bm, &format); expect(Ok, stat); - expect_guid(&format_ico, &format, __LINE__, TRUE); + expect_guid(&format_ico, &format, __LINE__, FALSE); cleanup: if (bm) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7318
On Wed Feb 12 19:52:48 2025 +0000, Nikolay Sivov wrote:
Those format guids have named constants defined in gdiplusimaging.h, I think we should be using that to make it both readable and prevent problems like that. The fix in principle looks fine, it was probably copied over from BMP format resulting in this typo. Good catch, I searched for the ids, but missed it because they are suffixed with `U`. Updated.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_94379
I feel like we've had issues with tests in the past with the current working directory not being writable when running tests. Is there an established best practice around that? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_94462
On Thu Feb 13 15:45:32 2025 +0000, Esme Povirk wrote:
I feel like we've had issues with tests in the past with the current working directory not being writable when running tests. Is there an established best practice around that? Some tests are using temp path for that reason.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_94463
On Thu Feb 13 15:45:32 2025 +0000, Nikolay Sivov wrote:
Some tests are using temp path for that reason. FWIW, for this case it may be easier to use an HGLOBAL stream. Or just, use the system already established for testing the other image formats.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_94464
The pipeline build failures will need to be addressed. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_94465
On Thu Feb 13 15:48:56 2025 +0000, Esme Povirk wrote:
The pipeline build failures will need to be addressed. The error is
../dlls/gdiplus/image.c:5072:38: error: initializer element is not a compile-time constant
/* FormatID */ ImageFormatBMP,
I'm a bit confused as to why it doesn't happen with gcc, but the compiler is right. Not sure how to fix that without avoiding `ImageFormatBMP` and using the GUID directly like it was before. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_94487
On Thu Feb 13 15:48:36 2025 +0000, Esme Povirk wrote:
FWIW, for this case it may be easier to use an HGLOBAL stream. Or just, use the system already established for testing the other image formats. FWIW, I adapted the test from `test_SavingImages`
Or just, use the system already established for testing the other image formats.
Not sure what exactly you mean. I tried to create the ico with gdiplus, but unfortunately there is no encoder. Unless you mean something different? I could rework it to use a stream instead of a file though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_94488
On Thu Feb 13 20:01:00 2025 +0000, Fabian Maurer wrote:
FWIW, I adapted the test from `test_SavingImages`
Or just, use the system already established for testing the other image formats. Not sure what exactly you mean. I tried to create the ico with gdiplus, but unfortunately there is no encoder. Unless you mean something different? I could rework it to use a stream instead of a file though. I think the point was that you don't need file access, and can use GdipLoadImageFromStream(). This is generally better, unless you're testing FromFile() API.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_94489
On Thu Feb 13 20:09:17 2025 +0000, Nikolay Sivov wrote:
I think the point was that you don't need file access, and can use GdipLoadImageFromStream(). This is generally better, unless you're testing FromFile() API. I mean this: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/gdiplus/tests/image.c...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_94509
On Thu Feb 13 19:52:43 2025 +0000, Fabian Maurer wrote:
The error is ``` ../dlls/gdiplus/image.c:5072:38: error: initializer element is not a compile-time constant /* FormatID */ ImageFormatBMP, ``` I'm a bit confused as to why it doesn't happen with gcc, but the compiler is right. Not sure how to fix that without avoiding `ImageFormatBMP` and using the GUID directly like it was before. How should I continue here?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_99338
On Sat Mar 29 19:12:05 2025 +0000, Fabian Maurer wrote:
How should I continue here? I would just keep hard-coding the GUIDs. I don't see a way around that while preserving the ability to blit the whole struct.
In theory, we could store a pointer instead, and not have to hard-code that, but then we'd have to dereference it when we return the codec info. I don't think it'd be an overall improvement. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7318#note_99343
participants (4)
-
Esme Povirk (＠madewokherd) -
Fabian Maurer
-
Fabian Maurer (＠DarkShadow44)
-
Nikolay Sivov (＠nsivov)