Re: [5/5] gdiplus: Add some tests for GdipBitmapLockBits/GdipBitmapUnlockBits.
Shouldn't your second LockBits in this loop always use PixelFormat24bppRGB and ImageLockModeRead? As it is, if the LockBits/UnlockBits is writing but not reading, you won't be able to tell. Also, you might want to try flags == 0 in some pixel format other than 24bppRGB. As I understand it, all modes without ImageLockModeUserInputBuf are treated as read+write regardless of flags when the bitmap is locked in its native format, as an optimization. You may simply be running into that optimization. On Wed, Jul 11, 2012 at 2:49 AM, Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
With additional tests for flags set to ImageLockModeUserInputBuf only. --- dlls/gdiplus/tests/image.c | 184 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 184 insertions(+)
diff --git a/dlls/gdiplus/tests/image.c b/dlls/gdiplus/tests/image.c index 29182aa..6ce100d 100644 --- a/dlls/gdiplus/tests/image.c +++ b/dlls/gdiplus/tests/image.c @@ -3342,6 +3342,189 @@ todo_wine GdipDisposeImage(image); }
+static void test_bitmapbits(void) +{ + + static const BYTE pixels_24[24] = {0xff,0xff,0xff, 0,0,0, 0xff,0xff,0xff, 0,0,0, + 0xff,0xff,0xff, 0,0,0, 0xff,0xff,0xff, 0,0,0}; + static const BYTE pixels_24_00[24] = {0,0,0, 0,0,0, 0,0,0, 0,0,0, + 0,0,0, 0,0,0, 0,0,0, 0,0,0}; + static const BYTE pixels_24_77[32] = {0xff,0xff,0xff, 0,0,0, 0xff,0xff,0xff, 0,0,0,0x77,0x77,0x77,0x77, + 0xff,0xff,0xff, 0,0,0, 0xff,0xff,0xff, 0,0,0,0x77,0x77,0x77,0x77}; + static const BYTE pixels_24_88[32] = {0xff,0xff,0xff, 0,0,0, 0xff,0xff,0xff, 0,0,0,0x88,0x88,0x88,0x88, + 0xff,0xff,0xff, 0,0,0, 0xff,0xff,0xff, 0,0,0,0x88,0x88,0x88,0x88}; +#if 0 /* FIXME: these tests crash gdiplus in Wine */ + static const BYTE pixels_8[8] = {0x01,0,0x01,0,0x01,0,0x01,0}; + static const BYTE pixels_8_00[8] = {0,0,0,0,0,0,0,0}; + static const BYTE pixels_8_77[32] = {0x01,0,0x01,0,0x77,0x77,0x77,0x77, + 0x77,0x77,0x77,0x77,0x77,0x77,0x77,0x77, + 0x01,0,0x01,0,0x77,0x77,0x77,0x77, + 0x77,0x77,0x77,0x77,0x77,0x77,0x77,0x77}; + static const BYTE pixels_8_88[32] = {0x01,0,0x01,0,0x88,0x88,0x88,0x88, + 0x88,0x88,0x88,0x88,0x88,0x88,0x88,0x88, + 0x01,0,0x01,0,0x88,0x88,0x88,0x88, + 0x88,0x88,0x88,0x88,0x88,0x88,0x88,0x88}; + static const BYTE pixels_a7[32] = {0xa7,0x77,0x77,0x77,0x77,0x77,0x77,0x77, + 0x77,0x77,0x77,0x77,0x77,0x77,0x77,0x77, + 0xa7,0x77,0x77,0x77,0x77,0x77,0x77,0x77, + 0x77,0x77,0x77,0x77,0x77,0x77,0x77,0x77}; + static const BYTE pixels_1[8] = {0xa0,0,0,0,0xa0,0,0,0}; +#endif + static const BYTE pixels_77[32] = {0x77,0x77,0x77,0x77,0x77,0x77,0x77,0x77, + 0x77,0x77,0x77,0x77,0x77,0x77,0x77,0x77, + 0x77,0x77,0x77,0x77,0x77,0x77,0x77,0x77, + 0x77,0x77,0x77,0x77,0x77,0x77,0x77,0x77}; + static const BYTE pixels_88[32] = {0x88,0x88,0x88,0x88,0x88,0x88,0x88,0x88, + 0x88,0x88,0x88,0x88,0x88,0x88,0x88,0x88, + 0x88,0x88,0x88,0x88,0x88,0x88,0x88,0x88, + 0x88,0x88,0x88,0x88,0x88,0x88,0x88,0x88}; + static const struct test_data + { + PixelFormat format; + ImageLockMode mode; + UINT width, height, stride, size; + const BYTE *pixels; + const BYTE *pixels_unlocked; + } td[] = + { + { PixelFormat24bppRGB, 0xfff0, 4, 2, 12, 24, pixels_24, pixels_24_00 }, + { PixelFormat24bppRGB, 0, 4, 2, 12, 24, pixels_24, pixels_24_00 }, + + { PixelFormat24bppRGB, ImageLockModeRead, 4, 2, 12, 24, pixels_24, pixels_24_00 }, + { PixelFormat24bppRGB, ImageLockModeWrite, 4, 2, 12, 24, pixels_24, pixels_24_00 }, + { PixelFormat24bppRGB, ImageLockModeRead|ImageLockModeWrite, 4, 2, 12, 24, pixels_24, pixels_24_00 }, + { PixelFormat24bppRGB, ImageLockModeRead|ImageLockModeUserInputBuf, 4, 2, 16, 32, pixels_24_77, pixels_24_88 }, + { PixelFormat24bppRGB, ImageLockModeWrite|ImageLockModeUserInputBuf, 4, 2, 16, 32, pixels_77, pixels_88 }, + { PixelFormat24bppRGB, ImageLockModeUserInputBuf, 4, 2, 16, 32, pixels_77, pixels_88 }, +#if 0 /* FIXME: these tests crash gdiplus in Wine */ + { PixelFormat8bppIndexed, ImageLockModeRead, 4, 2, 4, 8, pixels_8, pixels_8 }, + { PixelFormat8bppIndexed, ImageLockModeWrite, 4, 2, 4, 8, pixels_8, pixels_8 }, + { PixelFormat8bppIndexed, ImageLockModeRead|ImageLockModeWrite, 4, 2, 4, 8, pixels_8, pixels_8_00 }, + { PixelFormat8bppIndexed, ImageLockModeRead|ImageLockModeUserInputBuf, 4, 2, 16, 32, pixels_8_77, pixels_8_88 }, + { PixelFormat8bppIndexed, ImageLockModeWrite|ImageLockModeUserInputBuf, 4, 2, 16, 32, pixels_77, pixels_88 }, + { PixelFormat8bppIndexed, ImageLockModeUserInputBuf, 4, 2, 16, 32, pixels_77, pixels_88 }, + + { PixelFormat1bppIndexed, ImageLockModeRead, 4, 2, 4, 8, pixels_1, pixels_1 }, + { PixelFormat1bppIndexed, ImageLockModeWrite, 4, 2, 4, 8, pixels_1, pixels_1 }, + { PixelFormat1bppIndexed, ImageLockModeRead|ImageLockModeWrite, 4, 2, 4, 8, pixels_1, pixels_1 }, + { PixelFormat1bppIndexed, ImageLockModeRead|ImageLockModeUserInputBuf, 4, 2, 16, 32, pixels_a7, pixels_a7 }, +#endif + }; + BYTE buf[32]; + GpStatus status; + GpBitmap *bitmap; + UINT i; + BitmapData data; + struct + { + ColorPalette pal; + ARGB entries[1]; + } palette; + + for (i = 0; i < sizeof(td)/sizeof(td[0]); i++) + { + BYTE pixels[sizeof(pixels_24)]; + memcpy(pixels, pixels_24, sizeof(pixels_24)); + status = GdipCreateBitmapFromScan0(4, 2, 12, PixelFormat24bppRGB, pixels, &bitmap); + expect(Ok, status); + + /* associate known palette with pixel data */ + palette.pal.Flags = PaletteFlagsGrayScale; + palette.pal.Count = 2; + palette.pal.Entries[0] = 0; + palette.pal.Entries[1] = 0xffffffff; + status = GdipSetImagePalette((GpImage *)bitmap, &palette.pal); + expect(Ok, status); + + memset(&data, 0xfe, sizeof(data)); + if (td[i].mode & ImageLockModeUserInputBuf) + { + memset(buf, 0x77, sizeof(buf)); + data.Scan0 = buf; + data.Stride = 16; + } + status = GdipBitmapLockBits(bitmap, NULL, td[i].mode, td[i].format, &data); + ok(status == Ok || broken(status == InvalidParameter) /* XP */, "%u: GdipBitmapLockBits error %d\n", i, status); + if (status != Ok) + { + GdipDisposeImage((GpImage *)bitmap); + continue; + } + ok(td[i].width == data.Width, "%u: expected %d, got %d\n", i, td[i].width, data.Width); + ok(td[i].height == data.Height, "%u: expected %d, got %d\n", i, td[i].height, data.Height); + ok(td[i].stride == data.Stride, "%u: expected %d, got %d\n", i, td[i].stride, data.Stride); + ok(td[i].format == data.PixelFormat, "%u: expected %d, got %d\n", i, td[i].format, data.PixelFormat); + ok(td[i].size == data.Height * data.Stride, "%u: expected %d, got %d\n", i, td[i].size, data.Height * data.Stride); + if (td[i].mode & ImageLockModeUserInputBuf) + ok(data.Scan0 == buf, "%u: got wrong buffer\n", i); + if (td[i].size == data.Height * data.Stride) + { + int match = memcmp(data.Scan0, td[i].pixels, td[i].size) == 0; + if ((td[i].mode & (ImageLockModeRead|ImageLockModeWrite|ImageLockModeUserInputBuf)) == ImageLockModeWrite && td[i].format != PixelFormat24bppRGB) + ok(!match, "%u: data shouldn't match\n", i); + else + { + /* copying to user buffer is very buggy even in win7 */ + ok(match || broken(!match && (td[i].mode & ImageLockModeUserInputBuf)), + "%u: data should match\n", i); + if (!match) + { + UINT j; + BYTE *bits = data.Scan0; + printf("%u: data mismatch for format %#x:", i, td[i].format); + for (j = 0; j < td[i].size; j++) + printf(" %02x", bits[j]); + printf("\n"); + } + } + + memset(data.Scan0, 0, td[i].size); + } + + status = GdipBitmapUnlockBits(bitmap, &data); + ok(status == Ok, "%u: GdipBitmapUnlockBits error %d\n", i, status); + + memset(&data, 0xfe, sizeof(data)); + if (td[i].mode & ImageLockModeUserInputBuf) + { + memset(buf, 0x88, sizeof(buf)); + data.Scan0 = buf; + data.Stride = 16; + } + status = GdipBitmapLockBits(bitmap, NULL, td[i].mode, td[i].format, &data); + ok(status == Ok, "%u: GdipBitmapLockBits error %d\n", i, status); + ok(td[i].width == data.Width, "%u: expected %d, got %d\n", i, td[i].width, data.Width); + ok(td[i].height == data.Height, "%u: expected %d, got %d\n", i, td[i].height, data.Height); + ok(td[i].stride == data.Stride, "%u: expected %d, got %d\n", i, td[i].stride, data.Stride); + ok(td[i].format == data.PixelFormat, "%u: expected %d, got %d\n", i, td[i].format, data.PixelFormat); + ok(td[i].size == data.Height * data.Stride, "%u: expected %d, got %d\n", i, td[i].size, data.Height * data.Stride); + if (td[i].mode & ImageLockModeUserInputBuf) + ok(data.Scan0 == buf, "%u: got wrong buffer\n", i); + /* copying indexed data without associated palette from BitmapData leads + to funny effects, that's a design flaw with lock/unlock pixel data */ + if (td[i].size == data.Height * data.Stride && !(td[i].format & PixelFormatIndexed)) + { + int match = memcmp(data.Scan0, td[i].pixels_unlocked, td[i].size) == 0; + ok(match, "%u: data should match\n", i); + if (!match) + { + UINT j; + BYTE *bits = data.Scan0; + printf("%u: data mismatch for format %#x:", i, td[i].format); + for (j = 0; j < td[i].size; j++) + printf(" %02x", bits[j]); + printf("\n"); + } + } + + status = GdipBitmapUnlockBits(bitmap, &data); + ok(status == Ok, "%u: GdipBitmapUnlockBits error %d\n", i, status); + + status = GdipDisposeImage((GpImage *)bitmap); + expect(Ok, status); + } +} + START_TEST(image) { struct GdiplusStartupInput gdiplusStartupInput; @@ -3354,6 +3537,7 @@ START_TEST(image)
GdiplusStartup(&gdiplusToken, &gdiplusStartupInput, NULL);
+ test_bitmapbits(); test_tiff_palette(); test_GdipGetAllPropertyItems(); test_tiff_properties(); -- 1.7.11.1
Vincent Povirk <madewokherd(a)gmail.com> wrote:
Shouldn't your second LockBits in this loop always use PixelFormat24bppRGB and ImageLockModeRead? As it is, if the LockBits/UnlockBits is writing but not reading, you won't be able to tell.
Probably that would simplify the test a bit, but I don't see a problem with testing the same format.
Also, you might want to try flags == 0 in some pixel format other than 24bppRGB. As I understand it, all modes without ImageLockModeUserInputBuf are treated as read+write regardless of flags when the bitmap is locked in its native format, as an optimization. You may simply be running into that optimization.
Probably, but that's a different case IMO. -- Dmitry.
participants (2)
-
Dmitry Timoshkov -
Vincent Povirk