Added example project of Win32 and OpenGL implementation. #2772

danec020 · 2019-08-31T22:29:53Z

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.

corentin-plouet

The main issue is the way you've integrated the loader; you've hard-wired glad in. You should be able to make it work with gl3w. Which of course does not prevent you from using glad in your own project (I use glad myself).

The second issue is the way you have disrupted the way windows.h is included. I'm pretty sure it's fine the way it is currently.

Otherwise there are a few whitespace changes that add noise.

corentin-plouet · 2019-09-01T11:42:13Z

examples/example_win32_opengl3/main.cpp

+#include "glad/glad.h"
+#include <tchar.h>
+
+#define IMGUI_IMPL_OPENGL_LOADER_GLAD


This has no effect here.

corentin-plouet · 2019-09-01T11:44:33Z

examples/example_win32_opengl3/main.cpp

@@ -0,0 +1,229 @@
+#include <Windows.h>


Should be lowercase windows.h (is it even required to include this one here?)
Generally, it's a good practice to order your headers from most specific to least specific, such that you would have first file-specific headers (in this case the ones related to win32 and opengl3) then project-specific headers (imgui) then third-party library and finally system headers like stdio.
This way you can detect when your own headers have implicit dependencies that you forgot to add.

corentin-plouet · 2019-09-01T11:47:24Z

examples/imgui_impl_opengl3.h

@@ -33,7 +33,7 @@
 && !defined(IMGUI_IMPL_OPENGL_LOADER_GLEW)     \
 && !defined(IMGUI_IMPL_OPENGL_LOADER_GLAD)     \
 && !defined(IMGUI_IMPL_OPENGL_LOADER_CUSTOM)
-#define IMGUI_IMPL_OPENGL_LOADER_GL3W
+#define IMGUI_IMPL_OPENGL_LOADER_GLAD


See general comment regarding glad, below.
This line should be reverted; it breaks the Linux/macOS builds. The logic is wrong anyway: there is already a test for GLAD right above.

corentin-plouet · 2019-09-01T11:48:23Z

examples/imgui_impl_win32.cpp

+    g_glcontext = (HGLRC)glcontext;
+    g_focused_hWnd = g_hWnd;
+
+    ImGuiIO& io = ImGui::GetIO(); (void)io;


What's that cast to void on the right?

corentin-plouet · 2019-09-01T11:48:54Z

examples/imgui_impl_win32.cpp

-        MAP_ANALOG(ImGuiNavInput_LStickDown,    gamepad.sThumbLY,  -XINPUT_GAMEPAD_LEFT_THUMB_DEADZONE, -32767);
-        #undef MAP_BUTTON
-        #undef MAP_ANALOG
+#define MAP_BUTTON(NAV_NO, BUTTON_ENUM)     { io.NavInputs[NAV_NO] = (gamepad.wButtons & BUTTON_ENUM) ? 1.0f : 0.0f; }


Completely gratuitous white space changes here.

corentin-plouet · 2019-09-01T11:49:22Z

examples/imgui_impl_win32.cpp

    data->Hwnd = ::CreateWindowEx(
        data->DwExStyle, _T("ImGui Platform"), _T("Untitled"), data->DwStyle,   // Style, class name, window name
        rect.left, rect.top, rect.right - rect.left, rect.bottom - rect.top,    // Window area
        parent_window, NULL, ::GetModuleHandle(NULL), NULL);                    // Parent window, Menu, Instance, Param
+


Gratuitous whitespace change.

corentin-plouet · 2019-09-01T11:50:14Z

