Skin support for plugin window controls

[thinktink]

I tested your plugin (embedded window) and found that WADlg_DrawChildWindowBorders() function draws wrong borders for edit control. It draws one extra pixel:

...

It's not my function, it's part of the SDK, I can't change the SDK. To be honest, I'm not sure that's actually an error.

Also when switching the focus between winamp windows, the edit control's borders are flickers.

I don't doubt that this is an issue for you, unfortunately I've not been able to replicate it to test.

Combobox cannot have a style CBS_DROPDOWNLIST because winamp doesn't skin the combobox's button.
It should be set CBS_DROPDOWN style instead.

It seems to be working with CBS_DROPDOWNLIST fine for me, button and everything. Works on Windows 10 and Windows XP running inside Oracle VM VirtualBox.

embedwnd.cpp file:
//line 233
HWND mlwnd=NULL;

if(!IsWindow(mlwnd)) 

Here IsWindow() always takes NULL.

Fixed in version 1.3. Thank you.

[thinktink]

Version 1.3 (unauthorized) now available.

Changes:

• Fixed always NULL condition as indicated by Ingvar in the ML window handle retrieval function: http://forums.winamp.com/showthread....32#post3227532
• Updated reskin/refresh code to get consistent and expected results
• Added "CreateSkinFont" function to load/reload the current skin's font
• WM_SETFONT on every text displaying/handling control due to oddities
• Added posting of WM_DISPLAYCHANGE during WM_INITDIALOG due to oddities
• Added WM_APP handler to refresh certain aspects after skin switching completed as it wasn't working correctly all inside WM_DISPLAYCHANGE

Attached.

[Ingvar]

please mark the errors

I tested your plugin (embedded window) and found that WADlg_DrawChildWindowBorders() function draws wrong borders for edit control. It draws one extra pixel:



To fix this I changed the WADlg_DrawChildWindowBorders() function code a bit:

if((a & 0xffff0000) == DCW_SUNKENBORDER)
{
MoveToEx(ps->hdc, r.left, r.bottom, NULL);
LineTo(ps->hdc, r.right, r.bottom);
MoveToEx(ps->hdc, r.right, r.bottom - 1, NULL);
LineTo(ps->hdc, r.right, r.top - 1); 

WADlg_DrawChildWindowBorders() function cannot be used in WM_PAINT handler when in WM_PAINT already has some kind of drawing defined. Because the WADlg_DrawChildWindowBorders() is re-calling BeginPaint() function, it returns an empty HDC.
For my purposes I have fixed it.

Also when switching the focus between winamp windows, the edit control's borders are flickers.

Combobox cannot have a style CBS_DROPDOWNLIST because winamp doesn't skin the combobox's button.
It should be set CBS_DROPDOWN style instead.

embedwnd.cpp file:
//line 233
HWND mlwnd=NULL;

if(!IsWindow(mlwnd)) 

Here IsWindow() always takes NULL.

[thinktink]

Despite some errors in your code...

If you can, please mark the errors so that I can correct them.

[Ingvar]

Despite some errors in your code, the principle of skinning the controls is clear.

In short, in order to skin the controls, the developer needs to either draw them completely (owner drawing) or use a wrapper functions from wa_dlg.h, or request the API of the media library plugin to perform the skinning of the controls.

I also noticed that when using Media library API you must also use the WADlg_DrawChildWindowBorders() function for drawing borders, because Media library API does not draw it. It's strange that drawing borders is not part of skinning support of the Media library API.

To perform skinning of the edit control, I tried to use the media library API:

//on WM_INITDIALOG
MLSKINWINDOW mlsw = {0};
mlsw.hwndToSkin = GetDlgItem(hwnd, IDC_EDIT1);
mlsw.skinType = SKINNEDWND_TYPE_EDIT;
mlsw.style = SWS_USESKINCOLORS | SWS_USESKINCURSORS | SWS_USESKINFONT;
MLSkinWindow(ML_hWnd, &mlsw);

//on WM_PAINT draw borders (because ML API doesn't support it)
WADlg_DrawChildWindowBorders(...) 

And it works well except that it keeps using the default font and not the skin font.
As you can see the required bit SWS_USESKINFONT is set.



The only way to set the correct font for an edit control is to create a font and set it with WM_SETFONT for Edit control and not set SWS_USESKINFONT bit in mlsw.style.

I just don't understand why SWS_USESKINFONT bit doesn't work.

[thinktink]

Hope this helps...

[thinktink]

I'm working on an unauthorized update to the example plugin originally created by DrO with skinned controls and menus. It should answer a number of questions. Just need a little time.

[Ingvar]

Ok, so how do I get or calculate the text color for an inactive button?

For example, for a Bento skin the inactive text color should be: 91, 94, 95:
And for Winamp Modern Skin - 147, 149, 155

I checked all colors that WADlg_getColor() function returns. There is no such colors! Also I checked the contents of the skin bitmap (IPC_GET_GENSKINBITMAP). There is no such colors also.

It's like the text color for an inactive button is calculated by some formula.

I found the formula inside wa_dlg.h file:

fg = wadlg_colors[WADLG_WNDFG];
bg = wadlg_colors[WADLG_WNDBG];
colour = RGB((GetRValue(fg)+GetRValue(bg))/2, (GetGValue(fg)+GetGValue(bg))/2, (GetBValue(fg)+GetBValue(bg))/2); 

But this calculation gives wrong color. For example, for Bento:

fg = 210, 210, 210
bg = 51, 55, 56
colour = (210+51)/2, (210+55)/2, (210+56)/2 = 130, 132, 133

When DrawText() with this color, then we will see:



Similarly for Winamp Modern:

fg = 255, 255, 255
bg = 127, 135, 149
colour = (255+127)/2, (255+135)/2, (255+149)/2 = 191, 195, 202

When DrawText() with this color, then we will see:



As you can see, this is an incorrect calculation formula.

For example in JTF plugin inactive buttons has the correct text color.


So how do you calculate the correct color values for inactive text?

[Ingvar]

You can also try just not using checkboxes

Why?

use toggle buttons

What is the toggle buttons?

As far as I know JTFE was never open source so the code is not available. It could be an incompatible resource style set on the edit control

I have looked at the resources in gen_jumpex.dll file. But there is no such dialog. It looks like it is created dynamically using CreateWindow(). But Spy++ shows these styles:

Class Name: Edit

WS_CHILDWINDOW
WS_VISIBLE
WS_TABSTOP
ES_LEFT
ES_AUTHOHSCROLL

WS_EX_LEFT
WS_EX_LTRREADING
WS_EX_RIGHTSCROLLBAR
WS_EX_NOPARENTNOTIFY

I created an edit control with the same styles:

code:
CreateWindowEx(WS_EX_LEFT | WS_EX_LTRREADING | WS_EX_RIGHTSCROLLBAR | WS_EX_NOPARENTNOTIFY, L"EDIT", L"Edit control", WS_CHILDWINDOW | WS_VISIBLE | WS_TABSTOP | ES_LEFT | ES_AUTOHSCROLL, 20, 20, 75, 28, hwnd, NULL, plugin.hDllInstance, NULL);

But it doesn't look like it in Jump to File window:



As you can see there is no bottom and right side borders.

I also noticed that button with BS_OWNERDRAW style flickers when switching between the plugin window and the main window and back.
I also have set the WS_CLIPSIBLINGS and WS_CLIPCHILDREN styles for the button's parent window (i.e. for the dialog).

But with this:

code:
	case WM_DRAWITEM:

	{

	DRAWITEMSTRUCT *di = (DRAWITEMSTRUCT *)lParam;

	FillRect(di->hDC, &di->rcItem, blue_brush);

	return true;

	}

or with this:

code:
	case WM_DRAWITEM:

	{

	return WADlg_handleDialogMsgs(hwnd, msg, wParam, lParam);

	}

button flickers:



However, in the JTF plugin window, the buttons do not blink! However, despite the fact that JTF's buttons have BS_OWNERDRAW style, their parent window (dialog) does not receive WM_DRAWITEM message (I see it in Spy++). But why??
According to documentation:

for BS_OWNERDRAW style: The owner window receives a WM_DRAWITEM message when a visual aspect of the button has changed

Nonetheless, how to get rid of the flickering of the ownerdraw buttons?

[thinktink]

Do you mean that if WM_DESTROY event occurs and we try to assign a window procedure that holds in memory of already unloaded at this time plugin, then an AV error will occurs?

Potentially yes, it greatly depends on the order which Winamp unloads the various types of plugins during Winamp's shutdown/close. I've seen it before with my plugins.

On my last screenshot I overridden its WM_PAINT handler and did ownerdraw checkbox painting. Should it look different? How?

I would think so, yes. How you do it is up to you since you would have to code it yourself like I had to for my plugins. You can also try just not using checkboxes and use toggle buttons or even combo boxes (which requires ML skinning support currently) that have native support. However, if you do it yourself, try to make them look like mine: http://forums.winamp.com/showpost.ph...1&postcount=63

It looks like there is skinning support for push buttons and scrollbar only.
I used code to get skin HBITMAP and save it to file:

code:
SendMessage(plugin.hwndParent, WM_WA_IPC, 0, IPC_GET_GENSKINBITMAP);

And skin HBITMAP contains this:


As we can see, there glyphs for push-button and scrollbar are only.

Yup.

By the way, where is this image stored? In what file? I looked through all the graphic and resources files in the Winamp folder but didn't find these glyphs.

For modern skins there is no image store, I believe it's generated internally by Winamp based on the current color scheme of the selected modern skin, but that's a guess. For classic skins they exist inside the classic skin archive (or folder if it's not in an archive), I forgot the name of the file.

Ok, how to fix it to assign edit control skin support? How is it done in Jump to File plugin, for example?



Here we can see specific themed borders at the bottom and right side of EDIT control. I tried to change the styles of the control, but I could not get such borders.

Is the source code of Jump to File plugin available? I can't find it. I would be able to see the implementation of the edit control.

As far as I know JTFE was never open source so the code is not available. It could be an incompatible resource style set on the edit control but I've never seen that before like that. I would suggest ripping the dialog resource out of JTFE and comparing the Style and Extended Style flags of the edit control itself for clues.

[Ingvar]

Winamp is not guaranteed to unload other plugins that also subclass the Winamp windows in the correct order.

Do you mean that if WM_DESTROY event occurs and we try to assign a window procedure that holds in memory of already unloaded at this time plugin, then an AV error will occurs?

The checkboxes also aren't completely skinned

On my last screenshot I overridden its WM_PAINT handler and did ownerdraw checkbox painting. Should it look different? How?

there's currently no native support for skinning checkboxes or radio buttons

It looks like there is skinning support for push buttons and scrollbar only.
I used code to get skin HBITMAP and save it to file:

code:
SendMessage(plugin.hwndParent, WM_WA_IPC, 0, IPC_GET_GENSKINBITMAP);

And skin HBITMAP contains this:


As we can see, there glyphs for push-button and scrollbar are only.

By the way, where is this image stored? In what file? I looked through all the graphic and resources files in the Winamp folder but didn't find these glyphs.

it looks like there's a white border around what looks like edit controls, which is not part of the selected skin

Ok, how to fix it to assign edit control skin support? How is it done in Jump to File plugin, for example?



Here we can see specific themed borders at the bottom and right side of EDIT control. I tried to change the styles of the control, but I could not get such borders.

Is the source code of Jump to File plugin available? I can't find it. I would be able to see the implementation of the edit control.

[thinktink]

Thanks!

You're welcome.

I think you understood me correctly. Anyway it would be correct to move unsubclass code to the parent window's dialog proc at the WM_DESTROY.

Nope, as I said before, Winamp is not guaranteed to unload other plugins that also subclass the Winamp windows in the correct order. In this case, at least WM_NCDESTROY (if not more messages) will be called after your attempt to unsubclass after the call to WM_DESTROY completes, which can potentially break the subclass chain. If it does then it will cause an access violation if the plugin's subclass proc become unavailable. You might not even see the AV error unless you put Winamp under a debugger. To be honest, the unsubclassing code should just be outright deleted. However, I'm not the original developer of the plugin so I can only guess what the original developer intended.

Thanks! Now it looks right:

It's definitely an improvement but not entirely correct, it looks like there's a white border around what looks like edit controls, which is not part of the selected skin. The checkboxes also aren't completely skinned, but as I said before, there's currently no native support for skinning checkboxes or radio buttons.

It's weird but I downloaded Winamp SDK 5.5 from here https://download.nullsoft.com/winamp...WA5.55_SDK.exe and Winamp/gen.h file contains only #define GEN_INIT_SUCCESS 0.

Ok, that's odd. I may have put in the other #define into the SDK source myself and forgot that I did so. If you don't want to modify the SDK headers then just add...

#ifndef GEN_INIT_FAILURE
#define GEN_INIT_FAILURE 1
#endif //GEN_INIT_FAILURE 

...right after the GEN.H include. This way will prevent any warning in the future about duplicate definitions if a new version of the SDK is published with new/updated #define(s).

While looking through your code and using the debugger, I found another one bug. Looks like it's a winamp bug.
Look at this code:

code:
	else if(msg == WM_WA_IPC)

	{

		if(lParam == IPC_CB_MISC && (wParam == IPC_CB_MISC_TITLE || wParam == IPC_CB_MISC_TITLE_RATING))

		{



		//THIS BLOCK RUNS 4 TIMES !!!



			// art change

			PostMessage(myWndChild,WM_USER,0,0);

		}

As I marked, this block runs 4 times when track changing or when you press ffwd/rwd. I think you need to handle another notification to monitor the track change.

I'm not the original developer of the plugin so I can only assume or guess the original intent. It may very well be that those messages are all included to catch all instances of changes for peculiar or specific situations and multiple calls are simply an unfortunate result of the requirements during normal operations.

Offtopic. In Visual Studio 2012 I set configuration to "Release" and compiled my plugin dll. But Winamp loads it only on my PC.
I sent it to another PC and also tried it on a virtual system using the latest Winamp, but in these cases Winamp doesn't load my plugin.
In the "Preferences" window I see "NOT LOADED" message. And log file \AppData\Roaming\Winamp\winamp.log contains error:

For example, gen_classicart.dll works fine.
What other project settings do I need to set to compile my plugin to distribute it on other PCs?

I don't remember the name of the settings but what you're describing sounds like a missing dependency on the computer where the plugin is not loading. Google would be a better resource to figure out what's missing than me. I don't use Visual Studio on a regular basis, only when I absolutely have to, so I'm not uber familiar with specifics or details.

[Ingvar]

I finally found the example package

Thanks!

That is actually a good thing, it means that the window is being destroyed BEFORE your plugin is unloaded from memory.

I think you understood me correctly. Anyway it would be correct to move unsubclass code to the parent window's dialog proc at the WM_DESTROY.

Since that window never had standard controls to skin, the original developer never put in the code to skin any. Go ahead and add the handler inside "art_dlgproc" switch/case statements

Thanks! Now it looks right:

Those are defined in GEN.H in the Winamp SDK

It's weird but I downloaded Winamp SDK 5.5 from here https://download.nullsoft.com/winamp...WA5.55_SDK.exe and Winamp/gen.h file contains only #define GEN_INIT_SUCCESS 0.

While looking through your code and using the debugger, I found another one bug. Looks like it's a winamp bug.
Look at this code:

code:
	else if(msg == WM_WA_IPC)

	{

		if(lParam == IPC_CB_MISC && (wParam == IPC_CB_MISC_TITLE || wParam == IPC_CB_MISC_TITLE_RATING))

		{



		//THIS BLOCK RUNS 4 TIMES !!!



			// art change

			PostMessage(myWndChild,WM_USER,0,0);

		}

As I marked, this block runs 4 times when track changing or when you press ffwd/rwd. I think you need to handle another notification to monitor the track change.

Offtopic. In Visual Studio 2012 I set configuration to "Release" and compiled my plugin dll. But Winamp loads it only on my PC.
I sent it to another PC and also tried it on a virtual system using the latest Winamp, but in these cases Winamp doesn't load my plugin.
In the "Preferences" window I see "NOT LOADED" message. And log file \AppData\Roaming\Winamp\winamp.log contains error:

[5.9.0.9999] [error] Error when loading the plugin 'gen_wa_plugin.dll'! Error code : 126!

For example, gen_classicart.dll works fine.
What other project settings do I need to set to compile my plugin to distribute it on other PCs?

[thinktink]

Found it!

I finally found the example package.

Attached.

[thinktink]

I didn't generate value for MY_PLUGINMENU_ID. Just assign a value:

code:
UINT MY_PLUGINMENU_ID = 0xa2bc;...

Go ahead and do it the way I showed, it will help prevent future conflicts with other plugins and is the SDK way to do it.

...


But I found a bug in your code too . Look at quit() function:

code:
	// restores the original playlist window proc now that we are closing and if the window was subclassed

	HWND pe_wnd = (HWND)SendMessage(plugin.hwndParent,WM_WA_IPC,IPC_GETWND_PE,IPC_GETWND);

	if(GetWindowLongPtr(pe_wnd,GWLP_WNDPROC) == (LONG_PTR)SubclassPlaylistProc){

		SetWindowLongPtr(pe_wnd,GWLP_WNDPROC,(LONG_PTR)oldPlaylistWndProc);		

	}

At this moment, the GetWindowLongPtr() function will return 0x00 because the parent window no longer exists. You can verify it using debugger.
Therefore, this code should be moved to the parent window's dialog proc into the WM_DESTROY handler.

That is actually a good thing, it means that the window is being destroyed BEFORE your plugin is unloaded from memory.
Trying to "unsubclass" a shared window is problematic at best as not all plugins are guaranteed to be loaded and unloaded in the correct order to prevent subclass procedure chain breaks, which is very bad. If your subclass proceedure is unloaded (by dint of your plugin being unloaded) before the window it's connected to is destroyed it will cause Access Violations when the (now non-existent) procedure is called.

But now it's time to return to the main question of the topic.
I opened your "gen_classicart" project and edited the resource dialog: I placed some controls - button, text label and a edit control. Compile and run it in Winamp:



It looks like skin support isn't working for controls. I didn't modify source code except GEN_INIT_FAILURE replace with 1 (I didn't find a definition). So how to theme the controls?

Since that window never had standard controls to skin, the original developer never put in the code to skin any. Go ahead and add the handler inside "art_dlgproc" switch/case statements, between the WM_DESTROY and WM_PAINT case statements seems like a good place.

case WM_CTLCOLORLISTBOX:
case WM_CTLCOLORDLG:
case WM_CTLCOLORBTN:
case WM_CTLCOLORSTATIC:
case WM_CTLCOLOREDIT:
case WM_DRAWITEM:
{
return WADlg_handleDialogMsgs(hwnd, msg, wParam, lParam);
} 

#define GEN_INIT_FAILURE 1
#define GEN_INIT_SUCCESS 0 

Those are defined in GEN.H in the Winamp SDK