search

vsg-dev / vsgImGui

Integration of VulkanSceneGraph with ImGui
MIT License
43 stars 28 forks source link

Fixes (#34) and upgrades to ImGui 1.89.2 #35

Closed kannode closed 1 year ago

kannode commented 1 year ago
kannode commented 1 year ago

@robertosfield Would be good to squash my commits before merging. This fixes (#34) on Windows and Linux. I expect on Mac as well as there are no longer any platform specific constants that are used.

kannode commented 1 year ago

@rolandhill Hi Roland, I initially looked at how you attempted the upgrade to 1.89 and AddKeyEvent. Thanks for taking a stab at it. I opened pull request #35, which fixes #34 and likely subsumes pull requests #30 and #32. Could you please check if #30 and #32 are still needed and perhaps close them if not?

robertosfield commented 1 year ago

I won't have time to do a full review this afternoon so have had just done a quick run through.

No need to squash the commits, I prefer being able to review the PR and have the history as this often provides insights to what changes are made for what reason.

The change to README.md doesn't look to be an appropriate place to record changes from one PR to another, it's better just to list them in the PR itself, and the commit message. If we start putting every change into the README.md it'd quickly become just list of commit messages.

kannode commented 1 year ago

Sounds good. I will make cleaner WIP commits in the future in a manner that is useful.

robertosfield commented 1 year ago

I have just checked out the PR as a branch and on compiling get the errors:

10%] Building CXX object src/CMakeFiles/vsgImGui.dir/vsgImGui/SendEventsToImGui.cpp.o /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp: In member function ‘void vsgImGui::SendEventsToImGui::_updateModifier(ImGuiIO&, vsg::KeyModifier&, bool)’: /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:226:24: error: ‘ImGuiMod_Ctrl’ was not declared in this scope; did you mean ‘ImGuiModFlags_Ctrl’? 226 io.AddKeyEvent(ImGuiMod_Ctrl, pressed); ^~~~~ ImGuiModFlags_Ctrl /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:230:24: error: ‘ImGuiMod_Shift’ was not declared in this scope; did you mean ‘ImGuiModFlags_Shift’? 230 io.AddKeyEvent(ImGuiMod_Shift, pressed); ^~~~~~ ImGuiModFlags_Shift /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:234:24: error: ‘ImGuiMod_Alt’ was not declared in this scope 234 io.AddKeyEvent(ImGuiMod_Alt, pressed); ^~~~ /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:238:24: error: ‘ImGuiMod_Super’ was not declared in this scope; did you mean ‘ImGuiModFlags_Super’? 238 io.AddKeyEvent(ImGuiMod_Super, pressed); ^~~~~~ ImGuiModFlagsSuper /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp: In member function ‘virtual void vsgImGui::SendEventsToImGui::apply(vsg::KeyPressEvent&)’: /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:250:25: error: invalid conversion from ‘int’ to ‘ImGuiKey’ [-fpermissive] 250 imguiKey = itr->second; ~^~
int
/home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp: In member function ‘virtual void vsgImGui::SendEventsToImGui::apply(vsg::KeyReleaseEvent&)’: /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:278:25: error: invalid conversion from ‘int’ to ‘ImGuiKey_’ [-fpermissive] 278 imguiKey = itr->second; ~^~
int

make[2]: [src/CMakeFiles/vsgImGui.dir/build.make:174: src/CMakeFiles/vsgImGui.dir/vsgImGui/SendEventsToImGui.cpp.o] Error 1 make[1]: [CMakeFiles/Makefile2:342: src/CMakeFiles/vsgImGui.dir/all] Error 2 make: *** [Makefile:136: all] Error 2

robertosfield commented 1 year ago

After explicitly checking out ImGui v1.89.2 and then removing the include/vsg/imgui* files and doing a clean cmake run the headers are copied across correctly so I now get a clean build.

kannode commented 1 year ago

Let me figure it out. Suresh.

On Sun, Jan 15, 2023 at 11:54 AM Robert Osfield @.***> wrote:

I have just checked out the PR as a branch and on compiling get the errors:

10%] Building CXX object src/CMakeFiles/vsgImGui.dir/vsgImGui/SendEventsToImGui.cpp.o /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp: In member function ‘void vsgImGui::SendEventsToImGui::

*updateModifier(ImGuiIO&, vsg::KeyModifier&, bool)’: /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:226:24: error: ‘ImGuiMod_Ctrl’ was not declared in this scope; did you mean ‘ImGuiModFlags_Ctrl’? 226 io.AddKeyEvent(ImGuiMod_Ctrl, pressed); ^~~~~ ImGuiModFlags_Ctrl /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:230:24: error: ‘ImGuiMod_Shift’ was not declared in this scope; did you mean ‘ImGuiModFlags_Shift’? 230 io.AddKeyEvent(ImGuiMod_Shift, pressed); ^~~~~~ ImGuiModFlags_Shift /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:234:24: error: ‘ImGuiMod_Alt’ was not declared in this scope 234 io.AddKeyEvent(ImGuiMod_Alt, pressed); ^~~~ /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:238:24: error: ‘ImGuiMod_Super’ was not declared in this scope; did you mean ‘ImGuiModFlags_Super’? 238 io.AddKeyEvent(ImGuiMod_Super, pressed); ^~~~~~ ImGuiModFlags_Super /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp: In member function ‘virtual void vsgImGui::SendEventsToImGui::apply(vsg::KeyPressEvent&)’: /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:250:25: error: invalid conversion from ‘int’ to ‘ImGuiKey*’ [-fpermissive] 250 imguiKey = itr->second; ~^~
int
/home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp: In member function ‘virtual void vsgImGui::SendEventsToImGui::apply(vsg::KeyReleaseEvent&)’: /home/robert/Dev/vsgImGui/src/vsgImGui/SendEventsToImGui.cpp:278:25: error: invalid conversion from ‘int’ to ‘ImGuiKey_’ [-fpermissive] 278 imguiKey = itr->second; ~^~
int

make[2]: [src/CMakeFiles/vsgImGui.dir/build.make:174: src/CMakeFiles/vsgImGui.dir/vsgImGui/SendEventsToImGui.cpp.o] Error 1 make[1]: [CMakeFiles/Makefile2:342: src/CMakeFiles/vsgImGui.dir/all] Error 2 make: *** [Makefile:136: all] Error 2

— Reply to this email directly, view it on GitHub https://github.com/vsg-dev/vsgImGui/pull/35#issuecomment-1383199321, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHWFHIPE3QSUA236WE4T2ULWSQTU5ANCNFSM6AAAAAATZL7VHY . You are receiving this because you authored the thread.Message ID: @.***>

-- S https://www.nodein.com/uresh K. Kannan, Ph.D., Chief Scientist, Nodein https://www.nodein.com

kannode commented 1 year ago

Yes, I had to do that. I feel the copying of imgui works, but is brittle for subsequent refreshes of imgui. This likely could be improved.

robertosfield commented 1 year ago

Testing on my Kubuntu system looks solid so I've merged the branch into vsgImGui master.

rolandhill commented 1 year ago

Thanks, Suresh. I've been away and just got back to my desk. Thanks for continuing with this, it looks like a great job. I've got a few more things to add that will come through as separate PRs based on your work. I think my original PRs have already been closed.

Regards, Roland

On Fri, 13 Jan 2023 at 02:31, Suresh Kannan @.***> wrote:

@rolandhill https://github.com/rolandhill Hi Roland, I initially looked at how you attempted the upgrade to 1.89 and AddKeyEvent. Thanks for taking a stab at it. I opened pull request #35 https://github.com/vsg-dev/vsgImGui/pull/35, which fixes #34 https://github.com/vsg-dev/vsgImGui/issues/34 and likely subsumes pull requests #30 https://github.com/vsg-dev/vsgImGui/pull/30 and #32 https://github.com/vsg-dev/vsgImGui/pull/32. Could you please check if

30 https://github.com/vsg-dev/vsgImGui/pull/30 and #32

https://github.com/vsg-dev/vsgImGui/pull/32 are still needed and perhaps close them if not?

— Reply to this email directly, view it on GitHub https://github.com/vsg-dev/vsgImGui/pull/35#issuecomment-1380569279, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPOEQ3IRUISANWXCAJH7DTWSAPVNANCNFSM6AAAAAATZL7VHY . You are receiving this because you were mentioned.Message ID: @.***>