From fd01add9cfbdfd2d170b125723732e825ecc892e Mon Sep 17 00:00:00 2001 From: Timo Kreuzer Date: Thu, 17 Dec 2015 22:05:06 +0000 Subject: [PATCH] [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! svn path=/trunk/; revision=70388 --- reactos/win32ss/gdi/eng/lineto.c | 4 +- reactos/win32ss/gdi/ntgdi/brush.cpp | 11 +++++ reactos/win32ss/gdi/ntgdi/pen.c | 68 ++++++++++++++++++++--------- 3 files changed, 61 insertions(+), 22 deletions(-) diff --git a/reactos/win32ss/gdi/eng/lineto.c b/reactos/win32ss/gdi/eng/lineto.c index 12e09220bf3..630592a08fc 100644 --- a/reactos/win32ss/gdi/eng/lineto.c +++ b/reactos/win32ss/gdi/eng/lineto.c @@ -49,13 +49,13 @@ HandleStyles( { 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; } diff --git a/reactos/win32ss/gdi/ntgdi/brush.cpp b/reactos/win32ss/gdi/ntgdi/brush.cpp index 1049684aade..210297693c0 100644 --- a/reactos/win32ss/gdi/ntgdi/brush.cpp +++ b/reactos/win32ss/gdi/ntgdi/brush.cpp @@ -45,6 +45,11 @@ BRUSH::BRUSH( 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( @@ -63,6 +68,12 @@ BRUSH::~BRUSH( 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); + } } VOID diff --git a/reactos/win32ss/gdi/ntgdi/pen.c b/reactos/win32ss/gdi/ntgdi/pen.c index cbfa4ce405a..2dbd4fcb796 100644 --- a/reactos/win32ss/gdi/ntgdi/pen.c +++ b/reactos/win32ss/gdi/ntgdi/pen.c @@ -128,7 +128,7 @@ IntGdiExtCreatePen( 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 @@ IntGdiExtCreatePen( 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 @@ IntGdiExtCreatePen( 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; @@ -228,6 +220,8 @@ IntGdiExtCreatePen( } } + NT_ASSERT((pbrushPen->dwStyleCount == 0) || (pbrushPen->pStyle != NULL)); + PEN_UnlockPen(pbrushPen); return hPen; @@ -295,8 +289,9 @@ PEN_GetObject(PBRUSH pbrushPen, INT cbCount, PLOGPEN pBuffer) } 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 @@ PEN_GetObject(PBRUSH pbrushPen, INT cbCount, PLOGPEN pBuffer) 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 @@ NtGdiExtCreatePen( 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 @@ NtGdiExtCreatePen( { ProbeForRead(pUnsafeStyle, dwStyleCount * sizeof(DWORD), 1); RtlCopyMemory(pSafeStyle, - pUnsafeStyle, - dwStyleCount * sizeof(DWORD)); + pUnsafeStyle, + dwStyleCount * sizeof(DWORD)); } _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) {