ocornut / imgui

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

Sharing layout settings between multiple dockspaces #6487

Open BobbyAnguelov opened 1 year ago

BobbyAnguelov commented 1 year ago
Dear ImGui 1.89.5 (18950)
--------------------------------

sizeof(size_t): 8, sizeof(ImDrawIdx): 4, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: IMGUI_DISABLE_OBSOLETE_FUNCTIONS
define: IMGUI_DISABLE_OBSOLETE_KEYIO
define: _WIN32
define: _WIN64
define: _MSC_VER=1936
define: _MSVC_LANG=201703
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------

io.BackendPlatformName: imgui_impl_win32
io.BackendRendererName: NULL
io.ConfigFlags: 0x00000441
 NavEnableKeyboard
 DockingEnable
 ViewportsEnable
io.ConfigViewportsNoDecoration
io.ConfigViewportsNoDefaultParent
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigWindowsMoveFromTitleBarOnly
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x00001C06
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 HasMouseHoveredViewport
 RendererHasViewports
--------------------------------

io.Fonts: 8 fonts, Flags: 0x00000000, TexSize: 4096,4096
io.DisplaySize: 2379.00,1755.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------

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

My Issue/Question:

I currently have a use case where I create multiple "workspaces/tools" that have a set of windows that are restricted to that instance using the "window class". Basically think of these as child-applications within the main application, I call them workspaces. See the screenshot below where I have two "animation clip workspaces" open.

Right now, I'm generating a unique ID for each instance of a 'workspace' that I then append to all window titles. This allows me to have some degree of saving/loading layout changes via INI since the unique ID are consistent between runs. This unfortunately has the side effect of massively bloating my INI file as well as now having unique layouts per file that I open in what is conceptually the same workspace.

I'm wondering if there is some way that I can have a single shared layout that all workspaces of the same type share. And have it saved each time a workspace is closed. And then reloaded whenever that workspace is opened. I was looking at trying to leverage some of the layout loading/saving examples I've seen here, but the IDs won't match. Does anyone have a good idea on how to achieve this or something similar?

Screenshots/Video

image

GamingMinds-DanielC commented 1 year ago

If you search imgui.h for LoadIniSettingsFromDisk, you will find a block of functions that you can use to manage your settings. Resetting your context when loading a layout won't be enough though if you can have multiple workspaces open at the same time. Interfaces for loading and saving settings would solve your problem, then you could tamper with the IDs on the fly. But without that, I can only think of solutions that are pretty much overkill.

You could probably open a new ImGui context for each workspace to share settings, then render the context's content into a texture each and display these inside of windows of a governing context. Much like you seem to do with your 3d viewports now. You would have to forward inputs correctly of course. Ambitious, but should be possible. Some things like a menu extending beyond its windows would be a problem though.

ocornut commented 1 year ago

I have an example getting toward this, I'll post some details tomorrow.

BobbyAnguelov commented 1 year ago

Thanks Omar! Looking forward to your suggestion

@GamingMinds-DanielC - I explored a few options similar to this. The one that felt simplest was to have a defined format for the workspace window IDs so that I could auto-detect it and then at load time read the ini as text, modify it for the specific workspace and use those settings. It's rather messy and error prone.

One solution to solve this is to somehow allow users to specify the ID that should be used for save/load. Basically creating a parallel ID stack for loading/saving of IDs, but that would be a significant change to all ID handling so probably too big a change.

Anyways, looking forward to see your example.

ocornut commented 1 year ago

Apologies for late answer, I had random personal issues this week + exams to pass. All now cleared.

I'm wondering if there is some way that I can have a single shared layout that all workspaces of the same type share. And have it saved each time a workspace is closed. And then reloaded whenever that workspace is opened. I was looking at trying to leverage some of the layout loading/saving examples I've seen here, but the IDs won't match. Does anyone have a good idea on how to achieve this or something similar?

It's a difficult problem to design a solution, and not particularly because of dear imgui codebase/features limitations. If you allow multiple native windows then you generally allow extracting a tool out of the big workspace. As soon as you do that, it doesn't make sense that the layout is identical between both workspace as they are at different locations of your desktop.

