[WIN32K]

Rewrite SURFACE_AllocSurface(), which now does the part of the former SURFACE_bSetBitmapBits() as well. This is to allocate the bits for a bitmap directly with the SURFACE object in one allocation. Also don't use kernel mode sections anymore by default, but paged pool memory.

svn path=/trunk/; revision=56460

reactos/win32ss/gdi/eng/surface.c

@@ -15,7 +15,7 @@
 #define NDEBUG
 #include <debug.h>
 
-ULONG giUniqueSurface = 0;
+LONG giUniqueSurface = 0;
 
 UCHAR
 gajBitsPerFormat[11] =
@@ -34,7 +34,9 @@ gajBitsPerFormat[11] =
 };
 
 
-ULONG FASTCALL BitmapFormat(ULONG cBits, ULONG iCompression)
+ULONG
+FASTCALL
+BitmapFormat(ULONG cBits, ULONG iCompression)
 {
     switch (iCompression)
     {
@@ -107,15 +109,10 @@ SURFACE_Cleanup(PVOID ObjectBody)
                 ASSERT(FALSE);
             }
         }
-        else if (psurf->SurfObj.fjBitmap & BMF_RLE_HACK)
+        else if (psurf->SurfObj.fjBitmap & BMF_POOLALLOC)
         {
-            /* HACK: Free RLE decompressed bits */
+            /* Free a pool allocation */
-            EngFreeMem(pvBits);
+            ExFreePool(pvBits);
-        }
-        else
-        {
-            /* There should be nothing to free */
-            ASSERT(psurf->SurfObj.fjBitmap & BMF_DONT_FREE);
         }
     }
 
