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

Assert in BeginDragDropSource when dragging window to bottom of screen #4515

Closed
aclysma opened this issue Sep 6, 2021 · 5 comments
Closed
Labels
bug drag drop drag and drop

Comments

@aclysma
Copy link

aclysma commented Sep 6, 2021

Version/Branch of Dear ImGui:

Version: 1.84.1 (58f5092)
Branch: docking

Back-end/Renderer/Compiler/OS

Back-ends: SDL/OpenGL3 (Modified the example_sdl_opengl3 example)
Operating System: MacOS

My Issue/Question:

I am getting this fatal assert: Assertion failed: (0), function BeginDragDropSource, file ../../imgui.cpp, line 10561.

Steps:

  1. Modify example_sdl_opengl3
    a. Removed io.ConfigFlags |= ImGuiConfigFlags_ViewportsEnable;
    b. Removed the existing gui (everything between ImGui::NewFrame(); and ImGui::Render();)
    c. Added the below code and called it between ImGui::NewFrame(); and ImGui::Render();
  2. Drag the window past the bottom of the screen

I'm not trying to use multiple viewports/create a window outside the window (I disabled ImGuiConfigFlags_ViewportsEnable). It just happens that if you drag a floating window down there with the below code, it will fatal assert.

Screenshots/Video

imgui_fatal_assert

Standalone, minimal, complete and verifiable example: (see #2261)

void test_app() {
    if (ImGui::Begin("ExtraWindow")) {

        // Does not occur without this BeginChild/EndChild call
        ImGui::BeginChild("##Test", ImGui::GetContentRegionAvail(), false);

        // Does not occur if loop count <= 1
        for (int i = 0; i < 2; ++i) {
            // Does not occur without the BeginGroup/EndGroup call
            ImGui::PushID(1 + i);
            ImGui::BeginGroup();

            ImGui::Button("item");
            if (ImGui::BeginDragDropSource()) {
                int x = 0;
                ImGui::SetDragDropPayload("ITEM_TYPE", &x, sizeof(int), 0);
                ImGui::EndDragDropSource();
            }

            ImGui::EndGroup();
            ImGui::PopID();
        }

        ImGui::EndChild();
    }

    ImGui::End();
}

Other Notes

The comments around the assert are here. I read the comments and the code below it. My take from reading it was that as long as the widget has a unique ID, I should be fine. But I'm not certain I understand what's being said.

// If you want to use BeginDragDropSource() on an item with no unique identifier for interaction, such as Text() or Image(), you need to:
// A) Read the explanation below, B) Use the ImGuiDragDropFlags_SourceAllowNullID flag, C) Swallow your programmer pride.
if (!(flags & ImGuiDragDropFlags_SourceAllowNullID))
{
    IM_ASSERT(0);
    return false;
}

I also tried changing the button to:

char id[128];
sprintf(id, "item##%d", i);
ImGui::Button(id);

But I was expecting by pushing the ID, that this would not be necessary. (But really not sure)

Adding the flag to the call (i.e. ImGui::BeginDragDropSource(ImGuiDragDropFlags_SourceAllowNullID)) skips the assert, but I'm not sure what the downside of this is. I thought, especially with the change to sprintf a unique ID, that I would not need this.

Paste from About ImGui

Dear ImGui 1.84.1 (18403)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: __APPLE__
define: __GNUC__=4
define: __clang_version__=12.0.0 (clang-1200.0.32.29)
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: imgui_impl_sdl
io.BackendRendererName: imgui_impl_opengl3
io.ConfigFlags: 0x00000041
 NavEnableKeyboard
 DockingEnable
io.ConfigViewportsNoDecoration
io.ConfigMacOSXBehaviors
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000140E
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 RendererHasVtxOffset
 RendererHasViewports
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 1280.00,720.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00
@ocornut
Copy link
Owner

ocornut commented Sep 6, 2021

Interesting, the issue is because the child window is entirely out of view (BeginChild() returned false): all items are now coarse clipped early on so e.g. the Button() is not submitted and LastItemID is always zero.

The workaround/fix is to enclose your child contents submission under a if (BeginChild()) { } block (also generally better).

As per specs we should fix it tho.

@ocornut ocornut added bug drag drop drag and drop labels Sep 6, 2021
@ocornut
Copy link
Owner

ocornut commented Sep 6, 2021

The assert is misleading here: it is meant for the actual case of using items without ID as a drag source, e.g. a Text() element. In this case because of the clipping the assert erroneously triggers.

Fix coming soon.

ocornut added a commit that referenced this issue Sep 6, 2021
…inChild() that returned false. (#4515) + BeginDragDropTarget()

Note how 79ae6d3 adedd a SkipItems test in BeginDragDropTargetCustom() only.
Catching this similar to work needed to neatly represent the error in #4375 #4158, #4008, #2562
@ocornut
Copy link
Owner

ocornut commented Sep 6, 2021

Pushed a fix now! Thanks for the report and careful repro (we'll be adding some regression tests based on it).

A similar issue would manifest with either BeginDragDropSource() or BeginDragDropTarget() in docking branch when submitting items in a Begin() returning false (by dragging e.g. over the tab for that window).

@ocornut ocornut closed this as completed Sep 6, 2021
@aclysma
Copy link
Author

aclysma commented Sep 6, 2021

Thanks! BTW, do you think BeginDragDropTargetCustom should get the same treatment as BeginDragDropTarget? I have no idea, just noticed the code is very similar.

@ocornut
Copy link
Owner

ocornut commented Sep 7, 2021

do you think BeginDragDropTargetCustom should get the same treatment as BeginDragDropTarget

It already had that check in.

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

No branches or pull requests

2 participants