ImGui 1.92.+ support#251
Conversation
|
The main problem was with the MacOS - did you have a chance to test it? |
Not yet. I'm new to Mac development but I'll try to test it. |
|
Ok, I got it working on MacOS. Tested in XCode with OpenGL and Vulkan (via MoltenVK) backends and everything looks good. The problem was that the function ImGui_ImplOSX_HandleEvent(NSEvent*, NSView*) has been made private in newer imgui versions. From what I've seen in the source, the backend registers a custom input monitor and is supposed to handle events on its own. But this didn't work for me. Something in Diligent native/sample app osx code is consuming the events, or preventing them from reaching the internal imgui monitor. I tried to figure it out, forward the events to imgui somehow, but I gave up and I simply copied the internal function over so everything works like before. I hope this is acceptable. After all this backend should not be used in production - there's a comment at the top of the file "Not well tested. If you want a portable application, prefer using the GLFW or SDL platform Backends on Mac.". But for running Diligent samples it should be fine. I also got rid of the extra imgui_v1.85 target/thirdparty folder that for some reason was used to host the old osx imgui backend files. |
|
@mctb32 Thanks for submitting this change and sorry for delay. This unfortunately does not fully work on MacOS. While rendering is OK, the app crashes when handling the input. For example, clicking on input field reproduces the issue: Screen.Recording.2025-07-24.at.7.57.21.PM.movThe problem is that Diligent examples use separate threads for rendering and input handling, while ImGui only supports single-threaded. |
|
@TheMostDiligent Yes, I can see this happening for me too. Thanks for the info and sorry I missed that. |
|
Thank you! |
| // ---------------------------------------------------------------- | ||
| // 1) CREATE | ||
| // ---------------------------------------------------------------- | ||
| if (tex->Status == ImTextureStatus_WantCreate) |
There was a problem hiding this comment.
if - else if - else if will look a bit cleaner than blocks if multiple returns
|
|
||
| ITexture* pTexture = nullptr; | ||
| m_pDevice->CreateTexture(desc, &init, &pTexture); | ||
| pTexture->AddRef(); |
There was a problem hiding this comment.
This seems to be redundant. CreateTexture already returns a texture with one reference, this adds another one. So after DestroyTexture releases one reference, another still remains.
| ITexture* pTexture = nullptr; | ||
| m_pDevice->CreateTexture(desc, &init, &pTexture); | ||
| pTexture->AddRef(); | ||
| ITextureView* ptexView = pTexture->GetDefaultView(TEXTURE_VIEW_SHADER_RESOURCE); |
There was a problem hiding this comment.
There is no need to AddRef the texture view. The texture is kept in the BackendUserData, so all its views will be kept alive until it is released.
| { | ||
| Box dstBox{Uint32(tex->UpdateRect.x), Uint32(tex->UpdateRect.x + tex->UpdateRect.w), | ||
| Uint32(tex->UpdateRect.y), Uint32(tex->UpdateRect.y + tex->UpdateRect.h), | ||
| 0, 1}; // Z range |
There was a problem hiding this comment.
Z range may be omitted - it is set to 0..1 by default.
| if (ITextureView* pTextureView = reinterpret_cast<ITextureView*>(tex->GetTexID())) | ||
| { | ||
| pTextureView->Release(); | ||
| } |
There was a problem hiding this comment.
No need to keep the texture view reference.
| if (pDrawData->Textures != nullptr) | ||
| for (ImTextureData* tex : *pDrawData->Textures) | ||
| if (tex->Status != ImTextureStatus_OK) | ||
| UpdateTexture(pCtx, tex); |
There was a problem hiding this comment.
Let's add braces for ratability.
|
I created a branch for imgui update. |
code readability
Great! Implemented the changes you requested. |
|
Can you remove all macos changes from this PR - we will handle MacOS separately |
|
Yes - no problem, but if I do that, the MacOS build will be failing. Is that ok? |
|
We can make some temporary fixes to make the files compile. We will fix them later. This is a work branch |
|
@TheMostDiligent Ok. I reverted the macos related changes. But then I also updated the legacy 1.85 backend to use the new imgui IO api so it now compiles with 1.92.1+. I used some AI help + some bits from the new backends, but I also did some more extensive testing this time so I hope it works as expected. |
f145b8f to
7013506
Compare
7013506 to
6dc7440
Compare
|
Yes. There were quite a lot of changes needed this time, hopefully it will be smoother from here. |
In regards to this issue: #249
I tested everything on Windows in all backends and it works nice. Other platforms should work as well as the changes were relatively straightforward, but some testing would of course be needed.
I noticed that some Diligent samples use invalid ImGui flags for some controls and this is now being caught by assertions inside ImGui in debug builds. (for example ImGui::InputFloat() does not support ImGuiInputTextFlags_EnterReturnsTrue flag). These should be fixed, but they are in another repo.