@@ -132,126 +129,162 @@ SURFACE_Cleanup(PVOID ObjectBody)
 PSURFACE
 NTAPI
 SURFACE_AllocSurface(
-    IN USHORT iType,
+    _In_ USHORT iType,
-    IN ULONG cx,
+    _In_ ULONG cx,
-    IN ULONG cy,
+    _In_ ULONG cy,
-    IN ULONG iFormat)
+    _In_ ULONG iFormat,
+    _In_ ULONG fjBitmap,
+    _In_opt_ ULONG cjWidth,
+    _In_opt_ PVOID pvBits)
 {
+    ULONG cBitsPixel, cjBits, cjObject;
     PSURFACE psurf;
     SURFOBJ *pso;
+    PVOID pvSection;
+
+    ASSERT(!pvBits || (iType == STYPE_BITMAP));
+    ASSERT(pvBits || !(fjBitmap & BMF_DONT_FREE));
+    ASSERT(!pvBits || !(fjBitmap & BMF_SINGLEALLOC));
 
     /* Verify format */
-    if (iFormat < BMF_1BPP || iFormat > BMF_PNG)
+    if ((iFormat < BMF_1BPP) || (iFormat > BMF_PNG))
     {
         DPRINT1("Invalid bitmap format: %ld\n", iFormat);
         return NULL;
     }
 
-    /* Allocate a SURFACE object */
-    psurf = (PSURFACE)GDIOBJ_AllocObjWithHandle(GDI_OBJECT_TYPE_BITMAP, sizeof(SURFACE));
-
-    if (psurf)
-    {
-        /* Initialize the basic fields */
-        pso = &psurf->SurfObj;
-        pso->hsurf = psurf->BaseObject.hHmgr;
-        pso->sizlBitmap.cx = cx;
-        pso->sizlBitmap.cy = cy;
-        pso->iBitmapFormat = iFormat;
-        pso->iType = iType;
-        pso->iUniq = InterlockedIncrement((PLONG)&giUniqueSurface);
-
-        /* Assign a default palette and increment its reference count */
-        psurf->ppal = appalSurfaceDefault[iFormat];
-        GDIOBJ_vReferenceObjectByPointer(&psurf->ppal->BaseObject);
-    }
-
-    return psurf;
-}
-
-BOOL
-NTAPI
-SURFACE_bSetBitmapBits(
-    IN PSURFACE psurf,
-    IN ULONG fjBitmap,
-    IN ULONG ulWidth,
-    IN PVOID pvBits OPTIONAL)
-{
-    SURFOBJ *pso = &psurf->SurfObj;
-    PVOID pvSection;
-    UCHAR cBitsPixel;
-
-    /* Only bitmaps can have bits */
-    ASSERT(psurf->SurfObj.iType == STYPE_BITMAP);
-
     /* Get bits per pixel from the format */
-    cBitsPixel = gajBitsPerFormat[pso->iBitmapFormat];
+    cBitsPixel = gajBitsPerFormat[iFormat];
 
-    /* Is a width in bytes given? */
+    /* Are bits and a width in bytes given? */
-    if (ulWidth)
+    if (pvBits && cjWidth)
     {
         /* Align the width (Windows compatibility, drivers expect that) */
-        ulWidth = WIDTH_BYTES_ALIGN32((ulWidth << 3) / cBitsPixel, cBitsPixel);
+        cjWidth = WIDTH_BYTES_ALIGN32((cjWidth << 3) / cBitsPixel, cBitsPixel);
     }
-	else
+    else
-	{
+    {
         /* Calculate width from the bitmap width in pixels */
-        ulWidth = WIDTH_BYTES_ALIGN32(pso->sizlBitmap.cx, cBitsPixel);
+        cjWidth = WIDTH_BYTES_ALIGN32(cx, cBitsPixel);
-	}
+    }
 
     /* Calculate the bitmap size in bytes */
-    pso->cjBits = ulWidth * pso->sizlBitmap.cy;
+    cjBits = cjWidth * cy;
 
-    /* Did the caller provide bits? */
+    /* Check if we need an extra large object */
-    if (pvBits)
+    if ((iType == STYPE_BITMAP) && (pvBits == NULL) &&
+        !(fjBitmap & BMF_USERMEM) && !(fjBitmap & BMF_KMSECTION))
     {
-        /* Yes, so let him free it */
+        /* Allocate an object large enough to hold the bits */
-        fjBitmap |= BMF_DONT_FREE;
+        cjObject = sizeof(SURFACE) + cjBits;
     }
-    else if (pso->cjBits)
+    else
     {
-        /* We must allocate memory, check what kind */
+        /* Otherwise just allocate the SURFACE structure */
-        if (fjBitmap & BMF_USERMEM)
+        cjObject = sizeof(SURFACE);
+    }
+
+    /* Check for arithmetic overflow */
+    if ((cjBits < cjWidth) || (cjObject < sizeof(SURFACE)))
+    {
+        /* Fail! */
+        return NULL;
+    }
+
+    /* Allocate a SURFACE object */
+    psurf = (PSURFACE)GDIOBJ_AllocObjWithHandle(GDI_OBJECT_TYPE_BITMAP, cjObject);
+    if (!psurf)
+    {
+        return NULL;
+    }
+
+    /* Initialize the basic fields */
+    pso = &psurf->SurfObj;
+    pso->hsurf = psurf->BaseObject.hHmgr;
+    pso->sizlBitmap.cx = cx;
+    pso->sizlBitmap.cy = cy;
+    pso->iBitmapFormat = iFormat;
+    pso->iType = iType;
+    pso->fjBitmap = (USHORT)fjBitmap;
+    pso->iUniq = InterlockedIncrement(&giUniqueSurface);
+    pso->cjBits = cjBits;
+
+    /* Check if we need a bitmap buffer */
+    if (iType == STYPE_BITMAP)
+    {
+        /* Check if we got one or if we need to allocate one */
+        if (pvBits != NULL)
+        {
+            /* Use the caller provided buffer */
+            pso->pvBits = pvBits;
+        }
+        else if (fjBitmap & BMF_USERMEM)
         {
             /* User mode memory was requested */
-            pvBits = EngAllocUserMem(pso->cjBits, 0);
+            pso->pvBits = EngAllocUserMem(cjBits, 0);
+
+            /* Check for failure */
+            if (!pso->pvBits)
+            {
+                GDIOBJ_vDeleteObject(&psurf->BaseObject);
+                return NULL;
+            }
+        }
+        else if (fjBitmap & BMF_KMSECTION)
+        {
+            /* Use a kernel mode section */
+            pso->pvBits = EngAllocSectionMem(&pvSection,
+                                             (fjBitmap & BMF_NOZEROINIT) ?
+                                                 0 : FL_ZERO_MEMORY,
+                                             cjBits, TAG_DIB);
+
+            /* Check for failure */
+            if (!pso->pvBits)
+            {
+                GDIOBJ_vDeleteObject(&psurf->BaseObject);
+                return NULL;
+            }
+
+            /* Free the section already, but keep the mapping */
+            EngFreeSectionMem(pvSection, NULL);
         }
         else
         {
-            /* Use a kernel mode section */
+            /* Buffer is after the object */
-            fjBitmap |= BMF_KMSECTION;
+            pso->pvBits = psurf + 1;
-            pvBits = EngAllocSectionMem(&pvSection,
-                                        (fjBitmap & BMF_NOZEROINIT) ?
-                                                0 : FL_ZERO_MEMORY,
-                                        pso->cjBits, TAG_DIB);
 
-            /* Free the section already, but keep the mapping */
+            /* Zero the buffer, except requested otherwise */
-            if (pvBits) EngFreeSectionMem(pvSection, NULL);
+            if (!(fjBitmap & BMF_NOZEROINIT))
+            {
+                RtlZeroMemory(pso->pvBits, cjBits);
+            }
         }
-
+    }
-        /* Check for failure */
+    else
-        if (!pvBits) return FALSE;
+    {
+        /* There are no bitmap bits */
+        pso->pvBits = NULL;
     }
 
-    /* Set pvBits, pvScan0 and lDelta */
+    /* Set pvScan0 and lDelta */
-    pso->pvBits = pvBits;
     if (fjBitmap & BMF_TOPDOWN)
     {
         /* Topdown is the normal way */
         pso->pvScan0 = pso->pvBits;
-        pso->lDelta = ulWidth;
+        pso->lDelta = cjWidth;
     }
     else
     {
         /* Inversed bitmap (bottom up) */
-        pso->pvScan0 = (PVOID)((ULONG_PTR)pso->pvBits + pso->cjBits - ulWidth);
+        pso->pvScan0 = ((PCHAR)pso->pvBits + pso->cjBits - cjWidth);
-        pso->lDelta = -(LONG)ulWidth;
+        pso->lDelta = -(LONG)cjWidth;
     }
 
-    pso->fjBitmap = (USHORT)fjBitmap;
+    /* Assign a default palette and increment its reference count */
+    psurf->ppal = appalSurfaceDefault[iFormat];
+    GDIOBJ_vReferenceObjectByPointer(&psurf->ppal->BaseObject);
 
-    /* Success */
+    return psurf;
-    return TRUE;
 }
 
 HBITMAP
