SetWindowSubclass conflict in user plugin using the windows api

[agoschie]

My user plugin dll needs to call the windows api function: SetWindowSubclass(hwnd, (SUBCLASSPROC)proc, (UINT_PTR)id, (DWORD_PTR)&spevList)

My plugin has no issues and works as expected if I remove the SWS dll plugin. However, it crashes at reaper startup if I load SWS. If I call the same conflicted function after SWS is already initialized, then there is no crash.

I suspect there is a resource conflict from SWS using deprecated windows functionality or something similar. SetWindowSubclass is a highly reliable function that has no conflicts unlike those possibly related to SetWindowLong

[agoschie]

I also use: RemoveWindowSubclass(hwnd, (SUBCLASSPROC)proc, (UINT_PTR)id)

during initialization, as reaper closes windows and my code handles that automatically and calls the above function.

If SetWindowLong is depending upon my procedure as its next to call, would it crash SWS and thus reaper?

[cfillion]

Subclass the main window using SetWindowLongPtr(GWLP_WNDPROC) instead (like SWS and other extensions such as ReaImGui does). This way is also supported on macOS and Linux via SWELL.

LONG_PTR newProc { reinterpret_cast<LONG_PTR>(&mainProcOverride) },
         oldProc { SetWindowLongPtr(GetMainHwnd(), GWLP_WNDPROC, newProc) };
g_mainProc = reinterpret_cast<WNDPROC>(oldProc);

LRESULT CALLBACK mainProcOverride(HWND hwnd, unsigned int msg, WPARAM wParam, LPARAM lParam)
{
  return CallWindowProc(g_mainProc, hwnd, msg, wParam, lParam);
}

Other extensions may do the same after yours, so can't undo this later by blindly re-setting g_mainProc.

[agoschie]

Subclass the main window using SetWindowLongPtr(GWLP_WNDPROC) instead (like SWS and other extensions such as ReaImGui does). This way is also supported on macOS and Linux via SWELL.

LONG_PTR newProc { reinterpret_cast<LONG_PTR>(&mainProcOverride) },
         oldProc { SetWindowLongPtr(GetMainHwnd(), GWLP_WNDPROC, newProc) };
g_mainProc = reinterpret_cast<WNDPROC>(oldProc);

LRESULT CALLBACK mainProcOverride(HWND hwnd, unsigned int msg, WPARAM wParam, LPARAM lParam)
{
  return CallWindowProc(g_mainProc, hwnd, msg, wParam, lParam);
}

Other extensions may do the same after yours, so can't undo this later by blindly re-setting g_mainProc.

SetWindowSubclass is the new standard that replaces SetWindowLongPtr. SetWindowLongPtr is unsafe because it relies on global variables to store the original procedure, while SetWindowSubclass automatically calls the next proc using return DefSubclassProc(hwnd, msg, wParam, lParam);. Furthermore, SetWindowSubclass is more reliable when it comes to persistent data, as it automatically appends or removes data from a special window property atom named "UxSubclassInfo" when you use the dwRefData parameter, so you dont need to use that "other" hidden property that SetWindowLongPtr is often used with and overyone has to implicitely agree with how they will use it.

I think the issue is SWS thinks the window procedure can never change, which is not true if other extensions are running before it most likely. For instance, its considered good to cleanup the subclassed procedure when the window receives the destroy message, my procedures automatically remove using RemoveWindowSubclass.

It would be a lot of work and I don't think it would be a good idea for me to revert. I dont have my project on this github account, but I can begin a branch on my local system to see if its even possible for us to reliably make a dynamic system, but it seems like it might not be possible.

[cfillion]

SetWindowSubclass from ComCtl32.dll v6 converts the window to Unicode if it isn't already. REAPER's windows are ANSI. Messing with that is bound to break things in unsuspecting REAPER and other extensions.

The crash happens as a side-effect of this unintended conversion: Get/SetWindowLongPtrA(GWLP_WNDPROC) on a Unicode window returns a thunk that SWS attempts to call directly instead of through CallWindowProc.

Fixing that since a direct call is technically incorrect, but you shouldn't be using SetWindowSubclass on an ANSI window in the first place either.

SetWindowSubclass is the new standard that replaces SetWindowLongPtr

It dates from XP (98 unofficially) so it's not exactly "new"... Certainly newer though. But it doesn't matter how great it is: it's only good for Unicode windows (plus it's not implemented in SWELL).

[agoschie]

I was going to say, I think we found the exact same issue. Those direct calls to previous procedures. They are wrapped in a thunk, so they need callwindowproc.

EDIT: And I dont think its unicode thats breaking SWS there, its the thunk needing to pull the window atom data, and that needs callwindowproc

I did not realize reaper was still using ANSI window procedures. Why is reaper not crashing or showing any errors from its windows being converted to unicode?

[agoschie]

I'm probably just going to write my own SetWindowSubclassA and RemoveWindowSubclassA. I cant just sit in this crab bucket.