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

[MSVC compiler] Cleanup crash when having one OutPin connected with multiple InPin's #25

Open
RoBaertschi opened this issue May 11, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@RoBaertschi
Copy link

RoBaertschi commented May 11, 2024

Version: master branch
ImGui: 1.90.5-docking

Issue

Connecting a OutPin with 2 or more InPin's and then close the program. The deconstructor from ImFlow::Link produces a "Access violation on reading location ...".

image

One fix that I tried, was to replace m_left with m_right but that seems to break other things, like the node deletion logic.

Link::~Link() {
-    m_left->deleteLink();
+    m_right->deleteLink();
}

Code

I minimized my code to be still functional and replicate the issue here. It is not the most good looking code, but should be readable enough.

Also, I added all c++ code in the spoilers following.

main.cpp

#include "Window.h"

int main() {
    CWindow window{};
    window.run();
    return 0;
}

Window.h

#ifndef BOOLEANTABLE_WINDOW_H
#define BOOLEANTABLE_WINDOW_H

#include <stdexcept>


#include <SDL3/SDL.h>
#include <glad/gl.h>
#include <imgui.h>
#include <imgui_impl_sdl3.h>
#include <imgui_impl_opengl3.h>
#include <ImNodeFlow.h>

const int cWinHeight = 400;
const int cWinWidth = 600;

class CWindow {
    SDL_Window* window;
    SDL_GLContext context;

    bool running = true;

    std::shared_ptr<ImFlow::ImNodeFlow> imNodeFlow;
    std::vector<std::string> variables;

public:
    explicit CWindow();
    ~CWindow();

    void run();
};


#endif //BOOLEANTABLE_WINDOW_H

Window.cpp

#include "Window.h"
#include "nodes/AndNode.h"

CWindow::CWindow() : imNodeFlow(std::make_shared<ImFlow::ImNodeFlow>("Main Grid")) {
    if (SDL_Init(SDL_INIT_VIDEO | SDL_INIT_TIMER | SDL_INIT_GAMEPAD) != 0) {
        throw std::runtime_error("Could not initialize SDL3");
    }

    SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1);
    SDL_GL_SetAttribute(SDL_GL_CONTEXT_MAJOR_VERSION, 4);
    SDL_GL_SetAttribute(SDL_GL_CONTEXT_MINOR_VERSION, 5);
    SDL_GL_SetAttribute(SDL_GL_CONTEXT_PROFILE_MASK, SDL_GL_CONTEXT_PROFILE_CORE);

    SDL_SetHint(SDL_HINT_IME_SHOW_UI, "1");
    SDL_GL_SetAttribute(SDL_GL_DOUBLEBUFFER, 1);
    SDL_GL_SetAttribute(SDL_GL_DEPTH_SIZE, 24);
    SDL_GL_SetAttribute(SDL_GL_STENCIL_SIZE, 8);

    window = SDL_CreateWindow("Boolean Tables", cWinWidth, cWinHeight, SDL_WINDOW_OPENGL | SDL_WINDOW_RESIZABLE);
    if (window == nullptr) {
        throw std::runtime_error("Could not create a SDL3 Window");
    }

    context = SDL_GL_CreateContext(window);
    if (context == nullptr) {
        throw std::runtime_error("Could not create a OpenGL Context");
    }
    SDL_GL_MakeCurrent(window, context);

    SDL_GL_SetSwapInterval(1);

    int gl_version = gladLoadGL(SDL_GL_GetProcAddress);
    if (gl_version == 0) {
        throw std::runtime_error("Could not initialize OpenGL context");
    }

    IMGUI_CHECKVERSION();
    ImGui::CreateContext();
    ImGuiIO& io = ImGui::GetIO(); (void)io;
    io.ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard;
    io.ConfigFlags |= ImGuiConfigFlags_NavEnableGamepad;
    io.ConfigFlags |= ImGuiConfigFlags_DockingEnable;
    io.ConfigFlags |= ImGuiConfigFlags_ViewportsEnable;

    ImGui::StyleColorsDark();

    // When viewports are enabled we tweak WindowRounding/WindowBg so platform windows can look identical to regular ones.
    ImGuiStyle& style = ImGui::GetStyle();
    if (io.ConfigFlags & ImGuiConfigFlags_ViewportsEnable)
    {
        style.WindowRounding = 0.0f;
        style.Colors[ImGuiCol_WindowBg].w = 1.0f;
    }

    // Setup Platform/Renderer backends
    ImGui_ImplSDL3_InitForOpenGL(window,context);
    ImGui_ImplOpenGL3_Init();



    imNodeFlow->addNode<CAndNode>(ImVec2{2, 2});

    imNodeFlow->droppedLinkPopUpContent([this](ImFlow::Pin *pin) {

        if (ImGui::Button("And")) {
            auto node = this->imNodeFlow->placeNode<CAndNode>();
            pin->createLink(node->first.get());
        }
    });
}


