GNOME Bugzilla – Bug 740795
GDK: Add OpenGL Support for Windows
Last modified: 2014-12-17 08:14:03 UTC
Hi, As GTK+ 3.16 is aiming to have GLArea support for all the supported platforms, this is the tracking bug for the ongoing work to adding GLArea support for Windows in GDK. So far, I was able to get the GLArea demo to display the triangle/pyramid, but am still working on getting the updating part to work (which, as it seems to me, is using FBOs or pixel buffers). I will post my progress for what I have so far later. With blessings, thank you!
thanks for working on this
Created attachment 291715 [details] [review] Current Progress of adding GL Area support for GDK-Win32 (take i) Hi, This is the obviously-not-ready-for-going-in patch that indicates the current progress of this drive. With blessings, thank you!
Created attachment 291965 [details] Screenshot of current progress Hi, I managed to fix the part where the updates are being done, so that the gdkgears can show the gears turning in motion, and that one can try to turn the triangle around using the x-, y- and z-axis, as shown in the screenshot. I am still figuring out the reason behind the invisible GTK UI widgets (they are there, but aren't visible, as the screenshot tries to show as the triangle is turned :| ), otherwise I think there should be sufficient support for GL in the Windows GDK backend. :) As trying to do what X11 does by using Bitmaps (like in gdk_x11_gl_context_texture_from_surface () where pixmaps are used), it turns out that we can't just use CreateDIBSection (), which is the recommended way that MSDN suggests[1] as the GL context created against a Bitmap will automatically fall back to the Microsoft (software/stock) OpenGL 1.1 implementation which means: -When using Epoxy, since the function pointers are pointing against the Video Driver OpenGL implementation (nVidia GTX675m, latest drivers on Windows 8.1, which claims support for OpenGL 4.4), upon doing any real GL work would cause a crash. -There are a pretty good number of GL commands that aren't supported due to the lack of support of them in Microsoft's stock OpenGL implementation (meaning we still need to get the function pointers and macros via Epoxy). -If we use pBuffers, the overheads of context creation and switching is not currently worthwhile compared against the stock software fallback in gdk_gl_texture_from_surface (). I will post my other round of progress in the code in a bit. With blessings, thank you! [1]: http://msdn.microsoft.com/en-us/library/dd368997%28v=vs.85%29.aspx
Created attachment 291967 [details] [review] Current Progress of adding GL Area support for GDK-Win32 (take ii) Hi, This is the current state of the progress in adding OpenGL support in GDK-Win32. Any suggestions on the invisible GTK+ UI widgets in the GL Area demo, or on other items as well, are highly appreciated. With blessings, thank you!
Created attachment 291968 [details] [review] gtkgears.c: Use G_PI instead of M_PI Hi, By the way, this is the update to tests/gtkgears.c for compilers such as Visual Studio 2008-2012 that do not have M_PI defined. With blessings, thank you!
Review of attachment 291968 [details] [review]: the internet tells me something about #define _USE_MATH_DEFINES to get M_PI in visual c. this patch is fine too, though
Review of attachment 291968 [details] [review]: Hello Matthias, Thanks for the review and note-you're right about it. I committed the patch as fbe4f945 though, anyways. With blessings, thank you!
Texture from pixmap being software on win32 is less of a problem than on win32. Most cairo surfaces will be dib-sections already, so cairo_surface_map_to_image() will map to the existing image cairo surface, so there is no copy involved there. Of course, it also means the surface will not ever be in GPU memory, making texturing a bit slower. In the case where the surface is a DDB maybe there is some win32 opengl extension to use it as a texture source, but i doubt it.
Review of attachment 291967 [details] [review]: Of the controls don't work that means gdk_gl_texture_from_surface() is broken, as what happens is the regular gtk stuff is rendered to an offscreen cairo surface that is then drawn to the final window via gl. The software fallback works by just reading back surface image data via cairo_surface_map_to_image() which is uploaded as texture data using glTexImage2D. I can think of some reasons this is failing: * Maybe there is a cairo win32 issue with such readback. Verify using cairo_surface_write_to_png () that the returned image surfaces contain the right data. * Maybe the source format used "GL_BGRA, GL_UNSIGNED_INT_8_8_8_8_REV" is not right for win32 surfaces * maybe there issue with alignment or stride of the data ::: gdk/win32/gdkglcontext-win32.c @@ +173,3 @@ + const PIXELFORMATDESCRIPTOR *pfb) +{ + /* Always prefer a format with a stencil buffer */ We never use stencil buffers, so this is not needed. We don't even use a depth buffer for the back buffer, just a pure color buffer. Anything that needs other buffers needs to do that themselves by using gl framebuffer object with extra buffers attached to those. What we *do* need though is an alpha channel, if the target GdkWindow has alpha.
Hello Alex, Thanks for the notes... For the GTK+ GUI items, the controls do work but it's like poking around in the dark (so I hit the part of the window where the buttons/sliders are and it changes the image/animation that is displayed)-it does seem that I need to try creating a child window within the GtkWindow (I saw the X11 backend doing this as well, missed that part)... Will get back on this part ASAP though. (In reply to comment #9) > > What we *do* need though is an alpha channel, if the target GdkWindow has > alpha. I removed the check on the depth bits and stencil bits, but it does seem to me that I need to defer this a bit, due to bug 727316, which indicates that Cairo needs to be investigated as well for this. I could try to get the PFD to acquire the alpha bits first, and get support for checking on that in later. With blessings, thank you!
Fan: No, we should *not* need to create a (native) child window in the GtkWindow. That is the entire point of this (new) approach to OpenGL rendering. We use OpenGL to draw only to the toplevel, and any child (e.g. a GtkGLArea) renders to an offscreen gl framebuffer which we then draw onto the toplevel using opengl. I understand that the "ui" works (i.e. the events etc are right). The thing that fails is just painting the ui that was drawn using cairo to the toplevel, which is done by gdk_gl_texture_from_surface(). I agree that having proper toplevel alpha support on win32 would be very nice, although I don't think it necessarily is what is blocking this bug (other than via your limited time).
I guess you're talking about dummy_xwin? The reason we have that is more of an internal details of OpenGL in X11. What we want is that "attached" gl contexts are "normal" gl contexts for the toplevel window, but "non-attached" (which are all gl contexts that app authors see) are not really connected to any particular window or default framebuffer at all. You can only draw to them if you use OpenGL APIs to create your own gl framebuffer object and make them current. There are some extensions that let you create window-less gl contexts. For instance, on wayland we use EGL_KHR_surfaceless_context if available. If something like that is doable on win32 that would be much preferred.
Created attachment 292729 [details] Screenshot of Current Progress of GL support in GDK-Win32 Hi, This is the current progress of the ongoing work to add GL support in the GDK Win32 backend. The glaring issue is that the UI regions beside and above the areas where the GL graphics are drawn are still not displayed, so still looking into this part. I will post the code for the current progress in a bit...
Created attachment 292731 [details] [review] Current Progress of adding GL Area support for GDK-Win32 (take iii) Hello Alex, Here comes the current progress, code-wise... (In reply to comment #9) > We never use stencil buffers, so this is not needed. > We don't even use a depth buffer for the back buffer... > > What we *do* need though is an alpha channel, if the target GdkWindow has > alpha. (In reply to comment #11) > I agree that having proper toplevel alpha support on win32 would be very nice, > although I don't think it necessarily is what is blocking this bug (other than > via your limited time). I have added in the progress update that: -Alpha channel is preferred, and when we eventually get toplevel alpha support for Windows, it would be required. This seems like what the X11/Wayland backends are doing as well, as far as I can tell; so this means in the meanwhile alpha channel is preferred when we are selecting a pixel format for WGL. -The requirement/preference for stencil and depth are taken out. (In reply to comment #12) > I guess you're talking about dummy_xwin? The reason we have that is more of an > internal details of OpenGL in X11. > There are some extensions that let you create window-less gl contexts. I see, so it seems that this is like WGL where we need a WGL context that is made current to do many GL stuff, including querying, hence "dummy" :). Thanks for clearing this up for me :). (In reply to comment #9) > Of the controls don't work that means gdk_gl_texture_from_surface() is broken, > as what > happens is the regular gtk stuff is rendered to an offscreen cairo surface that > is then > drawn to the final window via gl. Hmm, I changed the glTexImage2D() call by overriding gdk_gl_texture_from_surface() (like what the X11 backend does). It seems that the UI beneath the region where the GL graphics are drawn is now shown correctly, but the UI beside and above the regions are still not being drawn :|. Need to look more into this. Thanks for all the notes and review. With blessings, thank you!
Can you retry with the latest master I fixed some generic issues with the software fallbacks in the gl case.
Created attachment 292789 [details] Screenshot of GL support in GDK-Win32 Hello Alex, This is a screenshot of the current state of the GL support in GDK-Win32. So it seems to me that the updates you had works, as I carried over the changes to GDK-Win32 (the glTexImage2D() still needs to be called with different formats and parameters). Thanks very much for the tips and help along the way-I will post a proposed patch for this bug shortly. With blessings, thank you!
Created attachment 292790 [details] [review] Proposed Patch for Adding OpenGL Support for the GDK Windows Backend Hi, As the patch description suggests, this is my proposed patch for adding support for OpenGL in the GDK Windows backend. It tries to model what are done in the X11 (and possibly Wayland) backends as far as possible (AFAICT). One of the major things that is not carried over from the X11 backend, is that the ->texture_from_surface() implementation is largely carried over from the stock implementation from gtkgl.c (gdk_gl_texture_from_surface()), as the call to glTexImage2D() needs different formats and parameters, which means that the implementation is using software fallback. The use of the software fallback is due to the case that we aren't able to do something similar to creating pixmaps in the Windows backend, which involves using Windows bitmaps via CreateDIBSection(), which will force us to use the stock Microsoft-supplied software-only OpenGL 1.1 implementation (which is insufficient for our use case). There may or may not be WGL extensions to help us get around this part, but not that I know at the moment. With blessings, thank you!
(P.S.: I will update the Visual Studio build files once the sources for this bug gets into master)
Review of attachment 292790 [details] [review]: ::: gdk/win32/gdkdisplay-win32.h @@ +34,3 @@ + guint pixel_format; + + gint hasWglARBCreateContext : 1; For bitfield bools like this it is preferable to use guint. For a signed one-bit value the two possible values are 0 and -1, which is often suprising. For guint it is instead 0 and 1, which is more natural. ::: gdk/win32/gdkglcontext-win32.c @@ +270,3 @@ + glPixelStorei (GL_UNPACK_ALIGNMENT, 4); + glPixelStorei (GL_UNPACK_ROW_LENGTH, cairo_image_surface_get_stride (image)/4); + glTexImage2D (target, 0, GL_RGBA, e.width, e.height, 0, GL_BGRA, GL_UNSIGNED_BYTE, Hmmm, I dislike that we copy the entire function for such a small change. Especially since the function is kind of complex and has had bugs before. Also, I expect various drivers to require similar sorts of tweaks in the future, even on X. So, i would prefer if we instead made this a vfunc on the cairo context like this: void (*upload_texture) (GdkGLContext *context, cairo_surface_t *image_surface, int width, int height, guint texture_target); With the current code as the default implementation. Does that work for you? @@ +397,3 @@ + /* Prefer a bigger color buffer */ + if (pfb->cColorBits > pfa->cColorBits) + return TRUE; Some formats have weird sizes like high-bit-depth colors. I wonder if we should really always pick the highest. I know on X/GLX the formats you get back are ordered in some kind of "most useful first" order, so that you just pick the first that satisfies your requirement, and that way you don't accidentally get some weird (possibly slow) format. This seems like something we can fix later though, as we start having some experience with different GL drivers on win32. ::: gdk/win32/gdkglcontext-win32.h @@ +43,3 @@ + + /* other items */ + gint is_attached : 1; guint here too
"on the gl context" i mean, not the cairo context.
Other than that this looks good to me to push.
Created attachment 292840 [details] [review] gdlgl.c: Optionally use vfunc to upload textures in gdk_gl_texture_from_surface() Hello Alex, (In reply to comment #19) > So, i would prefer if we instead made this a vfunc on the OpenGL context > like this: > > void (*upload_texture) (GdkGLContext *context, > cairo_surface_t *image_surface, > int width, int height, > guint texture_target); Here's my proposed patch to allow the overriding of the image uploading in gdk_gl_texture_from_surface(), if needed, by the backend. With blessings, thank you!
Created attachment 292841 [details] [review] Proposed Patch for Adding OpenGL Support for the GDK Windows Backend (take ii) Hello Alex, I updated the patch for adding GL support in the GDK Windows backend... (In reply to comment #19) > ::: gdk/win32/gdkdisplay-win32.h > For bitfield bools like this it is preferable to use guint. For a signed > one-bit value the two possible values are 0 and -1, which is often suprising. > For guint it is instead 0 and 1, which is more natural. Got it. > Some formats have weird sizes like high-bit-depth colors. I wonder if we should > really always pick the highest. > I know on X/GLX the formats you get back are ordered in some kind of "most > useful first" order, so that you just pick the first that satisfies your > requirement, and that way you don't accidentally get some weird (possibly slow) > format. Perhaps, like in this updated patch, use ChoosePixelFormat() instead and trust Windows to pick the best format, as it advertises in MSDN? > ::: gdk/win32/gdkglcontext-win32.h > @@ +43,3 @@ > + > + /* other items */ > + gint is_attached : 1; > Got it in the updated patch as well. With blessings, thank you!
Review of attachment 292840 [details] [review]: ::: gdk/gdkgl.c @@ +722,3 @@ + /* We might have a different alignment, stride or format, so allow overriding here if needed */ + if (GDK_GL_CONTEXT_GET_CLASS (paint_context)->upload_texture) Instead of this check, can you make a default implementation in GdkGLContext which does the below, and initialize it in gdk_gl_context_class_init.
Review of attachment 292841 [details] [review]: Looks good to me. ::: gdk/win32/gdkglcontext-win32.c @@ +296,3 @@ + pfd->dwFlags = PFD_SUPPORT_OPENGL | PFD_DRAW_TO_WINDOW | PFD_DOUBLEBUFFER; + pfd->iPixelType = PFD_TYPE_RGBA; + pfd->cColorBits = 32; Should we not look at the GdkWindow visual for cColorBits?
Created attachment 292864 [details] [review] gdkglcontext: Use vfunc for uploading textures Hello Alex, (In reply to comment #24) > Instead of this check, can you make a default implementation in GdkGLContext > which does the below, and initialize it in gdk_gl_context_class_init. OK, here it goes...
Created attachment 292865 [details] [review] Proposed Patch for Adding OpenGL Support for the GDK Windows Backend (take iii) Hello Alex, Here comes the updated patch for adding GL support in the GDK Windows backend, which is cleaned up a little bit (we don't really need to save the pixel format, as that was initially intended to be used to create a child window, which we didn't want). (In reply to comment #25) > Should we not look at the GdkWindow visual for cColorBits? I updated this part, but since we already have the HDC from the HWND from the GdkWindow (and the dummy window as we created it) during context creation, we could just use GetDeviceCaps() on the HDC to get the value for cColorBits, as this is Windows-only territory. Thanks for the pointers to make this work better. With blessings, thank you!
Review of attachment 292864 [details] [review]: Looks good to commit ::: gdk/gdkglcontextprivate.h @@ +83,3 @@ + int width, + int height, + guint texture_target); You can drop this one.
Review of attachment 292865 [details] [review]: looks good. Please commit!
Hello Alex, Thanks much for the reviews and tips along the way, the patches have been pushed as: Attachment 292864 [details]: 9fd9f61 Attachment 292865 [details]: 536fa88 The Windows GDK GL support has now landed! (In reply to comment #28) > You can drop this one. By the way, due to C89 compatibility, I moved up the stock implementation (as in commit 9fd9f61) for gdk_gl_context_upload_texture(), so that gdk_gl_context_class_init() can "see" it declared beforehand, when we do not have a prototype in the private header. With blessings, thank you!