comctl32: Make ImageList_Read and ImageList_Write compatible with each other, simplify the code

Dmitry Timoshkov dmitry at codeweavers.com
Mon Feb 12 02:47:56 CST 2007


Hello,

while investigating a regression in Outlook XP caused by recent changes
in internal image management, I discovered that ImageList_Write tries
to be smart by re-arranging DIB bits, but ImageList_Read is not able
to handle the result, that leads to black squares instead of images.

I decided to have a look how native comctl32 saves an image list in
ImageList_Write: it simply writes the whole bitmap it has internally,
avoiding any conversion.

This patch changes how ImageList_Read and ImageList_Write work, greatly
simplifies the code, and fixes Outlook XP for me.

Changelog:
    comctl32: Make ImageList_Read and ImageList_Write compatible with
    each other, simplify the code.

---
 dlls/comctl32/imagelist.c |  200 ++++++++++++++++++++++-----------------------
 1 files changed, 97 insertions(+), 103 deletions(-)

diff --git a/dlls/comctl32/imagelist.c b/dlls/comctl32/imagelist.c
index 54248bc..972a84a 100644
--- a/dlls/comctl32/imagelist.c
+++ b/dlls/comctl32/imagelist.c
@@ -1853,92 +1853,97 @@ ImageList_Merge (HIMAGELIST himl1, INT i1, HIMAGELIST himl2, INT i2,
 }
 
 
