Option to add the installation of an event filter to an existing Win3…

[640kb]

When integrating Vulkranscenegraph into an existing Win32 application (MFC program) I noticed that there is no event handler. With the flag installEventHandler in the WindowTraits class, the existing event handler in Win32_Window can be used if required.

The default value of the flag is set to False to avoid unwanted side effects (like in vsgQt).

I have adapted the solution from the Openscenegraph project, as you should recognize.

[robertosfield]

I'm not a Windows/MFC dev so could you please provide some background to what is done in this PR, with links to resources that would help inform of what is being done and why. If there is an open source example of this being done that would be perfect. Thanks.

[640kb]

I am also not an experienced MFC application developer (I only mentioned MFC as an example). But I have a small test application with which you can reproduce the problem.

The patch is intended to improve the following situation:
If you create your own Win32 window (or have it created by MFC, for example) and integrate VSG into it, then an event handler for controlling the VSG scene is missing. With this patch you should get the option to use the standard event handler of VSG, so that e.g. the interaction with the mouse works.

The code Win32_Window.cpp:318 checks whether the HWND handle is set in the window traits. If this is the case, the existing event handler is not linked to the HWND handle. This only happens if VSG creates the window itself, as can be seen in the following lines.

With the patch, you can set the option installEventHandler to True in the traits. If the option is True, the callback Win32WindowProc (Win32_Window.cpp:55) from VSG should be linked to the transferred HWND. When the VSG callback is set with SetWindowLongPtr, the Windows API returns a pointer to the existing callback linked to the HWND. This pointer is saved in the _windowProcedure variable. The CallWindowProc function is used to forward the events from the VSG callback to the stored callback so that it can continue to process events.

Further information on SetWindowLongPtr can be found here:
https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setwindowlongptra

Function CallWindowProc:
https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-callwindowproca

I have attached a small example.
If the program is started with the parameter "--install", the option installEventHandler in the traits is set to True and the VSG event handler is installed.
test-vsg-window.zip

[robertosfield]

Thanks for the info. To get the ball rolling I have merged the PR as a branch:

https://github.com/vsg-dev/VulkanSceneGraph/tree/640kb-master

I will need to learn about the topic and see if I can try things out on Windows.

[robertosfield]

I have finally had a chance to look at this PR. I am still somewhat rooting around the dark, but learning as I go...

First up I've merged this PR as a branch: https://github.com/vsg-dev/VulkanSceneGraph/tree/640kb-master

I'm now reviewing the example, this really helps thanks. Still feels a bit more convoluted than it should be, perhaps this is just the "Windows Way" but it looks like you are registering a callback with Windows, then in the Win32_Window.cpp you are retrieving that callback and then calling it late once the VSG has handled events.

The thing I find odd is why there is the whole set the function, then retrieve it later.

Could one not just have a the WINDPROC _windowProdedure added to Win32_Window.cpp be a public member that can be set by applications directly rather than having this whole get the procedure?

This would simplify the Win32_Window.cpp code and simplify the set up code in the application, and avoid putting a platform specific hint into vsg::WindowTraits.

[robertosfield]

@640kb I'm looking at the Windows docs in CallWindowProc: https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-callwindowproca

And it mentions handling STRICT and non STRICT types differently. Do we need to declare Win32_Window.cpp windowProcedure using their suggestion?

I tried your example but I'm such a Windows noob that while I got cmake-gui.ext to generate the project files and got VisualStudio to compile the source without errors, I get this error when I attempt to run it:

Any ideas?

[robertosfield]

@640kb I have created a windowProcedure branch with just added the Win32_Window::windowProcedure function pointer rather than the WindowTraits mechanism for doing this. If this works then I'd prefer it as it simpler and keeps all the code local to the Win32_Window class:

https://github.com/vsg-dev/VulkanSceneGraph/tree/windowProcedure

Changes are:
master...windowProcedure

[640kb]

I tried your example but I'm such a Windows noob that while I got cmake-gui.ext to generate the project files and got VisualStudio to compile the source without errors, I get this error when I attempt to run it:

Any ideas?

I do not use the standard CMake generator under Windows. In the past I always had problems with some projects when I wanted to create Visual Studio project files. Instead, I use the "NMake Makefiles JOM" generator, which can also be used and loaded with QtCreator, if you want to use an IDE.

For example:

cmake -G "NMake Makefiles JOM"  -DCMAKE_INSTALL_PREFIX="D:/projects/debug/msvc2022" -DCMAKE_INCLUDE_PATH="D:/projects/debug/msvc2022/include" -DCMAKE_LIBRARY_PATH="D:/projects/debug/msvc2022/lib" -DCMAKE_PREFIX_PATH="D:/projects/debug/msvc2022" -DCMAKE_BUILD_TYPE=Debug

Then you can compile with nmake or with jom (jom supports multithreading).

Under D:/projects/debug/msvc2022 I have installed libraries such as VulkanSceneGraph or Qt in this case.

[640kb]

@640kb I have created a windowProcedure branch with just added the Win32_Window::windowProcedure function pointer rather than the WindowTraits mechanism for doing this. If this works then I'd prefer it as it simpler and keeps all the code local to the Win32_Window class:

