-
-
Notifications
You must be signed in to change notification settings - Fork 300
Enable Docking #1046
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
base: master
Are you sure you want to change the base?
Enable Docking #1046
Conversation
|
If this is a draft mark it as one. Otherwise may have a look later (unsure if today but should be able to tomorrow). |
|
Saying that as you seem to be pushing still unsure if this is final |
|
yeah it's done now, already marked it as a non draft. |
src/d3d12/D3D12.cpp
Outdated
|
|
||
| if (d3d12.m_trapInputInImGui) // TODO: look into io.WantCaptureMouse and io.WantCaptureKeyboard | ||
| { | ||
| ImGui_ImplWin32_WndProcHandler(ahWnd, auMsg, awParam, alParam); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why move this down here and ignore the result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The not handling of the result is an oversight on my part, will fix that.
Initially I moved it here in this pr because I noticed you can drag windows around that are always drawn even when the overlay is closed which I assumed was due to the Dockingspace over viewport but after now testing it again it turns out it was an issue even before that.
Moving it to only handle input then the overlay is open also fixes the issue of the os cursor not hiding, which is the reason the NoMouseCursorChange was needed in the first place.
If you prefer I can move this (and the flag removal) out to a separate pr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah is good likely but also sth I want to overall check with the rest soon tm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing that isnt good though is the return, that has to happen to block input passthrough to the game. Some change that we did in imgui_impl_win32 likely causes some issues due to which you had to remove the return.
But yeah have to overall check as mentioned
| ImGui::GetStyle().ScaleAllSizes(scaleFromReference); | ||
|
|
||
| ImGui::GetIO().ConfigFlags |= ImGuiConfigFlags_NoMouseCursorChange; // Do not modify cursor from ImGui backend | ||
| ImGui::GetIO().ConfigFlags |= ImGuiConfigFlags_DockingEnable; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why remove the other flag? Does it prevent docking in some way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it should solve the cursor not changing based on if you are over the text, selecting or draging n such but unsure. Also unsure why it was even added now overall but that is one of the things I wanted to check later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave this open for now, would likely also open a thread
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the change in when imgui can handle the input it is no longer needed as it was used to prevent the os cursor from being visible when the overlay is closed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and yeah removing that flag restores the cursor changes by imgui with e.g. the text cursor or the drag/arrow cursor when resizing etc.
| if (!m_initialized) | ||
| return; | ||
|
|
||
| ImGui::DockSpaceOverViewport(0, ImGui::GetMainViewport(), ImGuiDockNodeFlags_PassthruCentralNode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expose related docking functions to Lua also please, and update the sol_ImGui readme. Also, this will likely allow toolbar and few other things to get docked as it is now which I dont really like personally. It would be better if the toolbar was docked by default in some way and prevent manipulation, sth I wanted to do with the docking (and reason it was not added still kinda, wanted to do it more properly or how to call it).
Toolbar shouldnt be a floating window with the docking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the toolbar it should be a main menu bar then which is always at the top of the viewport, and that is possible regardless of docking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah also had that idea overall, but just the menu bar didnt quite looked right. Think it would be better to split the main viewport into 2 docking zones and just dock the toolbar into the top zone. This requires a bit more of a setup to split the docking zones and also make the top docking zone fixed, along with programatically docking the toolbar into the top zone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are unsure how to do it may dig up piece of code from the legendary 2.0 had this working (and imo it looked just better than a simple menu)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by docking related methods are you referring to the DockBuider API?
Not sure what a reasonable approach would be for implementing that, might need it's own event since it seems to need to be done before the main docking space draws from just looking at the example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a visual example of what kind of menu you had in mind would be helpful too, pretty sure the main menu bar can be heavily customized too and is by default excluded from docking, immovable etc. so it might be better to use it rather than a manual window.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, possible, hadnt truly tried to customize it. Like if you have some idea feel free to try to make sth and post some screen how it looks.
And yes, youd want to make a new event onDockingSetup or something like that and call it once after ImGui initialization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that "once" may be multiple times, you want to reset the flag when the ImGui is reinitialized during reset of state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I thought now - this may overall baloon a bit with these additions, we may do it separately (Lua API).
But Id like to resolve the toolbar at least with this.
Enable Docking
Added:
Changed:
Removed: