vsg-dev / vsgImGui

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

Integrate ImPlot into vsgImGui #23

Closed rainergericke closed 1 year ago

rainergericke commented 1 year ago

Since the repository is too big, I try this PR.

robertosfield commented 1 year ago

I can't see any source files in the PR.

Currently ImGui is pulled in as a submodule then the header is copied by CMake, I'm think the same could be done for ImPlot.

The other alternative is to keep ImPlot separate and just provide some easy hooks to add it.

rainergericke commented 1 year ago

Sorry, an example is helpful. MyPlotVsgApplication.zip For full support following change is necessary in imconfig.h:

define ImDrawIdx unsigned int

The above line has to be commented out.

rainergericke commented 1 year ago

Anhang verfügbar bis 28.11.2022 PR didn’t work indeed.

Next trial:

Zum Laden klicken https://www.icloud.com/attachment/?u=https%3A%2F%2Fcvws.icloud-content.com%2FB%2FAX2Frlu5oNVR9LxVeN6NppYjirM9AbeN3nzfwHdFXomWswFAzY-SbOpE%2F%24%7Bf%7D%3Fo%3DAs7MZeRqgT9ilVUjmlH0WZvHLJK9YK6e3eeJU_srKJrD%26v%3D1%26x%3D3%26a%3DCAoge5q9GShc4ddAvANFCAtjqqse2OeXcPfrUyg843ix4cUSdBDm3Z6WwjAY5u2Z6sswIgEAKgkC6AMA_1s4gnhSBCOKsz1aBJJs6kRqJAI66Q9UKipd6f-u5z0eayNR16RmPt2q9xCd1vElkOldawCgOnIkWlt6IITtiDh9c5mx2_VTjeKYQ8JaLUGjHpjtXkZ-b8wNawDV%26e%3D1669622822%26fl%3D%26r%3D023B29C3-C779-4635-B66B-033697D6E2C9-1%26k%3D%24%7Buk%7D%26ckc%3Dcom.apple.largeattachment%26ckz%3D611C43B6-7CE5-42A6-BD21-64B75183D44E%26p%3D35%26s%3DDbIK8wuQgZqQ7VSCGJs7UWBIGi4&uk=r1BF1lIkh5M-UBX-2Fc0lQ&f=vsgImGuiWithImPlot.zip&sz=98675895vsgImGuiWithImPlot.zip 98,7 MB Best

Rainer

Am 28.10.2022 um 14:48 schrieb Robert Osfield @.***>:

I can't see any source files in the PR.

Currently ImGui is pulled in as a submodule then the header is copied by CMake, I'm think the same could be done for ImPlot.

The other alternative is to keep ImPlot separate and just provide some easy hooks to add it.

— Reply to this email directly, view it on GitHub https://github.com/vsg-dev/vsgImGui/pull/23#issuecomment-1294960263, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHOLYFLZFA5VNBBGYX62CADWFPDSDANCNFSM6AAAAAARQ7LXCI. You are receiving this because you authored the thread.

robertosfield commented 1 year ago

98.7M MB of what?

rainergericke commented 1 year ago

You should have got a zipped repository file of my modified vsgImgui library. I hope, at least something is working with the data transfer. Filename: vsgImguiWithImPlot.zip.

robertosfield commented 1 year ago

@rainergericke I haven't had a chance to do a review of this submissions yet. I've merged a number of other vsgImGui PR's today including updates to ImGui 1.89.2, would these affect the ImPlot integration?

kannode commented 1 year ago

I would like to take a stab at the implot integration as I need it as well.

rainergericke commented 1 year ago

I just made a test with the most actual code of the imgui and implot. vsgimgui_example runs as usual, the additional code does not disturb it. The special implot test example also works well, included the 2d histogram.

I made a script to generate a combined imgui/implot directory for the Mac. It should also work on Linux, but not on Windows, Windows can use the ready made combo directory yet.

genVsgImGuiPlot_sh.txt vsgImGuiPlot_diff.txt

You also need the imgui and the implot repository, rsp. the desired releases.

rainergericke commented 1 year ago

Here is the test example. MyPlotVsgApplication.zip

kannode commented 1 year ago

@robertosfield @rainergericke This would involve adding another submodule to vsgImGui. While it would be convenient to have implot, as a part of the imgui integration, it would result in unnecessary coupling.

implot does not require special code to integrate once imgui has been integrated. Hence users of vsg can simply integrate implot into their own projects without any special code. I am going to verify this.

robertosfield commented 1 year ago

Perhaps an example in vsgExamples would suffice.

rainergericke commented 1 year ago

Right. My example is a modified vsgimgui_example with an additional menu entry. The coupling is not unnecessary but unavoidable. It is more than just having two parallel functions, due to the installation process. See https://github.com/ocornut/imgui/issues/2591

robertosfield commented 1 year ago

I just read the thread, but am not clear on what it means for integrating vsgImGui with ImPlot.

rainergericke commented 1 year ago

When both libraries are used alongside without any change some functions of implot would crash the program. That's what the imgui issue is about. This can (must) be avoided by changing the imconfig.h file. The change is easy: //#define ImDrawIdx unsigned int must be #define ImDrawIdx unsigned int

robertosfield commented 1 year ago

@rainergericke could you review @kannode's new PR, it looks more extensive than this one, if your happy then I'd close this one and we can pick up any required changes there.

Do we need to patch the imconfig.h with cmake automatically for the unsigned int indexing issue? It's copied across into public headers from the imgui submodule, so if we update we'll loose any local modifications, so a cmake change at the point of copy seems sensible.

rainergericke commented 1 year ago

I tried to pull PR#38 with 'gh pr checkout 38'. Gh is authorized. It worked. Configuring with cmake leads to these errors on my machine: _-- The CXX compiler identification is AppleClang 14.0.0.14000029 -- Detecting CXX compiler ABI info -- Detecting CXX compiler ABI info - done -- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped -- Detecting CXX compile features -- Detecting CXX compile features - done -- Found Vulkan: /usr/local/lib/libvulkan.dylib (found suitable version "1.3.236", minimum required is "1.1.70.0") found components: glslc glslangValidator -- Performing Test CMAKE_HAVE_LIBC_PTHREAD -- Performing Test CMAKE_HAVE_LIBCPTHREAD - Success -- Found Threads: TRUE
-- Found glslang: /usr/local/lib/libglslang.a -- Reading 'vsg
...' macros from /Users/rainerge/Soft/Packs/VSG/lib/cmake/vsg/vsgMacros.cmake - look there for documentation Cloning into '/Users/rainerge/Documents/GitHub/kannode/src/implot'... git@github.com: Permission denied (publickey). fatal: Could not read from remote repository.

Please make sure you have the correct access rights and the repository exists. fatal: clone of 'git@github.com:epezent/implot.git' into submodule path '/Users/rainerge/Documents/GitHub/kannode/src/implot' failed Failed to clone 'src/implot'. Retry scheduled Cloning into '/Users/rainerge/Documents/GitHub/kannode/src/implot'... git@github.com: Permission denied (publickey). fatal: Could not read from remote repository.

Please make sure you have the correct access rights and the repository exists. fatal: clone of 'git@github.com:epezent/implot.git' into submodule path '/Users/rainerge/Documents/GitHub/kannode/src/implot' failed Failed to clone 'src/implot' a second time, aborting CMake Error at CMakeLists.txt:39 (message): git submodule update --init --recursive failed with 1, please checkout submodules

-- Configuring incomplete, errors occurred! See also "/Volumes/Ramdisk/kannode/CMakeFiles/CMakeOutput.log". See also "/Volumes/Ramdisk/kannode/CMakeFiles/CMakeError.log"._

What is wrong?

rainergericke commented 1 year ago

Is there a better opportunity to apply this change?

Am 16.01.2023 um 09:57 schrieb Robert Osfield @.***>:

@rainergericke https://github.com/rainergericke could you review @kannode https://github.com/kannode's new PR, it looks more extensive than this one, if your happy then I'd close this one and we can pick up any required changes there.

Do we need to patch the imconfig.h with cmake automatically for the unsigned int indexing issue? It's copied across into public headers from the imgui submodule, so if we update we'll loose any local modifications, so a cmake change at the point of copy seems sensible.

— Reply to this email directly, view it on GitHub https://github.com/vsg-dev/vsgImGui/pull/23#issuecomment-1383688869, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHOLYFI6YCKYQ6BJW4CRD2TWSUEOFANCNFSM6AAAAAARQ7LXCI. You are receiving this because you were mentioned.

kannode commented 1 year ago

Rainer, it's likely gh login issue on your end. You could do a gh auth login and choose https instead. My fork of vsgImGui is public.

Try the following and checkout the implot branch in each.

git clone https://github.com/kannode/vsgImGui.git
git checkout implot

git clone https://github.com/kannode/vsgExamples.git
git checkout implot
rainergericke commented 1 year ago

I tried your proposed commands. I got the same errors yet.

rainergericke commented 1 year ago

Good News!

The problem was, implot was loaded via ssh and it was not configured on my machine. After fixing this I could configure and build vsgImGui and vsgExamples and the example works well.

Thanks

kannode commented 1 year ago

Great! I updated the PR to use https for implot.

On Mon, Jan 16, 2023 at 10:54 AM Rainer Gericke @.***> wrote:

Good News!

The problem was, implot was loaded via ssh and it was not configured on my machine. After fixing this I could configure and build vsgImGui and vsgExamples and the example works well.

Thanks

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

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

kannode commented 1 year ago

@robertosfield @rainergericke

robertosfield commented 1 year ago

I'm closing this PR as it's no longer required :-)