[tkreuzer] 70388: [WIN32K] Fix bugs in pen implementation: - Do not use the x coordinate to adjust styles, instead they start where the lines start - Don't leak allocated styles - Make sure the PEN...

Author: tkreuzer
Date: Thu Dec 17 22:05:06 2015
New Revision: 70388

URL: http://svn.reactos.org/svn/reactos?rev=70388&view=rev
Log:
[WIN32K]
Fix bugs in pen implementation:
- Do not use the x coordinate to adjust styles, instead they start where the lines start
- Don't leak allocated styles
- Make sure the PEN fields are initialized correctly, even for BRUSHES, so that the destructor can do it's cleanup work
- Fix numerous parameter checks
gdi32_apitest:pen now shows 0 failures!

Modified:
    trunk/reactos/win32ss/gdi/eng/lineto.c
    trunk/reactos/win32ss/gdi/ntgdi/brush.cpp
    trunk/reactos/win32ss/gdi/ntgdi/pen.c

Modified: trunk/reactos/win32ss/gdi/eng/lineto.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/gdi/eng/lineto.c?re...
==============================================================================
--- trunk/reactos/win32ss/gdi/eng/lineto.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/gdi/eng/lineto.c	[iso-8859-1] Thu Dec 17 22:05:06 2015
@@ -49,13 +49,13 @@
     {
         if (deltax > deltay)
         {
-            offStyle = (x - Translate->x) % pebo->pbrush->ulStyleSize;
+            offStyle = (- Translate->x) % pebo->pbrush->ulStyleSize;
             diStyle = dx;
             lStyleMax = x;
         }
         else
         {
-            offStyle = (y - Translate->y) % pebo->pbrush->ulStyleSize;
+            offStyle = (- Translate->y) % pebo->pbrush->ulStyleSize;
             diStyle = dy;
             lStyleMax = y;
         }

Modified: trunk/reactos/win32ss/gdi/ntgdi/brush.cpp
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/gdi/ntgdi/brush.cpp...
==============================================================================
--- trunk/reactos/win32ss/gdi/ntgdi/brush.cpp	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/gdi/ntgdi/brush.cpp	[iso-8859-1] Thu Dec 17 22:05:06 2015
@@ -45,6 +45,11 @@
     this->ulSurfTime = 0;
     this->pvRBrush = NULL;
     this->hdev = NULL;
+
+    /* FIXME: should be done only in PEN constructor,
+       but our destructor needs it! */
+    this->dwStyleCount = 0;
+    this->pStyle = NULL;
 }

BRUSH::~BRUSH(
@@ -62,6 +67,12 @@
     {
         GreSetBitmapOwner(this->hbmPattern, BASEOBJECT::OWNER::POWNED);
         GreDeleteObject(this->hbmPattern);
+    }
+
+    /* Delete styles */
+    if ((this->pStyle != NULL) && !(this->flAttrs & BR_IS_DEFAULTSTYLE))
+    {
+        ExFreePoolWithTag(this->pStyle, GDITAG_PENSTYLE);
     }
 }

Modified: trunk/reactos/win32ss/gdi/ntgdi/pen.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/gdi/ntgdi/pen.c?rev...
==============================================================================
--- trunk/reactos/win32ss/gdi/ntgdi/pen.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/gdi/ntgdi/pen.c	[iso-8859-1] Thu Dec 17 22:05:06 2015
@@ -128,7 +128,7 @@
     pbrushPen->iBrushStyle = ulBrushStyle;
     // FIXME: Copy the bitmap first ?
     pbrushPen->hbmClient = (HANDLE)ulClientHatch;
-    pbrushPen->dwStyleCount = dwStyleCount;
+    pbrushPen->dwStyleCount = 0;
     pbrushPen->pStyle = NULL;
     pbrushPen->ulStyleSize = 0;

@@ -152,31 +152,31 @@
         break;

case PS_ALTERNATE:
-        pbrushPen->flAttrs |= BR_IS_SOLID;
+        pbrushPen->flAttrs |= BR_IS_SOLID | BR_IS_DEFAULTSTYLE;
         pbrushPen->pStyle = aulStyleAlternate;
         pbrushPen->dwStyleCount = _countof(aulStyleAlternate);
         break;

case PS_DOT:
-        pbrushPen->flAttrs |= BR_IS_SOLID;
+        pbrushPen->flAttrs |= BR_IS_SOLID | BR_IS_DEFAULTSTYLE;
         pbrushPen->pStyle = aulStyleDot;
         pbrushPen->dwStyleCount = _countof(aulStyleDot);
         break;

case PS_DASH:
-        pbrushPen->flAttrs |= BR_IS_SOLID;
+        pbrushPen->flAttrs |= BR_IS_SOLID | BR_IS_DEFAULTSTYLE;
         pbrushPen->pStyle = aulStyleDash;
         pbrushPen->dwStyleCount = _countof(aulStyleDash);
         break;

