Closed dcnieho closed 1 year ago
Ah, rethinking this, the crash makes sense: hello_imgui is iterating a bunch of docking windows, and i just changed that vector of windows while iterating. That's no good ;)
But when then can i change it?
Hi
there is a callback named PreNewFrame in the hello imgui params. I suspect it might help.
please keep me informed whether it helps
Another solution would be to change the loop inside abstractRunner so that it stores a list of pointers before looping, iterates on this list, and thus can resist to a change of the content. May be that would be less surprising for users.
Edit: this would not work, as dockingWindows may be changed when presented.
the PreNewFrame callback does the trick for this, nice! A simple callback like this works well:
def _update_windows(self):
if self._window_list:
hello_imgui.get_runner_params().docking_params.dockable_windows = self._window_list
self._window_list = []
where i fill self._window_list when i need to change the windows, instead of directly manipulating the list of windows as in the above crashing code
One problem though is that a window i create like this is not docked as indicated (temp.dock_space_name = "MainDockSpace"
), but just floating on the interface ((I can drag and doc it myself). I am not sure if i can fix this myself using imgui.internal.dock_builder_dock_window
somewhere in my code, how would i get the dock node id that needs? And i think i can't just call it anywhere either.
Furthermore, I would like to be able to set the ImGuiDockNodeFlags
of a dock space (to be able to hide the tab bar using ImGuiDockNodeFlags_AutoHideTabBar
or ImGuiDockNodeFlags_NoTabBar
) for one of my dock spaces. Should that be part of hello_imgui.DockingSplit()
as a field like new_dock_flags
or something like that? I may also want to use ImGuiDockNodeFlags_NoSplit
and ImGuiDockNodeFlags_NoResize
and some stuff from ImGuiDockNodeFlagsPrivate_
, not sure yet what will work best for this app.
So far i am loving hello_imgui for the more complex app i'm building now (and i don't expect that to change), nice work man! Really a good framework to make it easy to use even with more advanced layouts, etc.
One problem though is that a window i create like this is not docked as indicated (temp.dock_space_name = "MainDockSpace"), but just floating on the interface
The docking layout may be modified by the user, so that it is applied only at the start, or when the user clicks the menu "View/Restore Layout". This menu item only sets dockingParams.layoutReset
to true.
You can get the same effect with:
hello_imgui.get_runner_params().docking_params.layout_reset = True
However, this will reset any previous modifications to the layout that the user may have applied.
If we want to add the window and keep the user modifications, this would require a deep dive in the ImGui docking api which is not that easy to navigate. I'm not sure it is easily feasible, since I did not yet find an easy way to list the available splits in ImGui's context itself (see https://github.com/ocornut/imgui/blob/f5c5a710aa38b91be39b1c219f3bb90e2e15df80/imgui_internal.h#L1696-L1703)
Furthermore, I would like to be able to set the ImGuiDockNodeFlags of a dock space (to be able to hide the tab bar using ImGuiDockNodeFlags_AutoHideTabBar or ImGuiDockNodeFlags_NoTabBar) for one of my dock spaces. Should that be part of hello_imgui.DockingSplit() as a field like new_dock_flags or something like that? I may also want to use ImGuiDockNodeFlags_NoSplit and ImGuiDockNodeFlagsNoResize and some stuff from ImGuiDockNodeFlagsPrivate, not sure yet what will work best for this app.
Hum, this is getting quite advanced. Could you do some tests on your side and tell me if it helps? It's easier to make tests in C++ first (may be by hacking docking_details.cpp first)
So far i am loving hello_imgui for the more complex app i'm building now (and i don't expect that to change), nice work man! Really a good framework to make it easy to use even with more advanced layouts, etc.
Thanks!
Hmm, that restore layout wouldn't work indeed for that reason. The hiding the tab bar was easy-ish:
diff --git a/bindings/imgui_bundle/demos_cpp/demos_immapp/demo_docking.cpp b/bindings/imgui_bundle/demos_cpp/demos_immapp/demo_docking.cpp
index 97d1250..95fe0e1 100644
--- a/bindings/imgui_bundle/demos_cpp/demos_immapp/demo_docking.cpp
+++ b/bindings/imgui_bundle/demos_cpp/demos_immapp/demo_docking.cpp
@@ -13,6 +13,7 @@ It demonstrates:
#include "hello_imgui/hello_imgui.h"
#include "hello_imgui/icons_font_awesome.h"
#include "imgui.h"
+#include "imgui_internal.h"
#include "imgui_md_wrapper/imgui_md_wrapper.h"
#include "immapp/immapp.h"
#include "immapp/clock.h"
@@ -237,6 +238,7 @@ int main(int, char**)
splitMainLeft.newDock = "LeftSpace";
splitMainLeft.direction = ImGuiDir_Left;
splitMainLeft.ratio = 0.25f;
+ splitMainLeft.nodeFlags = ImGuiDockNodeFlags_NoTabBar;
// Finally, transmit these splits to HelloImGui
runnerParams.dockingParams.dockingSplits = {splitMainBottom, splitMainLeft};
diff --git a/src/hello_imgui/docking_params.h b/src/hello_imgui/docking_params.h
index a1652ae..06b7add 100644
--- a/src/hello_imgui/docking_params.h
+++ b/src/hello_imgui/docking_params.h
@@ -100,13 +100,15 @@ Direction where this dock space should be created
struct DockingSplit
{
DockingSplit(const DockSpaceName& initialDock_ = "", const DockSpaceName& newDock_ = "",
- ImGuiDir_ direction_ = ImGuiDir_Down, float ratio_ = 0.25f)
- : initialDock(initialDock_), newDock(newDock_), direction(direction_), ratio(ratio_) {}
+ ImGuiDir_ direction_ = ImGuiDir_Down, float ratio_ = 0.25f,
+ ImGuiDockNodeFlags nodeFlags_ = ImGuiDockNodeFlags_None)
+ : initialDock(initialDock_), newDock(newDock_), direction(direction_), ratio(ratio_), nodeFlags(nodeFlags_) {}
DockSpaceName initialDock;
DockSpaceName newDock;
ImGuiDir_ direction;
float ratio = 0.25f;
+ ImGuiDockNodeFlags nodeFlags = ImGuiDockNodeFlags_None;
};
/**
diff --git a/src/hello_imgui/internal/docking_details.cpp b/src/hello_imgui/internal/docking_details.cpp
index 886c42c..b57b8e5 100644
--- a/src/hello_imgui/internal/docking_details.cpp
+++ b/src/hello_imgui/internal/docking_details.cpp
@@ -31,6 +31,10 @@ void DoSplit(const DockingSplit & dockingSplit)
gImGuiSplitIDs[dockingSplit.initialDock] = initalDock_imguiId;
gImGuiSplitIDs[dockingSplit.newDock] = newDock_imguiId;
+
+ // apply flags
+ ImGuiDockNode* newDockNode = ImGui::DockBuilderGetNode(newDock_imguiId);
+ newDockNode->SetLocalFlags(newDockNode->LocalFlags | dockingSplit.nodeFlags);
}
void ApplyDockingSplits(const std::vector<DockingSplit>& dockingSplits)
Would something like that be acceptable.
I try the other problem, which i could rephrase as forcing a specific window to be docked to a specific dockspace (so basically add an API to call DockBuilderDockWindow
on a specific window)
Programatically making a specific window dock programatically was not straightforward, because the case of a newly created window has to be handled separately (do the docking only on the next frame). My solution isn't pretty (ideally isNewWindow
should not be user-facing i guess), but works both to directly dock a new window, and to programmatically dock an already shown window:
diff --git a/src/hello_imgui/docking_params.h b/src/hello_imgui/docking_params.h
index a1652ae..2c1a551 100644
--- a/src/hello_imgui/docking_params.h
+++ b/src/hello_imgui/docking_params.h
@@ -159,6 +161,8 @@ struct DockableWindow
bool canBeClosed = true;
bool callBeginEnd = true;
bool includeInViewMenu = true;
+ bool shouldDock = false;
+ bool isNewWindow = false;
ImGuiWindowFlags imGuiWindowFlags = 0;
ImVec2 windowSize = ImVec2(0.f, 0.f);
diff --git a/src/hello_imgui/internal/docking_details.cpp b/src/hello_imgui/internal/docking_details.cpp
index 886c42c..c9a0738 100644
--- a/src/hello_imgui/internal/docking_details.cpp
+++ b/src/hello_imgui/internal/docking_details.cpp
@@ -39,14 +43,25 @@ void ApplyDockingSplits(const std::vector<DockingSplit>& dockingSplits)
DoSplit(dockingSplit);
}
-void ApplyWindowDockingLocations(
- const std::vector<DockableWindow> & dockableWindows)
+void DoDock(DockableWindow & dockableWindow)
{
- for (const auto & dockableWindow: dockableWindows)
+ if (!dockableWindow.isNewWindow)
+ {
ImGui::DockBuilderDockWindow(
dockableWindow.label.c_str(),
gImGuiSplitIDs[dockableWindow.dockSpaceName]
);
+
+ dockableWindow.shouldDock = false;
+ }
+ dockableWindow.isNewWindow = false;
+}
+
+void ApplyWindowDockingLocations(
+ std::vector<DockableWindow> & dockableWindows)
+{
+ for (auto& dockableWindow : dockableWindows)
+ DoDock(dockableWindow);
}
void MenuView_DockableWindows(RunnerParams& runnerParams)
@@ -223,6 +238,12 @@ void ApplyDockLayout(DockingParams& dockingParams)
ApplyWindowDockingLocations(dockingParams.dockableWindows);
dockingParams.layoutReset = false;
}
+
+ for (auto& dockableWindow : dockingParams.dockableWindows)
+ {
+ if (dockableWindow.shouldDock)
+ DoDock(dockableWindow);
+ }
}
void ProvideWindowOrDock(const ImGuiWindowParams& imGuiWindowParams, DockingParams &dockingParams)
{
And for the demo:
diff --git a/bindings/imgui_bundle/demos_cpp/demos_immapp/demo_docking.cpp b/bindings/imgui_bundle/demos_cpp/demos_immapp/demo_docking.cpp
index 97d1250..ee87ed0 100644
--- a/bindings/imgui_bundle/demos_cpp/demos_immapp/demo_docking.cpp
+++ b/bindings/imgui_bundle/demos_cpp/demos_immapp/demo_docking.cpp
@@ -30,6 +31,8 @@ struct AppState {
Launched
};
RocketState rocket_state = RocketState::Init;
+
+ std::vector<HelloImGui::DockableWindow> winVec;
};
@@ -55,6 +58,15 @@ void DemoHideWindow()
}
}
+void ExtraWindow()
+{
+ ImGui::Text("simple");
+ if (ImGui::Button("Dock window"))
+ {
+ HelloImGui::GetRunnerParams()->dockingParams.dockableWindows.back().shouldDock = true;
+ }
+}
+
void CommandGui(AppState& state)
{
@@ -119,6 +131,19 @@ void CommandGui(AppState& state)
You can even easily tweak their colors.
)");
+ if (ImGui::Button("Add window"))
+ {
+ HelloImGui::DockableWindow temp;
+ temp.label = "Testy";
+ temp.dockSpaceName = "MainDockSpace";
+ temp.GuiFunction = [] { ExtraWindow(); };
+ temp.shouldDock = true;
+ temp.isNewWindow = true;
+
+ state.winVec = HelloImGui::GetRunnerParams()->dockingParams.dockableWindows;
+ state.winVec.push_back(temp);
+ }
+
}
@@ -132,6 +157,15 @@ void StatusBarGui(AppState& app_state)
}
}
+void UpdateWindows(AppState& app_state)
+{
+ if (!app_state.winVec.empty())
+ {
+ HelloImGui::GetRunnerParams()->dockingParams.dockableWindows = app_state.winVec;
+ app_state.winVec.clear();
+ }
+}
+
int main(int, char**)
{
@@ -163,6 +197,7 @@ int main(int, char**)
// uncomment next line in order to hide the FPS in the status bar
// runnerParams.imGuiWindowParams.showStatusFps = false;
runnerParams.callbacks.ShowStatus = [&appState]() { StatusBarGui(appState); };
+ runnerParams.callbacks.PreNewFrame= [&appState]() { UpdateWindows(appState); };
//
// Menu bar
@@ -237,6 +272,7 @@ int main(int, char**)
splitMainLeft.newDock = "LeftSpace";
splitMainLeft.direction = ImGuiDir_Left;
splitMainLeft.ratio = 0.25f;
// Finally, transmit these splits to HelloImGui
runnerParams.dockingParams.dockingSplits = {splitMainBottom, splitMainLeft};
@@ -261,12 +297,17 @@ int main(int, char**)
dearImGuiDemoWindow.label = "Dear ImGui Demo";
dearImGuiDemoWindow.dockSpaceName = "MainDockSpace";
dearImGuiDemoWindow.GuiFunction = [] { ImGui::ShowDemoWindow(); };
+ HelloImGui::DockableWindow dearImGuiDemoWindow2;
+ dearImGuiDemoWindow2.label = "Dear ImGui Demo 2";
+ dearImGuiDemoWindow2.dockSpaceName = "MainDockSpace";
+ dearImGuiDemoWindow2.GuiFunction = [] { ImGui::Text("yo test"); };
// Finally, transmit these windows to HelloImGui
runnerParams.dockingParams.dockableWindows = {
commandsWindow,
logsWindow,
dearImGuiDemoWindow,
+ dearImGuiDemoWindow2,
};
//###############################################################################################
Hi,
I just saw your propositions.
The one concerning which adds nodeFlags seems ok to me (although it should document this new field in the doc at the top of the struct). Thanks!
I would like to try the other one. Is there a branch where I can checkout your code?
Hello again,
I applied the patch concerning shouldDock
.
There is a probable issue with it. Let me explain:
First, I changed this after your patch: I added an assert to check that gImGuiSplitIDs contains the dockableWindow.dockSpaceName (the code accessed it via the dreaded std::map[] operator which silently adds a value when you think your are querying it in read only mode...)
docking_details.cpp: line 42
void DoDock(DockableWindow & dockableWindow)
{
if (!dockableWindow.isNewWindow)
{
IM_ASSERT(gImGuiSplitIDs.find(dockableWindow.dockSpaceName) != gImGuiSplitIDs.end());
ImGui::DockBuilderDockWindow(
dockableWindow.label.c_str(),
gImGuiSplitIDs.at(dockableWindow.dockSpaceName)
);
dockableWindow.shouldDock = false;
}
dockableWindow.isNewWindow = false;
}
Then in the demo I selected "BottomSpace" to make sure we do not use the first dock space.
demo_docking.cpp: line 131
if (ImGui::Button("Add window"))
{
HelloImGui::DockableWindow temp;
temp.label = "Testy";
temp.dockSpaceName = "BottomSpace";
temp.GuiFunction = [] { ExtraWindow(); };
temp.shouldDock = false;
temp.isNewWindow = true;
state.winVec = HelloImGui::GetRunnerParams()->dockingParams.dockableWindows;
state.winVec.push_back(temp);
}
This will work if you start the app, click on "Add Window", then click on "Dock window". However, you can trigger a bug like this:
=> the assertion IM_ASSERT(gImGuiSplitIDs.find(dockableWindow.dockSpaceName) != gImGuiSplitIDs.end());
will be triggered
This is due to the fact that DockingParams.layoutReset
is false by default. In that case, the docking layout is not reapplied upon starting the application (since we want to keep the user setting). In that case, the layout is restored by ImGui.
However, I did not find an easy way to list the available splits via ImGui. `
Below is the content of Docking_demo.ini
(i.e. the imgui.ini file for this app):
[Window][MainDockSpace]
Pos=0,0
Size=1000,771
Collapsed=0
[Window][Commands]
Pos=0,21
Size=249,561
Collapsed=0
DockId=0x00000003,0
[Window][Logs]
Pos=0,584
Size=1000,187
Collapsed=0
DockId=0x00000002,0
[Window][Dear ImGui Demo]
Pos=251,21
Size=749,561
Collapsed=0
DockId=0x00000004,0
[Window][Dear ImGui Demo 2]
Pos=251,21
Size=749,561
Collapsed=0
DockId=0x00000004,1
[Window][Debug##Default]
Pos=60,60
Size=400,400
Collapsed=0
[Window][StatusBar]
Pos=0,770
Size=1000,32
Collapsed=0
[Window][Testy]
Pos=60,60
Size=102,77
Collapsed=0
[Docking][Data]
DockSpace ID=0xC3C5E3D9 Window=0xDEDC5B90 Pos=364,150 Size=1000,750 Split=Y
DockNode ID=0x00000001 Parent=0xC3C5E3D9 SizeRef=1000,561 Split=X
DockNode ID=0x00000003 Parent=0x00000001 SizeRef=249,561 Selected=0x346CB028
DockNode ID=0x00000004 Parent=0x00000001 SizeRef=749,561 CentralNode=1 Selected=0xC3642376
DockNode ID=0x00000002 Parent=0xC3C5E3D9 SizeRef=1000,187 Selected=0x65EF497E
Note: the docking splits name is an artifact added by HelloImGui. ImGui does not know about it, and only deals with IDs.
Note: the docking splits name is an artifact added by HelloImGui. ImGui does not know about it, and only deals with IDs.
And so, I begin to think HelloImGui might need to store the correspondence between the name of the DockSplit and its ID in its own ini file.
I'll try to study that tomorrow.
@pthom: I'll send a pull request for the node flags.
Hmm, that indeed makes the other patch harder to use. So the problem is that your map is empty when loading from ini (makes sense you don't re-apply, its already applied), correct? I guess even if you store your own docksplit<->ID correspondence, you have an issue because you wont know when users manually move around windows or make new splits or whatever, right?
An alternative: should we be thinking in terms of splits or dock spaces/nodes? Its still hard for me to follow, but the latter is listed in that ini file... It does seem you are using the correct api for all this.
I'll have a look as well into the API regarding splits, but i find the API hard to follow, so lets not get our hopes up. If i'm wrong above and you can track user changes, then storing your own correspondence would make sense. Or parse the imgui.ini on startup, if there is one.
So the problem is that your map is empty when loading from ini (makes sense you don't re-apply, its already applied), correct?
Correct
I guess even if you store your own docksplit<->ID correspondence, you have an issue because you wont know when users manually move around windows or make new splits or whatever, right?
HelloImGui will not know about newly created splits. It will only know the Ids of the splits created by code. However, there is a good chance that those IDs will remain.
An alternative: should we be thinking in terms of splits or dock spaces/nodes? Its still hard for me to follow, but the latter is listed in that ini file...
The imgui docking API is indeed hard to follow; and this is why I provided an alternative simpler API in HelloImGui. What HelloImGui calls a split corresponds to a DockSpace Node, and DockingSplit enables to create the same kind of tree like structure; except that we use names for the splits, which makes it more user friendly.
It does seem you are using the correct api for all this.
I think so. The docking API may have evolved since the time I wrote this (in 2020).
If i'm wrong above and you can track user changes, then storing your own correspondence would make sense.
I cannot track user changes. However, I tested that calling ImGui::DockBuilderDockWindow
with a non existing split Id does not cause the program to crash (it simply does not dock the window).
Or parse the imgui.ini on startup, if there is one.
imgui.ini will not contains the split/node names, so it would not help that much.
I do some basic parsing inbool AbstractRunner::HasUserDockingSettingsIniIniFile()
(check that dockable windows seem to be already docked)
Hmm, I've been thinking about this. Indeed, the only thing you can do is store your own name/split to ID mapping. Then at least programatically you can add things to splits you know should exist. It would be nice if it is possible to also query which windows are attached to a certain named split, and whether the split still exists. Does that look possible?
If the user themselves creates more splits, I do not know what one as a programmer against hello_imgui would want to do with those.
Though I just noted what the docs say: - Do not hold on ImGuiDockNode* pointers! They may be invalidated by any split/merge/remove operation and every frame.
Hi,
I do not have much time this week-end.
It would be nice if it is possible to also query which windows are attached to a certain named split, and whether the split still exists. Does that look possible?
It is certainly possible, since it is exported in the ini file. However this will require some reverse engineering.
If you open the imgui online manual https://pthom.github.io/imgui_manual_online/manual/imgui_manual.html, you can see some debug tools:
They seem to be provided by ImGui::ShowMetricsWindow(bool* p_open)
I'm starting to think my use case is too complex/niche to befoul your nice simple framework that is sufficient for most people with ;) Shall we turn this into a discussion? What you have now does the trick for most simple apps i think. Just those who have more windows appear when buttons are pressed may need this, but then they probably want to lock down a lot of the docking functionality anyway so that users can't create more splits or remove some (for instance)--i know i do. And then its workable, though again only when a clean interface is presented (which is something i'll also do, reapply on each opening of the app) because else you have a problem with your cache as we have established.. an unsolvable problem (with a reasonable amount of effort) it seems.
I have solved my specific problem in a cheap way:
runner_params.docking_params.layout_condition = hello_imgui.DockingLayoutCondition.application_start
, so i know my initial docking is always appliedmain_dock_node_id = imgui.get_window_dock_id()
runner_params.callbacks.pre_new_frame
, I also dock them: imgui.internal.dock_builder_dock_window(window_name, self._main_dock_node_id)
This could be made easier if you would make the dock node ids resulting from a split accessible through the API (read only!) but no biggie
Hi,
Sorry for not answering sooner, I was on holiday. Today, I worked a few hours on this, and I decided to postpone the work on this, since there are quite a few corner cases with the caching, and it does prove to be cumbersome for a small benefit.
So, I think we should may be transfer this as a discussion (I will do it next week).
Anyhow I added a method dockSpaceIdFromName in DockingParams:
optional<ImGuiID> dockSpaceIdFromName(const std::string& dockSpaceName)
: may return the ImGuiID corresponding to the dockspace with this name.
Warning: this will work reliably only if layoutCondition = DockingLayoutCondition::ApplicationStart. In other
cases, the ID may be cached by ImGui himself at the first run, and HelloImGui will not know it on subsequent runs!See commit in HellImGui (https://github.com/pthom/hello_imgui/commit/cac16c5dbe78bd0bea8430411126529faa95eb6d ) and in ImGuiBundle
Super, thanks! Man, all the integrated libraries is making it super easy for me to develop the GUI around my application. So happy with this for me very timely initiative. And its cool that it supports more complicated applications than quick tools as well.
I would like to add a window to a dockspace programmatically. However, trying to do so crashes. I take the source of
demo_docking.py
and add the following to the bottom ofcommand_gui()
:The crash i then get is:
Is this the correct way to add more dockable windows to a program? And will they correctly be docked to the indicated dockspace, not free floating?