My solution below essentially does the following, which was inspired from what Unreal does:

Comments:

I posted this while ago: https://github.com/ocornut/imgui/issues/2109#issuecomment-459409449

Attaching a slight update (will back-link from there to here): imgui_unreal_style_mdi_v53.zip

image

"What it is doing, super roughly:

BobbyAnguelov commented 1 year ago

Thanks for the detailed reply, I'll dig into this in detail over the weekend and post a follow-up.

BobbyAnguelov commented 1 year ago

So funnily enough going through that MDI example, my editor architecture was very similar so porting it over gave me exactly what I was looking for! Thanks for that!

https://github.com/ocornut/imgui/assets/10767490/01735ac8-522a-479f-bb81-b80d6e1ab947

ocornut commented 1 year ago

While I'm not ready to officially support this (with e.g. a demo branch), attaching a slight update. imgui_unreal_style_mdi_v55.zip

I made two changes:

When merging back, changed the code to prioritize the target layout (instead of overwrite target layout with dragged layout)

Essentially before it did:

MyEditor_LayoutCopy(doc->PrevDockspaceID, doc->CurrDockspaceID);

Now it is doing:

#if MYEDITOR_CONFIG_ONMERGE_OVERWRITE_WITH_SOURCE_LAYOUT
        // Always overwrite with dragged layout
        MyEditor_LayoutCopy(doc->PrevDockspaceID, doc->CurrDockspaceID);
#else
        // Priority existing layout
        if (curr_dockspace_ref_count <= 1)
            MyEditor_LayoutCopy(doc->PrevDockspaceID, doc->CurrDockspaceID);
#endif

Uncommented and fixed+simplified the code to delete old settings on merge

I think you may want to explicitly forget window and docking settings when you close a "secondary" dockspace, that is tied to a floating location. We GC dock trees on load if there are no references from window, but as we cannot eagerly throw away window settings, the dock tree will only be GC-ed if you delete window settings. So you may decide to do that manually on closure that you want to forget data for.

The idea is that our document ID is encoded inside window names, so instead of tracking all possibles windows linked to this document names, we parse the settings list and use that:

// Delete settings of old windows
// Rely on window names ending with "##XXXXXXX" to ditch their .ini settings forever..
char window_suffix[16];
ImFormatString(window_suffix, IM_ARRAYSIZE(window_suffix), "##%08X", doc->PrevDockspaceID);
size_t window_suffix_len = strlen(window_suffix);

ImGuiContext& g = *GImGui;
for (ImGuiWindowSettings* settings = g.SettingsWindows.begin(); settings != NULL; settings = g.SettingsWindows.next_chunk(settings))
{
    if (settings->ID == 0)
        continue;
    const char* window_name = settings->GetName();
    size_t window_name_len = strlen(window_name);
    if (window_name_len >= window_suffix_len && strcmp(window_name + window_name_len - window_suffix_len, window_suffix) == 0) // Compare suffix
        ImGui::ClearWindowSettings(window_name);
}

ClearWindowSettings() takes a const char and not e.g. a ImGuiWindow because it acts on both runtime windows and stored settings.

Then an open problem with is that if you crash with a multi-window document open, and your document ID allocation never or is unlikely reuse the same ID on subsequent uses, you will end up accumulating ini garbage.

I think the best solution for it is to allocate and reuse sequentially allocated numbers instead of DockspaceID, So instead of using LocationID in the dockspace ID:

    ImGuiID CalcDockspaceID() const
    {
        ImU32 s = 0;
#if MYEDITOR_CONFIG_SAME_LOCATION_SHARE_LAYOUT
        // Shared dockspace per-document type in a same tab-bar
        // - Pros: Always sync by default.
        // - Cons: Ambiguous/weird merging/re-docking.
        s = CurrLocationID;
        s = ImHashData(&Type, sizeof(Type), s);
#else
        ...
#endif 
        return s;
   }

You would add an indirection so that a small sequential number is used, and reused is available. This way, even with crashes or misclears, the data is unlikely to accumulate over time.

BobbyAnguelov commented 1 year ago

When merging back, changed the code to prioritize the target layout (instead of overwrite target layout with dragged layout)

In the final solution, this should likely be user configurable, as there really isn't a correct answer and there's arguments for both mechanism. Personally, I think the dragged one should take precedence as it is the "active/focused" window and will be the shown window when docked. Imagine you have floating workspace (X) of type 'anim', and a dock with several workspaces of several different types ( 'skeleton', 'mesh', etc...) including some of type 'anim' but with a different layout. In that dockspace, all anim workspaces are not visible and a 'mesh' workspace is active and visible. It would be jarring to to the use to have workspace X immediately change its layout when docking (just because of a hidden anim workspace). As a user I would expect it to keep its layout (and have the priority), since it is the thing, I am focused on and interacting with.

But that said, this is something that can be made user configurable trivially and so it probably should be.

The rest makes sense. Just a question about the sequential number, I'm not entirely sure how that works? Could you elaborate a bit more on the idea?

ocornut commented 8 months ago

In the final solution, this should likely be user configurable, as there really isn't a correct answer and there's arguments for both mechanism. [...] But that said, this is something that can be made user configurable trivially and so it probably should be.

My intuition is that this logic (the one I provide in the zip file above, and similarly the one you have) is unlikely make sense as a default/standardized component. Dear ImGui job would be to ensure you can trivially implement it, by providing better helper and better docs/demos, and we'll keep pushing for that within available resources, but it's possible that this decision would always be done to what user code does anyway.

The rest makes sense. Just a question about the sequential number, I'm not entirely sure how that works? Could you elaborate a bit more on the idea?

Sorry for late answer. The idea would be that instead of using unique hashes for each "location + document type" idea the code could allocate small numbers, which would facilitate reuse (after a layout is dropped you can reuse that value) and generally facilitate debugging/auditing the data. In my comment it's also a workaround to reduce the amount of possible settings garbage accumulating on crashes: if all your settings data are identified by uniquely-active-but-reused sequential numbers then settings garbage is less likely to grow even if you don't have proactive garbage collection going. That said, maybe it's a silly consideration because having proactive garbage collection is not difficult (pseudo-code in my previous code) if you can encode from document id in your window names. Also see my answer for #3468.

BobbyAnguelov commented 8 months ago

Just an update on this... Since the 1.90 update, it seems something in the docking code has change wrt to dockspace ID or windows ID. So the MDI example and my port of it are crashing now. I've kinda protected against the crashes in Esoterica but I have kinda weird behavior:

https://github.com/ocornut/imgui/assets/10767490/61c2101f-5dd4-4fc8-a1bb-da978cca7c6e

I was also failing to find a previously valid window ID.

Using the MDI example in the imgui dx11 example, I get crashes too:

image

I'm a little out of my depth trying to debug this stuff but maybe you might have any idea what changes could have resulting in this happening?

I'm this version of imgui: https://github.com/ocornut/imgui/commit/20e1caec858caa8123a6d52d410fa3f2578d3054

ocornut commented 8 months ago

So the MDI example and my port of it are crashing now. I've kinda protected against the crashes in Esoterica but I have kinda weird behavior:

Where does it crash? (asserts in provided screnshot or somewhere else?). What do you mean "I've kinda protected against the crashes" ?

Using the MDI example in the imgui dx11 example, I get crashes too:

What are the steps to get a crash with said example? I can't seem to get it to crash.

I'm getting odd behaviors indeed with today's version, such as a section disappearing until I click again. I will investigate this first and it'll probably lead me to something (probably all related to a same issue).

ocornut commented 8 months ago

Oddly enough if I backtrack to even 1.88 i get the issue.

In my example, the document contents appears to be empty because of this app-side filtering code aimed at not displaying floating tools associated to given document:

        if (editor->OptFloatingToolsOnlyVisibleWhenParentIsFocused && !is_last_focused_doc)
            if (ImGuiWindow* window = ImGui::FindWindowByName(buf))
                if (window->DockNode == NULL || ImGui::DockNodeGetRootNode(window->DockNode)->ID != dockspace_id)
                    continue;

It gets a false positive on the "Properties" "Log" etc window because DockNode==NULL. The logic is a little chicken-and-eggy dubious, somehow another change (I don't know what/where yet) is clearing DockNode for those tool window and because the filter stop submitting the windows, the field never gets updated.

In theory that logic should be doing the checks based on window->DockId. Rewriting the logic as:

        if (editor->OptFloatingToolsOnlyVisibleWhenParentIsFocused && !is_last_focused_doc)
            if (ImGuiWindow* window = ImGui::FindWindowByName(buf))
            {
                ImGuiDockNode* window_node = window->DockNode;
                if (window_node == NULL && window->DockId != 0)
                    window_node = ImGui::DockContextFindNodeByID(ImGui::GetCurrentContext(), window->DockId);
                if (window_node == NULL || ImGui::DockNodeGetRootNode(window_node)->ID != dockspace_id)
                    continue;
            }

Seemingly fixes it for it. But there's something dodgy with clearing window->DockNode in the first place.

ocornut commented 8 months ago

Could you clarify with IMGUI_VERSION_NUM you were on before, and if possible try to narrow it down slightly?

EDIT Update to the code imgui_unreal_style_mdi_v57.zip Three changes since v55:

I'd also like to confirm you that filter on even on your game viewport, since even that is not showing with your video showcase of the bug? But assuming it's a docked window with _NoTabBar and used same filter code as all "child" of a window, it would also be affected by the bug.

(I apologize that code is not on git, it's a branch on my private repo, I'm just trying to not give it too much visibility until things are working better)

BobbyAnguelov commented 8 months ago

Version

I was on 1.89.5 before, I dont recall having any issues on that version.

#define IMGUI_VERSION               "1.89.5"
#define IMGUI_VERSION_NUM           18950

Local Fix

The fix I needed to make on my end was to wrap this code:

// Perform the cloning
ImGui::DockBuilderCopyDockSpace(src_dockspace_id, dst_dockspace_id, &window_remap_pairs);
ImGui::DockBuilderFinish(dst_dockspace_id);

with a check that the source dockspace exists:

// Perform the cloning
if ( ImGui::DockContextFindNodeByID( ImGui::GetCurrentContext(), src_dockspace_id ) )
{
    ImGui::DockBuilderCopyDockSpace(src_dockspace_id, dst_dockspace_id, &window_remap_pairs);
    ImGui::DockBuilderFinish(dst_dockspace_id);
}

Note this only seems to be necessary in my integration of the code, that said it's also not consistent, i can dock undock things just fine then once in a while this happens. It's similar to the crash in the example code, where some docking operations are fine and others are not.

Crash with imgui example

  So to replicate the crash with the example code. You need to replace the show_demo_window and the "show simple window" code in main.cpp with this (i'm using the v55 version):

MyEditor_ShowDemo(&g_Editor);

Then use this uploaded ini file:

imgui.zip

and then do the following steps:

https://github.com/ocornut/imgui/assets/10767490/60b8fa20-7dfa-470a-923f-97c3076a1e05

Changes to v57

I had already made the change to use 'ImGuiFocusedFlags_DockHierarchy' on my end it seems. I will try to add the fix mentioned above in my integration.

BobbyAnguelov commented 8 months ago

Just tried your fix above and it does seem to fix the docked windows showing up as "blank" until clicked.

And yes, the main game viewport is identical to all other docked "tool workspaces/windows". I basically integrated the MDI example into my base code for editor tools.

ocornut commented 7 months ago

Update to the code imgui_unreal_style_mdi_v58.zip

Changes since v57:

   // Before Document window
    if (doc->Unsaved)
        window_flags |= ImGuiWindowFlags_UnsavedDocument;
    doc->ToolWindowsClass.FocusRouteParentWindowId = doc->WIndow->ID;        // Setup shortcut routing (#6798)
[...]
  // In Document window
    if (ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_S))
        doc->Save();
[...]
   // In tools window
        if (ImGui::SmallButton("Dummy Button"))
            doc->Unsaved = true;

If the tool window also used Shortcut(ImGuiMod_Ctrl | ImGuiKey_S) it'll get a higher score and essentially override the document one.

firesgc commented 5 months ago

Is there a dedicated thread specifically for discussing the 'imgui_unreal_style_mdi' example? I really appreciate it and I dont want to miss any changes or updates to it. I looked for it, but I havnt found anything. Thank you!

xpenatan commented 4 months ago

Update to the code imgui_unreal_style_mdi_v58.zip

Changes since v57:

  • Demonstrate using FocusRouteParentWindowId to chain tool window to their parent.
   // Before Document window
    if (doc->Unsaved)
        window_flags |= ImGuiWindowFlags_UnsavedDocument;
    doc->ToolWindowsClass.FocusRouteParentWindowId = doc->WIndow->ID;        // Setup shortcut routing (#6798)
[...]
  // In Document window
    if (ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_S))
        doc->Save();
