Image border color modify the behavior of widgets

[Anthony-Verdon]

Version/Branch of Dear ImGui:

Version 1.91.1, 1.91.2, 1.91.6

Back-ends:

imgui_impl_opengl3.cpp + imgui_impl_glfw.cpp

Compiler, OS:

Windows 11 + clang

Full config/build information:

No response

Details:

Hello. I wanted to be able to re-organize assets in a window, like the example "Widgets/Drag and Drop/Drag to reorder items (simple)", but with images. I add a bug, which was that multiples swap where donne on a single frame. I search why and I think it come from the border color argument of the Image function.

first clip, you can see the bug, second you can't. The only modification I made was to remove ImGui::GetStyleColorVec4(ImGuiCol_Border) from the call of ImGui::Image()

My code is a mix of asset browser example (to draw an image over a selectable) and reorder items example (for the switch)

Screenshots/Video:

Enregistrement.de.l.ecran.2024-12-17.234603.mp4

Enregistrement.de.l.ecran.2024-12-17.234732.mp4

Minimal, Complete and Verifiable Example code:

/*
what my code does is:
loop over frame vector (a vector containing sprites, there is also a number vector containing int and which is the same size as frame vector)
create a selectable and an image at the same place
add the possibility to swap them
*/
const ImVec2 size = ImVec2(TILE_SIZE * 2.0f, TILE_SIZE * 2.0f);
const ImVec4 tint_col = ImVec4(1, 1, 1, 1);
ImGui::PushItemFlag(ImGuiItemFlags_AllowDuplicateId, true);
for (size_t i = 0; i < frames.size(); i++)
{
    ImGui::SameLine();
    
    ImVec2 uv0 = ImVec2(1.0f / frames[i].textureSize.x * frames[i].spriteCoords.x, 1.0f / frames[i].textureSize.y * frames[i].spriteCoords.y); 
    ImVec2 uv1 = ImVec2(1.0f / frames[i].textureSize.x * (frames[i].spriteCoords.x + 1), 1.0f / frames[i].textureSize.y * (frames[i].spriteCoords.y + 1));
    ImVec2 selectable_pos = ImGui::GetCursorScreenPos();

    ImGui::Selectable(std::to_string(numbers[i]).c_str(), false, ImGuiSelectableFlags_AllowOverlap, size);
    if (ImGui::IsItemActive() && !ImGui::IsItemHovered())
    {
        int i_next = i + (ImGui::GetMouseDragDelta(0).x < 0.f ? -1 : 1);
        if (i_next >= 0 && i_next < (int)frames.size())
        {
            std::swap(frames[i], frames[i_next]);
            std::swap(numbers[i], numbers[i_next]);
            ImGui::ResetMouseDragDelta();
        }
    }
    ImGui::SetCursorScreenPos(selectable_pos);
    ImGui::Image((ImTextureID)(intptr_t)RessourceManager::GetTexture(frames[i].textureName)->getID(), size, uv0, uv1, tint_col, ImGui::GetStyleColorVec4(ImGuiCol_Border)); // <-- last argument create the problem I think
}
ImGui::PopItemFlag();

[Anthony-Verdon]

sorry my code may not work, but I tried to be as clear as possible

[GamingMinds-DanielC]

When you give ImGui::Image() a border color with an alpha value > 0.0f, it will add a pixel of padding to your size for the border to be visible, and thus change the size slightly. This adds a gap and might mess up hovering calculations since your image and selectable are no longer aligned properly.

Maybe just a personal opinion, but I don't like the approach to swap items in a list (or any container for that matter) while working through the list. If you swap an item to the right, it effectively gets its selectable duplicated in this frame, making the item still active in the next slot. The flag ImGuiItemFlags_AllowDuplicateId doesn't really change any behavior except for error reporting as far as I know. Also since you make the swap before displaying the image, the number displayed and the image don't match for a frame.

The demo code does basically the same, but it is still shaky, as the code comment admits. So shaky in fact, that a slight modification like adding a gap due to a border color breaks it. My suggestion would be to try a slightly different approach. First walk through the entire list, but instead of swapping, just memorize the indices to swap and that a swap needs to be made. Then, when you are done displaying all of the items in the list, perform the swap. This should be more robust since it doesn't introduce any duplicate ids. Yes, it adds a delay of one frame for the list to display properly (there are ways to get around that if you don't mind some extra steps), but this should be no problem since your number and image didn't match for the frame anyway.

One approach without a frame delay would be to use invisible buttons in a first step for the entire list, then perform the swap and finally draw the updated list on top, using a draw list.

[Anthony-Verdon]

This adds a gap and might mess up hovering calculations since your image and selectable are no longer aligned properly.

Yeah I think it's what happens, but I find it strange since the Image isn't interactable, that's why I report it.

But like you said the way I do it isn't really great.

[GamingMinds-DanielC]

Non-interactable widgets like texts and images still have to advance the cursor based on their size, otherwise there would be lots of unwanted overlaps everywhere. Since the border is added on top of the given size, the image is bigger than the underlying selectable, advancing the cursor a bit further and introducing an unwanted gap because of that.

You could also adjust the size of the selectable to accommodate for the border, but since the border size is an implementation detail, this might unexpectedly break in the future.