[PATCH 0/1] MR2323: gdiplus: Use transparency instead of background color if transparent color flag is set in GIF GCE.
From: Piotr Caban <piotr(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54563 --- dlls/gdiplus/image.c | 16 ++-------- dlls/gdiplus/tests/image.c | 65 +++++++++++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 14 deletions(-) diff --git a/dlls/gdiplus/image.c b/dlls/gdiplus/image.c index 101e7ae1c93..3b9ce3c14f1 100644 --- a/dlls/gdiplus/image.c +++ b/dlls/gdiplus/image.c @@ -3881,20 +3881,10 @@ static DWORD get_gif_background_color(GpBitmap *bitmap) UINT i; for(i=0; i<bitmap->prop_count; i++) { - if(bitmap->prop_item[i].id == PropertyTagIndexBackground) { + if(bitmap->prop_item[i].id == PropertyTagIndexBackground) bgcolor_idx = *(BYTE*)bitmap->prop_item[i].value; - break; - } - } - - for(i=0; i<bitmap->prop_count; i++) { - if(bitmap->prop_item[i].id == PropertyTagIndexTransparent) { - BYTE transparent_idx; - transparent_idx = *(BYTE*)bitmap->prop_item[i].value; - - if(transparent_idx == bgcolor_idx) - return 0; - } + else if(bitmap->prop_item[i].id == PropertyTagIndexTransparent) + return 0; } for(i=0; i<bitmap->prop_count; i++) { diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 9e35cf33eb4..e32b4c81240 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -2899,10 +2899,11 @@ static const unsigned char gifanimation[72] = { * -dispose previous -page +4+0 -size 2x2 xc:green \ * -dispose undefined -page +6+0 -size 2x2 xc:gray \ * test.gif + * Background color index changed to 1. */ static const unsigned char gifanimation2[] = { 0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0x08, 0x00, - 0x02, 0x00, 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x02, 0x00, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x21, 0xf9, 0x04, 0x01, 0x64, 0x00, 0x00, 0x00, 0x21, 0xff, 0x0b, 0x4e, 0x45, 0x54, 0x53, 0x43, 0x41, 0x50, 0x45, 0x32, 0x2e, @@ -2937,6 +2938,43 @@ static ARGB gifanimation2_pixels[5][4] = { {0xffff0000, 0, 0, 0xff7e7e7e} }; +static const unsigned char gifanimation3[] = { + 0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0x08, 0x00, + 0x02, 0x00, 0xf0, 0x01, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x21, 0xf9, 0x04, 0x00, 0x64, + 0x00, 0x00, 0x00, 0x21, 0xff, 0x0b, 0x4e, 0x45, + 0x54, 0x53, 0x43, 0x41, 0x50, 0x45, 0x32, 0x2e, + 0x30, 0x03, 0x01, 0x00, 0x00, 0x00, 0x2c, 0x00, + 0x00, 0x00, 0x00, 0x08, 0x00, 0x02, 0x00, 0x00, + 0x02, 0x04, 0x84, 0x8f, 0x09, 0x05, 0x00, 0x21, + 0xf9, 0x04, 0x05, 0x64, 0x00, 0x10, 0x00, 0x2c, + 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x02, 0x00, + 0x81, 0xff, 0x00, 0x00, 0xff, 0x00, 0x00, 0xff, + 0x00, 0x00, 0xff, 0x00, 0x00, 0x02, 0x03, 0x44, + 0x34, 0x05, 0x00, 0x21, 0xf9, 0x04, 0x09, 0x64, + 0x00, 0x10, 0x00, 0x2c, 0x02, 0x00, 0x00, 0x00, + 0x02, 0x00, 0x02, 0x00, 0x81, 0x00, 0x00, 0xff, + 0x00, 0x00, 0xff, 0x00, 0x00, 0xff, 0x00, 0x00, + 0xff, 0x02, 0x03, 0x44, 0x34, 0x05, 0x00, 0x21, + 0xf9, 0x04, 0x0d, 0x64, 0x00, 0x10, 0x00, 0x2c, + 0x04, 0x00, 0x00, 0x00, 0x02, 0x00, 0x02, 0x00, + 0x81, 0x00, 0x80, 0x00, 0x00, 0x80, 0x00, 0x00, + 0x80, 0x00, 0x00, 0x80, 0x00, 0x02, 0x03, 0x44, + 0x34, 0x05, 0x00, 0x21, 0xf9, 0x04, 0x01, 0x64, + 0x00, 0x10, 0x00, 0x2c, 0x06, 0x00, 0x00, 0x00, + 0x02, 0x00, 0x02, 0x00, 0x80, 0x7e, 0x7e, 0x7e, + 0x00, 0x00, 0x00, 0x02, 0x02, 0x84, 0x51, 0x00, + 0x3b +}; + +static ARGB gifanimation3_pixels[5][4] = { + {0xff000000, 0xff000000, 0xff000000, 0xff000000}, + {0xffff0000, 0xff000000, 0xff000000, 0xff000000}, + {0xffff0000, 0xff0000ff, 0xff000000, 0xff000000}, + {0xffff0000, 0xff000000, 0xff008000, 0xff000000}, + {0xffff0000, 0xff000000, 0xff000000, 0xff7e7e7e} +}; + static void test_multiframegif(void) { LPSTREAM stream; @@ -3185,6 +3223,31 @@ static void test_multiframegif(void) } GdipDisposeImage((GpImage*)bmp); + + hglob = GlobalAlloc (0, sizeof(gifanimation3)); + data = GlobalLock (hglob); + memcpy(data, gifanimation3, sizeof(gifanimation3)); + GlobalUnlock(hglob); + + hres = CreateStreamOnHGlobal(hglob, TRUE, &stream); + ok(hres == S_OK, "Failed to create a stream\n"); + + stat = GdipCreateBitmapFromStream(stream, &bmp); + ok(stat == Ok, "Failed to create a Bitmap\n"); + IStream_Release(stream); + + for(i=0; i<6; i++) { + stat = GdipImageSelectActiveFrame((GpImage*)bmp, &dimension, i%5); + expect(Ok, stat); + + for(j=0; j<4; j++) { + stat = GdipBitmapGetPixel(bmp, j*2, 0, &color); + expect(Ok, stat); + ok(gifanimation3_pixels[i%5][j] == color, "at %d,%d got %lx, expected %lx\n", i, j, color, gifanimation3_pixels[i%5][j]); + } + } + + GdipDisposeImage((GpImage*)bmp); } static void test_rotateflip(void) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2323
I'm having trouble understanding the logic to this. If there is a transparent index set, then "restore to background" becomes "restore to transparency" instead? Is that in the spec? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2323#note_26125
On Fri Mar 3 20:35:26 2023 +0000, Esme Povirk wrote:
I'm having trouble understanding the logic to this. If there is a transparent index set, then "restore to background" becomes "restore to transparency" instead? Is that in the spec? I have written old code hoping to match the spec (that is vague in this regard). New code is written to match tests and fix real applications.
Most modern implementations implement restore to background as restore to transparency (e.g. Firefox, Chrome) and ignore background color completely. This is something invented in gdi+. It's not a trustful source but a good description what gdi+ does is available [here](https://stackoverflow.com/questions/48015999/interpretation-of-background-co...). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2323#note_26131
On Fri Mar 3 20:35:26 2023 +0000, Piotr Caban wrote:
I have written old code hoping to match the spec (that is vague in this regard). New code is written to match tests and fix real applications. Most modern implementations implement restore to background as restore to transparency (e.g. Firefox, Chrome) and ignore background color completely. This is something invented in gdi+. It's not a trustful source but a good description what gdi+ does is available [here](https://stackoverflow.com/questions/48015999/interpretation-of-background-co...). Huh, OK. Thank you for explaining.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2323#note_26143
This merge request was approved by Esme Povirk. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2323
participants (3)
-
Esme Povirk (＠madewokherd) -
Piotr Caban
-
Piotr Caban (＠piotr)