[RAPPS] Load icons on background thread #6881
Conversation
- Load icons on background thread to massively reduce loading time. - Use SM_CXICON sized icons consistently instead of hardcoding 32 in some places.
base/applications/rapps/appview.cpp
Outdated
| ItemCount = 0; | ||
| CheckedItemCount = 0; | ||
|
|
||
| ListView_Scroll(m_hWnd, 0, 0x7fff * -1); // Fixes a bug in Wine ComCtl32 where VScroll is not reset after deleting items |
There was a problem hiding this comment.
Not that I know of.
| } | ||
|
|
||
| VOID | ||
| CMainWindow::UpdateApplicationsList(AppsCategories EnumType, BOOL bReload, BOOL bCheckAvailable) | ||
| { | ||
| bUpdating = TRUE; | ||
|
|
||
| if (HCURSOR hCursor = LoadCursor(NULL, IDC_APPSTARTING)) |
There was a problem hiding this comment.
What's the difference with IDC_WAIT ?
Wouldn't https://git.reactos.org/?p=reactos.git;a=blob;f=sdk/include/reactos/ui/CWaitCursor.h;h=38bc74d614d59934030539d718e1bb7cd720d0bd;hb=HEAD
be of any use?
There was a problem hiding this comment.
IDC_WAIT is the big hourglass and IDC_APPSTARTING is the arrow with small hourglass and is less jarring when the wait is somewhat short.
| if (!AppInfo.RetrieveIcon(szIconPath)) | ||
| return NULL; | ||
| SIZE_T cch = szIconPath.GetLength() + 1, cbstr = cch * sizeof(WCHAR); | ||
| CAsyncLoadIcon *task = (CAsyncLoadIcon*)LocalAlloc(LMEM_FIXED, sizeof(CAsyncLoadIcon) + cbstr); |
There was a problem hiding this comment.
why not using new operator?
There was a problem hiding this comment.
Because it would not let me use try/catch because of project settings.
There was a problem hiding this comment.
I suppose I could use new std::nothrow.
| static DWORD CALLBACK | ||
| AsyncLoadIconProc(LPVOID Param) | ||
| { | ||
| for (CAsyncLoadIcon *task = (CAsyncLoadIcon*)Param, *old; task; old->Free()) |
There was a problem hiding this comment.
What's this "old" variable ? Where is it defined?
There was a problem hiding this comment.
What do you mean, it's defined right there (and initialized and the end of the loop). It is walking the linked list and exists because you can't access the next pointer after freeing.
| CopyMemory(task->Location, szIconPath.GetBuffer(), cbstr); | ||
| szIconPath.ReleaseBuffer(); | ||
| task->pNext = g_AsyncIconTasks; | ||
| g_AsyncIconTasks = task; |
There was a problem hiding this comment.
Can this CAsyncLoadIcon::Queue() helper be called concurrently?
If so, this g_AsyncIconTasks should probably be InterlockedExchangePointer 'ed...
Or maybe, use a SLIST or something where queuing can be done atomically.
There was a problem hiding this comment.
It cannot, all of this is happening on the UI thread. And the assert is there to make sure it stays that way.
Load icons on a background thread to massively reduce loading time.
Use SM_CXICON sized icons consistently instead of hard-coding 32 in some places.