@@ -267,7 +300,13 @@ EngCreateBitmap(
     HBITMAP hbmp;
 
     /* Allocate a surface */
-    psurf = SURFACE_AllocSurface(STYPE_BITMAP, sizl.cx, sizl.cy, iFormat);
+    psurf = SURFACE_AllocSurface(STYPE_BITMAP,
+                                 sizl.cx,
+                                 sizl.cy,
+                                 iFormat,
+                                 fl,
+                                 lWidth,
+                                 pvBits);
     if (!psurf)
     {
         DPRINT1("SURFACE_AllocSurface failed.\n");
@@ -277,15 +316,6 @@ EngCreateBitmap(
     /* Get the handle for the bitmap */
     hbmp = (HBITMAP)psurf->SurfObj.hsurf;
 
-    /* Set the bitmap bits */
-    if (!SURFACE_bSetBitmapBits(psurf, fl, lWidth, pvBits))
-    {
-        /* Bail out if that failed */
-        DPRINT1("SURFACE_bSetBitmapBits failed.\n");
-        GDIOBJ_vDeleteObject(&psurf->BaseObject);
-        return NULL;
-    }
-
     /* Set public ownership */
     GDIOBJ_vSetObjectOwner(&psurf->BaseObject, GDI_OBJ_HMGR_PUBLIC);
 
@@ -308,10 +338,17 @@ EngCreateDeviceBitmap(
     HBITMAP hbmp;
 
     /* Allocate a surface */
-    psurf = SURFACE_AllocSurface(STYPE_DEVBITMAP, sizl.cx, sizl.cy, iFormat);
+    psurf = SURFACE_AllocSurface(STYPE_DEVBITMAP,
+                                 sizl.cx,
+                                 sizl.cy,
+                                 iFormat,
+                                 0,
+                                 0,
+                                 NULL);
     if (!psurf)
     {
-        return 0;
+        DPRINT1("SURFACE_AllocSurface failed.\n");
+        return NULL;
     }
 
     /* Set the device handle */
@@ -339,10 +376,17 @@ EngCreateDeviceSurface(
     HSURF hsurf;
 
     /* Allocate a surface */
-    psurf = SURFACE_AllocSurface(STYPE_DEVICE, sizl.cx, sizl.cy, iFormat);
+    psurf = SURFACE_AllocSurface(STYPE_DEVICE,
+                                 sizl.cx,
+                                 sizl.cy,
+                                 iFormat,
+                                 0,
+                                 0,
+                                 NULL);
     if (!psurf)
     {
-        return 0;
+        DPRINT1("SURFACE_AllocSurface failed.\n");
+        return NULL;
     }
 
     /* Set the device handle */

reactos/win32ss/gdi/eng/surface.h

@@ -83,7 +83,8 @@ typedef struct _SURFACE
 
 #define BMF_DONT_FREE 0x100
 #define BMF_RLE_HACK  0x200
-
+#define BMF_SINGLEALLOC 0x400
+#define BMF_POOLALLOC 0x800
 
 /*  Internal interface  */
 
@@ -99,31 +100,30 @@ typedef struct _SURFACE
 #define  SURFACE_ShareUnlockSurface(pBMObj)  \
   GDIOBJ_vDereferenceObject ((POBJ)pBMObj)
 
-BOOL NTAPI SURFACE_Cleanup(PVOID ObjectBody);
-
-PSURFACE
-NTAPI
-SURFACE_AllocSurface(
-    IN USHORT iType,
-    IN ULONG cx,
-    IN ULONG cy,
-    IN ULONG iFormat);
-
-BOOL
-NTAPI
-SURFACE_bSetBitmapBits(
-    IN PSURFACE psurf,
-    IN ULONG fjBitmap,
-    IN ULONG ulWidth,
-    IN PVOID pvBits OPTIONAL);
-
 #define GDIDEV(SurfObj) ((PDEVOBJ *)((SurfObj)->hdev))
 #define GDIDEVFUNCS(SurfObj) ((PDEVOBJ *)((SurfObj)->hdev))->DriverFunctions
 
-ULONG FASTCALL BitmapFormat(ULONG cBits, ULONG iCompression);
 extern UCHAR gajBitsPerFormat[];
 #define BitsPerFormat(Format) gajBitsPerFormat[Format]
 
 #define WIDTH_BYTES_ALIGN32(cx, bpp) ((((cx) * (bpp) + 31) & ~31) >> 3)
 #define WIDTH_BYTES_ALIGN16(cx, bpp) ((((cx) * (bpp) + 15) & ~15) >> 3)
 
+ULONG
+FASTCALL
+BitmapFormat(ULONG cBits, ULONG iCompression);
+
+BOOL
+NTAPI
+SURFACE_Cleanup(PVOID ObjectBody);
+
+PSURFACE
+NTAPI
+SURFACE_AllocSurface(
+    _In_ USHORT iType,
+    _In_ ULONG cx,
+    _In_ ULONG cy,
+    _In_ ULONG iFormat,
+    _In_ ULONG fjBitmap,
+    _In_opt_ ULONG cjWidth,
+    _In_opt_ PVOID pvBits);

reactos/win32ss/gdi/ntgdi/bitblt.c

@@ -1160,34 +1160,36 @@ NtGdiGetPixel(
     }
 
     /* Allocate a surface */
-    psurfDest = SURFACE_AllocSurface(STYPE_BITMAP, 1, 1, BMF_32BPP);
+    psurfDest = SURFACE_AllocSurface(STYPE_BITMAP,
+                                     1,
+                                     1,
+                                     BMF_32BPP,
+                                     BMF_DONT_FREE,
+                                     0,
+                                     &ulRGBColor);
     if (psurfDest)
     {
-        /* Set the bitmap bits */
+        RECTL rclDest = {0, 0, 1, 1};
-        if (SURFACE_bSetBitmapBits(psurfDest, 0, 0, &ulRGBColor))
+        EXLATEOBJ exlo;
-        {
-            RECTL rclDest = {0, 0, 1, 1};
-            EXLATEOBJ exlo;
 
-            /* Translate from the source palette to RGB color */
+        /* Translate from the source palette to RGB color */
-            EXLATEOBJ_vInitialize(&exlo,
+        EXLATEOBJ_vInitialize(&exlo,
-                                  psurfSrc->ppal,
+                              psurfSrc->ppal,
-                                  &gpalRGB,
+                              &gpalRGB,
-                                  0,
+                              0,
-                                  RGB(0xff,0xff,0xff),
+                              RGB(0xff,0xff,0xff),
-                                  RGB(0,0,0));
+                              RGB(0,0,0));
 
-            /* Call the copy bits function */
+        /* Call the copy bits function */
-            EngCopyBits(&psurfDest->SurfObj,
+        EngCopyBits(&psurfDest->SurfObj,
-                        &psurfSrc->SurfObj,
+                    &psurfSrc->SurfObj,
-                        NULL,
+                    NULL,
-                        &exlo.xlo,
+                    &exlo.xlo,
-                        &rclDest,
+                    &rclDest,
-                        &ptlSrc);
+                    &ptlSrc);
 
-            /* Cleanup the XLATEOBJ */
+        /* Cleanup the XLATEOBJ */
-            EXLATEOBJ_vCleanup(&exlo);
+        EXLATEOBJ_vCleanup(&exlo);
-        }
 
         /* Delete the surface */
         GDIOBJ_vDeleteObject(&psurfDest->BaseObject);

reactos/win32ss/gdi/ntgdi/bitmaps.c

@@ -56,25 +56,36 @@ GreCreateBitmapEx(
 {
     PSURFACE psurf;
     HBITMAP hbmp;
+    PVOID pvCompressedBits;
 
     /* Verify format */
     if (iFormat < BMF_1BPP || iFormat > BMF_PNG) return NULL;
 
+    /* The infamous RLE hack */
+    if ((iFormat == BMF_4RLE) || (iFormat == BMF_8RLE))
+    {
+        pvCompressedBits = pvBits;
+        pvBits = NULL;
+        iFormat = (iFormat == BMF_4RLE) ? BMF_4BPP : BMF_8BPP;
+    }
+
     /* Allocate a surface */
-    psurf = SURFACE_AllocSurface(STYPE_BITMAP, nWidth, nHeight, iFormat);
+    psurf = SURFACE_AllocSurface(STYPE_BITMAP,
+                                 nWidth,
+                                 nHeight,
+                                 iFormat,
+                                 fjBitmap,
+                                 cjWidthBytes,
+                                 pvBits);
     if (!psurf)
     {
         DPRINT1("SURFACE_AllocSurface failed.\n");
         return NULL;
     }
 
-    /* Get the handle for the bitmap */
-    hbmp = (HBITMAP)psurf->SurfObj.hsurf;
-
     /* The infamous RLE hack */
-    if (iFormat == BMF_4RLE || iFormat == BMF_8RLE)
+    if ((iFormat == BMF_4RLE) || (iFormat == BMF_8RLE))
     {
-        PVOID pvCompressedBits;
         SIZEL sizl;
         LONG lDelta;
 
@@ -82,34 +93,17 @@ GreCreateBitmapEx(
         sizl.cy = nHeight;
         lDelta = WIDTH_BYTES_ALIGN32(nWidth, gajBitsPerFormat[iFormat]);
 
-        pvCompressedBits = pvBits;
+        pvBits = psurf->SurfObj.pvBits;
-        pvBits = EngAllocMem(FL_ZERO_MEMORY, lDelta * nHeight, TAG_DIB);
-        if (!pvBits)
-        {
-            EngSetLastError(ERROR_NOT_ENOUGH_MEMORY);
-            GDIOBJ_vDeleteObject(&psurf->BaseObject);
-            return NULL;
-        }
         DecompressBitmap(sizl, pvCompressedBits, pvBits, lDelta, iFormat);
-        fjBitmap |= BMF_RLE_HACK;
+        psurf->SurfObj.fjBitmap |= BMF_RLE_HACK;
-
-        iFormat = iFormat == BMF_4RLE ? BMF_4BPP : BMF_8BPP;
-        psurf->SurfObj.iBitmapFormat = iFormat;
     }
 
+    /* Get the handle for the bitmap */
+    hbmp = (HBITMAP)psurf->SurfObj.hsurf;
+
     /* Mark as API bitmap */
     psurf->flags |= (flags | API_BITMAP);
 
-    /* Set the bitmap bits */
-    if (!SURFACE_bSetBitmapBits(psurf, fjBitmap, cjWidthBytes, pvBits))
-    {
-        /* Bail out if that failed */
-        DPRINT1("SURFACE_bSetBitmapBits failed.\n");
-        EngSetLastError(ERROR_NOT_ENOUGH_MEMORY);
-        GDIOBJ_vDeleteObject(&psurf->BaseObject);
-        return NULL;
-    }
-
     /* Unlock the surface and return */
     SURFACE_UnlockSurface(psurf);
     return hbmp;
@@ -149,8 +143,9 @@ NtGdiCreateBitmap(
     IN OPTIONAL LPBYTE pUnsafeBits)
 {
     HBITMAP hbmp;
-    ULONG cRealBpp, cjWidthBytes, iFormat;
+    ULONG cRealBpp, cjWidthBytes, iFormat, fjBitmap;
     ULONGLONG cjSize;
+    PSURFACE psurf;
 
     /* Calculate bitmap format and real bits per pixel. */
     iFormat = BitmapFormat(cBitsPixel * cPlanes, BI_RGB);
@@ -170,12 +165,27 @@ NtGdiCreateBitmap(
         return NULL;
     }
 
-    /* Call internal function. */
+    /* Allocate the surface (but don't set the bits) */
-    hbmp = GreCreateBitmapEx(nWidth, nHeight, 0, iFormat, 0, 0, NULL, DDB_SURFACE);
+    psurf = SURFACE_AllocSurface(STYPE_BITMAP,
-
+                                 nWidth,
-    if (pUnsafeBits && hbmp)
+                                 nHeight,
+                                 iFormat,
+                                 fjBitmap,
+                                 0,
+                                 NULL);
+    if (!psurf)
     {
-        PSURFACE psurf = SURFACE_ShareLockSurface(hbmp);
+        DPRINT1("SURFACE_AllocSurface failed.\n");
+        return NULL;
+    }
+
+    /* Mark as API and DDB bitmap */
+    psurf->flags |= (API_BITMAP | DDB_SURFACE);
+
+    /* Check if we have bits to set */
+    if (pUnsafeBits)
+    {
+        /* Protect with SEH and copy the bits */
         _SEH2_TRY
         {
             ProbeForRead(pUnsafeBits, (SIZE_T)cjSize, 1);
@@ -187,9 +197,18 @@ NtGdiCreateBitmap(
             _SEH2_YIELD(return NULL;)
         }
         _SEH2_END
-
-        SURFACE_ShareUnlockSurface(psurf);
     }
+    else
+    {
+        /* Zero the bits */
+        RtlZeroMemory(psurf->SurfObj.pvBits, psurf->SurfObj.cjBits);
+    }
+
+    /* Get the handle for the bitmap */
+    hbmp = (HBITMAP)psurf->SurfObj.hsurf;
+
+    /* Unlock the surface */
+    SURFACE_UnlockSurface(psurf);
 
     return hbmp;
 }
@@ -324,7 +343,8 @@ NtGdiCreateCompatibleBitmap(
     HBITMAP Bmp;
     PDC Dc;
 
-    if (Width <= 0 || Height <= 0 || (Width * Height) > 0x3FFFFFFF)
+    /* Check parameters */
+    if ((Width <= 0) || (Height <= 0) || ((Width * Height) > 0x3FFFFFFF))
     {
         EngSetLastError(ERROR_INVALID_PARAMETER);
         return NULL;
@@ -351,17 +371,19 @@ NtGdiCreateCompatibleBitmap(
     return Bmp;
 }
 
-BOOL APIENTRY
+BOOL
+APIENTRY
 NtGdiGetBitmapDimension(
     HBITMAP hBitmap,
-    LPSIZE Dimension)
+    LPSIZE psizDim)
 {
     PSURFACE psurfBmp;
-    BOOL Ret = TRUE;
+    BOOL bResult = TRUE;
 
     if (hBitmap == NULL)
         return FALSE;
 
+    /* Lock the bitmap */
     psurfBmp = SURFACE_ShareLockSurface(hBitmap);
     if (psurfBmp == NULL)
     {
@@ -369,20 +391,22 @@ NtGdiGetBitmapDimension(
         return FALSE;
     }
 
+    /* Use SEH to copy the data to the caller */
     _SEH2_TRY
     {
-        ProbeForWrite(Dimension, sizeof(SIZE), 1);
+        ProbeForWrite(psizDim, sizeof(SIZE), 1);
-        *Dimension = psurfBmp->sizlDim;
+        *psizDim = psurfBmp->sizlDim;
     }
     _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
     {
-        Ret = FALSE;
+        bResult = FALSE;
     }
     _SEH2_END
 
+    /* Unlock the bitmap */
     SURFACE_ShareUnlockSurface(psurfBmp);
 
-    return Ret;
+    return bResult;
 }
 
 
@@ -580,7 +604,7 @@ BITMAP_CopyBitmap(HBITMAP hBitmap)
                                psurfSrc->SurfObj.sizlBitmap.cy,
                                abs(psurfSrc->SurfObj.lDelta),
                                psurfSrc->SurfObj.iBitmapFormat,
-                               psurfSrc->SurfObj.fjBitmap,
+                               psurfSrc->SurfObj.fjBitmap & BMF_TOPDOWN,
                                psurfSrc->SurfObj.cjBits,
                                NULL,
                                psurfSrc->flags);