Closed Sergy2001 closed 7 months ago
Not sure the impact of this change.
I add it to my build of Notepad2.
And it works...
I'm sorry. I forget that Notepad2 DPI aware program. So corrected version of function:
static HBITMAP getBitmapForFile(LPCWSTR path) { const long const bitmapSize = MulDiv(16, g_uCurrentDPI, 96); HBITMAP bitmap = nullptr; SHFILEINFO sfi = { 0 }; HIMAGELIST imageList = (HIMAGELIST)SHGetFileInfo(path, FILE_ATTRIBUTE_NORMAL, &sfi, sizeof(SHFILEINFO), SHGFI_USEFILEATTRIBUTES | SHGFI_SYSICONINDEX | SHGFI_SMALLICON);
if (imageList) {
HDC dcDesktop = GetDC(nullptr);
if (dcDesktop) {
HDC dc = CreateCompatibleDC(dcDesktop);
if (dc) {
bitmap = CreateCompatibleBitmap(dcDesktop, bitmapSize, bitmapSize);
HBITMAP oldBitmap = (HBITMAP)SelectObject(dc, bitmap);
RECT rect = { 0, 0, bitmapSize, bitmapSize };
FillRect(dc, &rect, (HBRUSH)LongToPtr(COLOR_MENUBAR + 1));
ImageList_Draw(imageList, sfi.iIcon, dc, 0, 0, ILD_TRANSPARENT);
SelectObject(dc, oldBitmap);
DeleteDC(dc);
}
ReleaseDC(nullptr, dcDesktop);
}
}
return bitmap;
}
It seems more code is needed:
When the menu is destroyed, these bitmaps are not destroyed; it is up to the application to destroy them.
per https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setmenuitembitmaps. also create bitmap for each file looks slow when some of them have same extension.
Ok! You are right.
The solution: Line: 5351. add (, i) - getBitmapForFile(path, i)
case WM_DESTROY: for (int i = 0; i < MRU_MAXITEMS; i++) // Clean up all handles. getBitmapForFile(nullptr, i);
// New function. static HBITMAP getBitmapForFile(LPCWSTR path, int index) { static HBITMAP bitmaps[MRU_MAXITEMS] = { 0 }; if (bitmaps[index]) { DeleteObject(bitmaps[index]); bitmaps[index] = nullptr; } if (path == nullptr) // We just clean up handles. return nullptr;
SHFILEINFO sfi = { 0 };
HIMAGELIST imageList = (HIMAGELIST)SHGetFileInfo(path, FILE_ATTRIBUTE_NORMAL, &sfi, sizeof(SHFILEINFO), SHGFI_USEFILEATTRIBUTES | SHGFI_SYSICONINDEX | SHGFI_SMALLICON);
if (imageList) {
HDC dcDesktop = GetDC(nullptr);
if (dcDesktop) {
HDC dc = CreateCompatibleDC(dcDesktop);
if (dc) {
long const bitmapSize = MulDiv(16, g_uCurrentDPI, 96);
bitmaps[index] = CreateCompatibleBitmap(dcDesktop, bitmapSize, bitmapSize);
HBITMAP oldBitmap = (HBITMAP)SelectObject(dc, bitmaps[index]);
RECT rect = { 0, 0, bitmapSize, bitmapSize };
FillRect(dc, &rect, (HBRUSH)LongToPtr(COLOR_MENUBAR + 1));
ImageList_Draw(imageList, sfi.iIcon, dc, 0, 0, ILD_TRANSPARENT);
SelectObject(dc, oldBitmap);
DeleteDC(dc);
}
ReleaseDC(nullptr, dcDesktop);
}
}
return bitmaps[index];
}
Another one variant with cache...
The solution: Line: 5351. just - getBitmapForFile(path)
case WM_DESTROY: getBitmapForFile(nullptr); // Clean up all handles.
struct BitmapCache { int iIcon; HBITMAP bitmap; };
static HBITMAP getBitmapForFile(LPCWSTR path) { static struct BitmapCache bitmapCaches[64] = { 0 }; SHFILEINFO sfi = { 0 }; HIMAGELIST imageList = (HIMAGELIST)SHGetFileInfo(path, FILE_ATTRIBUTE_NORMAL, &sfi, sizeof(SHFILEINFO), SHGFI_USEFILEATTRIBUTES | SHGFI_SYSICONINDEX | SHGFI_SMALLICON);
for (int index = 0; index < 64; index++) {
struct BitmapCache *bitmapHash = &bitmapCaches[index];
if (!path && !imageList) { // Clean up cached bitmaps.
if (bitmapHash->bitmap)
DeleteBitmap(bitmapHash->bitmap);
bitmapHash->bitmap = nullptr;
} else if (imageList) {
if (!bitmapHash->iIcon && !bitmapHash->bitmap) { // There is no cached bitmap.
HDC dcDesktop = GetDC(nullptr);
if (dcDesktop) {
HDC dc = CreateCompatibleDC(dcDesktop);
if (dc) {
long const bitmapSize = MulDiv(16, g_uCurrentDPI, 96);
bitmapHash->bitmap = CreateCompatibleBitmap(dcDesktop, bitmapSize, bitmapSize); // Cache bitmap.
bitmapHash->iIcon = sfi.iIcon; // Cache bitmap index.
HBITMAP oldBitmap = (HBITMAP)SelectObject(dc, bitmapHash->bitmap);
RECT rect = { 0, 0, bitmapSize, bitmapSize };
FillRect(dc, &rect, (HBRUSH)LongToPtr(COLOR_MENUBAR + 1));
ImageList_Draw(imageList, sfi.iIcon, dc, 0, 0, ILD_TRANSPARENT);
SelectObject(dc, oldBitmap);
DeleteDC(dc);
}
ReleaseDC(nullptr, dcDesktop);
}
return bitmapHash->bitmap;
}
else if (bitmapHash->iIcon == sfi.iIcon)
return bitmapHash->bitmap;
}
}
return nullptr; // Somthing get wrong.
}
And one more thing...
When you click in MRU popup menu at non existing file, Notepad offers to create new file. I suppose it would be better to do like in "Open Resent File" dialog.
The solution: (5009) Replace with this. if (path) { if (FileSave(FileSaveFlag_Ask)) { FileLoad(FileLoadFlag_DontSave, path); } } ____ With that ___ if (!PathIsFile(path)) { // Ask... if (IDYES == MsgBoxWarn(MB_YESNO, IDS_ERR_MRUDLG)) { MRU_Delete(pFileMRU, index); MRU_DeleteFileFromStore(pFileMRU, path); } } else if (FileSave(FileSaveFlag_Ask)) { FileLoad(FileLoadFlag_DontSave, path); }
When you click in MRU popup menu at non existing file, Notepad offers to create new file.
Committed the change as 2bdc01429ac0fe490331608eb2aef792bfe01864 with minor change to avoid use after free for path
:
MRU_DeleteFileFromStore(pFileMRU, path);
MRU_Delete(pFileMRU, index);
Dear Zufu Liu!
Will you add getBitmapForFile() function(cached variant) to Notepad2? I tested it and it works perfectly and destroys HBITMAP-s on VM_DESTROY. Microsoft function SHGetFileInfo I use as hash one, because it always return sfi.iIcon which unique for every file extension. So that we have simple hash for each file extension in system. imageList.count in my system returns 51 icons, that's why 64 elements in static struct BitmapCache bitmapCaches[64] = { 0 };
Will you add getBitmapForFile() function(cached variant) to Notepad2?
Yes, but with 32 (MRU_MAXITEMS
) instead of 64.
Hi @Sergy2001, can you give email and name to author the change?
Another one variant with cache...
Committed the change as 361920b1ed771219ea26ca7d47f67dc4dc3affc9.
There seem needs extra fixes: the menu bitmap isn't transparent (compared to explorer's context menu) when the menu is selected:
Hello Zufu Liu!
My name is Serg(in French manner) or Sergio(in Italian manner). You can use just Sergy.
About transparency. Of course I use function FillRect(dc, &rect, (HBRUSH)LongToPtr(COLOR_MENUBAR + 1)); Which fill bitmap with COLOR_MENUBAR.
Transparency:
BITMAPINFO bmi = { sizeof(BITMAPINFOHEADER), bitmapSize, bitmapSize, 1, 32, BI_RGB, bitmapSize bitmapSize 4, { 0 } }; bitmapHash->bitmap = CreateDIBSection(nullptr, (PBITMAPINFO)&bmi, DIB_RGB_COLORS, nullptr, nullptr, 0);
//bitmapHash->bitmap = CreateCompatibleBitmap(dcDesktop, bitmapSize, bitmapSize); // Cache bitmap. bitmapHash->iIcon = sfi.iIcon; // Cache bitmap index. HBITMAP oldBitmap = (HBITMAP)SelectObject(dc, bitmapHash->bitmap); //RECT rect = { 0, 0, bitmapSize, bitmapSize }; //FillRect(dc, &rect, (HBRUSH)LongToPtr(COLOR_MENUBAR + 1));
Use COLOR_MENUBAR
is same, image background is not set to selection color like TortoiseGit's menu (maybe it custom menu with png image).
Solution.
Comment lines in my function: //bitmapHash->bitmap = CreateCompatibleBitmap(dcDesktop, bitmapSize, bitmapSize); // Cache bitmap. //RECT rect = { 0, 0, bitmapSize, bitmapSize }; //FillRect(dc, &rect, (HBRUSH)LongToPtr(COLOR_MENUBAR + 1));
add those: BITMAPINFO bmi = { sizeof(BITMAPINFOHEADER), bitmapSize, bitmapSize, 1, 32, BI_RGB, bitmapSize bitmapSize 4, { 0 } }; bitmapHash->bitmap = CreateDIBSection(nullptr, (PBITMAPINFO)&bmi, DIB_RGB_COLORS, nullptr, nullptr, 0); // Cache bitmap.
And it will work with transparency. I've already tried it. And it works.
image background is not set to selection color
I mean some like following, the file icon in menu doesn't change back color to light blue on selecting like it in Windows explorer (or it's right click menu), it still has the COLOR_MENUBAR
or COLOR_MENU
white color, which looks a little ugly.
COLOR_MENUBAR
or CreateDIBSection
doesn't fix this. directly load IDB_NEXT16
does not work either.
I understand what you mean. But in my build it works correctly. Function:
struct BitmapCache { int iIcon; HBITMAP bitmap; };
static HBITMAP getBitmapForFile(LPCWSTR path) { static struct BitmapCache bitmapCaches[64] = { 0 }; SHFILEINFO sfi = { 0 }; HIMAGELIST imageList = (HIMAGELIST)SHGetFileInfo(path, FILE_ATTRIBUTE_NORMAL, &sfi, sizeof(SHFILEINFO), SHGFI_USEFILEATTRIBUTES | SHGFI_SYSICONINDEX | SHGFI_SMALLICON);
for (int index = 0; index < 64; index++) {
struct BitmapCache *bitmapHash = &bitmapCaches[index];
if (!path && !imageList) { // Clean up cached bitmaps.
if (bitmapHash->bitmap)
DeleteBitmap(bitmapHash->bitmap);
bitmapHash->bitmap = nullptr;
} else if (imageList) {
if (!bitmapHash->iIcon && !bitmapHash->bitmap) { // There is no cached bitmap.
HDC dcDesktop = GetDC(nullptr);
if (dcDesktop) {
HDC dc = CreateCompatibleDC(dcDesktop);
if (dc) {
long const bitmapSize = MulDiv(16, g_uCurrentDPI, 96);
BITMAPINFO bmi = { sizeof(BITMAPINFOHEADER), bitmapSize, bitmapSize, 1, 32, BI_RGB, bitmapSize * bitmapSize * 4, { 0 } };
bitmapHash->bitmap = CreateDIBSection(nullptr, (PBITMAPINFO)&bmi, DIB_RGB_COLORS, nullptr, nullptr, 0); // Cache bitmap.
bitmapHash->iIcon = sfi.iIcon; // Cache bitmap index.
HBITMAP oldBitmap = (HBITMAP)SelectObject(dc, bitmapHash->bitmap);
ImageList_Draw(imageList, sfi.iIcon, dc, 0, 0, ILD_TRANSPARENT);
SelectObject(dc, oldBitmap);
DeleteDC(dc);
}
ReleaseDC(nullptr, dcDesktop);
}
return bitmapHash->bitmap;
}
else if (bitmapHash->iIcon == sfi.iIcon)
return bitmapHash->bitmap;
}
}
return nullptr; // Somthing get wrong.
}
Thanks Serg :+1:, it works now.
Thank you for your patience.
Dear Zufu Liu!
Would you be so kindly to add this function:
static HBITMAP getBitmapForFile(LPCWSTR path) { HBITMAP bitmap = nullptr; SHFILEINFO sfi = { 0 }; HIMAGELIST imageList = (HIMAGELIST)SHGetFileInfo(path, FILE_ATTRIBUTE_NORMAL, &sfi, sizeof(SHFILEINFO), SHGFI_USEFILEATTRIBUTES | SHGFI_SYSICONINDEX | SHGFI_SMALLICON);
}
to Notepad2.c
And add line SetMenuItemBitmaps(subMenu, i + IDM_RECENT_HISTORY_START, MF_BITMAP | MF_BYCOMMAND, getBitmapForFile(path), nullptr);
after (5350) AppendMenu(subMenu, MF_STRING, i + IDM_RECENT_HISTORY_START, path);
I hope It will prettify your Notepad2.
And please add snippets support?
Thanks.