CWindow::~CWindow() {
    imNodeFlow.reset();

    ImGui_ImplOpenGL3_Shutdown();
    ImGui_ImplSDL3_Shutdown();
    ImGui::DestroyContext();

    SDL_GL_DeleteContext(context);
    SDL_DestroyWindow(window);
    SDL_Quit();
}

void CWindow::run() {
    ImGuiIO& io = ImGui::GetIO(); (void)io;
    while(running) {
        for(SDL_Event event; SDL_PollEvent(&event);) {
            ImGui_ImplSDL3_ProcessEvent(&event);
            switch (event.type) {
                case SDL_EVENT_QUIT:
                    running = false;
                    return;
                case SDL_EVENT_WINDOW_CLOSE_REQUESTED:
                    if (event.window.windowID == SDL_GetWindowID(window)) {
                        running = false;
                        return;
                    }
                    break;
                default:
                    break;
            }

        }

        ImGui_ImplOpenGL3_NewFrame();
        ImGui_ImplSDL3_NewFrame();
        ImGui::NewFrame();

        ImGui::Begin("Boolean Editor");
            imNodeFlow->update();
        ImGui::End();


        ImGui::Render();
        glViewport(0, 0, (int)io.DisplaySize.x, (int)io.DisplaySize.y);
        glClearColor(0.f, 0.f, 0.f, 1.f);
        glClear(GL_COLOR_BUFFER_BIT);
        ImGui_ImplOpenGL3_RenderDrawData(ImGui::GetDrawData());

        if (io.ConfigFlags & ImGuiConfigFlags_ViewportsEnable)
        {
            SDL_Window* backup_current_window = SDL_GL_GetCurrentWindow();
            SDL_GLContext backup_current_context = SDL_GL_GetCurrentContext();
            ImGui::UpdatePlatformWindows();
            ImGui::RenderPlatformWindowsDefault();
            SDL_GL_MakeCurrent(backup_current_window, backup_current_context);
        }

        SDL_GL_SwapWindow(window);
    }
}

AndNode.h

#ifndef BOOLEANTABLE_ANDNODE_H
#define BOOLEANTABLE_ANDNODE_H

#include <ImNodeFlow.h>

class CAndNode : public ImFlow::BaseNode {
public:
    std::shared_ptr<ImFlow::InPin<bool>> first;
    std::shared_ptr<ImFlow::InPin<bool>> second;
    CAndNode();

};


#endif //BOOLEANTABLE_ANDNODE_H

AndNode.cpp

#ifndef BOOLEANTABLE_ANDNODE_H
#define BOOLEANTABLE_ANDNODE_H

#include <ImNodeFlow.h>

class CAndNode : public ImFlow::BaseNode {
public:
    std::shared_ptr<ImFlow::InPin<bool>> first;
    std::shared_ptr<ImFlow::InPin<bool>> second;
    CAndNode();

};


#endif //BOOLEANTABLE_ANDNODE_H

@RoBaertschi RoBaertschi changed the title Cleanup crash when having one OutPin connected with multiple InPin's. Cleanup crash when having one OutPin connected with multiple InPin's May 12, 2024
@Fattorino Fattorino added the bug Something isn't working label May 12, 2024
@Fattorino
Copy link
Owner

Thanks for the detailed issue.
I haven't been able to reproduce the issue in the non-docking branch of ImGui, I'll have to try again with docking another day.

Also please note that in this minimal example, the end node will always return true, and it's also not taking advantage of the full potential of getInVal<>()

@Fattorino Fattorino self-assigned this May 12, 2024
@RoBaertschi
Copy link
Author

I haven't been able to reproduce the issue in the non-docking branch of ImGui, I'll have to try again with docking another day.

That is weird, I just moved to the non docking branch of imgui and that does not change anything. If you want to test that, I created a new branch in the repo. Also I switched to uids, but didn't change anything.

Maybe something is wrong with my setup, but I would not know what.

@RoBaertschi
Copy link
Author

I recorded a demo of what is happening here, maybe it could help with understanding what is going wrong.

cut-error-demostration.mp4

@RoBaertschi
Copy link
Author

So, I did some testing and found something. When I compile the program with MSVC, then it will crash. But with mingw, it does not crash and I don't know why. Also, using LLVM gives another error (These green bars can be ignored, they seem to come from some sort of other issue when taking the screenshot):
image
Maybe this could help, it is probably a MSVC issue.

@Fattorino Fattorino changed the title Cleanup crash when having one OutPin connected with multiple InPin's [MSVC compiler] Cleanup crash when having one OutPin connected with multiple InPin's Jun 7, 2024
@Fattorino Fattorino removed their assignment Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants