trlsmax / imgui-vtk

test on how to integrate vtk into glfw + imgui project
MIT License
108 stars 27 forks source link

Various fixes, changes, and improvements #14

Closed rajkundu closed 2 years ago

rajkundu commented 2 years ago

Please see https://github.com/rajkundu/imgui-vtk for full details.

First of all, thank you very much for this! This repository is amazing, and I am super excited to be able to use VTK with ImGui in such an integrated way. That being said, I started by trying to fix Issue #13 and kind of got carried away. I ended up making quite a few fixes, changes, and improvements. I believe they are quite beneficial, but they carry with them some serious changes – not least because I attempted to "clean" the repository as well.

vtk-imgui/VTKViewer demo

Changes

Summary

As you can see, quite a few changes have been made. For example, I opted to deprecate the imgui_impl_vtk files and instead use the object-oriented VtkViewer class. This now allows for multiple independent VTKViewer instances in a single ImGui application, which I think is quite beneficial. However, there are also many changes to style that are not strict improvements per se.

Note: the "broken" IO behavior to which I am referring is that the last click outside the VTK window's bounds – e.g., clicking on another ImGui window – was transmitted to VTK. Thus, the next time you click again into the VTK window, the camera moves pseudorandomly.

So, if you're interested in only merging some changes – e.g., the IO bugfixes and using VTK's framebuffer directly instead of blitting to an internal one – please let me know! It's my fault that these changes are all a bit tangled in the commit history. Also, if you think it's best for these repos to just remain separate, then I totally understand as well!

rajkundu commented 2 years ago

:warning: Testing

I forgot to include this important point in the original PR: I have only tested my fork on my computer. I have a 16" Intel MacBook Pro running macOS 12.5. I've compiled using Apple Clang 14.0.0. I am using VTK v9.1.0.

I would highly recommend testing on other platforms before merging this PR. Since it has only been tested on my computer, it is practically untested. I can test it on a Windows laptop next week, but if anyone else could test it (preferably on an older version of VTK), that would be greatly appreciated! Thank you!

rajkundu commented 2 years ago

✅ Tested

Tested my fork on Windows with VTK 9.2.2 today. I initially ran into some non-critical error messages from VTK regarding OpenGL versions. After digging around, I found that VTK has been required GL v3.2 since VTK v8.1.0:

After forcing glfw to use GL 3.2 (was previously using GL 3.0), these errors all went away.

I believe this branch is now ready to be merged. It brings many fixes and improvements with it, so I'd at least love to get some of your thoughts on it. Please let me know what you think!

trlsmax commented 2 years ago

Just tested this on manjaro linux and everything just works fine, THANK YOU. With all these changes, imgui-vtk is more flexible and easier to use.

rajkundu commented 2 years ago

You have no idea how happy it makes me to hear that! I'm really glad you found the changes useful! 😁