-/* helper for _read_bitmap currently unused */
-#if 0
-static int may_use_dibsection(HDC hdc) {
-    int bitspixel = GetDeviceCaps(hdc,BITSPIXEL)*GetDeviceCaps(hdc,PLANES);
-    if (bitspixel>8)
-	return TRUE;
-    if (bitspixel<=4)
-	return FALSE;
-    return GetDeviceCaps(hdc,CAPS1) & C1_DIBENGINE;
+/***********************************************************************
+ *           DIB_GetDIBWidthBytes
+ *
+ * Return the width of a DIB bitmap in bytes. DIB bitmap data is 32-bit aligned.
+ */
+static int DIB_GetDIBWidthBytes( int width, int depth )
+{
+    int words;
+
+    switch(depth)
+    {
+    case 1:  words = (width + 31) / 32; break;
+    case 4:  words = (width + 7) / 8; break;
+    case 8:  words = (width + 3) / 4; break;
+    case 15:
+    case 16: words = (width + 1) / 2; break;
+    case 24: words = (width * 3 + 3)/4; break;
+
+    default:
+        WARN("(%d): Unsupported depth\n", depth );
+        /* fall through */
+    case 32:
+        words = width;
+        break;
+    }
+    return 4 * words;
+}
+
+/***********************************************************************
+ *           DIB_GetDIBImageBytes
+ *
+ * Return the number of bytes used to hold the image in a DIB bitmap.
+ */
+static int DIB_GetDIBImageBytes( int width, int height, int depth )
+{
+    return DIB_GetDIBWidthBytes( width, depth ) * abs( height );
 }
-#endif
+
 
 /* helper for ImageList_Read, see comments below */
-static BOOL _read_bitmap(HIMAGELIST himl, HDC hdcIml, LPSTREAM pstm, int ilcFlag)
+static BOOL _read_bitmap(HIMAGELIST himl, HDC hdcIml, LPSTREAM pstm)
 {
-    HDC                 xdc = 0;
     BITMAPFILEHEADER	bmfh;
-    BITMAPINFOHEADER	bmih;
-    int			bitsperpixel,palspace,longsperline,width,height;
-    LPBITMAPINFO       bmi = NULL;
+    int bitsperpixel, palspace;
+    char bmi_buf[sizeof(BITMAPINFOHEADER) + sizeof(RGBQUAD) * 256];
+    LPBITMAPINFO bmi = (LPBITMAPINFO)bmi_buf;
     int                result = FALSE;
-    HBITMAP            hDIB = 0;
     LPBYTE             bits = NULL;
-    int i, j, nheight, nRows, nCols;
-    POINT pt;
-    int cy = himl->cy;
 
     if (!SUCCEEDED(IStream_Read ( pstm, &bmfh, sizeof(bmfh), NULL)))
-        return result;
+        return FALSE;
 
     if (bmfh.bfType != (('M'<<8)|'B'))
-        return result;
+        return FALSE;
 
-    if (!SUCCEEDED(IStream_Read ( pstm, &bmih, sizeof(bmih), NULL)))
-        return result;
+    if (!SUCCEEDED(IStream_Read ( pstm, &bmi->bmiHeader, sizeof(bmi->bmiHeader), NULL)))
+        return FALSE;
 
-    if ((bmih.biSize != sizeof(bmih)))
-	return 0;
+    if ((bmi->bmiHeader.biSize != sizeof(bmi->bmiHeader)))
+        return FALSE;
+
+    TRACE("width %u, height %u, planes %u, bpp %u\n",
+          bmi->bmiHeader.biWidth, bmi->bmiHeader.biHeight,
+          bmi->bmiHeader.biPlanes, bmi->bmiHeader.biBitCount);
 
-    bitsperpixel = bmih.biPlanes * bmih.biBitCount;
+    bitsperpixel = bmi->bmiHeader.biPlanes * bmi->bmiHeader.biBitCount;
     if (bitsperpixel<=8)
         palspace = (1<<bitsperpixel)*sizeof(RGBQUAD);
     else
         palspace = 0;
-    width = bmih.biWidth;
-    height = bmih.biHeight;
-    bmi = Alloc(sizeof(bmih)+palspace);
-    if (!bmi)
-        return result;
 
-    memcpy(bmi, &bmih, sizeof(bmih));
-    longsperline = ((width*bitsperpixel+31)&~0x1f)>>5;
-    bmi->bmiHeader.biSizeImage = (longsperline*height)<<2;
+    bmi->bmiHeader.biSizeImage = DIB_GetDIBImageBytes(bmi->bmiHeader.biWidth, bmi->bmiHeader.biHeight, bitsperpixel);
 
     /* read the palette right after the end of the bitmapinfoheader */
     if (palspace && !SUCCEEDED(IStream_Read(pstm, bmi->bmiColors, palspace, NULL)))
 	goto error;
 
-    xdc = GetDC(0);
-
-    nheight = cy;
-    nRows = height/cy;
-    nCols = width/himl->cx;
-
-    hDIB = CreateDIBSection(xdc, bmi, 0, (LPVOID*) &bits, 0, 0);
-    if (!hDIB)
+    bits = Alloc(bmi->bmiHeader.biSizeImage);
+    if (!bits)
         goto error;
     if (!SUCCEEDED(IStream_Read(pstm, bits, bmi->bmiHeader.biSizeImage, NULL)))
         goto error;
 
-    /* Copy the NxM bitmap into a 1x(N*M) bitmap we need, linewise */
-    /* Do not forget that windows bitmaps are bottom->top */
-    for (i=0; i < nRows; i++) {
-        for (j=0; j < nCols; j++) {
-            imagelist_point_from_index(himl, i*nCols + j, &pt);
-            StretchDIBits(hdcIml, pt.x, pt.y, himl->cx, cy,
-                      j*himl->cx, (nRows - 1 - i)*himl->cy, himl->cx, cy, bits,
-                      bmi, DIB_RGB_COLORS, SRCCOPY);
-        }
-    }
-
+    if (!StretchDIBits(hdcIml, 0, 0, bmi->bmiHeader.biWidth, bmi->bmiHeader.biHeight,
+                  0, 0, bmi->bmiHeader.biWidth, bmi->bmiHeader.biHeight,
+                  bits, bmi, DIB_RGB_COLORS, SRCCOPY))
+        goto error;
     result = TRUE;
+
 error:
-    if (xdc)	ReleaseDC(0,xdc);
-    Free(bmi);
-    if (hDIB)   DeleteObject(hDIB);
+    Free(bits);
     return result;
 }
 
@@ -1972,9 +1977,6 @@ error:
  *	RGBQUAD		rgbs[nr_of_paletted_colors];
  *
  *	BYTE			maskbits[imagesize];
- *
- * CAVEAT: Those images are within a NxM bitmap, not the 1xN we expect.
- *         _read_bitmap needs to convert them.
  */
 HIMAGELIST WINAPI ImageList_Read (LPSTREAM pstm)
 {
@@ -1991,16 +1993,23 @@ HIMAGELIST WINAPI ImageList_Read (LPSTREAM pstm)
     if (ilHead.usVersion != 0x101) /* probably version? */
 	return NULL;
 
+    TRACE("cx %u, cy %u, flags 0x%04x, cCurImage %u, cMaxImage %u\n",
+          ilHead.cx, ilHead.cy, ilHead.flags, ilHead.cCurImage, ilHead.cMaxImage);
+
     himl = ImageList_Create(ilHead.cx, ilHead.cy, ilHead.flags, ilHead.cCurImage, ilHead.cMaxImage);
-    if (!himl) {
+    if (!himl)
 	return NULL;
-    }
-    if (!_read_bitmap(himl, himl->hdcImage, pstm, ilHead.flags & ~ILC_MASK)) {
+
+    if (!_read_bitmap(himl, himl->hdcImage, pstm))
+    {
 	WARN("failed to read bitmap from stream\n");
 	return NULL;
     }
-    if (ilHead.flags & ILC_MASK) {
-	if (!_read_bitmap(himl, himl->hdcMask, pstm, 0)) {
+    if (ilHead.flags & ILC_MASK)
+    {
+        if (!_read_bitmap(himl, himl->hdcMask, pstm))
+        {
+            WARN("failed to read mask bitmap from stream\n");
 	    return NULL;
 	}
     }
@@ -2651,38 +2660,29 @@ ImageList_SetOverlayImage (HIMAGELIST himl, INT iImage, INT iOverlay)
  * currently everything is written as 24 bit RGB, except masks
  */
 static BOOL
-_write_bitmap(HBITMAP hBitmap, LPSTREAM pstm, int cx, int cy)
+_write_bitmap(HBITMAP hBitmap, LPSTREAM pstm)
 {
     LPBITMAPFILEHEADER bmfh;
     LPBITMAPINFOHEADER bmih;
-    LPBYTE data = NULL, lpBits = NULL, lpBitsOrg = NULL;
+    LPBYTE data = NULL, lpBits;
     BITMAP bm;
     INT bitCount, sizeImage, offBits, totalSize;
-    INT nwidth, nheight, nsizeImage, icount;
     HDC xdc;
     BOOL result = FALSE;
 
-
-    xdc = GetDC(0);
     if (!GetObjectW(hBitmap, sizeof(BITMAP), (LPVOID)&bm))
-        goto failed;
-
-    /* XXX is this always correct? */
-    icount = bm.bmWidth / cx;
-    nwidth = cx;
-    nheight = cy * icount;
+        return FALSE;
 
     bitCount = bm.bmBitsPixel == 1 ? 1 : 24;
-    sizeImage = ((((bm.bmWidth * bitCount)+31) & ~31) >> 3) * bm.bmHeight;
-    nsizeImage = ((((nwidth * bitCount)+31) & ~31) >> 3) * nheight;
+    sizeImage = DIB_GetDIBImageBytes(bm.bmWidth, bm.bmHeight, bitCount);
 
     totalSize = sizeof(BITMAPFILEHEADER) + sizeof(BITMAPINFOHEADER);
     if(bitCount != 24)
 	totalSize += (1 << bitCount) * sizeof(RGBQUAD);
     offBits = totalSize;
-    totalSize += nsizeImage;
+    totalSize += sizeImage;
 
-    data = (LPBYTE)LocalAlloc(LMEM_ZEROINIT, totalSize);
+    data = Alloc(totalSize);
     bmfh = (LPBITMAPFILEHEADER)data;
     bmih = (LPBITMAPINFOHEADER)(data + sizeof(BITMAPFILEHEADER));
     lpBits = data + offBits;
@@ -2707,25 +2707,15 @@ _write_bitmap(HBITMAP hBitmap, LPSTREAM pstm, int cx, int cy)
     bmih->biClrUsed       = 0;
     bmih->biClrImportant  = 0;
 
-    lpBitsOrg = (LPBYTE)LocalAlloc(LMEM_ZEROINIT, sizeImage);
-    if(!GetDIBits(xdc, hBitmap, 0, bm.bmHeight, lpBitsOrg,
-		  (BITMAPINFO *)bmih, DIB_RGB_COLORS))
+    xdc = GetDC(0);
+    result = GetDIBits(xdc, hBitmap, 0, bm.bmHeight, lpBits, (BITMAPINFO *)bmih, DIB_RGB_COLORS) == bm.bmHeight;
+    ReleaseDC(0, xdc);
+    if (!result)
 	goto failed;
-    else {
-	int i;
-	int obpl = (((bm.bmWidth*bitCount+31) & ~31)>>3);
-	int nbpl = (((nwidth*bitCount+31) & ~31)>>3);
-
-	for(i = 0; i < nheight; i++) {
-	    int ooff = ((nheight-1-i)%cy) * obpl + ((i/cy) * nbpl);
-	    int noff = (nbpl * (nheight-1-i));
-	    memcpy(lpBits + noff, lpBitsOrg + ooff, nbpl);
-	}
-    }
 
-    bmih->biWidth  = nwidth;
-    bmih->biHeight = nheight;
-    bmih->biSizeImage = nsizeImage;
+    TRACE("width %u, height %u, planes %u, bpp %u\n",
+          bmih->biWidth, bmih->biHeight,
+          bmih->biPlanes, bmih->biBitCount);
 
     if(bitCount == 1) {
         /* Hack. */
@@ -2739,10 +2729,8 @@ _write_bitmap(HBITMAP hBitmap, LPSTREAM pstm, int cx, int cy)
 
     result = TRUE;
 
-    failed:
-    ReleaseDC(0, xdc);
-    LocalFree((HLOCAL)lpBitsOrg);
-    LocalFree((HLOCAL)data);
+failed:
+    Free(data);
 
     return result;
 }
@@ -2771,6 +2759,8 @@ ImageList_Write (HIMAGELIST himl, LPSTREAM pstm)
     ILHEAD ilHead;
     int i;
 
+    TRACE("%p %p\n", himl, pstm);
+
     if (!is_valid(himl))
 	return FALSE;
 
@@ -2787,16 +2777,19 @@ ImageList_Write (HIMAGELIST himl, LPSTREAM pstm)
 	ilHead.ovls[i] = himl->nOvlIdx[i];
     }
 
+    TRACE("cx %u, cy %u, flags 0x04%x, cCurImage %u, cMaxImage %u\n",
+          ilHead.cx, ilHead.cy, ilHead.flags, ilHead.cCurImage, ilHead.cMaxImage);
+
     if(!SUCCEEDED(IStream_Write(pstm, &ilHead, sizeof(ILHEAD), NULL)))
 	return FALSE;
 
     /* write the bitmap */
-    if(!_write_bitmap(himl->hbmImage, pstm, himl->cx, himl->cy))
+    if(!_write_bitmap(himl->hbmImage, pstm))
 	return FALSE;
 
     /* write the mask if we have one */
     if(himl->flags & ILC_MASK) {
-	if(!_write_bitmap(himl->hbmMask, pstm, himl->cx, himl->cy))
+	if(!_write_bitmap(himl->hbmMask, pstm))
 	    return FALSE;
     }
 
@@ -2817,7 +2810,8 @@ static HBITMAP ImageList_CreateImage(HDC hdc, HIMAGELIST himl, UINT count, UINT
         VOID* bits;
         BITMAPINFO *bmi;
 
-        TRACE("Creating DIBSection: %d Bits per Pixel\n", himl->uBitsPixel);
+        TRACE("Creating DIBSection %d x %d, %d Bits per Pixel\n",
+              sz.cx, sz.cy, himl->uBitsPixel);
 
 	if (himl->uBitsPixel <= ILC_COLOR8)
 	{
-- 
1.4.4.4






More information about the wine-patches mailing list