examples/imgui_impl_win32.cpp

 }

 static bool ImGui_ImplWin32_GetWindowFocus(ImGuiViewport* viewport)
 {
    ImGuiViewportDataWin32* data = (ImGuiViewportDataWin32*)viewport->PlatformUserData;
    IM_ASSERT(data->Hwnd != 0);
+    ImDrawList* list = ImGui::GetBackgroundDrawList(viewport);
+    //return g_focused_hWnd == data->Hwnd;


Might want to remove that line instead of leaving it commented.

corentin-plouet · 2019-09-01T11:55:24Z

examples/libs/glad/include/KHR/khrplatform.h

@@ -0,0 +1,290 @@
+#ifndef __khrplatform_h_


There are two issues here:

glad files (maybe not this one, but the other two) are generated based on the developer or project's spec. Maybe not such a big deal for an example program though, but...

This adds an extra dependency. ImGui already ships with gl3w as a GL loader.

So it would be better to use gl3w instead of glad. Unless there is a good reason not too? I assume gl3w works on Windows but I have not tested myself.

Look at the existing examples to see how it's integrated. Generally you don't have to touch the code, there is already the #ifdef logic to pick the right one based on what you specify when building it.

corentin-plouet · 2019-09-01T11:59:28Z

examples/imgui_impl_win32.cpp

@@ -13,7 +13,6 @@
 #ifndef WIN32_LEAN_AND_MEAN
 #define WIN32_LEAN_AND_MEAN
 #endif
-#include <windows.h>


I'm not sure we want this one gone...

I had removed it and put it in the header. Which was probably a mistake left over when I was cleaning up the file. I will bring it back and remove the one in the header file so it is like it was before.

Then you should also move the MEAN_AND_LEAN stuff

corentin-plouet · 2019-09-01T12:02:49Z

examples/imgui_impl_win32.h

@@ -9,8 +9,9 @@
 //  [X] Platform: Multi-viewport support (multiple windows). Enable with 'io.ConfigFlags |= ImGuiConfigFlags_ViewportsEnable'.

 #pragma once
+#include <windows.h>


See the comment below.
You don't want to drag windows.h in other header files because it's enormous (even when defining WIN32_LEAN_AND_MEAN).

ocornut · 2019-09-01T12:49:55Z

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.

danec020 · 2019-09-01T13:28:42Z

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 · 2019-09-01T14:58:13Z

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.

ocornut · 2019-09-01T15:00:57Z

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.

danec020 · 2019-09-01T15:42:21Z

Done. Should now be a single commit.

corentin-plouet · 2019-09-02T01:59:24Z

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.

…Gl3 via Glad. Trying to remove the white space changes that were made to the original for no reason. Removed changes to Updating the mouse position. Removing more unnecessary changes. Cleaned up some more unneeded changes and whitespace. Switched the projec to using gl3w. Project should be cleaned of all uneccessary white spaces and code. Everything has tested and worked. Had to add back the windows header and delete to forward declarations. Was able to remove the windows dependence from the header with forward declaration. Setup the batch file to properly build the project from command line.

danec020 · 2019-09-02T14:01:13Z

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.

ocornut · 2019-09-02T16:35:57Z

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:
#1553 (comment)

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.

danec020 · 2019-09-04T13:29:40Z

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.

…is created it uses the main window's context and pixel format. Removed extra additions to the code that used a unique GL Context per viewport.

danec020 · 2019-09-08T02:40:56Z

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

danec020 · 2019-10-09T14:36:03Z

@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.

ocornut · 2019-10-09T15:00:52Z

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

LipkeGu · 2019-10-11T23:02:45Z

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

danec020 · 2019-10-11T23:06:47Z

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

LipkeGu · 2019-10-11T23:09:40Z

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

danec020 · 2019-10-11T23:11:41Z

I do not have that issue. Sorry.

strangezakary · 2019-10-16T02:42:55Z

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 · 2019-10-24T23:17:43Z

@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

ocornut · 2019-10-25T08:36:42Z

@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.

Donione · 2021-03-01T11:03:14Z

@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.

learn-more · 2021-03-02T09:23:05Z

examples/example_win32_opengl3/main.cpp

+    ImGui_ImplWin32_EnableDpiAwareness();
+
+    // Create application window
+    WNDCLASSEX wc = { sizeof(WNDCLASSEX), CS_CLASSDC, WndProc, 0L, 0L, GetModuleHandle(NULL), NULL, NULL, NULL, NULL, _T("ImGui Example"), NULL };


CS_CLASSDC means all windows created with this window class will share 1 Device context.
So no matter how many times you call GetDC (and no matter on what window with this class) they all share 1 DC.
Are you sure that is the correct option here?

So the big slowdown is not as much from calling GetDC repeatedly as it is from not calling ReleaseDC afterwards.
Is it generally bad idea to just store the DC once?

Attaching a profiler will tell you exactly where the slowdown comes from, instead of guessing at it.

learn-more · 2021-03-02T09:24:16Z

examples/imgui_examples.sln

-# Visual Studio 14
-VisualStudioVersion = 14.0.25420.1
+# Visual Studio 15
+VisualStudioVersion = 15.0.28307.779


You should probably revert this?

…2772, #2600, #2359, #2022, #1553) Removed mulit-viewpot stuff in this commit on master, will be re-added separately.

#2772, #2600, #2359, #2022, #1553)

…3218, #5170 and #6086, #2772, #2600, #2359, #2022, #1553)

, #5170 and #6086, #2772, #2600, #2359, #2022, #1553)

ocornut · 2023-04-19T14:56:38Z

Closed/solved, see #3218 (comment)

corentin-plouet reviewed Sep 1, 2019

View reviewed changes

danec020 force-pushed the docking branch from cf8b329 to 9380b8f Compare September 1, 2019 15:39

danec020 force-pushed the docking branch from f2ba7ea to 2f2a0b0 Compare September 2, 2019 13:56

ocornut added backends opengl multi-viewports labels Sep 2, 2019

Removed the need to pass in a GL Context. Also now when a new window …

61a7803

…is created it uses the main window's context and pixel format. Removed extra additions to the code that used a unique GL Context per viewport.

danec020 force-pushed the docking branch from 317cf9b to 61a7803 Compare September 11, 2019 00:46

ocornut force-pushed the docking branch 2 times, most recently from 4a6447b to 6822493 Compare October 8, 2020 14:06

learn-more reviewed Mar 2, 2021

View reviewed changes

ocornut force-pushed the docking branch from 44771a1 to 6afcfe3 Compare December 1, 2021 17:27

ocornut changed the title ~~Added example project of Win32 and OpenGl implementation.~~ Added example project of Win32 and OpenGL implementation. Jan 4, 2022

ocornut force-pushed the docking branch from cf2de07 to 101aec9 Compare June 16, 2022 11:13

ocornut force-pushed the docking branch from 7eaa1d3 to d822c65 Compare January 5, 2023 14:58

ocornut mentioned this pull request Jan 20, 2023

Added Win32_Opengl example #6086

Closed

ocornut force-pushed the docking branch from 7f662a0 to ad44f58 Compare March 21, 2023 16:05

ocornut mentioned this pull request Apr 7, 2023

Viewports for win32+opengl backend (fix #2600) #5170

Closed

ocornut pushed a commit that referenced this pull request Apr 19, 2023

Examples: Add Win32/Winapi + OpenGL example. (#3218, #5170 and #6086, #…

a566ecc

…2772, #2600, #2359, #2022, #1553) Removed mulit-viewpot stuff in this commit on master, will be re-added separately.

ocornut added a commit that referenced this pull request Apr 19, 2023

Examples: Amend Win32/Winapi + OpenGL example. (#3218, #5170 and #6086,

9308cfd

#2772, #2600, #2359, #2022, #1553)

ocornut pushed a commit that referenced this pull request Apr 19, 2023

Examples: Extend Win32/Winapi + OpenGL example for multi-viewport. (#…

4bc51c6

…3218, #5170 and #6086, #2772, #2600, #2359, #2022, #1553)

ocornut added a commit that referenced this pull request Apr 19, 2023

Examples: Amend Win32/Winapi + OpenGL example for multi-viewport. (#3218

f498f08

, #5170 and #6086, #2772, #2600, #2359, #2022, #1553)

ocornut mentioned this pull request Apr 19, 2023

Add native winapi OpenGL example #3218

Closed

ocornut closed this Apr 19, 2023

Added example project of Win32 and OpenGL implementation. #2772

Added example project of Win32 and OpenGL implementation. #2772

Conversation

danec020 commented Aug 31, 2019

corentin-plouet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocornut commented Sep 1, 2019 via email

danec020 commented Sep 1, 2019

danec020 commented Sep 1, 2019

ocornut commented Sep 1, 2019 via email

danec020 commented Sep 1, 2019

corentin-plouet commented Sep 2, 2019

danec020 commented Sep 2, 2019

ocornut commented Sep 2, 2019

danec020 commented Sep 4, 2019 • edited Loading

danec020 commented Sep 8, 2019

danec020 commented Oct 9, 2019

ocornut commented Oct 9, 2019

LipkeGu commented Oct 11, 2019

danec020 commented Oct 11, 2019

LipkeGu commented Oct 11, 2019 • edited Loading

danec020 commented Oct 11, 2019

strangezakary commented Oct 16, 2019

strangezakary commented Oct 24, 2019

ocornut commented Oct 25, 2019

Donione commented Mar 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ocornut commented Apr 19, 2023

danec020 commented Sep 4, 2019 •

edited

Loading

LipkeGu commented Oct 11, 2019 •

edited

Loading

Donione commented Mar 1, 2021 •

edited

Loading