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

Impl glfw backend multi context support by adding event queue #7186

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nyaoouo
Copy link

@nyaoouo nyaoouo commented Jan 2, 2024

Impl multi context support by adding event queue,
test code in windows

#include <GLFW/glfw3.h>
#include <imgui.h>
#include "imgui_impl_glfw.h"
#include "imgui_impl_opengl3.h"

int main() {

    if (!glfwInit()) {
        return -1;
    }
    const char *glsl_version = "#version 330";
    glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 3);
    glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 3);
    glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE);

    GLFWwindow *window1 = glfwCreateWindow(640, 480, "Window 1", NULL, NULL);
    if (!window1) {
        glfwTerminate();
        return -1;
    }
    glfwMakeContextCurrent(window1);
    IMGUI_CHECKVERSION();
    ImGuiContext *ctx1 = ImGui::CreateContext();
    ImGui::SetCurrentContext(ctx1);
    ImGui_ImplGlfw_InitForOpenGL(window1, true);
    ImGui_ImplOpenGL3_Init(glsl_version);
    GLFWwindow *window2 = glfwCreateWindow(640, 480, "Window 2", NULL, window1);
    if (!window2) {
        glfwDestroyWindow(window1);
        glfwTerminate();
        return -1;
    }
    glfwMakeContextCurrent(window2);
    IMGUI_CHECKVERSION();
    ImGuiContext *ctx2 = ImGui::CreateContext();
    ImGui::SetCurrentContext(ctx2);
    ImGui_ImplGlfw_InitForOpenGL(window2, true);
    ImGui_ImplOpenGL3_Init(glsl_version);

    while (!glfwWindowShouldClose(window1) && !glfwWindowShouldClose(window2)) {
        {
            glfwMakeContextCurrent(window1);
            ImGui::SetCurrentContext(ctx1);
            ImGui_ImplOpenGL3_NewFrame();
            ImGui_ImplGlfw_NewFrame();
            ImGui::NewFrame();
            ImGui::Begin("Hello from window 1!");
            ImGui::Text("This is window 1.");
            ImGui::End();

            ImGui::Render();
            int display_w, display_h;
            glfwGetFramebufferSize(window1, &display_w, &display_h);
            glViewport(0, 0, display_w, display_h);
            glClear(GL_COLOR_BUFFER_BIT);
            ImGui_ImplOpenGL3_RenderDrawData(ImGui::GetDrawData());

            glfwSwapBuffers(window1);
        }

        {
            glfwMakeContextCurrent(window2);
            ImGui::SetCurrentContext(ctx2);
            ImGui_ImplOpenGL3_NewFrame();
            ImGui_ImplGlfw_NewFrame();
            ImGui::NewFrame();
            ImGui::Begin("Hello from window 2!");
            ImGui::Text("This is window 2.");
            ImGui::End();

            ImGui::Render();
            int display_w, display_h;
            glfwGetFramebufferSize(window2, &display_w, &display_h);
            glViewport(0, 0, display_w, display_h);
            glClear(GL_COLOR_BUFFER_BIT);
            ImGui_ImplOpenGL3_RenderDrawData(ImGui::GetDrawData());

            glfwSwapBuffers(window2);
        }

        glfwPollEvents();
    }

    ImGui_ImplOpenGL3_Shutdown();
    ImGui_ImplGlfw_Shutdown();
    ImGui::DestroyContext();

    glfwDestroyWindow(window1);
    glfwDestroyWindow(window2);
    glfwTerminate();

    return 0;
}

@nyaoouo nyaoouo changed the title Impl multi context support by adding event queue Impl glfw backend multi context support by adding event queue Jan 2, 2024
@ocornut
Copy link
Owner

ocornut commented Jan 3, 2024

Hello,

Thanks for your PR. This is a backend-side implementation for what's suggested in e.g. #7155 #5671
Unfortunately we cannot merge a PR like this: we cannot use C++ std library in our code, and it is generally a rather noisy PR.

Here how I would like to rework it:

  • Store a single global ImGuiStorage in imgui_impl_glfw
  • Let the user know via comments they cannot add new context while others are processing (e.g. if multi-threaded)
  • Change ImGui_ImplGlfw_GetBackendData() by splitting into ImGui_ImplGlfw_GetBackendDataForCurrentContext() and ImGui_ImplGlfw_GetBackendDataForWindow(GLFWwindow*).
  • Most callbacks using GetBackendData have access to that window and can use ImGui_ImplGlfw_GetBackendDataForWindow().
  • Special handling for ImGui_ImplGlfw_WndProc.
  • ImGui_ImplGlfw_MonitorCallback() in docking branch need to iterate all created contexts and set WantUpdateMonitors=true in all of them.

@FunMiles
Copy link
Contributor

I am interested in using this PR's work. My goal is slightly more ambitious as I want to make ImGui thread safe.

Thanks for your PR. This is a backend-side implementation for what's suggested in e.g. #7155 #5671 Unfortunately we cannot merge a PR like this: we cannot use C++ std library in our code, and it is generally a rather noisy PR.

It is not possible to make any code thread safe in a multi-threaded system without at least some form of atomic and/or mutex. The most portable way to do this is to use std::atomic or std::mutex. For a single threaded system, one can provide a No-op equivalent. Though it would seem that one could leave the use of locks for which system libraries are needed to the user, I do not see how to do it efficiently. Example: the user's code, running on an independent thread could ask the main thread to give it the pending events. The main thread would swap the event queue with an empty one and put the full one somewhere for the user's code to get it. Then the user code could be notified to resume.
Yes, this scheme would work, but it comes at the cost of two full thread sequencing, which is notably expensive. Many system take about 100µs to do such operations. That, compared to the time for making 60fps or 120fps visualization, is a big chunk of time. With atomics or mutex, one can go down to a few nanoseconds for swapping queues.

Here how I would like to rework it:

  • Store a single global ImGuiStorage in imgui_impl_glfw
  • Let the user know via comments they cannot add new context while others are processing (e.g. if multi-threaded)

see below for why I think it is unnecessary.

  • Change ImGui_ImplGlfw_GetBackendData() by splitting into ImGui_ImplGlfw_GetBackendDataForCurrentContext() and ImGui_ImplGlfw_GetBackendDataForWindow(GLFWwindow*).
  • Most callbacks using GetBackendData have access to that window and can use ImGui_ImplGlfw_GetBackendDataForWindow().

I am doing this is my changes on top of #5856 so yes, I believe that is the way to go.

  • Special handling for ImGui_ImplGlfw_WndProc.
  • ImGui_ImplGlfw_MonitorCallback() in docking branch need to iterate all created contexts and set WantUpdateMonitors=true in all of them.

As can be found in the discussion of #5856 after having thought of that, I've come up with a much simpler approach.
If each context keeps a time at which it last acted on a monitor event, the event handler can simply record the change time in a global variable that each context can compare to the last time they acted on a change. That avoids having to keep a global table of active contexts and removes the constraint mentioned above.

@FunMiles
Copy link
Contributor

When a previous event handler existed, it is probably better to call it before queueing the event for ImGui processing.

@paulocoutinhox
Copy link

+1

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

Successfully merging this pull request may close these issues.

None yet

4 participants