[PATCH 0/2] MR7211: windowscodecs: Implement IWICBitmapFlipRotator(WICBitmapTransformRotate90) for bitmaps with bpp >= 8.
From: Dmitry Timoshkov dmitry@baikal.ru
Signed-off-by: Dmitry Timoshkov dmitry@baikal.ru --- dlls/windowscodecs/tests/bitmap.c | 65 +++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+)
diff --git a/dlls/windowscodecs/tests/bitmap.c b/dlls/windowscodecs/tests/bitmap.c index f070eaeb47f..85e4ebaa04d 100644 --- a/dlls/windowscodecs/tests/bitmap.c +++ b/dlls/windowscodecs/tests/bitmap.c @@ -1476,6 +1476,70 @@ static void test_IMILBitmap(void) IWICBitmap_Release(bitmap); }
+static void test_FlipRotator(void) +{ + static BYTE data[] = { 1,2,3, 4,5,6 }; + static BYTE data_rotate90[] = { 4,1, 5,2, 6,3 }; + HRESULT hr; + IWICBitmap *bitmap; + IWICPalette *palette; + IWICBitmapFlipRotator *fr; + WICColor colors[256]; + WICRect rc; + UINT width, height; + BYTE buf[sizeof(data)]; + int i, ret; + + hr = IWICImagingFactory_CreateBitmapFromMemory(factory, 3, 2, &GUID_WICPixelFormat8bppIndexed, + 3, sizeof(data), data, &bitmap); + ok(hr == S_OK, "got %#lx\n", hr); + + hr = IWICImagingFactory_CreatePalette(factory, &palette); + ok(hr == S_OK, "got %#lx\n", hr); + + for (i = 0; i < 256; i++) + colors[i] = i; + hr = IWICPalette_InitializeCustom(palette, colors, 256); + ok(hr == S_OK, "got %#lx\n", hr); + + hr = IWICBitmap_SetPalette(bitmap, palette); + ok(hr == S_OK, "got %#lx\n", hr); + + hr = IWICImagingFactory_CreateBitmapFlipRotator(factory, &fr); + ok(hr == S_OK, "got %#lx\n", hr); + + hr = IWICBitmapFlipRotator_Initialize(fr, (IWICBitmapSource *)bitmap, WICBitmapTransformRotate90); + ok(hr == S_OK, "got %#lx\n", hr); + + hr = IWICBitmapFlipRotator_GetSize(fr, &width, &height); + ok(hr == S_OK, "got %#lx\n", hr); + ok(width == 2, "got %u\n", width); + ok(height == 3, "got %u\n", height); + + rc.X = 0; + rc.Y = 0; + rc.Width = width; + rc.Height = height; + memset(buf, 0, sizeof(buf)); + hr = IWICBitmapFlipRotator_CopyPixels(fr, &rc, 2, sizeof(buf), buf); + todo_wine + ok(hr == S_OK, "got %#lx\n", hr); + ret = !memcmp(buf, data_rotate90, sizeof(data_rotate90)); + todo_wine + ok(ret, "data mismatch\n"); + if (!ret && winetest_debug > 1) + { + printf("got data:\n"); + for (i = 0; i < sizeof(buf); i++) + printf(" %u", buf[i]); + printf("\n"); + } + + IWICBitmapFlipRotator_Release(fr); + IWICPalette_Release(palette); + IWICBitmap_Release(bitmap); +} + START_TEST(bitmap) { HRESULT hr; @@ -1494,6 +1558,7 @@ START_TEST(bitmap) test_CreateBitmapFromHBITMAP(); test_clipper(); test_bitmap_scaler(); + test_FlipRotator();
IWICImagingFactory_Release(factory);
From: Dmitry Timoshkov dmitry@baikal.ru
Signed-off-by: Dmitry Timoshkov dmitry@baikal.ru --- dlls/windowscodecs/fliprotate.c | 56 +++++++++++++++++++++++++++---- dlls/windowscodecs/tests/bitmap.c | 2 -- 2 files changed, 50 insertions(+), 8 deletions(-)
diff --git a/dlls/windowscodecs/fliprotate.c b/dlls/windowscodecs/fliprotate.c index 7fc5959eeda..c880e10ea33 100644 --- a/dlls/windowscodecs/fliprotate.c +++ b/dlls/windowscodecs/fliprotate.c @@ -1,5 +1,6 @@ /* * Copyright 2010 Vincent Povirk for CodeWeavers + * Copyright 2024 Dmitry Timoshkov * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -37,6 +38,7 @@ typedef struct FlipRotator { int flip_x; int flip_y; int swap_xy; + UINT bpp; CRITICAL_SECTION lock; /* must be held when initialized */ } FlipRotator;
@@ -154,7 +156,7 @@ static HRESULT WINAPI FlipRotator_CopyPixels(IWICBitmapFlipRotator *iface, { FlipRotator *This = impl_from_IWICBitmapFlipRotator(iface); HRESULT hr; - UINT y; + UINT x, y, width, height; UINT srcy, srcwidth, srcheight; WICRect rc; WICRect rect; @@ -163,21 +165,20 @@ static HRESULT WINAPI FlipRotator_CopyPixels(IWICBitmapFlipRotator *iface,
if (!This->source) return WINCODEC_ERR_WRONGSTATE;
- if (This->swap_xy || This->flip_x) + if ((This->swap_xy || This->flip_x) && This->bpp < 8) { /* This requires knowledge of the pixel format. */ - FIXME("flipping x and rotating are not implemented\n"); + FIXME("flipping x and rotating are not implemented for %u bpp bitmap\n", This->bpp); return E_NOTIMPL; }
hr = IWICBitmapSource_GetSize(This->source, &srcwidth, &srcheight); if (FAILED(hr)) return hr;
+ hr = IWICBitmapFlipRotator_GetSize(iface, &width, &height); + if (FAILED(hr)) return hr; if (!prc) { - UINT width, height; - hr = IWICBitmapFlipRotator_GetSize(iface, &width, &height); - if (FAILED(hr)) return hr; rect.X = 0; rect.Y = 0; rect.Width = width; @@ -185,6 +186,44 @@ static HRESULT WINAPI FlipRotator_CopyPixels(IWICBitmapFlipRotator *iface, prc = ▭ }
+ if (This->swap_xy && This->flip_x) /* Rotate90 */ + { + UINT bytes_per_pixel = This->bpp / 8, stride = srcwidth * bytes_per_pixel; + BYTE *bits; + + bits = malloc(srcheight * stride); + if (!bits) return E_OUTOFMEMORY; + + rc.X = 0; + rc.Y = 0; + rc.Width = srcwidth; + rc.Height = srcheight; + hr = IWICBitmapSource_CopyPixels(This->source, &rc, stride, srcheight * stride, bits); + if (FAILED(hr)) + { + free(bits); + return hr; + } + + for (y = prc->Y; y - prc->Y < prc->Height; y++) + { + for (x = prc->X; x - prc->X < prc->Width; x++) + { + BYTE *src = bits + (width - x - 1) * stride + y * bytes_per_pixel; + BYTE *dst = pbBuffer + y * cbStride + x * bytes_per_pixel; + memcpy(dst, src, bytes_per_pixel); + } + } + + free(bits); + return S_OK; + } + else if (This->swap_xy || This->flip_x) + { + FIXME("flipping x and rotating are not implemented for %u bpp bitmap\n", This->bpp); + return E_NOTIMPL; + } + for (y=prc->Y; y - prc->Y < prc->Height; y++) { if (This->flip_y) @@ -212,6 +251,7 @@ static HRESULT WINAPI FlipRotator_Initialize(IWICBitmapFlipRotator *iface, IWICBitmapSource *pISource, WICBitmapTransformOptions options) { FlipRotator *This = impl_from_IWICBitmapFlipRotator(iface); + GUID pf; HRESULT hr=S_OK;
TRACE("(%p,%p,%u)\n", iface, pISource, options); @@ -242,6 +282,10 @@ static HRESULT WINAPI FlipRotator_Initialize(IWICBitmapFlipRotator *iface, if (options&WICBitmapTransformFlipVertical) This->flip_y = !This->flip_y;
+ hr = IWICBitmapSource_GetPixelFormat(pISource, &pf); + if (SUCCEEDED(hr)) + hr = get_pixelformat_bpp(&pf, &This->bpp); + IWICBitmapSource_AddRef(pISource); This->source = pISource;
diff --git a/dlls/windowscodecs/tests/bitmap.c b/dlls/windowscodecs/tests/bitmap.c index 85e4ebaa04d..f89e89d7459 100644 --- a/dlls/windowscodecs/tests/bitmap.c +++ b/dlls/windowscodecs/tests/bitmap.c @@ -1522,10 +1522,8 @@ static void test_FlipRotator(void) rc.Height = height; memset(buf, 0, sizeof(buf)); hr = IWICBitmapFlipRotator_CopyPixels(fr, &rc, 2, sizeof(buf), buf); - todo_wine ok(hr == S_OK, "got %#lx\n", hr); ret = !memcmp(buf, data_rotate90, sizeof(data_rotate90)); - todo_wine ok(ret, "data mismatch\n"); if (!ret && winetest_debug > 1) {
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=151091
Your paranoid android.
=== debian11b (64 bit WoW report) ===
user32: input.c:4306: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 00000000015700EE, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
Esme Povirk (@madewokherd) commented about dlls/windowscodecs/tests/bitmap.c:
- rc.Y = 0;
- rc.Width = width;
- rc.Height = height;
- memset(buf, 0, sizeof(buf));
- hr = IWICBitmapFlipRotator_CopyPixels(fr, &rc, 2, sizeof(buf), buf);
- todo_wine
- ok(hr == S_OK, "got %#lx\n", hr);
- ret = !memcmp(buf, data_rotate90, sizeof(data_rotate90));
- todo_wine
- ok(ret, "data mismatch\n");
- if (!ret && winetest_debug > 1)
- {
printf("got data:\n");for (i = 0; i < sizeof(buf); i++)printf(" %u", buf[i]);printf("\n");
Should we be using printf in tests?
Esme Povirk (@madewokherd) commented about dlls/windowscodecs/fliprotate.c:
prc = ▭ }
- if (This->swap_xy && This->flip_x) /* Rotate90 */
I don't think we should have to implement every permutation of these individually.
On Sat Feb 1 01:55:00 2025 +0000, Esme Povirk wrote:
Should we be using printf in tests?
I've copied it from other similar places in the windowscodecs tests.
On Sat Feb 1 01:58:24 2025 +0000, Esme Povirk wrote:
I don't think we should have to implement every permutation of these individually.
The application I've implemented this for needs only Rotate90 which I can readily verify. I guess that other cases could be added using this implementation with minor adjustments. However since I can't reliably test other cases I'd leave them to someone else to implement.
On Sat Feb 1 12:25:53 2025 +0000, Dmitry Timoshkov wrote:
The application I've implemented this for needs only Rotate90 which I can readily verify. I guess that other cases could be added using this implementation with minor adjustments. However since I can't reliably test other cases I'd leave them to someone else to implement.
Well, the point of splitting it into variables like this was that we'd only have to implement 3 operations (with flip_y already implemented) to cover all cases.
-
Dmitry Timoshkov -
Dmitry Timoshkov (＠dmitry)
-
Esme Povirk (＠madewokherd)
-
Marvin