[...]
   // In tools window
        if (ImGui::SmallButton("Dummy Button"))
            doc->Unsaved = true;

If the tool window also used Shortcut(ImGuiMod_Ctrl | ImGuiKey_S) it'll get a higher score and essentially override the document one.

Very nice.

There is an issue that when attaching window to another one with same ImGuiWindowClass can make it dock anywhere using v58. Example: Properties window from scene2 can be docked in scene1.

Is this a bug or its possible to prevent this from happening ?

https://github.com/ocornut/imgui/assets/6472084/22fd7119-083e-4686-89c6-96c79b260678

xpenatan commented 4 months ago

Update to the code imgui_unreal_style_mdi_v58.zip Changes since v57:

  • Demonstrate using FocusRouteParentWindowId to chain tool window to their parent.
   // Before Document window
    if (doc->Unsaved)
        window_flags |= ImGuiWindowFlags_UnsavedDocument;
    doc->ToolWindowsClass.FocusRouteParentWindowId = doc->WIndow->ID;        // Setup shortcut routing (#6798)
[...]
  // In Document window
    if (ImGui::Shortcut(ImGuiMod_Ctrl | ImGuiKey_S))
        doc->Save();
[...]
   // In tools window
        if (ImGui::SmallButton("Dummy Button"))
            doc->Unsaved = true;

If the tool window also used Shortcut(ImGuiMod_Ctrl | ImGuiKey_S) it'll get a higher score and essentially override the document one.

Very nice.

There is an issue that when attaching window to another one with same ImGuiWindowClass can make it dock anywhere using v58. Example: Properties window from scene2 can be docked in scene1.

Is this a bug or its possible to prevent this from happening ?

subwindow.mp4

Figured out where in code that allows split node dock anywhere.

https://github.com/ocornut/imgui/blob/085781f5ca5372d5fc804d7e44b5bf27a8994af7/imgui.cpp#L17588-L17589

    if (root_payload->DockNodeAsHost && root_payload->DockNodeAsHost->IsSplitNode()) // FIXME-DOCK: Missing filtering
        return true;

There is a fixme so I guess we need to wait for a better implementation that check if a splitnode can be docked to a window.

**EDIT

I used the idea from lines below in splitnode and it worked 😀

    if (root_payload->DockNodeAsHost && root_payload->DockNodeAsHost->IsSplitNode()) { // FIXME-DOCK: Missing filtering
        ImGuiDockNode* node1 = root_payload->DockNodeAsHost->ChildNodes[0];
        ImGuiDockNode* node2 = root_payload->DockNodeAsHost->ChildNodes[1];

        bool flag1 = false;
        bool flag2 = false;

        for (int payload_n = 0; payload_n < node1->Windows.Size; payload_n++)
        {
            ImGuiWindow* payload = node1->Windows[payload_n];
            if (DockNodeIsDropAllowedOne(payload, host_window)) {
                flag1 = true;
                break;
            }
        }

        for (int payload_n = 0; payload_n < node2->Windows.Size; payload_n++)
        {
            ImGuiWindow* payload = node2->Windows[payload_n];
            if (DockNodeIsDropAllowedOne(payload, host_window)) {
                flag2 = true;
                break;
            }
        }
         return flag1 && flag2;
    }