Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prettify Open popup menu #740

Closed
Sergy2001 opened this issue Dec 1, 2023 · 20 comments
Closed

Prettify Open popup menu #740

Sergy2001 opened this issue Dec 1, 2023 · 20 comments

Comments

@Sergy2001
Copy link

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);

if (imageList) {
	HDC dcDesktop = GetDC(nullptr);
	if (dcDesktop) {
		HDC dc = CreateCompatibleDC(dcDesktop);
		if (dc) {
			bitmap = CreateCompatibleBitmap(dcDesktop, 20, 20);
			HBITMAP oldBitmap = (HBITMAP)SelectObject(dc, bitmap);
			RECT rect = { 0, 0, 20, 20 };
			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;

}

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.

@zufuliu
Copy link
Owner

zufuliu commented Dec 1, 2023

Not sure the impact of this change.

@Sergy2001
Copy link
Author

I add it to my build of Notepad2.
And it works...
Notepad

@zufuliu zufuliu added this to the v4.24.01 milestone Dec 1, 2023
@Sergy2001
Copy link
Author

Sergy2001 commented Dec 1, 2023

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;

}

@zufuliu
Copy link
Owner

zufuliu commented Dec 1, 2023

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.

@Sergy2001
Copy link
Author

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];

}

@Sergy2001
Copy link
Author

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.

}

@Sergy2001
Copy link
Author

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);
}

@zufuliu
Copy link
Owner

zufuliu commented Dec 3, 2023

When you click in MRU popup menu at non existing file, Notepad offers to create new file.

Committed the change as 2bdc014 with minor change to avoid use after free for path:

MRU_DeleteFileFromStore(pFileMRU, path);
MRU_Delete(pFileMRU, index);

@Sergy2001
Copy link
Author

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 };

@zufuliu
Copy link
Owner

zufuliu commented Dec 5, 2023

Will you add getBitmapForFile() function(cached variant) to Notepad2?

Yes, but with 32 (MRU_MAXITEMS) instead of 64.

@zufuliu
Copy link
Owner

zufuliu commented Dec 9, 2023

Hi @Sergy2001, can you give email and name to author the change?

@zufuliu
Copy link
Owner

zufuliu commented Dec 10, 2023

Another one variant with cache...

Committed the change as 361920b.

@zufuliu
Copy link
Owner

zufuliu commented Dec 11, 2023

There seem needs extra fixes: the menu bitmap isn't transparent (compared to explorer's context menu) when the menu is selected:
image

@Sergy2001
Copy link
Author

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));

@zufuliu
Copy link
Owner

zufuliu commented Dec 12, 2023

Use COLOR_MENUBAR is same, image background is not set to selection color like TortoiseGit's menu (maybe it custom menu with png image).

@Sergy2001
Copy link
Author

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.

@zufuliu
Copy link
Owner

zufuliu commented Dec 13, 2023

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.

image

COLOR_MENUBAR or CreateDIBSection doesn't fix this. directly load IDB_NEXT16 does not work either.

@Sergy2001
Copy link
Author

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.

}

01

zufuliu added a commit that referenced this issue Dec 14, 2023
@zufuliu
Copy link
Owner

zufuliu commented Dec 14, 2023

Thanks Serg 👍, it works now.

@Sergy2001
Copy link
Author

Thank you for your patience.

@zufuliu zufuliu closed this as completed Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants