Support system imgui libs

[Apteryks]

This allows using the system provided utf8.h and imgui libraries. Tested using the GNU Guix package definition of OpenBoardView.

[piernov]

What does the ImGui system package look like?
On ArchLinux AUR ( https://aur.archlinux.org/packages/imgui ), it contains a CMake targets file that could be used instead of a custom CMake module, however it does not contain any of the backend headers.
Additionally, our trick to replace the OpenGL include with glad for the OpenGL2 (/1.2) backend wouldn't work with system headers, does it work with the regular gl.h header in your distribution?

[Apteryks]

@piernov Hi! It looks like this: https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/packages/toolkits.scm#n32. It currently does not use any build system (as it doesn't ship any). Here's what the package currently contains:

$ find /gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/backends
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/backends/imgui_impl_glfw.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/backends/imgui_impl_opengl3.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/backends/imgui_impl_opengl2.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/backends/imgui_impl_sdl.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/backends/imgui_impl_sdlrenderer.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/backends/imgui_impl_opengl3_loader.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/backends/imgui_impl_vulkan.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/misc
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/misc/cpp
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/misc/cpp/imgui_stdlib.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/misc/freetype
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/misc/freetype/imgui_freetype.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/misc/single_file
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/misc/single_file/imgui_single_file.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/imconfig.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/imgui.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/imgui_internal.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/imstb_rectpack.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/imstb_textedit.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/include/imgui/imstb_truetype.h
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/lib
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/lib/libimgui.so
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/share
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/share/doc
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/share/doc/imgui-1.87
/gnu/store/jbrb1wd54p5hak7mfycxrpsk7sjfkcqa-imgui-1.87/share/doc/imgui-1.87/LICENSE.txt

So yes, it includes the headers (and is built with support for) a few select backends. I first upgraded OpenBoardView from 8.95.1 to 8.95.2 without unbundling ImGui, so I could test both version, so it was easy to validate that 8.95.2 with its bundled copy of ImGui vs 8.95.2 using the system ImGui behaved the same.

I must say that 8.95.2 seems to have some graphical glitch on my setup;
it looks like this:

or this:

Going back to 8.95.1, I no longer have these graphical artifacts.

One thing that gets printed on the console with the system ImGui which I don't see with the bundled version are "Unknown key name" errors, like so:

$ /gnu/store/wzqlw4sxj5msqiiasjqp65q3r3k345ak-openboardview-8.95.2/bin/o
INFO: Initializing ImGuiRendererSDLGL3
INFO: 
-------------------------------------------------------------
GL Vendor     : nouveau
GL GLRenderer : NV50
GL Version    : 3.3 (Core Profile) Mesa 21.3.2
GLSL Version  : 3.30
-------------------------------------------------------------
ERROR: Unknown key name: Keypad 6
ERROR: Unknown key name: Keypad 4
ERROR: Unknown key name: Ctrl
ERROR: Unknown key name: Keypad 8
ERROR: Unknown key name: Ctrl
ERROR: Unknown key name: /
ERROR: Unknown key name: Ctrl
ERROR: Unknown key name: Shift
ERROR: Unknown key name: Return
ERROR: Unknown key name: Return
ERROR: Unknown key name: Keypad 5
ERROR: Unknown key name: Keypad +
ERROR: Unknown key name: =
ERROR: Unknown key name: .
ERROR: Unknown key name: Keypad .
ERROR: Unknown key name: -
ERROR: Unknown key name: Keypad -
ERROR: Unknown key name: ,
ERROR: Unknown key name: Keypad 0
ERROR: Unknown key name: Keypad 2

Would you know what could be causing them?

[piernov]

Ok, since the ImGui integration seems distribution-specific and there is not consensus on how to use it as a system package, it doesn't make a lot of sense to add support for a system-wide ImGui in OpenBoardView for now, so I am not going to merge this change yet. It seems to me that at least each backend should be built as a separate shared library (with its own set of dependencies).
Maybe you could get in touch with ArchLinux AUR imgui maintainer so that there are at least 2 distributions that agree on how to package it?
Also it looks like Debian has an ImGui package which includes backend headers in the system include directory, but also source code for the backends in the doc directory, I assume in order to be built with the application.

The utf8.h change is probably fine though, could you split the PR in two to merge that commit separately?
In fact I'm not sure the utf8.h library is actually useful for anything but for now we will keep it.

About the graphical glitches, please open an issue describing your software&hardware configuration.
I am not seeing this issue on several of my machines (Windows with AMD 200-series GPU, ArchLinux with AMD 200-series, Intel Gen6/Gen9 GPU, Nvidia 1000-series, macOS with NVidia 8000-series and Intel Gen7, and even Windows in VirtualBox) so I believe it is dependent on the graphics drivers and hardware.
EDIT: bug caused by a change in ImGui 1.86 GL3 backend (introduced by ocornut/imgui@389982e , similar report at ocornut/imgui#4832). Only happens with some specific hardware/software, I was able to reproduce with an NVidia 8800GTS (G80) on ArchLinux32 with nouveau at least.
GL2/1.2 backend is not affected so if you start OBV with -r 1 or set renderer=1 in obv.conf, everything should be working properly.

The errors you are seeing in the terminal are normal after updating to 8.95.2, this is due to a change in keyboard handling for ImGui 1.87, you will have reset the keyboard configuration, see the release notes: https://github.com/OpenBoardView/OpenBoardView/releases/tag/8.95.2 .

[Apteryks]

Hi,

Ok, since the ImGui integration seems distribution-specific and there is not consensus on how to use it as a system package, it doesn't make a lot of sense to add support for a system-wide ImGui in OpenBoardView for now, so I am not going to merge this change yet. It seems to me that at least each backend should be built as a separate shared library (with its own set of dependencies). Maybe you could get in touch with ArchLinux AUR imgui maintainer so that there are at least 2 distributions that agree on how to package it?

The Arch Linux package uses some Microsoft-provided CMakeLists.txt file but the end result is the same:

1. a libimgui.so shared library that contains support for the configured backends
2. the headers (which include that of the compiled in backends).

Which is the same in the case of the imgui Guix package. So it seems we already are in agreement :-). I'd be happy to have their feedback here, but I feel like this change would be useful and welcome to them as well, so my personal take would be that it could be merged already (it doesn't impact users of the bundled copy).

Also it looks like Debian has an ImGui package which includes backend headers in the system include directory, but also source code for the backends in the doc directory, I assume in order to be built with the application.

We can see their file list here. That is odd; they provide a static library (.a) and source code for the backends. It seems to me a better option would be to distribute a shared library which includes the support for the common GNU/Linux backends (OpenGL/Vulkan). Users can then use whatever they need and simply need linking against imgui. The transitive dependencies to support these backends doesn't change drastically anyway (it's mostly mesa).

The utf8.h change is probably fine though, could you split the PR in two to merge that commit separately? In fact I'm not sure the utf8.h library is actually useful for anything but for now we will keep it.

About the graphical glitches [...] EDIT: bug caused by a change in ImGui 1.86 GL3 backend (introduced by ocornut/imgui@389982e , similar report at ocornut/imgui#4832). Only happens with some specific hardware/software, I was able to reproduce with an NVidia 8800GTS (G80) on ArchLinux32 with nouveau at least. GL2/1.2 backend is not affected so if you start OBV with -r 1 or set renderer=1 in obv.conf, everything should be working properly.

Interesting. That's the very GPU I was testing with (nvidia 8800GTS); thanks for the research & workaround! Now the graphical glitches are gone, but the GUI is still difficult to use, due to seemingly not registering most of the clicks (opening a menu is rather difficult).

The errors you are seeing in the terminal are normal after updating to 8.95.2, this is due to a change in keyboard handling for ImGui 1.87, you will have reset the keyboard configuration, see the release notes: https://github.com/OpenBoardView/OpenBoardView/releases/tag/8.95.2 .

I confirm the keyword related errors went away after issuing rm -rf ~/.local/share/OpenBoardView/ && rm -rf ~/.config/OpenBoardView/; thank you!

[piernov]

Forgot to mention that I merged build system: Allow using utf8.h from the system in 1753efa.
(also uploaded an AUR package for the occasion https://aur.archlinux.org/packages/utf8.h)

As for ImGui, I still think it doesn't make much sense to merge a change that would only work on one specific distribution and could cause issues on others, since the rendering backends are not included as a library (not included at all in ArchLinux, and source code only on Debian). Include path are not consistent either.

In my opinion a better way to distribute ImGui in binary form and have applications be able to use it easily would be to provide a shared library for each supported rendering backend, and have a CMake file (pkg-config would also work) for each to include support if available.

The 3 different ways the distributions identified here (Debian/ArchLinux/Guix) do it do not make it easy to have a common build process in OpenBoardView to use the system library.
So unless multiple distributions agree on a common way to present the library or ImGui themselves provide a guideline, the support for ImGui as a system library should stay in an out-of-tree patch for each distribution (if needed).

[Apteryks]

While more fine grained controls could be nice, I think a imgui.so library that includes whatever backend support was linked in at build time would be OK as a first implementation, to keep things simple. Include headers not appearing at the same place on different distributions should be fixed by consensus ideally or alternatively by letting them specify it via a pkg-config file.

If users come and request such finer grained (and more complex) controls, we can improve on it, in my opinion.

That Arch doesn't currently ship any support for backends could be easily changed, given you seem to be an Arch contributor? :-)

[Apteryks]

For what it's worth, this still works with 9.95.0.