[PATCH v2] oleaut32: improve the icon size selection logic in OleLoadPictureEx
Damjan Jovanovic
damjan.jov at gmail.com
Mon May 11 10:34:58 CDT 2020
Currently OleLoadPictureEx() ignores the caller-desired icon size
and always loads the 32x32 icon, which sometimes has to be
scaled down to 16x16, resulting in quality loss. VB6 apps are
especially affected, due to their heavy use of that API
at a framework level. Change this to load the icon having the
desired size, or the 32x32 one if no desired size is requested,
and use the first icon in the file if nothing is found.
Try 2 adds thorough unit tests and improves the icon
selection logic to match what Windows does.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=20732
Signed-off-by: Damjan Jovanovic <damjan.jov at gmail.com>
---
dlls/oleaut32/olepicture.c | 29 ++++--
dlls/oleaut32/tests/olepicture.c | 147 +++++++++++++++++++++++++++++++
2 files changed, 170 insertions(+), 6 deletions(-)
-------------- next part --------------
diff --git a/dlls/oleaut32/olepicture.c b/dlls/oleaut32/olepicture.c
index 43a92ca58f..6366e075ab 100644
--- a/dlls/oleaut32/olepicture.c
+++ b/dlls/oleaut32/olepicture.c
@@ -151,6 +151,8 @@ typedef struct OLEPictureImpl {
BOOL bIsDirty; /* Set to TRUE if picture has changed */
unsigned int loadtime_magic; /* If a length header was found, saves value */
unsigned int loadtime_format; /* for PICTYPE_BITMAP only, keeps track of image format (GIF/BMP/JPEG) */
+ DWORD desiredWidth;
+ DWORD desiredHeight;
} OLEPictureImpl;
static inline OLEPictureImpl *impl_from_IPicture(IPicture *iface)
@@ -316,6 +318,8 @@ static HRESULT OLEPictureImpl_Construct(LPPICTDESC pictDesc, BOOL fOwn, OLEPictu
newObject->loadtime_magic = 0xdeadbeef;
newObject->loadtime_format = 0;
newObject->bIsDirty = FALSE;
+ newObject->desiredWidth = LP_DEFAULT;
+ newObject->desiredHeight = LP_DEFAULT;
if (pictDesc) {
newObject->desc = *pictDesc;
@@ -1251,14 +1255,23 @@ static HRESULT OLEPictureImpl_LoadIcon(OLEPictureImpl *This, BYTE *xbuf, ULONG x
return E_FAIL;
}
i=0;
- /* If we have more than one icon, try to find the best.
- * this currently means '32 pixel wide'.
- */
if (cifd->idCount!=1) {
- for (i=0;i<cifd->idCount;i++) {
- if (cifd->idEntries[i].bWidth == 32)
- break;
+ if (This->desiredWidth == LP_DEFAULT && This->desiredHeight == LP_DEFAULT) {
+ /* Windows returns various sizes here, unrelated to SM_CXICON/SM_CYICON.
+ * We'll return width 32. */
+ for (i=0;i<cifd->idCount;i++) {
+ if (cifd->idEntries[i].bWidth == 32)
+ break;
+ }
+ } else {
+ /* Exact match on the desired dimensions */
+ for (i=0;i<cifd->idCount;i++) {
+ if (cifd->idEntries[i].bWidth == This->desiredWidth &&
+ cifd->idEntries[i].bHeight == This->desiredHeight)
+ break;
+ }
}
+ /* Otherwise, return the first one, like Windows does. */
if (i==cifd->idCount) i=0;
}
if (xread < cifd->idEntries[i].dwDIBOffset + cifd->idEntries[i].dwDIBSize)
@@ -2297,6 +2310,7 @@ HRESULT WINAPI OleLoadPictureEx( LPSTREAM lpstream, LONG lSize, BOOL fRunmode,
{
LPPERSISTSTREAM ps;
IPicture *newpic;
+ OLEPictureImpl *pictureImpl;
HRESULT hr;
FIXME("(%p,%d,%d,%s,x=%d,y=%d,f=%x,%p), partially implemented.\n",
@@ -2305,6 +2319,9 @@ HRESULT WINAPI OleLoadPictureEx( LPSTREAM lpstream, LONG lSize, BOOL fRunmode,
hr = OleCreatePictureIndirect(NULL,riid,!fRunmode,(LPVOID*)&newpic);
if (hr != S_OK)
return hr;
+ pictureImpl = impl_from_IPicture(newpic);
+ pictureImpl->desiredWidth = xsiz;
+ pictureImpl->desiredHeight = ysiz;
hr = IPicture_QueryInterface(newpic,&IID_IPersistStream, (LPVOID*)&ps);
if (hr != S_OK) {
ERR("Could not get IPersistStream iface from Ole Picture?\n");
diff --git a/dlls/oleaut32/tests/olepicture.c b/dlls/oleaut32/tests/olepicture.c
index 6a5f7ee042..2f88f98805 100644
--- a/dlls/oleaut32/tests/olepicture.c
+++ b/dlls/oleaut32/tests/olepicture.c
@@ -169,6 +169,78 @@ static const unsigned char enhmetafile[] = {
0x14, 0x00, 0x00, 0x00
};
+static const unsigned char ico_16x16_32x32[518] = {
+ 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x10, 0x10, 0x02, 0x00, 0x01, 0x00, 0x01, 0x00, 0xb0, 0x00,
+ 0x00, 0x00, 0x26, 0x00, 0x00, 0x00, 0x20, 0x20, 0x02, 0x00, 0x01, 0x00, 0x01, 0x00, 0x30, 0x01,
+ 0x00, 0x00, 0xd6, 0x00, 0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x20, 0x00,
+ 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
+ 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x40, 0x00,
+ 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
+ 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
+static const unsigned char ico_32x32_16x16[518] = {
+ 0x00, 0x00, 0x01, 0x00, 0x02, 0x00, 0x20, 0x20, 0x02, 0x00, 0x01, 0x00, 0x01, 0x00, 0x30, 0x01,
+ 0x00, 0x00, 0x26, 0x00, 0x00, 0x00, 0x10, 0x10, 0x02, 0x00, 0x01, 0x00, 0x01, 0x00, 0xb0, 0x00,
+ 0x00, 0x00, 0x56, 0x01, 0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x40, 0x00,
+ 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
+ 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x20, 0x00,
+ 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
+ 0xff, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+
static HBITMAP stock_bm;
static HDC create_render_dc( void )
@@ -1293,6 +1365,80 @@ todo_wine
IPicture_Release(pic);
}
+static void test_ico_size_in_size_out(IStream *stream,
+ INT width_in, INT height_in,
+ INT width_out, INT height_out)
+{
+ LARGE_INTEGER seekto;
+ HRESULT hres;
+ IPicture* pict;
+ OLE_HANDLE handle;
+ ICONINFO iconInfo;
+ BITMAP DECLSPEC_ALIGN(4) bmp;
+
+ memset(&seekto, 0, sizeof(seekto));
+ hres = IStream_Seek(stream, seekto, SEEK_SET, NULL);
+ ok (hres == S_OK, "IStream_Seek failed, hres 0x%08x\n", hres);
+
+ ole_check(pOleLoadPictureEx(stream, 0, TRUE, &IID_IPicture,
+ width_in, height_in, LP_DEFAULT, (LPVOID *)&pict));
+ hres = IPicture_get_Handle(pict, &handle);
+ ok(SUCCEEDED(hres) && handle, "unexpected: res 0x%08x, handle %u\n", hres, handle);
+ ok(GetIconInfo(UlongToHandle(handle), &iconInfo), "GetIconInfo failed\n");
+ ok(GetObjectA(iconInfo.hbmColor, sizeof(BITMAP), &bmp), "GetObjectA failed\n");
+ ok(bmp.bmWidth == width_out && bmp.bmHeight == height_out,
+ "unexpected: (width, height) (%d, %d) -> (%d, %d) instead of (%d, %d)\n",
+ width_in, height_in, bmp.bmWidth, bmp.bmHeight, width_out, height_out);
+ DeleteObject(iconInfo.hbmColor);
+ DeleteObject(iconInfo.hbmMask);
+ IPicture_Release(pict);
+}
+
+static void icon_size_selection_tests(const unsigned char *icon_data, int icon_data_size, int first_icon_size)
+{
+ LPSTREAM stream;
+ HGLOBAL hglob;
+ LPBYTE data;
+
+ hglob = GlobalAlloc(GMEM_MOVEABLE, icon_data_size);
+ ok(hglob, "out of memory\n");
+ data = GlobalLock(hglob);
+ memcpy(data, icon_data, icon_data_size);
+ GlobalUnlock(hglob);
+
+ ole_check(CreateStreamOnHGlobal(hglob, TRUE, &stream));
+
+ /* Even these, which are expected to exact-match, return icons
+ * that are too large or too small on some Windows setups: */
+ if (0)
+ {
+ test_ico_size_in_size_out(stream, 16, 16, 16, 16);
+ test_ico_size_in_size_out(stream, 32, 32, 32, 32);
+ }
+ /* It various between 16x16 or 32x32 on Windows, independently of SM_CXICON/SM_CYICON: */
+ if (0)
+ test_ico_size_in_size_out(stream, LP_DEFAULT, LP_DEFAULT, GetSystemMetrics(SM_CXICON), GetSystemMetrics(SM_CYICON));
+
+ test_ico_size_in_size_out(stream, 16, LP_DEFAULT, first_icon_size, first_icon_size);
+ test_ico_size_in_size_out(stream, LP_DEFAULT, 16, first_icon_size, first_icon_size);
+ test_ico_size_in_size_out(stream, 32, LP_DEFAULT, first_icon_size, first_icon_size);
+ test_ico_size_in_size_out(stream, LP_DEFAULT, 32, first_icon_size, first_icon_size);
+
+ test_ico_size_in_size_out(stream, 16, 32, first_icon_size, first_icon_size);
+ test_ico_size_in_size_out(stream, 32, 16, first_icon_size, first_icon_size);
+ test_ico_size_in_size_out(stream, 8, 8, first_icon_size, first_icon_size);
+ test_ico_size_in_size_out(stream, 40, 40, first_icon_size, first_icon_size);
+ test_ico_size_in_size_out(stream, 8, 40, first_icon_size, first_icon_size);
+
+ IStream_Release(stream);
+}
+
+static void test_icon_size_selection(void)
+{
+ icon_size_selection_tests(ico_16x16_32x32, ARRAY_SIZE(ico_16x16_32x32), 16);
+ icon_size_selection_tests(ico_32x32_16x16, ARRAY_SIZE(ico_32x32_16x16), 32);
+}
+
static void test_load_save_empty_picture(void)
{
IPicture *pic;
@@ -1443,6 +1589,7 @@ START_TEST(olepicture)
test_load_save_bmp();
test_load_save_icon();
test_load_save_empty_picture();
+ test_icon_size_selection();
}
More information about the wine-devel
mailing list