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

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

Open
mistahmikey opened this issue Jun 19, 2024 · 12 comments

Comments

@mistahmikey
Copy link

mistahmikey commented Jun 19, 2024

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
Copy link
Owner

ocornut commented Jun 20, 2024

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
Copy link
Author

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
Copy link
Owner

ocornut commented Jun 20, 2024

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
Copy link
Author

mistahmikey commented Jun 20, 2024

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
Copy link
Owner

ocornut commented Jun 20, 2024

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
Copy link
Author

mistahmikey commented Jun 20, 2024

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
Copy link
Author

mistahmikey commented Jun 20, 2024

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
Copy link
Author

mistahmikey commented Jun 23, 2024

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
Copy link
Author

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
Copy link
Author

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

@ocornut
Copy link
Owner

ocornut commented Jun 27, 2024

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
Copy link
Owner

ocornut commented Jun 27, 2024

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());

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

No branches or pull requests

2 participants