ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
57.5k stars 9.89k forks source link

Mouse action functions (IsItemHovered, IsMouseClicked,IsMouseDown,IsMouseReleased, etc.) always return false for overlapped item. #7715

Open mistahmikey opened 1 week ago

mistahmikey commented 1 week ago

Version/Branch of Dear ImGui:

Version 1.90.8, Branch: master

Back-ends:

imgui_impl_XXX.cpp + imgui_impl_XXX.cpp

Compiler, OS:

Windows 11

Full config/build information:

Dear ImGui 1.90.8 (19080)
--------------------------------

sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=202101
define: _WIN32
define: _WIN64
define: _MSC_VER=1936
define: _MSVC_LANG=202004
define: __clang_version__=15.0.1 
--------------------------------

io.BackendPlatformName: NULL
io.BackendRendererName: NULL
io.ConfigFlags: 0x00000020
 NoMouseCursorChange
io.ConfigInputTextCursorBlink
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x00000000
--------------------------------

io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 2048,4096
io.DisplaySize: 3440.00,1369.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------

style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 0.00
style.FramePadding: 4.00,0.00
style.FrameRounding: 3.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,2.00
style.ItemInnerSpacing: 4.00,4.00

Details:

My Issue/Question:

We just updated an ancient version of IMGui to 1.90.8. Some of our code relied on SetItemAllowOverlap(), so we changed it to use the new SetNextItemAllowOverlap() function instead. Our code is written in LUA, so the order of calls is:

GUI:SetNextItemAllowOverlap() GUI:Selectable(...,GUI.SelectableFlags_SpanAllColumns + GUI.SelectableFlags_AllowOverlap) GUI:SameLine() -- draw the overlapped item GUI:IsMouseClicked(0) -- this now always returns false (as do any of the other similar functions)

In the previous version, that last mouse action function worked as expected - when the cursor was positioned over the overlapped item, it would return true as expected. Now it always returns false.

Please let me know what I am missing for this to once again work as I described. Because the code snippet shown above is called by a much more complex framework that simulates tables using the Column widget (that code is drawing a single cell of a table), and it is all written in LUA, I didn't feel it would be useful at this point to try to include all that detail as well here.

Thanks for any guidance you can provide.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

No response

ocornut commented 1 week ago

In the previous version, that last mouse action function worked as expected - when the cursor was positioned over the overlapped item, it would return true as expected. Now it always returns false.

Code as posted calls IsMouseClicked(), which doesn't test cursor overlap. There's probably a mistake in your statement or repro, because IsMouseClicked(), IsMouseDown() are unrelated to item logic.

mistahmikey commented 1 week ago

I've dug a bit more into this, and the problem is occurring with single clicks on the last column in the table (created using the Columns widget.) For some reason, the Selectable doesn't appear to be passing through the click on that column to the overlapped widget. If I drop back to the previous version (which uses the obsolesced SetItemAllowOverlap), it works fine - and the ONLY code difference between that version and the new version that in the new version, SetNextItemAllowOverlap is called before the Selectable call, while in the old version, SetItemAllowOverlap is called after the selectable call. Detecting single click works on all other table columns but the last - and the code that handles drawing each column and detecting the click is the same. So not sure what to make of it.

ocornut commented 1 week ago

I think you need to provide more pseudo code or repro and a more descriptive context (eg screenshots), words are too ambiguous and your first blurb of code is clearly omitting the important stuff.

mistahmikey commented 1 week ago

Here is about as simplified a "complete" snippet as I can provide. It is much simpler, but it accurately represents the structure of how the code is called:

GUI:Columns(5, "##selected_table_column_data_columns", true)
for row = 1, 5 do
    for column = 1,5 do
        GUI:SetColumnWidth(-1,10)
        GUI:SetNextItemAllowOverlap()
        GUI:Selectable("##selectable_data_cell_"..row.."_"..column,false,GUI.SelectableFlags_SpanAllColumns|GUI.SelectableFlags_AllowOverlap) GUI:SameLine()
        GUI:Text("Data Cell")
        local isItemHovered = GUI:IsItemHovered()
        if isItemHovered and GUI:IsMouseClicked(0) then
            print("Mouse Clicked on "..row..","..column)
        end
        GUI:NextColumn()
    end
end
GUI:Columns(1)

What I am seeing is the equivalent of never seeing a "Mouse Clicked" message when I click on any cell in the last column of the table. All other cells in the table respond as expected.

ocornut commented 1 week ago

Why are you submitting an all-row-spanning Selectable() in every column? You should only submit one.

I’ll try to recreate that repro soon.

mistahmikey commented 1 week ago

Because it hard to find authoritative information about this stuff for all use cases, I just guessed that is how it was supposed to be used. So should that code look more like the following?

GUI:Columns(5, "##selected_table_column_data_columns", true)
for row = 1, 5 do
    for column = 1,5 do
        GUI:SetColumnWidth(-1,10)
        if column == 1 then
            GUI:SetNextItemAllowOverlap()
            GUI:Selectable("##selectable_data_cell_"..row.."_"..column,false,GUI.SelectableFlags_SpanAllColumns|GUI.SelectableFlags_AllowOverlap) GUI:SameLine()
        end
        GUI:Text("Data Cell")
        local isItemHovered = GUI:IsItemHovered()
        if isItemHovered and GUI:IsMouseClicked(0) then
            print("Mouse Clicked on "..row..","..column)
        end
        GUI:NextColumn()
    end
end
GUI:Columns(1)
mistahmikey commented 1 week ago

FYI, I changed my code to essentially do the above, and now, none of the cells respond to the single mouse click. They do respond to double right clicks, however (as did the previous version.)

mistahmikey commented 1 week ago

Another update - we updated to the latest (1.90.8), and the single click detection problem is still present for all columns for snippet version 2 above. Changing back to snippet version 1 (calling Selectable for each cell), and the single click detection works for all columns but the final one again. I also updated the original post to reflect the latest version.

mistahmikey commented 1 week ago

Another observation - it looks like when using a selectable, you don't need both GUI:SetNextItemAllowOverlap() and SelectableFlags_AllowOverlap - using one or the other alone results in the same behavior.

mistahmikey commented 1 week ago

I also tried changing GUI:IsItemHovered() to GUI:IsItemHovered(GUI.HoveredFlags_AllowWhenOverlappedByItem), but it didn't change the behavior for either snippet version.

ocornut commented 3 days ago

I ported your code to C++ and understood the problem: a ImGui::Text() item doesn't react to mouse events and thus doesn't claim overlap from the Selectable(). So essentially SetNextItemAllowOverlap + Selectable + followed by just a Text won't work. But if you replace Text by an active element e.g. Button() it will work.

Your code:

ImGui::Begin("#7715");
ImGui::Columns(5, "##selected_table_column_data_columns", true);
for (int row = 0; row < 5; row++)
    for (int col = 0; col < 5; col++)
    {
        //ImGui::SetColumnWidth(-1, 10);
        ImGui::PushID(row * 5 + col);
        if (col == 0)
        {
            char buf[32];
            ImGui::SetNextItemAllowOverlap();
            ImGui::Selectable("##selectable_data_cell", false, ImGuiSelectableFlags_SpanAllColumns);// | GUI.SelectableFlags_AllowOverlap)
            ImGui::SameLine(0, 0);
        }
        ImGui::Button("Data Cell");
        bool isItemHovered = ImGui::IsItemHovered();
        if (isItemHovered && ImGui::IsMouseClicked(0))
            printf("Mouse Clicked on %d %d\n", row, col);
        ImGui::PopID();
        ImGui::NextColumn();
    }
ImGui::Columns(1);
ImGui::End();

However it seems like you may want want to do hit-testing on Text element, but rather on columns here?

Likely you should be querying hovered column instead:

ImGui::Begin("#7715");
if (ImGui::BeginTable("##selected_table_column_data_columns", 5, ImGuiTableFlags_BordersV))
{
    for (int row = 0; row < 5; row++)
    {
        ImGui::TableNextRow();
        bool row_clicked = false;
        for (int col = 0; col < 5; col++)
        {
            ImGui::TableNextColumn();
            ImGui::PushID(row * 5 + col);
            if (col == 0)
            {
                ImGui::SetNextItemAllowOverlap();
                row_clicked = ImGui::Selectable("##selectable_data_cell", false, ImGuiSelectableFlags_SpanAllColumns);
                ImGui::SameLine(0, 0);
            }
            ImGui::Text("Data Cell");

            if (row_clicked && (ImGui::TableGetColumnFlags() & ImGuiTableColumnFlags_IsHovered))
                printf("Mouse Clicked on %d %d\n", row, col);
            ImGui::PopID();
        }
    }
    ImGui::EndTable();
}
ImGui::End();
ocornut commented 3 days ago

Tangential: I decided to move TableGetHoveredColumn() to public API, which is a way to obtain the hovered column in a table without polling individual columns.

So you could change

if (row_clicked && (ImGui::TableGetColumnFlags() & ImGuiTableColumnFlags_IsHovered))
    printf("Mouse Clicked on %d %d\n", row, col);

to

if (row_clicked && (ImGui::TableGetHoveredColumn() == col)
    printf("Mouse Clicked on %d %d\n", row, col);

Or do:

if (ImGui::Selectable("##selectable_data_cell", false, ImGuiSelectableFlags_SpanAllColumns))
     printf("Mouse Clicked on %d %d\n", row, ImGui::TableGetHoveredColumn());