case PS_DASHDOT:
-        pbrushPen->flAttrs |= BR_IS_SOLID;
+        pbrushPen->flAttrs |= BR_IS_SOLID | BR_IS_DEFAULTSTYLE;
         pbrushPen->pStyle = aulStyleDashDot;
         pbrushPen->dwStyleCount = _countof(aulStyleDashDot);
         break;

case PS_DASHDOTDOT:
-        pbrushPen->flAttrs |= BR_IS_SOLID;
+        pbrushPen->flAttrs |= BR_IS_SOLID | BR_IS_DEFAULTSTYLE;
         pbrushPen->pStyle = aulStyleDashDotDot;
         pbrushPen->dwStyleCount = _countof(aulStyleDashDotDot);
         break;
@@ -186,14 +186,6 @@
         break;

case PS_USERSTYLE:
-        if ((dwPenStyle & PS_TYPE_MASK) == PS_COSMETIC)
-        {
-            /* FIXME: PS_USERSTYLE workaround */
-            DPRINT1("PS_COSMETIC | PS_USERSTYLE not handled\n");
-            pbrushPen->flAttrs |= BR_IS_SOLID;
-            break;
-        }
-        else
         {
             UINT i;
             BOOL has_neg = FALSE, all_zero = TRUE;
@@ -227,6 +219,8 @@
             pbrushPen->ulStyleSize += pbrushPen->pStyle[i];
         }
     }
+
+    NT_ASSERT((pbrushPen->dwStyleCount == 0) || (pbrushPen->pStyle != NULL));

PEN_UnlockPen(pbrushPen);
     return hPen;
@@ -295,8 +289,9 @@
     }
     else
     {
-        // FIXME: Can we trust in dwStyleCount being <= 16?
-        cbRetCount = sizeof(EXTLOGPEN) - sizeof(DWORD) + pbrushPen->dwStyleCount * sizeof(DWORD);
+        DWORD dwStyleCount = (pbrushPen->flAttrs & BR_IS_DEFAULTSTYLE) ?
+            0 : pbrushPen->dwStyleCount;
+        cbRetCount = sizeof(EXTLOGPEN) - sizeof(DWORD) + dwStyleCount * sizeof(DWORD);
         if (pBuffer)
         {
             ULONG i;
@@ -308,8 +303,8 @@
             pExtLogPen->elpBrushStyle = pbrushPen->iBrushStyle;
             pExtLogPen->elpColor = pbrushPen->BrushAttr.lbColor;
             pExtLogPen->elpHatch = (ULONG_PTR)pbrushPen->hbmClient;
-            pExtLogPen->elpNumEntries = pbrushPen->dwStyleCount;
-            for (i = 0; i < pExtLogPen->elpNumEntries; i++)
+            pExtLogPen->elpNumEntries = dwStyleCount;
+            for (i = 0; i < dwStyleCount; i++)
             {
                 pExtLogPen->elpStyleEntry[i] = pbrushPen->pStyle[i];
             }
@@ -375,6 +370,39 @@
         return 0;
     }

+    if (((dwPenStyle & PS_TYPE_MASK) == PS_COSMETIC) &&
+        (ulBrushStyle != BS_SOLID))
+    {
+        EngSetLastError(ERROR_INVALID_PARAMETER);
+        return 0;
+    }
+
+    if (((dwPenStyle & PS_STYLE_MASK) == PS_NULL) ||
+        (ulBrushStyle == BS_NULL))
+    {
+        return StockObjects[NULL_PEN];
+    }
+
+
+    if ((ulBrushStyle == BS_PATTERN) ||
+        (ulBrushStyle == BS_DIBPATTERN) ||
+        (ulBrushStyle == BS_DIBPATTERNPT))
+    {
+        ulColor = 0;
+    }
+    else if ((ulBrushStyle != BS_SOLID) &&
+             (ulBrushStyle != BS_HATCHED))
+    {
+        EngSetLastError(ERROR_INVALID_PARAMETER);
+        return 0;
+    }
+
+    if ((dwPenStyle & PS_STYLE_MASK) != PS_USERSTYLE)
+    {
+        dwStyleCount = 0;
+        pUnsafeStyle = NULL;
+    }
+
     if (dwStyleCount > 0)
     {
         if (pUnsafeStyle == NULL)
@@ -395,8 +423,8 @@
         {
             ProbeForRead(pUnsafeStyle, dwStyleCount * sizeof(DWORD), 1);
             RtlCopyMemory(pSafeStyle,
-            pUnsafeStyle,
-            dwStyleCount * sizeof(DWORD));
+                          pUnsafeStyle,
+                          dwStyleCount * sizeof(DWORD));
         }
         _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
         {