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

Backend api to better support multiple contexts #1851

Closed
sonoro1234 opened this issue Jun 1, 2018 · 15 comments
Closed

Backend api to better support multiple contexts #1851

sonoro1234 opened this issue Jun 1, 2018 · 15 comments

Comments

@sonoro1234
Copy link

sonoro1234 commented Jun 1, 2018

Hi Omar,

I think that with some minimal changes the different implementations could be directly used in bindings:

Basically an ImGuiContext should be asociated with the implementation and the following functions exported as C functions: impl_new, impl_delete, impl_Init, impl_NewFrame, impl_Render (which does Render and RenderDrawLists) and also the hook setting functions (Mouse, Key, Char and Scroll) or ProcessEvent in the sdl case or WndProcHandler in dx

This is what I did for example in:https://github.com/sonoro1234/LuaJIT-ImGui/blob/master_auto_implementations/extras/impl_glfw3/imgui_impl_glfw_gl3.h

which is used as in:https://github.com/sonoro1234/LuaJIT-ImGui/blob/master_auto_implementations/examples/minimal_main.lua
or multiwindow
https://github.com/sonoro1234/LuaJIT-ImGui/blob/master_auto_implementations/examples/minimal_multicontext.lua

@ocornut
Copy link
Owner

ocornut commented Jun 1, 2018

I'm not sure I understand, why did you wrap everything into a class with members and not just wrap the 5-6 functions of the original impl code exactly as they exist? What prevents anyone from using the existing functions and wrap them to be accessible in a binding?

@sonoro1234
Copy link
Author

sonoro1234 commented Jun 1, 2018

I just used a class to made things easier for me: ctx is a member of implementation so I can set global context from the class (see the cpp file)
https://github.com/sonoro1234/LuaJIT-ImGui/blob/master_auto_implementations/extras/impl_glfw3/imgui_impl_glfw_gl3.cpp#L121

@ocornut
Copy link
Owner

ocornut commented Jun 1, 2018 via email

@sonoro1234
Copy link
Author

sonoro1234 commented Jun 1, 2018

impl:Set() is used in several places of cpp file to set the class context as the global ImGuiContext. This allows me to have several instances of impl working at the same time, one for each window for example in a multiwindow application. Without the class it would be difficult to implement.
The only way to do it without a class would be to have a impl struct and some functions to work with it, but a class is just simpler for me.

@sonoro1234
Copy link
Author

Making implementations multi context ready.

@ocornut
Copy link
Owner

ocornut commented Jun 2, 2018 via email

@sonoro1234
Copy link
Author

the problem would be all those static variables
https://github.com/ocornut/imgui/blob/master/examples/sdl_opengl3_example/imgui_impl_sdl_gl3.cpp#L54
unless they are in a struct or class

@ocornut
Copy link
Owner

ocornut commented Jun 2, 2018 via email

@sonoro1234
Copy link
Author

sonoro1234 commented Jun 2, 2018

of course struct could be in the .cpp instead of .h file.
But all the functions will need an extra argument for the implementation struct

@sonoro1234
Copy link
Author

The main problem is with opengl data
https://github.com/ocornut/imgui/blob/master/examples/imgui_impl_opengl3.cpp#L33
being static makes imposible to have several windows each one with its own opengl data.

@ocornut
Copy link
Owner

ocornut commented Jun 28, 2018

being static makes imposible to have several windows each one with its own opengl data.

It works perfectly (the viewport branch is using it daily) as long as the OpenGL context are shared contexts. And I see no reason for having non-shared contexts.
PS: Please open separate topics if the discussion is not related to the C bindings.

@sonoro1234
Copy link
Author

sonoro1234 commented Jun 28, 2018

You are right:
Just saw this http://www.glfw.org/docs/latest/context.html#context_sharing

But the problem remains in g_Window
https://github.com/ocornut/imgui/blob/master/examples/imgui_impl_glfw.cpp#L52

This is related to bindings that also want to bind imgui implementations instead of provide its own implementation. (just dropped bindings from title)

@sonoro1234 sonoro1234 changed the title Implementations bindings proposal Implementations proposal Jun 28, 2018
@sonoro1234
Copy link
Author

with classes for multiwindow
gif
with standard imgui/examples implementation
gif2

@ocornut ocornut changed the title Implementations proposal Backend api to better support multiple contexts Dec 20, 2019
ocornut added a commit that referenced this issue Jun 29, 2021
…multi-contexts. (#586, #1851, #2004, #3012, #3934, #4141)

This is NOT enable multi-contexts for any backends
- in order to make this commit as harmless as possible, while containing all the cruft/renaming
-
ocornut added a commit that referenced this issue Jun 29, 2021
#1851, #2004, #3012, #3934, #4141)

I believe more renderer backends should work. GLFW/Win32/SDL/Vulkan probably have many issues.
@ocornut
Copy link
Owner

ocornut commented Jun 29, 2021

Hello,

Making implementations multi context ready.

This should now be solved by 70c6038 + 4cec3a0.

I confirmed that both OpenGL and DX11 backends worked my test bed.
Platform backends however are not confirmed to fully work but for your specific use case (nested, one blocking the other) it should work with no noticeable issue.

Details (posted in a few threads)

With 70c6038 i have rearranged all backends to pull their data from structures
This commit is rather large and noisy so I tried to make it as harmless a possible and therefore the commit as-is tries to NOT change anything in the backends, which include NOT enabling support for multiple imgui-context.
It was then enabled with a small patch in each backend 4cec3a0

PS: Current change being generaly healthy I don't mind it but I'm not very excited with promoting and encouraging support of multiple-imgui-context-each-in-their-own-backend-context. I think that feature is likely to be incompatible with multi-viewports (single imgui context + multiple backend windows) and I think instead we should aim at reworking this to let user register any existing window.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants