Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opengl loader dependencies #9

Closed
XPXPv2 opened this issue Nov 8, 2020 · 9 comments
Closed

Opengl loader dependencies #9

XPXPv2 opened this issue Nov 8, 2020 · 9 comments

Comments

@XPXPv2
Copy link

XPXPv2 commented Nov 8, 2020

I was thinking about this earlier but the - recent pull request reminded me. The opengl loader dependencie situation instead of trying to add all the dependencies for every library what if we had a option to use a dependencie like IMGUI_OPENGL_LOADER and the user could define that before they call the subproject I plan on implementing it but I wanted to check to make sure it's ok to do something like that

@eli-schwartz
Copy link
Member

I'm not sure I understand... you want to do dependency('IMGUI_OPENGL_LOADER')? I don't understand what that means...

Can you clarify the problem with the current meson_options.txt based approach?

@XPXPv2
Copy link
Author

XPXPv2 commented Jan 4, 2021

Ill use the current patch

meson_options.txt

#renderer backends
option('dx9', type : 'feature', value : 'auto')
option('dx10', type : 'feature', value : 'auto')
option('dx11', type : 'feature', value : 'auto')
option('dx12', type : 'feature', value : 'auto')
option('metal', type : 'feature', value : 'auto')
option('vulkan', type : 'feature', value : 'auto')

option('opengl_glew', type : 'feature', value : 'auto')
option('opengl_custom', type : 'feature', value : 'auto')

#platform backends
option('glfw', type : 'feature', value : 'auto')
option('sdl2', type : 'feature', value : 'auto')
option('osx', type : 'feature', value : 'auto')
option('win', type : 'feature', value : 'auto')

#frameworks (renderer + platform)
option('marmalade', type : 'feature', value : 'auto')
option('allegro5', type : 'feature', value : 'auto')

meson.build

glew_dep = dependency('glew', required: get_option('opengl_glew'))
if glew_dep.found()
sources += 'examples/imgui_impl_opengl3.cpp'
dependencies += glew_dep
endif
opengl_custom_dep = dependency('IMGUI_OPENGL_LOADER', required: get_option('opengl_custom'))
if opengl_custom_dep.found()
sources += 'examples/imgui_impl_opengl3.cpp'
dependencies += opengl_custom_dep
endif

end user meson.build using glad for example

meson.override_dependency('IMGUI_OPENGL_LOADER',glad_dep)

this can be done currently by redefining glew for example but this should be more readable and less likely to cause conflict in the future

@stephanlachnit
Copy link

I'm not sure I understand... you want to do dependency('IMGUI_OPENGL_LOADER')? I don't understand what that means...

Can you clarify the problem with the current meson_options.txt based approach?

If you look at imgui_impl_opengl3.h:
https://github.com/ocornut/imgui/blob/58075c4414b985b352d10718b02a8c43f25efd7c/backends/imgui_impl_opengl3.h#L66-L82
You'll see that you can select from multiple OpenGL loaders. We currently don't offer this option in the Wrap.

If you look in the meson.build:

imgui/meson.build

Lines 40 to 44 in 025c4df

glew_dep = dependency('glew', required: get_option('opengl'))
if glew_dep.found()
sources += 'examples/imgui_impl_opengl3.cpp'
dependencies += glew_dep
endif

You see that I simply replace I want OpenGL with I use OpenGL with GLEW. One could add more options and adding compile flags, but as I mentioned here, I don't really care about the other options (I also don't think there is a lot of demand for this).

@XPXPv2
Copy link
Author

XPXPv2 commented Jan 23, 2021

Yes, I also don't want to bother adding every single other OpenGL loader so I am instead proposing to add a more generic option so you don't have to override the glew dependency when someone wants to use something else.

@stephanlachnit
Copy link

Btw I will probably do this for 1.82 since MangoHud now started using a OpenGL loder (glad).

@martinhath
Copy link

What is the intended way of choosing the opengl loader with the current meson setup? My current solution is to manually change glew for glad, and add in the right definition, like so:

diff --git a/meson.build b/meson.build
index c7bd7da..d10f7fa 100644
--- a/meson.build
+++ b/meson.build
@@ -8,6 +8,7 @@ sources = ['imgui_demo.cpp', 'imgui_draw.cpp', 'imgui_tables.cpp', 'imgui_widget

 cpp = meson.get_compiler('cpp')
 dependencies = []
+cpp_args = []

 # renderer backends
 dx9_dep = cpp.find_library('d3d9', required: get_option('dx9'))
@@ -38,6 +39,7 @@ endif
 glad_dep = dependency('glad', required: get_option('opengl'))
 if glad_dep.found()
     sources += 'backends/imgui_impl_opengl3.cpp'
+    cpp_args += '-DIMGUI_IMPL_OPENGL_LOADER_GLAD'
     dependencies += glad_dep
 endif
 vulkan_dep = dependency('vulkan', required: get_option('vulkan'))
@@ -80,6 +82,7 @@ imgui = library('imgui',
     sources,
     dependencies: dependencies,
     include_directories: include_dirs,
+    cpp_args: cpp_args
 )

 imgui_dep = declare_dependency(include_directories: include_dirs, link_with: imgui)

but I'm not sure if this is really the best way, because now I'll have to manage my own wrapfile to have this work for other machines as well.

@XPXPv2
Copy link
Author

XPXPv2 commented Sep 23, 2021

Unfortunately their isn't a way of changing the opengl loader that is the topic of this discussion to add such a feature. It is possible to override the glew dependency with a custom glad dependency if you won't to use the standard wrap file.

@eli-schwartz
Copy link
Member

Is this something which could be solved via e.g.

# meson_options.txt
option('opengl-type', type : 'combo', choices : ['glew', 'glad', 'somethingelse'], value : 'glew')

# meson.build
opengl_dep = dependency(get_option('opengl_type'), required: get_option('opengl'))
if opengl_dep.found()
    sources += 'backends/imgui_impl_opengl3.cpp'
    dependencies += opengl_dep
    cpp_args += '-DIMGUI_IMPL_OPENGL_LOADER_' + get_option('opengl_type').to_upper()
endif

Then the value of -Dopengl-type=glad would cause it to look up dependency('glad') instead, and define -DIMGUI_IMPL_OPENGL_LOADER_GLAD. Are there other opengl backends to consider as well?

@stephanlachnit
Copy link

Actually, we can close this now: ocornut/imgui#4445

Dear ImGui now ships its own loader, meaning we don't have to deal with this anymore. Unfortunately, I'm very busy at the moment and don't have time to code until mid-October.

@XPXPv2 XPXPv2 closed this as completed Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants