[PATCH 0/2] MR4658: gidplus: Add GdipGetEffectParameterSize stub, fix GdipDeleteEffect and update test.

[PATCH 1/2] gidplus: Add GdipGetEffectParameterSize stub and fix GdipDeleteEffect

[Bartosz Kosiorek]

From: Bartosz Kosiorek

---
 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.@]
  */

[PATCH 2/2] gidplus/test: Add GdipGetEffectParameterSize test.

[Bartosz Kosiorek]

From: Bartosz Kosiorek

---
 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, &param_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, &param_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)

[PATCH 2/2] gidplus/test: Add GdipGetEffectParameterSize test.

[Marvin]

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

[Vijay Kiran Kamuju (＠infyquest)]

I have created a MR 4661 which tries to implement the functions, please check.

[Esme Povirk (＠madewokherd)]

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, &param_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, &param_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.

[Esme Povirk (＠madewokherd)]

There were some failures on testbot: https://testbot.winehq.org/JobDetails.pl?Key=141000#k301

[Esme Povirk (＠madewokherd)]

The commit messages have a typo: gidplus