imgui icon indicating copy to clipboard operation
imgui copied to clipboard

Added example project of Win32 and OpenGL implementation.

Open danec020 opened this issue 6 years ago • 20 comments

This pull request is for the example project show casing using Dear ImGui's docking branch working with Win32 window API and OpenGl3 using the Glad loader.

To use: -Pull commit. -Load ImGui solution. -Set imgui_impl_opengl3.h to use #define IMGUI_IMPL_OPENGL_LOADER_GLAD -Set included example project as startup. -Build and run.

Screenshot

danec020 avatar Aug 31 '19 22:08 danec020

I haven’t looked at this in details yet but I am also surprised you switched the example to Glad and added Glad to the repo. That itself is wholly different thing than just adding a win32+opengl example. If we ever decide to add Glad for whichever reason we can discuss it separately, but right now I think this example should use Gl3w like other ones.

ocornut avatar Sep 01 '19 12:09 ocornut

Thanks for the feedback and you are right. I will clean up those white space changes and remove the dependencies. I am sorry about adding Glad, I thought it would be ok since the project already supported Glad but I can use Gl3w and that should help with the changes to the branch.

danec020 avatar Sep 01 '19 13:09 danec020

Ok, the latest version should have only the necessary changes. I tried to be extra careful cleaning it up for you so please let me know if I missed anything. Will the pull request get the history of all my commits or just the latest? If I need to, I can delete the fork and start a new one with just one commit with the latest changes so we don't clutter the history.

danec020 avatar Sep 01 '19 14:09 danec020

You should be able to rebase and squash all your commits locally into 1 commit, then force push the branch. That will update the existing PR with a single commit version.

ocornut avatar Sep 01 '19 15:09 ocornut

Done. Should now be a single commit.

danec020 avatar Sep 01 '19 15:09 danec020

Much better indeed. The batch file to build it is still broken though. You should be able to look at the other ones for Windows and the SDL OpenGL one to make something that works.

I don't think we have Windows CI at the moment. I might look at that at some point.

corentin-plouet avatar Sep 02 '19 01:09 corentin-plouet

Really sorry for all the time of yours that I have wasted not doing this correctly. I have updated the batch file now and ran it in the command line with a successful build.

danec020 avatar Sep 02 '19 14:09 danec020

Thank you @danec020 for taking the time to update this, much appreciated. Also thanks to @corentin-plouet for helping with the review :)

As is, I believe the changes in imgui_impl_win32.cpp would be breaking all other back-ends since they create an OpenGL context. This topic has been approach various time but it never got fully solved right yet. Linking to #2359, #2290 , #2022, #1553, However I believe we now have enough reference to design a working thing.

In my attempt to do it for multi-viewports here: https://github.com/ocornut/imgui/pull/1553#issuecomment-455519832

I added the win32<>gl glue code in the main.cpp code. I belive that win32 glue code which you currently added in imgui_impl_win32.cpp should somehow be moved either to the app (main.cpp) either the renderer backend (imgui_impl_openglX.cpp) (preferably) but for the later it would need an indication from the platform whether the context was already created or not and that indication would need to be stored in a back-end agnostic manner (e.g. something in ImGuiPlatformIO).

The problem stem from the fact that our platform<>renderer split approach doesn't perfectly fit the situation for GL+Win32.

I think we need to be looking at those 4 topics together, find a suitable solution for multi-viewports, once that solution is agreed upon we can add the example to master and then add the multi-viewports compliant feature.

To be honest I believe the situation is a little bit hairy. Bonus point is that when we solve it we'll be solving FIVE issues at once and that prospect keeps me awake at night in excitement. I'll be looking at it when I have time.

ocornut avatar Sep 02 '19 16:09 ocornut

Which sections of code do you feel would be non breaking? I have removed the code creating the second GLContext and updated it to use the PixelFormat and GlContext of the main window. This way whatever the implementation the user uses for their window, our child window would be setup the same way.

This eleminated the need for the glue code in the imgui_impl_win32.cpp. Does the new RenderWindow and SwapBuffer methods cause any concern for issues? That shouldn't break anything if the user had already supplied their own? Let me know what you think about the changes and we can keep nudging it in the right direction. I should have the changes up soon.

danec020 avatar Sep 04 '19 13:09 danec020

Just checking in to see if you seen my edited comment. Does this address some of the concerns?

danec020 avatar Sep 08 '19 02:09 danec020

@ocornut I also have discovered that when you have multiple Viewports outside the main window the program slows tremendously. The cpu usage and ram isn't changing but the response in imgui is very laggy and nonresponsive.

danec020 avatar Oct 09 '19 14:10 danec020

It's probably mishandling vsync/swap, should be fixed.

ocornut avatar Oct 09 '19 15:10 ocornut

It seems also it doenst respond to any Input resize etc...

LipkeGu avatar Oct 11 '19 23:10 LipkeGu

I am not sure I follow what you mean by input resize? Do you have a screenshot?

danec020 avatar Oct 11 '19 23:10 danec020

When i try to resize the imgu window, it doesnt respond to mouseInput. neighter i can move it to another Position... imgui_input

LipkeGu avatar Oct 11 '19 23:10 LipkeGu

I do not have that issue. Sorry.

danec020 avatar Oct 11 '19 23:10 danec020

Hi @danec020 !

I just happened to stumble upon this PR so i figured I’d add some feedback / food for thought here.

Firstly, the way your are initializing OpenGL isn’t really “modern” or the way you supposed to do it anymore. The way you are doing it is fine for quickly getting a context up and running, but not how you are supposed to do it for applications that will run on many PCs. I’m not sure if this is really a concern or not given that people aren’t supposed to just copy these examples (even though I imagine many do). In my experience creating OpenGL contexts the “modern” way (wglCreateContextAttribsArb) also reduces driver to driver output variation like we are getting above.

Secondly (this one is easier to digest), since this is just a window app it is recommended to use WinMain this will automatically give you a hInstance so you won’t have to call GetModuleHandle anymore.

I’d be able to go ahead and make these changes if desired.

Thanks Zak

strangezakary avatar Oct 16 '19 02:10 strangezakary

@ocornut Hey Omar. Is this example something that you are interested in adding to the repo. If so i can take the weekend and put something together as it shouldn't be too much work

strangezakary avatar Oct 24 '19 23:10 strangezakary

@StrangeZak I am very interested in this but every variations of this PR comes with their own sets of issues and I don't have much of the energy to be wading through them and providing the feedback/fixes at the moment. Happy to hear suggestion of changes to the initialization process, ideally backed with data/references. Your first suggestion looks adequate but the second suggestion feels offtopic and unnecessary.

ocornut avatar Oct 25 '19 08:10 ocornut

@ocornut I also have discovered that when you have multiple Viewports outside the main window the program slows tremendously. The cpu usage and ram isn't changing but the response in imgui is very laggy and nonresponsive.

It's probably mishandling vsync/swap, should be fixed.

I did some poking around and this seems to be caused by the constant GetDC calls in the RenderWindow/SwapBuffers functions. Saving the HDC in ImGuiViewportDataWin32 on window creation is a way to solve this.

Donione avatar Mar 01 '21 11:03 Donione

Closed/solved, see https://github.com/ocornut/imgui/pull/3218#issuecomment-1514886161

ocornut avatar Apr 19 '23 14:04 ocornut