https://github.com/vsg-dev/VulkanSceneGraph/tree/windowProcedure

Changes are: master...windowProcedure

I have tried out the new branch windowProcedure.
I like the simpler approach. However, the Win32WindowProc callback must still be set to public so that it can be registered with the self-created window. Then it works for me.

See also: 640kb@920a176

I have updated the example in the attachment.
test-vsg-window-2.zip

[640kb]

And it mentions handling STRICT and non STRICT types differently. Do we need to declare Win32_Window.cpp windowProcedure using their suggestion?

I'm also unsure about how to treat STRICT. This is the first time I've come across it. But I've just done some more research and came across this documentation.

https://learn.microsoft.com/en-us/windows/win32/winprog/enabling-strict

[robertosfield]

I'm just reviewing the suggested placement of LRESULT CALLBACK Win32WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) into the Win32_Window.h. I'm not the author of Win32_Window or have expertise under Windows so I'm just trying to figure things out as I go.

If users are attaching the vsg::Win32_Window to a preexisting window and this the registering of the Win32WindowProc callback would happen, so I presume this is left up to the class doing the integration. In which case they would be responsible for calling Win32WindowProc, if so couldn't they just call

 win->handleWin32Messages(msg, wParam, lParam)

From their own equivalent callback? Or would end user application just want to use Win32WindowProc directly?

When reviewing the code I was surprised that the Win32_Window class itself isn't declared with VSG_DECLSPEC so if I'm reading it correctly if users built the VSG as a DLL then they won't be able to use the Win32_Window directly, instead would have to rely upon the VSG's own window creation functions.

The Win32WindowProc is declared with a CALLBACK so I presume that's a Windows #define but makes me wonder what will be happening with exporting this function in the case of DLL. If we do put Win32WindowProc in the public scope then we'd need to use VSG_DECLSPEC.

Thoughts?

[robertosfield]

Looking at the Win32WindowProc implementation:

    // our windows events callback
    LRESULT CALLBACK Win32WindowProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
    {
        Win32_Window* win = reinterpret_cast<Win32_Window*>(GetWindowLongPtr(hwnd, GWLP_USERDATA));
        if (win != nullptr) return win->handleWin32Messages(msg, wParam, lParam);
        return ::DefWindowProc(hwnd, msg, wParam, lParam);
    }

It relies upon the assignment of the GWLP_USERDATA which is done within the if (createWindow) {} block of Win32_Window::Win32_Window(vsg::ref_ptr traits) constructor:

        // set window handle user data pointer to hold ref to this so we can retrieve in WindowsProc
        SetWindowLongPtr(_window, GWLP_USERDATA, reinterpret_cast<LONG_PTR>(this));

This will call will be skipped if Win32_Window createWindow path isn't taken, so users that have their own window that they want the Win32_Window to piggy back off will need to set this pointer themselves in order to use the Win32WindowProc.

If they already need the knowledge that this is required then we'd need to make this explicit in the docs or provide an example in vsgExamples that illustrates this. I'm inclined towards use needing such an example but it doing all it's own callbacks and just calling Win32_Window::handleWin32Messages() itself rather than relying upon an internal callback.

Another possible refinement might be for Win32_Window to not call DefWindowProc at all, and just return, but then have the Win32WindowProc do the ::DefWindowProc(hwnd, msg, wParam, lParam); when required. This approach would mean that we don't need the Win32_Window::windowProcedure member at all.

I'll create a branch with this approach as I think it might just be the simplest and more flexible approach.

[robertosfield]

OK, here's my proposal of moving responibility for calling DefWindowProc out of the handleWin32Messages method:

https://github.com/vsg-dev/VulkanSceneGraph/tree/Win32WindowProc

I have also added what I think is missing VSG_DCLSPEC export.

For uses adapting existing windows to Win32_Window they'll need to set up their own equivalent to Win32WindowProc but I think this is more flexible and better than hiding this functionality within a shell function that does stuff for them, I'd much rather applications explicitly implement this type of stuff as it gives them full control, and it's only power users that will be tackling this.

What would be nice is an example for vsgExamples to test this all out. As I'm no Windows expert I'll have to defer to others on this.

[640kb]

I am testing the new branch and will adapt my small example. If it works so far, I could submit it for vsgExamples.

[640kb]

I have added an example, see: vsg-dev/vsgExamples#303

[robertosfield]

Thanks for the example. Does this mean the Win32WindowProc branch works fine and should be merged with VSG master?

[robertosfield]

Just merged vsgwin32 with vsgExamples and tested it under Windows 11 and it works fine with the Win32WindowProc branch so I'll merge it with VSG master.

[robertosfield]

I have merged the Win32WindowProc branch: #1153 and the new vsgwin32 example. I'll now close this PR as it's no longer required :-)

[640kb]

Thanks for the example. Does this mean the Win32WindowProc branch works fine and should be merged with VSG master?

Just made several tests, just to be sure. Everything works fine for me. Thanks for merging and your feedback :)