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

ImGuiListClipper behaviour with garbage collected instance (when not calling End) #4822

Closed
sonoro1234 opened this issue Dec 22, 2021 · 8 comments

Comments

@sonoro1234
Copy link

Version: 1.86
Branch:docking

with LuaJIT-ImGui

running this program

local igwin = require"imgui.window"
local win = igwin:GLFW(800,400, "list clipper")

function win:draw(ig)
    local cols = 10
    local clipper = ig.ImGuiListClipper()
    clipper:Begin(1000)
    while (clipper:Step()) do
        for line = clipper.DisplayStart,clipper.DisplayEnd-1 do
            for N=line*cols+1,line*cols+cols do
                if ig.Button(string.format("%d",N)) then
                    print(N)
                end
                if not ((N)%cols == 0) then ig.SameLine() end
            end
        end
    end
end

win:start()

I am getting this assertion:

Assertion failed!

Program: c:\anima\luajit.exe
File: C:\LuaGL\gitsources\anima\LuaJIT-ImGui\cimgui\imgui\imgui.cpp, Line 2443

Expression: data->ListClipper == this

I dont know if it is due to bug or misuse (Althought it used to work before)

@sonoro1234
Copy link
Author

sonoro1234 commented Dec 23, 2021

This does not happen with main.cpp modified for having ImGuiListClipper.
It does not happen in LuaJIT-ImGui if code is modified for making sure ImGuiListClipper is destroyed before creating a new one with code:

local igwin = require"imgui.window"
local win = igwin:SDL(800,400, "list clipper")

function win:draw(ig)
    local cols = 10
    if ig.Begin("testt") then
        local clipper = ig.ImGuiListClipper()
        clipper:Begin(1000)
        while (clipper:Step()) do
            for line = clipper.DisplayStart,clipper.DisplayEnd-1 do
                for N=line*cols+1,line*cols+cols do
                    if ig.Button(string.format("%d",N)) then
                        print(N)
                    end
                    if not ((N)%cols == 0) then ig.SameLine() end
                end
            end
        end
    end
    ig.End()
    collectgarbage()
end

win:start()

But if collectgarbage is not used (it calls __gc on clipper) so that there are several ImGuiListClipper creations before destroy is called the problem reappears.
It seems dificult to mimic this behaviour in C++: We should store ImGuiListClippers in an array and call destructor when the array exceds some given size.
Perhaps the cause is in imgui.cpp:2424 where TempData is set?

@sonoro1234 sonoro1234 changed the title ImGuiListClipper bug or misuse ImGuiListClipper behaviour with garbage collection Dec 23, 2021
@ocornut
Copy link
Owner

ocornut commented Dec 23, 2021

What happen if you simply call clipper.End() at the end of your loop ?

@sonoro1234
Copy link
Author

sonoro1234 commented Dec 23, 2021

What happen if you simply call clipper.End() at the end of your loop ?

if collectgarbage is active there is a crash without assertion in the first iteration

gdb posts SIGSEGV in memcpy

@sonoro1234
Copy link
Author

correction: with clipper.End() at the end of my loop problem is gone

@ocornut
Copy link
Owner

ocornut commented Dec 23, 2021 via email

@sonoro1234
Copy link
Author

Having to call clipper.End at the end of Step loop seems to me a clearer pattern for the casual ImGui user

@ocornut ocornut changed the title ImGuiListClipper behaviour with garbage collection ImGuiListClipper behaviour with garbage collected instance (when not calling End) Jan 3, 2022
@ocornut ocornut added the bug label Feb 17, 2022
@ocornut
Copy link
Owner

ocornut commented Feb 17, 2022

seems to me a clearer pattern for the casual ImGui user

Yes but technically we have a regression now, and Rust users are also affected. I'll look into it now.

@ocornut
Copy link
Owner

ocornut commented Feb 17, 2022

Pushed a fix now d9e60d2

Thanks for reporting!

@ocornut ocornut closed this as completed Feb 17, 2022
@ocornut ocornut reopened this Feb 17, 2022
@ocornut ocornut closed this as completed Feb 18, 2022
edvn0 pushed a commit to edvn0/imgui that referenced this issue Feb 24, 2022
edvn0 pushed a commit to edvn0/imgui that referenced this issue Feb 24, 2022
sergeyn pushed a commit to sergeyn/imgui that referenced this issue Mar 19, 2022
sergeyn pushed a commit to sergeyn/imgui that referenced this issue Mar 19, 2022
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