[PATCH 0/2] MR4658: gidplus: Add GdipGetEffectParameterSize stub, fix GdipDeleteEffect and update test.
From: Bartosz Kosiorek <gang65(a)poczta.onet.pl> --- dlls/gdiplus/gdiplus.spec | 2 +- dlls/gdiplus/image.c | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/dlls/gdiplus/gdiplus.spec b/dlls/gdiplus/gdiplus.spec index 178592e35d9..0f17bb02d4d 100644 --- a/dlls/gdiplus/gdiplus.spec +++ b/dlls/gdiplus/gdiplus.spec @@ -612,7 +612,7 @@ 612 stdcall GdipGetImageItemData(ptr ptr) 613 stdcall GdipCreateEffect(int128 ptr) 614 stdcall GdipDeleteEffect(ptr) -615 stub GdipGetEffectParameterSize +615 stdcall GdipGetEffectParameterSize(ptr ptr) 616 stub GdipGetEffectParameters 617 stdcall GdipSetEffectParameters(ptr ptr long) 618 stdcall GdipInitializePalette(ptr long long long ptr) diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index 806da0cb696..dca27c63f9e 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -5592,11 +5592,26 @@ GpStatus WINGDIPAPI GdipCreateEffect(const GUID guid, CGpEffect **effect) GpStatus WINGDIPAPI GdipDeleteEffect(CGpEffect *effect) { FIXME("(%p): stub\n", effect); + + if (!effect) + return InvalidParameter; /* note: According to Jose Roca's GDI+ Docs, this is not implemented * in Windows's gdiplus */ return NotImplemented; } +/***************************************************************************** + * GdipGetEffectParameterSize [GDIPLUS.@] + */ +GpStatus WINGDIPAPI GdipGetEffectParameterSize(CGpEffect *effect, UINT *size) +{ + FIXME("(%p,%p): stub\n", effect, size); + + if (!effect || !size) + return InvalidParameter; + return NotImplemented; +} + /***************************************************************************** * GdipSetEffectParameters [GDIPLUS.@] */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4658
From: Bartosz Kosiorek <gang65(a)poczta.onet.pl> --- dlls/gdiplus/tests/image.c | 51 +++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 8fc9aa78225..28061185781 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -5407,20 +5407,40 @@ static void test_createeffect(void) static const GUID noneffect = { 0xcd0c3d4b, 0xe15e, 0x4cf2, { 0x9e, 0xa8, 0x6e, 0x1d, 0x65, 0x48, 0xc5, 0xa5 } }; GpStatus (WINAPI *pGdipCreateEffect)( const GUID guid, CGpEffect **effect); GpStatus (WINAPI *pGdipDeleteEffect)( CGpEffect *effect); + GpStatus (WINAPI *pGdipGetEffectParameterSize)(CGpEffect *effect, UINT *size); + GpStatus (WINAPI *pGdipGetEffectParameters)(CGpEffect *effect, const VOID *params, const UINT size); GpStatus stat; CGpEffect *effect; HMODULE mod = GetModuleHandleA("gdiplus.dll"); int i; - const GUID * const effectlist[] = - {&BlurEffectGuid, &SharpenEffectGuid, &ColorMatrixEffectGuid, &ColorLUTEffectGuid, - &BrightnessContrastEffectGuid, &HueSaturationLightnessEffectGuid, &LevelsEffectGuid, - &TintEffectGuid, &ColorBalanceEffectGuid, &RedEyeCorrectionEffectGuid, &ColorCurveEffectGuid}; + UINT param_size; + + static const struct test_data { + const GUID *effect; + const UINT parameters_number; + const BOOL todo; + } td[] = + { + { &BlurEffectGuid, 8, TRUE }, + { &BrightnessContrastEffectGuid, 8, TRUE }, + { &ColorBalanceEffectGuid, 12, TRUE }, + { &ColorCurveEffectGuid, 12, TRUE }, + { &ColorLUTEffectGuid, 1024, TRUE }, + { &ColorMatrixEffectGuid, 100, TRUE }, + { &HueSaturationLightnessEffectGuid, 12, TRUE }, + { &LevelsEffectGuid, 12, TRUE }, + { &RedEyeCorrectionEffectGuid, 8, TRUE }, + { &SharpenEffectGuid, 8, TRUE }, + { &TintEffectGuid, 8, TRUE }, + }; pGdipCreateEffect = (void*)GetProcAddress( mod, "GdipCreateEffect"); pGdipDeleteEffect = (void*)GetProcAddress( mod, "GdipDeleteEffect"); - if(!pGdipCreateEffect || !pGdipDeleteEffect) + pGdipGetEffectParameterSize = (void*)GetProcAddress( mod, "GdipGetEffectParameterSize"); + pGdipGetEffectParameters = (void*)GetProcAddress( mod, "GdipGetEffectParameters"); + if (!pGdipCreateEffect || !pGdipDeleteEffect || !pGdipGetEffectParameterSize || !pGdipGetEffectParameters) { - /* GdipCreateEffect/GdipDeleteEffect was introduced in Windows Vista. */ + /* GdipCreateEffect/GdipDeleteEffect/GdipGetEffectParameterSize/GdipGetEffectParameters were introduced in Windows Vista. */ win_skip("GDIPlus version 1.1 not available\n"); return; } @@ -5431,16 +5451,29 @@ static void test_createeffect(void) stat = pGdipCreateEffect(noneffect, &effect); todo_wine expect(Win32Error, stat); - for(i=0; i < ARRAY_SIZE(effectlist); i++) + param_size = 0; + stat = pGdipGetEffectParameterSize(NULL, ¶m_size); + expect(InvalidParameter, stat); + expect(0, param_size); + + for (i = 0; i < ARRAY_SIZE(td); i++) { - stat = pGdipCreateEffect(*effectlist[i], &effect); - todo_wine expect(Ok, stat); + stat = pGdipCreateEffect(*(td[i].effect), &effect); + todo_wine_if(td[i].todo) expect(Ok, stat); + + stat = pGdipGetEffectParameterSize(effect, ¶m_size); + todo_wine_if(td[i].todo) expect(Ok, stat); + todo_wine_if(td[i].todo) expect(td[i].parameters_number, param_size); + if(stat == Ok) { stat = pGdipDeleteEffect(effect); expect(Ok, stat); } } + + stat = pGdipDeleteEffect(NULL); + expect(InvalidParameter, stat); } static void test_getadjustedpalette(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4658
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=141000 Your paranoid android. === w7pro64 (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w864 (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w1064v1507 (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w1064v1809 (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w1064_2qxl (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w1064_adm (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w1064_tsign (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w10pro64 (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w10pro64_ar (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w10pro64_ja (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w10pro64_zh_CN (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16 === w11pro64_amd (64 bit report) === gdiplus: image.c:5466: Test failed: Expected 8, got 16
I have created a MR 4661 which tries to implement the functions, please check. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4658#note_55599
Esme Povirk (@madewokherd) commented about dlls/gdiplus/tests/image.c:
todo_wine expect(Win32Error, stat);
- for(i=0; i < ARRAY_SIZE(effectlist); i++) + param_size = 0; + stat = pGdipGetEffectParameterSize(NULL, ¶m_size); + expect(InvalidParameter, stat); + expect(0, param_size); + + for (i = 0; i < ARRAY_SIZE(td); i++) { - stat = pGdipCreateEffect(*effectlist[i], &effect); - todo_wine expect(Ok, stat); + stat = pGdipCreateEffect(*(td[i].effect), &effect); + todo_wine_if(td[i].todo) expect(Ok, stat); + + stat = pGdipGetEffectParameterSize(effect, ¶m_size);
If `GdipCreateEffect` doesn't zero `effect` in case of failure, `effect` will be used uninitialized here. This will be a problem when `GdipGetEffectParameterSize` is implemented in Wine. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4658#note_55608
There were some failures on testbot: https://testbot.winehq.org/JobDetails.pl?Key=141000#k301 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4658#note_55609
The commit messages have a typo: gidplus -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4658#note_55610
participants (5)
-
Bartosz Kosiorek -
Bartosz Kosiorek (＠gang65)
-
Esme Povirk (＠madewokherd)
-
Marvin -
Vijay Kiran Kamuju (＠infyquest)