[PATCH v2 0/2] MR6401: uxtheme: Optimize drawing by avoiding unneeded TransparentBlt().
-- v2: uxtheme: Try to avoid TransparentBlt() when possible. https://gitlab.winehq.org/wine/wine/-/merge_requests/6401
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/uxtheme/draw.c | 7 +++---- dlls/uxtheme/msstyles.h | 2 ++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dlls/uxtheme/draw.c b/dlls/uxtheme/draw.c index 3286d70d4d1..eb400571197 100644 --- a/dlls/uxtheme/draw.c +++ b/dlls/uxtheme/draw.c @@ -502,7 +502,7 @@ static inline void get_transparency (HTHEME hTheme, int iPartId, int iStateId, if (hasImageAlpha) { *transparent = ALPHABLEND_FULL; - *transparentcolor = RGB (255, 0, 255); + *transparentcolor = DEFAULT_TRANSPARENT_COLOR; } else { @@ -514,8 +514,7 @@ static inline void get_transparency (HTHEME hTheme, int iPartId, int iStateId, if(FAILED(GetThemeColor(hTheme, iPartId, iStateId, glyph ? TMT_GLYPHTRANSPARENTCOLOR : TMT_TRANSPARENTCOLOR, transparentcolor))) { - /* If image is transparent, but no color was specified, use magenta */ - *transparentcolor = RGB(255, 0, 255); + *transparentcolor = DEFAULT_TRANSPARENT_COLOR; } } else @@ -1968,7 +1967,7 @@ static HRESULT create_image_bg_region(HTHEME theme, int part, int state, const R OffsetRect(&r, -r.left, -r.top); if (FAILED(GetThemeColor(theme, part, state, TMT_TRANSPARENTCOLOR, &transcolour))) - transcolour = RGB(255, 0, 255); /* defaults to magenta */ + transcolour = DEFAULT_TRANSPARENT_COLOR; dc = CreateCompatibleDC(NULL); if (!dc) { diff --git a/dlls/uxtheme/msstyles.h b/dlls/uxtheme/msstyles.h index b2b0e0d8f22..7de41340b86 100644 --- a/dlls/uxtheme/msstyles.h +++ b/dlls/uxtheme/msstyles.h @@ -27,6 +27,8 @@ #define MAX_THEME_CLASS_NAME 60 #define MAX_THEME_VALUE_NAME 60 +#define DEFAULT_TRANSPARENT_COLOR RGB(255, 0, 255) + typedef struct _THEME_PROPERTY { int iPrimitiveType; int iPropertyId; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6401
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/uxtheme/draw.c | 32 ++++++++++++++++-------------- dlls/uxtheme/msstyles.c | 43 +++++++++++++++++++++++++++++++++++------ dlls/uxtheme/msstyles.h | 5 +++-- 3 files changed, 57 insertions(+), 23 deletions(-) diff --git a/dlls/uxtheme/draw.c b/dlls/uxtheme/draw.c index eb400571197..0d403dc9514 100644 --- a/dlls/uxtheme/draw.c +++ b/dlls/uxtheme/draw.c @@ -250,7 +250,7 @@ static PTHEME_PROPERTY UXTHEME_SelectImage(HTHEME hTheme, int iPartId, int iStat BOOL hasAlpha; lstrcpynW(szPath, fileProp->lpValue, min(fileProp->dwValueLen+1, ARRAY_SIZE(szPath))); - hBmp = MSSTYLES_LoadBitmap(hTheme, szPath, &hasAlpha); + hBmp = MSSTYLES_LoadBitmap(hTheme, szPath, &hasAlpha, NULL); if(!hBmp) continue; GetThemeEnumValue(hTheme, iPartId, iStateId, TMT_IMAGELAYOUT, &imagelayout); @@ -289,7 +289,7 @@ static PTHEME_PROPERTY UXTHEME_SelectImage(HTHEME hTheme, int iPartId, int iStat */ static HRESULT UXTHEME_LoadImage(HTHEME hTheme, int iPartId, int iStateId, const RECT *pRect, BOOL glyph, HBITMAP *hBmp, RECT *bmpRect, BOOL *hasImageAlpha, - int *imageDpi) + BOOL *hasDefaultTransparentColour, int *imageDpi) { int imagelayout = IL_HORIZONTAL; int imagecount = 1; @@ -304,7 +304,7 @@ static HRESULT UXTHEME_LoadImage(HTHEME hTheme, int iPartId, int iStateId, const return E_PROP_ID_UNSUPPORTED; } lstrcpynW(szPath, tp->lpValue, min(tp->dwValueLen+1, ARRAY_SIZE(szPath))); - *hBmp = MSSTYLES_LoadBitmap(hTheme, szPath, hasImageAlpha); + *hBmp = MSSTYLES_LoadBitmap(hTheme, szPath, hasImageAlpha, hasDefaultTransparentColour); if(!*hBmp) { TRACE("Failed to load bitmap %s\n", debugstr_w(szPath)); return HRESULT_FROM_WIN32(GetLastError()); @@ -496,7 +496,7 @@ static inline BOOL UXTHEME_SizedBlt (HDC hdcDst, int nXOriginDst, int nYOriginDs * depend on whether the image has full alpha or whether it is * color-transparent or just opaque. */ static inline void get_transparency (HTHEME hTheme, int iPartId, int iStateId, - BOOL hasImageAlpha, INT* transparent, + BOOL hasImageAlpha, BOOL hasDefaultTransparentColour, INT* transparent, COLORREF* transparentcolor, BOOL glyph) { if (hasImageAlpha) @@ -516,6 +516,8 @@ static inline void get_transparency (HTHEME hTheme, int iPartId, int iStateId, transparentcolor))) { *transparentcolor = DEFAULT_TRANSPARENT_COLOR; } + if (!hasDefaultTransparentColour && *transparentcolor == DEFAULT_TRANSPARENT_COLOR) + *transparent = ALPHABLEND_NONE; } else *transparent = ALPHABLEND_NONE; @@ -541,7 +543,7 @@ static void reset_dc_alpha_values(HTHEME htheme, HDC hdc, int part_id, int state return; if (FAILED(UXTHEME_LoadImage(htheme, part_id, state_id, rect, FALSE, &hbmp, &image_rect, - &has_alpha, NULL)) || has_alpha) + &has_alpha, NULL, NULL)) || has_alpha) return; bitmap_info.bmiHeader.biSize = sizeof(BITMAPINFOHEADER); @@ -576,10 +578,10 @@ static HRESULT UXTHEME_DrawImageGlyph(HTHEME hTheme, HDC hdc, int iPartId, POINT dstSize; POINT srcSize; POINT topleft; - BOOL hasAlpha; + BOOL hasAlpha, hasDefaultTransparentColour; hr = UXTHEME_LoadImage(hTheme, iPartId, iStateId, pRect, TRUE, &bmpSrc, &rcSrc, &hasAlpha, - NULL); + &hasDefaultTransparentColour, NULL); if(FAILED(hr)) return hr; hdcSrc = CreateCompatibleDC(hdc); if(!hdcSrc) { @@ -593,8 +595,8 @@ static HRESULT UXTHEME_DrawImageGlyph(HTHEME hTheme, HDC hdc, int iPartId, srcSize.x = rcSrc.right-rcSrc.left; srcSize.y = rcSrc.bottom-rcSrc.top; - get_transparency (hTheme, iPartId, iStateId, hasAlpha, &transparent, - &transparentcolor, TRUE); + get_transparency (hTheme, iPartId, iStateId, hasAlpha, hasDefaultTransparentColour, &transparent, + &transparentcolor, TRUE); GetThemeEnumValue(hTheme, iPartId, iStateId, TMT_VALIGN, &valign); GetThemeEnumValue(hTheme, iPartId, iStateId, TMT_HALIGN, &halign); @@ -659,7 +661,7 @@ static HRESULT get_image_part_size(HTHEME hTheme, int iPartId, int iStateId, REC BOOL hasAlpha; hr = UXTHEME_LoadImage(hTheme, iPartId, iStateId, prc, FALSE, &bmpSrc, &rcSrc, &hasAlpha, - &imageDpi); + NULL, &imageDpi); if (FAILED(hr)) return hr; switch (eSize) @@ -760,10 +762,10 @@ static HRESULT UXTHEME_DrawImageBackground(HTHEME hTheme, HDC hdc, int iPartId, int sizingtype = ST_STRETCH; INT transparent; COLORREF transparentcolor = 0; - BOOL hasAlpha; + BOOL hasAlpha, hasDefaultTransparentColour; hr = UXTHEME_LoadImage(hTheme, iPartId, iStateId, pRect, FALSE, &bmpSrc, &rcSrc, &hasAlpha, - NULL); + &hasDefaultTransparentColour, NULL); if(FAILED(hr)) return hr; hdcSrc = CreateCompatibleDC(hdc); if(!hdcSrc) { @@ -774,7 +776,7 @@ static HRESULT UXTHEME_DrawImageBackground(HTHEME hTheme, HDC hdc, int iPartId, rcDst = *pRect; - get_transparency (hTheme, iPartId, iStateId, hasAlpha, &transparent, + get_transparency (hTheme, iPartId, iStateId, hasAlpha, hasDefaultTransparentColour, &transparent, &transparentcolor, FALSE); dstSize.x = rcDst.right-rcDst.left; @@ -2224,10 +2226,10 @@ BOOL WINAPI IsThemeBackgroundPartiallyTransparent(HTHEME hTheme, int iPartId, if (bgtype != BT_IMAGEFILE) return FALSE; if (FAILED(UXTHEME_LoadImage(hTheme, iPartId, iStateId, &rect, FALSE, &bmpSrc, &rcSrc, - &hasAlpha, NULL))) + &hasAlpha, NULL, NULL))) return FALSE; - get_transparency (hTheme, iPartId, iStateId, hasAlpha, &transparent, + get_transparency (hTheme, iPartId, iStateId, hasAlpha, TRUE, &transparent, &transparentcolor, FALSE); return (transparent != ALPHABLEND_NONE); } diff --git a/dlls/uxtheme/msstyles.c b/dlls/uxtheme/msstyles.c index 71603953ed2..aa78c372d15 100644 --- a/dlls/uxtheme/msstyles.c +++ b/dlls/uxtheme/msstyles.c @@ -1181,19 +1181,43 @@ PTHEME_PROPERTY MSSTYLES_FindProperty(PTHEME_CLASS tc, int iPartId, int iStateId } /* Prepare a bitmap to be used for alpha blending */ -static BOOL prepare_alpha (HBITMAP bmp, BOOL* hasAlpha) +static BOOL prepare_alpha (HBITMAP bmp, BOOL* hasAlpha, BOOL *hasDefaultTransparentColour) { DIBSECTION dib; - int n; + int n, stride; BYTE* p; *hasAlpha = FALSE; + *hasDefaultTransparentColour = FALSE; if (!bmp || GetObjectW( bmp, sizeof(dib), &dib ) != sizeof(dib)) return FALSE; - if (dib.dsBm.bmBitsPixel != 32 || dib.dsBmih.biCompression != BI_RGB) - /* nothing to do */ + if (dib.dsBmih.biCompression != BI_RGB) + return TRUE; + + if (dib.dsBm.bmBitsPixel == 24) + { + int y; + + stride = (dib.dsBmih.biWidth * 3 + 3) & ~3; + p = dib.dsBm.bmBits; + for (y = 0; y < dib.dsBmih.biHeight; ++y) + { + p = (BYTE *)dib.dsBm.bmBits + stride * y; + for (n = 0; n < dib.dsBmih.biWidth; ++n, p += 3) + { + if (RGB(p[0], p[1], p[2]) == DEFAULT_TRANSPARENT_COLOR) + { + *hasDefaultTransparentColour = TRUE; + return TRUE; + } + } + } + return TRUE; + } + + if (dib.dsBm.bmBitsPixel != 32) return TRUE; /* If all alpha values are 0xff, don't use alpha blending */ @@ -1219,11 +1243,13 @@ static BOOL prepare_alpha (HBITMAP bmp, BOOL* hasAlpha) return TRUE; } -HBITMAP MSSTYLES_LoadBitmap (PTHEME_CLASS tc, LPCWSTR lpFilename, BOOL* hasAlpha) +HBITMAP MSSTYLES_LoadBitmap (PTHEME_CLASS tc, LPCWSTR lpFilename, BOOL* hasAlpha, BOOL *hasDefaultTransparentColour) { WCHAR szFile[MAX_PATH]; LPWSTR tmp; PTHEME_IMAGE img; + BOOL has_default; + lstrcpynW(szFile, lpFilename, ARRAY_SIZE(szFile)); tmp = szFile; do { @@ -1240,6 +1266,8 @@ HBITMAP MSSTYLES_LoadBitmap (PTHEME_CLASS tc, LPCWSTR lpFilename, BOOL* hasAlpha { TRACE ("found %p %s: %p\n", img, debugstr_w (img->name), img->image); *hasAlpha = img->hasAlpha; + if (hasDefaultTransparentColour) + *hasDefaultTransparentColour = img->hasDefaultTransparentColour; return img->image; } img = img->next; @@ -1247,8 +1275,11 @@ HBITMAP MSSTYLES_LoadBitmap (PTHEME_CLASS tc, LPCWSTR lpFilename, BOOL* hasAlpha /* Not found? Load from resources */ img = malloc(sizeof(*img)); img->image = LoadImageW(tc->hTheme, szFile, IMAGE_BITMAP, 0, 0, LR_CREATEDIBSECTION); - prepare_alpha (img->image, hasAlpha); + prepare_alpha (img->image, hasAlpha, &has_default); img->hasAlpha = *hasAlpha; + img->hasDefaultTransparentColour = has_default; + if (hasDefaultTransparentColour) + *hasDefaultTransparentColour = has_default; /* ...and stow away for later reuse. */ lstrcpyW (img->name, szFile); img->next = tc->tf->images; diff --git a/dlls/uxtheme/msstyles.h b/dlls/uxtheme/msstyles.h index 7de41340b86..25380af400f 100644 --- a/dlls/uxtheme/msstyles.h +++ b/dlls/uxtheme/msstyles.h @@ -67,7 +67,8 @@ typedef struct _THEME_CLASS { typedef struct _THEME_IMAGE { WCHAR name[MAX_PATH]; HBITMAP image; - BOOL hasAlpha; + unsigned int hasAlpha : 1; + unsigned int hasDefaultTransparentColour : 1; struct _THEME_IMAGE *next; } THEME_IMAGE, *PTHEME_IMAGE; @@ -103,7 +104,7 @@ PTHEME_PARTSTATE MSSTYLES_FindPart(PTHEME_CLASS tc, int iPartId); PTHEME_PARTSTATE MSSTYLES_FindPartState(PTHEME_CLASS tc, int iPartId, int iStateId, PTHEME_CLASS *tcNext); PTHEME_PROPERTY MSSTYLES_FindProperty(PTHEME_CLASS tc, int iPartId, int iStateId, int iPropertyPrimitive, int iPropertyId); PTHEME_PROPERTY MSSTYLES_FindMetric(int iPropertyPrimitive, int iPropertyId); -HBITMAP MSSTYLES_LoadBitmap(PTHEME_CLASS tc, LPCWSTR lpFilename, BOOL* hasAlpha); +HBITMAP MSSTYLES_LoadBitmap(PTHEME_CLASS tc, LPCWSTR lpFilename, BOOL* hasAlpha, BOOL *hasDefaultTransparentColour); HRESULT MSSTYLES_GetPropertyBool(PTHEME_PROPERTY tp, BOOL *pfVal); HRESULT MSSTYLES_GetPropertyColor(PTHEME_PROPERTY tp, COLORREF *pColor); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6401
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=148171 Your paranoid android. === debian11b (64 bit WoW report) === ddraw: ddraw2.c:3814: Test failed: Expected (0,0)-(640,480), got (0,0)-(1024,768). ddraw2.c:3839: Test failed: Expected (0,0)-(640,480), got (0,0)-(1024,768). ddraw4.c:3969: Test failed: Expected message 0x5, but didn't receive it.
v2: - use hasDefaultTransparentColour name; - use bitfields for hasAlpha and hasDefaultTransparentColour; - return early in prepare_alpha(); - check for the colour being default also for explicitly provided colour. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6401#note_80904
On Tue Sep 3 16:29:01 2024 +0000, Paul Gofman wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/6401/diffs?diff_id=129991&start_sha=c203750fa40cded26440a45362b0fe6304efb025#0e16d58d07b0b600d8a924944c3568c8c481d755_1282_1280) I used bitfields for hasAlpha and hasDefaultTransparentColour as suggested below, and taking address of bitfields doesn't work nice, so I changed this place in a bit different way.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6401#note_80906
On Tue Sep 3 10:44:08 2024 +0000, Zhiyi Zhang wrote:
Does this MR completely remove the slowdown for the application? If not, I suggest fixing this for listbox as well. Try to avoid redraws for listbox whenever it's possible. Well, the slowdown is not completely absent even on Windows and it is not *completely* removed by this patch, although it reduces load time ~2.5 times and I believe this way the load time is tolerable (while still ~2-3 times longer than on Windows). FWIW this is technically a regression from introducing theming, with theming disabled the time roughly matches Windows (while of course it is not possible to display themed controls without introducing just any overhead).
The app is doing rather unoptimal thing, adds a huge amount of items to listbox without explicitly disabling redraws. My main motivation with this patch is to make some sensible and universally useful optimization without too much complication. There are normal cases when listbox has to be updated and the current update time being so big is the core problem. Then maybe listbox has some room for optimization too but I suspect in this case the more likely outcome is not that the redraw can be avoided completely based on the listbox state (while I didn't exclude that possibility) but maybe it can determine that with given item count and scrollbar metrics the update will yield no changes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6401#note_80910
This merge request was approved by Zhiyi Zhang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6401
participants (4)
-
Marvin -
Paul Gofman -
Paul Gofman (＠gofman)
-
Zhiyi Zhang (＠zhiyi)