After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 741946 - OpenGL context should allow for GL attribute selection
OpenGL context should allow for GL attribute selection
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: GtkGLArea
3.15.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 742893 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-12-24 18:02 UTC by Pavol Klačanský
Modified: 2015-02-12 13:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GL: Split GL context creation in two phases (29.87 KB, patch)
2015-01-28 12:05 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
gl: Add context options (9.96 KB, patch)
2015-01-28 12:05 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
x11/gl: Use the GdkGLContext options (5.00 KB, patch)
2015-01-28 12:05 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
GL: Split GL context creation in two phases (30.22 KB, patch)
2015-01-28 12:40 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
gl: Add context options (9.96 KB, patch)
2015-01-28 12:40 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
x11/gl: Use the GdkGLContext options (5.00 KB, patch)
2015-01-28 12:40 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
wayland/gl: Use the GdkGLContext options (1.84 KB, patch)
2015-01-28 12:40 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
GL: Split GL context creation in two phases (31.85 KB, patch)
2015-01-28 12:52 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
gl: Add context options (9.96 KB, patch)
2015-01-28 12:52 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
x11/gl: Use the GdkGLContext options (5.00 KB, patch)
2015-01-28 12:52 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
wayland/gl: Use the GdkGLContext options (1.84 KB, patch)
2015-01-28 12:52 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
GDK-Win32: Split GL Context Creation in 2 Phases (6.43 KB, patch)
2015-01-30 05:57 UTC, Fan, Chun-wei
none Details | Review
GDK-Win32: Use the GdkCLContext options (3.93 KB, patch)
2015-01-30 06:34 UTC, Fan, Chun-wei
none Details | Review
GL: Split GL context creation in two phases (31.85 KB, patch)
2015-02-05 17:47 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
gl: Add context options (9.96 KB, patch)
2015-02-05 17:47 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
x11/gl: Use the GdkGLContext options (5.00 KB, patch)
2015-02-05 17:48 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
wayland/gl: Use the GdkGLContext options (1.84 KB, patch)
2015-02-05 17:48 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
GDK-Win32: Split GL context creation in two phases (6.42 KB, patch)
2015-02-05 17:49 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
win32/gl: Use the GdkGLContext options (3.91 KB, patch)
2015-02-05 17:49 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
Switch GDK_GL_PROFILE_DEFAULT to mean 3_2_CORE (1.99 KB, patch)
2015-02-05 17:49 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
gl: Move getters for context options to the public API (4.80 KB, patch)
2015-02-05 17:50 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
gl: Drop OpenGL legacy profile (5.61 KB, patch)
2015-02-05 17:50 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
gl: Do not use the extension API for core GL (903 bytes, patch)
2015-02-05 17:50 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
glarea: Do not use extension API (1.62 KB, patch)
2015-02-05 17:50 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
gl: Add more debugging notes (1.33 KB, patch)
2015-02-05 17:51 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
tests: Update testglarea (8.13 KB, patch)
2015-02-05 17:51 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
demos: Update the GtkGLArea demo code (9.31 KB, patch)
2015-02-05 17:51 UTC, Emmanuele Bassi (:ebassi)
none Details | Review

Description Pavol Klačanský 2014-12-24 18:02:19 UTC
The GLX attributes cannot be controlled currently when context is created.

These attributes should be configurable:
GLX_CONTEXT_MAJOR_VERSION_ARB
GLX_CONTEXT_MINOR_VERSION_ARB
GLX_CONTEXT_FLAGS_ARB
GLX_CONTEXT_PROFILE_MASK_ARB

Flags:
GLX_CONTEXT_DEBUG_BIT_ARB
GLX_CONTEXT_FORWARD_COMPATIBLE_BIT_ARB
Comment 1 Emmanuele Bassi (:ebassi) 2014-12-25 12:09:18 UTC
we don't have API to let you set the context attributes directly when creating a GL context for various reasons, mostly to avoid an explosion of API and to keep the GL context creation under our control.

we can probably come to some form of compromise in terms of what we can add to gdk_window_create_gl_context() and/or to GtkGLArea, but what I *really* don't want to have in the public API is an array of enumeration values and integers. it's not 1997 any more.

one option may be to have a platform-specific gdk_$BACKEND_gl_context_new_from_foreign() that lets you create a GdkGLContext implementation with a given platform-specific GL context that you created the way you want it.
Comment 2 Pavol Klačanský 2014-12-25 12:37:19 UTC
The setting of version number should be supported to avoid the need of Gtk rebuild every time a new OpenGL version is released.

The forward compatibility flag is important for Mac compatibility. The debug option can be similar to gtk_gl_area_set_has_debug.

Just to note, EXT are part of the core GL, so I had to remove it from function calls and enums.


(In reply to comment #1)
> it's not 1997 any more.
Yet, the default version of OpenGL is the legacy one.


>one option may be to have a platform-specific
>gdk_$BACKEND_gl_context_new_from_foreign() that lets you create a GdkGLContext
>implementation with a given platform-specific GL context that you created the
>way you want it.
I am not sure if this is an significant improvement over taking a drawing area's x window and creating context.
Comment 3 Emmanuele Bassi (:ebassi) 2014-12-25 20:18:24 UTC
(In reply to comment #2)
> The setting of version number should be supported to avoid the need of Gtk
> rebuild every time a new OpenGL version is released.

there aren't many OpenGL releases, and even if you request a specific GL version you have no guarantees that you'll get one — which means that you already need to code defensively in terms of extensions.

the main reason we have a LEGACY/CORE_3_2 split is because of the change in behaviour between the two profiles, not because of the change in extensions 3.2 provides as core functionality.

> The forward compatibility flag is important for Mac compatibility.

care to explain this bit?

> The debug
> option can be similar to gtk_gl_area_set_has_debug.
> 
> Just to note, EXT are part of the core GL, so I had to remove it from function
> calls and enums.
> 
> 
> (In reply to comment #1)
> > it's not 1997 any more.
> Yet, the default version of OpenGL is the legacy one.

that has no bearing on the type of API we expose to the developer. we're also checking for Core 3.2 contexts first internally.
 
> >one option may be to have a platform-specific
> >gdk_$BACKEND_gl_context_new_from_foreign() that lets you create a GdkGLContext
> >implementation with a given platform-specific GL context that you created the
> >way you want it.
> I am not sure if this is an significant improvement over taking a drawing
> area's x window and creating context.

that's entirely *not* the same *at all*. the GL capabilities inside GDK are fully integrated with the GDK frame cycle and drawing API; you cannot just create a GL context and draw on the backing X11 window of a GdkWindow — if you actually try that, you'll see that it breaks pretty badly.
Comment 4 Pavol Klačanský 2014-12-25 20:39:35 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > The setting of version number should be supported to avoid the need of Gtk
> > rebuild every time a new OpenGL version is released.
> 
> there aren't many OpenGL releases, and even if you request a specific GL
> version you have no guarantees that you'll get one — which means that you
> already need to code defensively in terms of extensions.
> 
> the main reason we have a LEGACY/CORE_3_2 split is because of the change in
> behaviour between the two profiles, not because of the change in extensions 3.2
> provides as core functionality.
I am mostly the sole user, so this is non-issue. It is easier to use a minimum version as requirement than to check all extensions.


> 
> > The forward compatibility flag is important for Mac compatibility.
> 
> care to explain this bit?
Mac supports only forward compatible profile from GL 3.2.

> 
> > The debug
> > option can be similar to gtk_gl_area_set_has_debug.
> > 
> > Just to note, EXT are part of the core GL, so I had to remove it from function
> > calls and enums.
> > 
> > 
> > (In reply to comment #1)
> > > it's not 1997 any more.
> > Yet, the default version of OpenGL is the legacy one.
> 
> that has no bearing on the type of API we expose to the developer. we're also
> checking for Core 3.2 contexts first internally.
I apologize. Why does gtk_gl_area uses internally EXT type of extension? (this requires renaming the functions when core is used)

> 
> > >one option may be to have a platform-specific
> > >gdk_$BACKEND_gl_context_new_from_foreign() that lets you create a GdkGLContext
> > >implementation with a given platform-specific GL context that you created the
> > >way you want it.
> > I am not sure if this is an significant improvement over taking a drawing
> > area's x window and creating context.
> 
> that's entirely *not* the same *at all*. the GL capabilities inside GDK are
> fully integrated with the GDK frame cycle and drawing API; you cannot just
> create a GL context and draw on the backing X11 window of a GdkWindow — if you
> actually try that, you'll see that it breaks pretty badly.
How does it break? I just disabled double buffering to avoid flicker and had no problems otherwise. It was however tedious to set up. Now it is easier, I just remove EXT and _EXT, add version, debug and forward flag and build gtk. However it would be even better to avoid that step.
Comment 5 Emmanuele Bassi (:ebassi) 2014-12-25 23:01:35 UTC
(In reply to comment #4)

> > > The forward compatibility flag is important for Mac compatibility.
> > 
> > care to explain this bit?
> Mac supports only forward compatible profile from GL 3.2.

thanks; I guess we'll have to ensure this is always set for the MacOS implementation.

> I apologize. Why does gtk_gl_area uses internally EXT type of extension? (this
> requires renaming the functions when core is used)

no, it does not. the EXT_* are kept as aliases as per specification. also, libepoxy shields us from a lot of this stuff.
 
> > that's entirely *not* the same *at all*. the GL capabilities inside GDK are
> > fully integrated with the GDK frame cycle and drawing API; you cannot just
> > create a GL context and draw on the backing X11 window of a GdkWindow — if you
> > actually try that, you'll see that it breaks pretty badly.
> How does it break? I just disabled double buffering to avoid flicker and had no
> problems otherwise.

you cannot portably disable double buffering (e.g. it won't work on Wayland, or on any other windowing system that relies on double buffering), and the flickering is precisely how it breaks. the GDK OpenGL code works, portably, with double buffering.

> It was however tedious to set up. Now it is easier, I just
> remove EXT and _EXT, add version, debug and forward flag and build gtk. However
> it would be even better to avoid that step.

being able to create your own GL context with your own flags, and then creating a GdkGLContext instance out of it would not negate that; it would only side-step the GL context creation inside GDK, but the rest would work precisely as it does now.
Comment 6 Pavol Klačanský 2014-12-25 23:33:38 UTC
(In reply to comment #5)

> thanks; I guess we'll have to ensure this is always set for the MacOS
> implementation.
It would not hurt to set it everywhere as it disables only wide lines (which are not guaranteed anyway).

> no, it does not. the EXT_* are kept as aliases as per specification. also,
> libepoxy shields us from a lot of this stuff.
Can you try to get 3.3 core context and check if it works, please? It did not work automatically on my machine and I would like to know what I need to change.


> being able to create your own GL context with your own flags, and then creating
> a GdkGLContext instance out of it would not negate that; it would only
> side-step the GL context creation inside GDK, but the rest would work precisely
> as it does now.
Fair enough, can I expect this being in the next Gtk release? GLFW, Qt offers version, debug, multisampling settings - and there is a chance users will require these settings.
Comment 7 Niels Nesse 2015-01-14 07:45:13 UTC
*** Bug 742893 has been marked as a duplicate of this bug. ***
Comment 8 Niels Nesse 2015-01-14 08:11:21 UTC
I filed a bug a bug similar to this (742893) just now which I just closed as duplicate.

I think that all context creation attributes shared by all supported platforms (especially the context version) should be exposed to the user. A way to externally create the context will work for me but then you may just get many developers implementing the same context creation abstraction in order to control of the GL version. Moreover some people will probably take the limitation as a reason to use a different API.
Comment 9 Niels Nesse 2015-01-14 20:24:44 UTC
I've reread this thread and did some investigation as to how contexts are created by GdkGLContext and the implementation seems to reflect a misinterpretation of the GLX_ARB_create_context extension.

At present the code to create a 3.2 context sets the core profile bit and nothing else. The core profile bit is default in any case so the invocation is equivalent to passing no attributes at all. Since no version is specified the default version of 1.0 will be assumed and the driver will return a context that is backwards compatible OpenGL 1.0. The "ARB_create_context" spec states "If the requested OpenGL version is less than 3.2, GLX_CONTEXT_PROFILE_MASK_ARB is ignored and the functionality of the context is determined solely by the requested version." So if the driver is implemented correctly the "3_2_CORE" profile option will create an arbitrary legacy context.

I tried adding the flags to request an actual 3.2 context and I got a totally black window. Some part of the code must have legacy dependencies. I'm trying to investigate myself but I think this issue needs a priority bump as well.

I also wanted to address a comment made early on in this thread:

>here aren't many OpenGL releases, and even if you request a specific GL
>version you have no guarantees that you'll get one

The "ARB_create_context" spec guarantees you will get at least get a context that is backwards compatible with the version you requested and my experience so far has been that it gives me the exact version. Yes you should still make sure that you query for any extensions you use but I don't see how this makes creating core contexts >3.2 not worth supporting. I would like to create newer contexts for two reasons:

1) To keep my code free of features that have been deprecated. Features are usually removed because they have no efficient mapping in hardware or more flexible ways to get the same functionality have been provided. Using newer core contexts helps me to avoid these pitfalls and write better GL code.

2) To experiment with the 4.x API series without having to query for a long list of extensions on top of 3.2.
Comment 10 Pavol Klačanský 2015-01-18 15:01:34 UTC
Is there any reason for supporting pre-core feature set? Would it be more maintainable to support only core profile and >= GL 3.2? (Mesa, OS X support only this)
Comment 11 Emmanuele Bassi (:ebassi) 2015-01-28 12:05:49 UTC
Created attachment 295645 [details] [review]
GL: Split GL context creation in two phases

One of the major requests by OpenGL users has been the ability to
specify settings when creating a GL context, like the version to use
or whether the debug support should be enabled.

We have a couple of requirements in terms of API:

 • avoid, if at all possible, the "C arrays of integers with
   attribute, value pairs", which are hard to write and hard
   to bind in non-C languages.
 • allow failing in a recoverable way.
 • do not make the GL context creation API a mess of arguments.

Looking at prior art, it seems that a common pattern is to split the
construction phase in two:

 • a first phase that creates a GL context wrapper object and
   does preliminary checks on the environment.
 • a second phase that creates the backend-specific GL object.

We adopted a similar pattern:

 • gdk_window_create_gl_context() creates a GdkGLContext
 • gdk_gl_context_realize() creates the underlying resources

Calling gdk_gl_context_make_current() also realizes the context, so
simple GL users do not need to care. Advanced users will want to
call gdk_window_create_gl_context(), set up the optional requirements,
and then call gdk_gl_context_realize(). If either of these two steps
fails, it's possible to recover by changing the requirements, or simply
creating a new GdkGLContext instance.
Comment 12 Emmanuele Bassi (:ebassi) 2015-01-28 12:05:54 UTC
Created attachment 295646 [details] [review]
gl: Add context options

Users of the GdkGLContext API should be allowed to set properties on the
shim GdkGLContext instance prior to realization, so that the
backend-specific implementation can use the value of those properties
when creating the windowing system specific resources.

The main three options are:

 • a major/minor version tuple, to request a specific GL version
 • a debug bit, to request a "debug context", which provides additional
   validation and run time checking
 • a forward compatibility bit, to request a context that does not
   have deprecated functionality

See also:
 - https://www.opengl.org/registry/specs/ARB/glx_create_context.txt
Comment 13 Emmanuele Bassi (:ebassi) 2015-01-28 12:05:59 UTC
Created attachment 295647 [details] [review]
x11/gl: Use the GdkGLContext options

When creating an OpenGL context using the glXCreateContextAttribs()
function.
Comment 14 Emmanuele Bassi (:ebassi) 2015-01-28 12:08:52 UTC
after discussing this topic with Alex at the DX hackfest, this is the solution we came up with.

current users of GdkGLContext do not need to do anything; users that wish to specify options to the GL context, can create the GdkGLContext, set the options on it, and call gdk_gl_context_realize() explicitly. calling make_current() will implicitly realize the GdkGLContext first.
Comment 15 Emmanuele Bassi (:ebassi) 2015-01-28 12:10:35 UTC
we also discussed dropping the legacy GL profile altogether, and given that Mesa seems to not support a shared core-3.2/legacy profiles scenario, it seems that dropping legacy is the only way for us to move forward. this will require changing the example code we ship, but it shouldn't be terrible.
Comment 16 Emmanuele Bassi (:ebassi) 2015-01-28 12:40:17 UTC
Created attachment 295657 [details] [review]
GL: Split GL context creation in two phases

One of the major requests by OpenGL users has been the ability to
specify settings when creating a GL context, like the version to use
or whether the debug support should be enabled.

We have a couple of requirements in terms of API:

 • avoid, if at all possible, the "C arrays of integers with
   attribute, value pairs", which are hard to write and hard
   to bind in non-C languages.
 • allow failing in a recoverable way.
 • do not make the GL context creation API a mess of arguments.

Looking at prior art, it seems that a common pattern is to split the
construction phase in two:

 • a first phase that creates a GL context wrapper object and
   does preliminary checks on the environment.
 • a second phase that creates the backend-specific GL object.

We adopted a similar pattern:

 • gdk_window_create_gl_context() creates a GdkGLContext
 • gdk_gl_context_realize() creates the underlying resources

Calling gdk_gl_context_make_current() also realizes the context, so
simple GL users do not need to care. Advanced users will want to
call gdk_window_create_gl_context(), set up the optional requirements,
and then call gdk_gl_context_realize(). If either of these two steps
fails, it's possible to recover by changing the requirements, or simply
creating a new GdkGLContext instance.
Comment 17 Emmanuele Bassi (:ebassi) 2015-01-28 12:40:20 UTC
Created attachment 295658 [details] [review]
gl: Add context options

Users of the GdkGLContext API should be allowed to set properties on the
shim GdkGLContext instance prior to realization, so that the
backend-specific implementation can use the value of those properties
when creating the windowing system specific resources.

The main three options are:

 • a major/minor version tuple, to request a specific GL version
 • a debug bit, to request a "debug context", which provides additional
   validation and run time checking
 • a forward compatibility bit, to request a context that does not
   have deprecated functionality

See also:
 - https://www.opengl.org/registry/specs/ARB/glx_create_context.txt
Comment 18 Emmanuele Bassi (:ebassi) 2015-01-28 12:40:24 UTC
Created attachment 295659 [details] [review]
x11/gl: Use the GdkGLContext options

When creating an OpenGL context using the glXCreateContextAttribs()
function.
Comment 19 Emmanuele Bassi (:ebassi) 2015-01-28 12:40:28 UTC
Created attachment 295660 [details] [review]
wayland/gl: Use the GdkGLContext options
Comment 20 Alexander Larsson 2015-01-28 12:42:57 UTC
Review of attachment 295645 [details] [review]:

::: gdk/gdkwindow.c
@@ +2760,3 @@
+  gdk_gl_context_realize (window->impl_window->gl_paint_context, error);
+  if (error != NULL && *error != NULL)
+    g_clear_object (&(window->impl_window->gl_paint_context));

Doesn't this have to use a local error to ensure we clear the pointer if the realize fails, indepenent of whether the user passed in a *error or not.

@@ +2778,3 @@
  *
+ * Before using the returned #GdkGLContext, you will need to
+ * call gdk_gl_context_realize().

Thats not quite true, since we realize it in make_current.
Comment 21 Emmanuele Bassi (:ebassi) 2015-01-28 12:52:28 UTC
Created attachment 295662 [details] [review]
GL: Split GL context creation in two phases

One of the major requests by OpenGL users has been the ability to
specify settings when creating a GL context, like the version to use
or whether the debug support should be enabled.

We have a couple of requirements in terms of API:

 • avoid, if at all possible, the "C arrays of integers with
   attribute, value pairs", which are hard to write and hard
   to bind in non-C languages.
 • allow failing in a recoverable way.
 • do not make the GL context creation API a mess of arguments.

Looking at prior art, it seems that a common pattern is to split the
construction phase in two:

 • a first phase that creates a GL context wrapper object and
   does preliminary checks on the environment.
 • a second phase that creates the backend-specific GL object.

We adopted a similar pattern:

 • gdk_window_create_gl_context() creates a GdkGLContext
 • gdk_gl_context_realize() creates the underlying resources

Calling gdk_gl_context_make_current() also realizes the context, so
simple GL users do not need to care. Advanced users will want to
call gdk_window_create_gl_context(), set up the optional requirements,
and then call gdk_gl_context_realize(). If either of these two steps
fails, it's possible to recover by changing the requirements, or simply
creating a new GdkGLContext instance.
Comment 22 Emmanuele Bassi (:ebassi) 2015-01-28 12:52:33 UTC
Created attachment 295663 [details] [review]
gl: Add context options

Users of the GdkGLContext API should be allowed to set properties on the
shim GdkGLContext instance prior to realization, so that the
backend-specific implementation can use the value of those properties
when creating the windowing system specific resources.

The main three options are:

 • a major/minor version tuple, to request a specific GL version
 • a debug bit, to request a "debug context", which provides additional
   validation and run time checking
 • a forward compatibility bit, to request a context that does not
   have deprecated functionality

See also:
 - https://www.opengl.org/registry/specs/ARB/glx_create_context.txt
Comment 23 Emmanuele Bassi (:ebassi) 2015-01-28 12:52:38 UTC
Created attachment 295664 [details] [review]
x11/gl: Use the GdkGLContext options

When creating an OpenGL context using the glXCreateContextAttribs()
function.
Comment 24 Emmanuele Bassi (:ebassi) 2015-01-28 12:52:43 UTC
Created attachment 295665 [details] [review]
wayland/gl: Use the GdkGLContext options
Comment 25 Alexander Larsson 2015-01-28 14:22:51 UTC
Review of attachment 295646 [details] [review]:

::: gdk/gdkglcontextprivate.h
@@ +78,3 @@
+                                                                 int            *major,
+                                                                 int            *minor);
+gboolean                gdk_gl_context_get_debug_enabled        (GdkGLContext   *context);

Why are these private? Seems like they could be somewhat useful in 3rd party code.
Comment 26 Fan, Chun-wei 2015-01-29 17:29:35 UTC
Hi,

I think I might need to jump into this as probably work will need to be done to the Windows backend as well, so changing the OS and component tags, for records.

With blessings.
Comment 27 Fan, Chun-wei 2015-01-30 05:57:17 UTC
Created attachment 295789 [details] [review]
GDK-Win32: Split GL Context Creation in 2 Phases

Hi,

This is a follow up patch when we split the GL context creation in 2 phases, for Windows, which would depend on Emmanuele's patch for that (currently attachment 295662 [details] [review]).  I will do another patch for using the GdkGLContext options a bit later.

With blessings, thank you!
Comment 28 Fan, Chun-wei 2015-01-30 06:34:04 UTC
Created attachment 295791 [details] [review]
GDK-Win32: Use the GdkCLContext options

Hi,

This is the add-on patch to GDK-Win32 that would add support to use GdkGLContext options, which would depend on the patch in GDK-Win32 to split GL Context creation in 2 phases (currently attachment 295789 [details] [review]).

With blessings, thank you!
Comment 29 Emmanuele Bassi (:ebassi) 2015-02-05 16:45:48 UTC
I've updated all patches in the wip/ebassi/gl-context-realize branch; need to apply Fan's patches there as well.

the branch includes dropping the legacy compatibility profile, and updates the examples. it's missing the GdkGLGears port to modern GL (because I forgot about it), but I'll try to get to it ASAP.

I tested the testglarea and the gtk3-demo code, and they both work fine. I'll have a look at Gthree, to see if it needs changing.
Comment 30 Emmanuele Bassi (:ebassi) 2015-02-05 17:47:33 UTC
Created attachment 296217 [details] [review]
GL: Split GL context creation in two phases

One of the major requests by OpenGL users has been the ability to
specify settings when creating a GL context, like the version to use
or whether the debug support should be enabled.

We have a couple of requirements in terms of API:

 • avoid, if at all possible, the "C arrays of integers with
   attribute, value pairs", which are hard to write and hard
   to bind in non-C languages.
 • allow failing in a recoverable way.
 • do not make the GL context creation API a mess of arguments.

Looking at prior art, it seems that a common pattern is to split the
construction phase in two:

 • a first phase that creates a GL context wrapper object and
   does preliminary checks on the environment.
 • a second phase that creates the backend-specific GL object.

We adopted a similar pattern:

 • gdk_window_create_gl_context() creates a GdkGLContext
 • gdk_gl_context_realize() creates the underlying resources

Calling gdk_gl_context_make_current() also realizes the context, so
simple GL users do not need to care. Advanced users will want to
call gdk_window_create_gl_context(), set up the optional requirements,
and then call gdk_gl_context_realize(). If either of these two steps
fails, it's possible to recover by changing the requirements, or simply
creating a new GdkGLContext instance.
Comment 31 Emmanuele Bassi (:ebassi) 2015-02-05 17:47:59 UTC
Created attachment 296218 [details] [review]
gl: Add context options

Users of the GdkGLContext API should be allowed to set properties on the
shim GdkGLContext instance prior to realization, so that the
backend-specific implementation can use the value of those properties
when creating the windowing system specific resources.

The main three options are:

 • a major/minor version tuple, to request a specific GL version
 • a debug bit, to request a "debug context", which provides additional
   validation and run time checking
 • a forward compatibility bit, to request a context that does not
   have deprecated functionality

See also:
 - https://www.opengl.org/registry/specs/ARB/glx_create_context.txt
Comment 32 Emmanuele Bassi (:ebassi) 2015-02-05 17:48:17 UTC
Created attachment 296219 [details] [review]
x11/gl: Use the GdkGLContext options

When creating an OpenGL context using the glXCreateContextAttribs()
function.
Comment 33 Emmanuele Bassi (:ebassi) 2015-02-05 17:48:36 UTC
Created attachment 296220 [details] [review]
wayland/gl: Use the GdkGLContext options
Comment 34 Emmanuele Bassi (:ebassi) 2015-02-05 17:49:10 UTC
Created attachment 296221 [details] [review]
GDK-Win32: Split GL context creation in two phases

Like what is being done in the X11 and Wayland backends, create the
GdkWin32GLContext in 2 steps, where we only create the actual WGL context
in _gdk_win32_gl_context_realize().
Comment 35 Emmanuele Bassi (:ebassi) 2015-02-05 17:49:29 UTC
Created attachment 296222 [details] [review]
win32/gl: Use the GdkGLContext options
Comment 36 Emmanuele Bassi (:ebassi) 2015-02-05 17:49:54 UTC
Created attachment 296223 [details] [review]
Switch GDK_GL_PROFILE_DEFAULT to mean 3_2_CORE

Instead of LEGACY.
Comment 37 Emmanuele Bassi (:ebassi) 2015-02-05 17:50:10 UTC
Created attachment 296224 [details] [review]
gl: Move getters for context options to the public API

They can be useful for third party code as well.
Comment 38 Emmanuele Bassi (:ebassi) 2015-02-05 17:50:25 UTC
Created attachment 296225 [details] [review]
gl: Drop OpenGL legacy profile

We simply don't want to care about legacy OpenGL.

All supported platforms also have support for OpenGL ≥ 3.2; it would
complicate the internal code; and would force us to use legacy GL
contexts internally if the first context created by the user is a legacy
GL context, and disable creation of core-3.2 contexts after that.

We will need to fix all our code examples to use the Core 3.2 profile.
Comment 39 Emmanuele Bassi (:ebassi) 2015-02-05 17:50:41 UTC
Created attachment 296226 [details] [review]
gl: Do not use the extension API for core GL

Since we are using a Core GL profile, we need to drop the
extension-based API.
Comment 40 Emmanuele Bassi (:ebassi) 2015-02-05 17:50:56 UTC
Created attachment 296227 [details] [review]
glarea: Do not use extension API

We are using GL contexts with Core GL profiles, so we need to use the
proper API, not the one provided by extensions.
Comment 41 Emmanuele Bassi (:ebassi) 2015-02-05 17:51:19 UTC
Created attachment 296228 [details] [review]
gl: Add more debugging notes
Comment 42 Emmanuele Bassi (:ebassi) 2015-02-05 17:51:33 UTC
Created attachment 296229 [details] [review]
tests: Update testglarea

Since we dropped the legacy OpenGL compatibility profile, we need to use
recent OpenGL APi and concepts. This also means that the example code
gets a tad more complicated.
Comment 43 Emmanuele Bassi (:ebassi) 2015-02-05 17:51:46 UTC
Created attachment 296230 [details] [review]
demos: Update the GtkGLArea demo code

Same way we updated the testglarea test code.
Comment 44 Fan, Chun-wei 2015-02-06 04:56:48 UTC
Hi,

Somehow, on Windows, when I was running the gdkgears program (in tests), the image of the gears are not showing up, neither are the images that I tried to use in Alex's GThree demo programs.  Wondering whether I missed something?

p.s. The testglarea program and the GtkGLArea demo
     code works fine for me, on Windows, with the updates
     in this bug.

With blessings, thank you!
Comment 45 Emmanuele Bassi (:ebassi) 2015-02-06 10:00:47 UTC
(In reply to comment #44)
> Hi,
> 
> Somehow, on Windows, when I was running the gdkgears program (in tests), the
> image of the gears are not showing up, neither are the images that I tried to
> use in Alex's GThree demo programs.  Wondering whether I missed something?

no, it's correct. both GtkGears and Gthree need to be fixed not to use the legacy GL API. I've already started fixing GtkGears by using the es2gears implementation that comes with Mesa. I'll also have a look at Gthree soon.

> p.s. The testglarea program and the GtkGLArea demo
>      code works fine for me, on Windows, with the updates
>      in this bug.

unlike GtkGears and Gthree, I ported the testglarea code (which is also what the GLArea widget demo uses) to use OpenGL ≥ 3.2 API, so if they work on Windows it's a confirmation that the approach is correct. :-)

thanks for testing!
Comment 46 Fan, Chun-wei 2015-02-06 11:09:14 UTC
Hello Emmanuele,

Nice to hear that! :)

With blessings, thank you!
Comment 47 Emmanuele Bassi (:ebassi) 2015-02-10 00:06:15 UTC
while Bugzilla was down, I pushed the latest version of the patches to add gdk_gl_context_realize() to master, as well as the patches that remove the support for legacy OpenGL profiles:

6bf55142 demo: Change the resource path for the shaders
843475bd gl: Drop GdkGLContextClass.upload_texture()
3b4bf963 demo: Move the GLSL shaders to resources
2d35053d gl: Clean up the required version accessors
4ad887f4 docs: We do not support non-core GL profiles
601c49ef gl: Clean up pre-requisite checks for GdkGLContext setters
2d9081d1 wayland/gl: Ensure we use the 3.2 core profile
cc45e828 x11/gl: Ensure we use the 3.2 core profile
01d1cdc7 demos: Update the GtkGLArea demo code
b87034a9 tests: Update testglarea
1d3cc65e gl: Add more debugging notes
5a3b28aa glarea: Do not use extension API
395125bc gl: Do not use the extension API for core GL
4b8b3b43 gl: Drop OpenGL legacy profile
6aaa6c33 gl: Move getters for context options to the public API
f7497dae Switch GDK_GL_PROFILE_DEFAULT to mean 3_2_CORE
4c091db6 win32/gl: Use the GdkGLContext options
ba56f117 GDK-Win32: Split GL context creation in two phases
42a895e8 wayland/gl: Use the GdkGLContext options
3425f7fe x11/gl: Use the GdkGLContext options
fa900522 gl: Add context options
22e6f37c GL: Split GL context creation in two phases

I'm closing this bug; any other missing functionality should get its own bug report.

thanks for the feedback on the API.
Comment 48 Fan, Chun-wei 2015-02-10 08:26:38 UTC
Hi,

(In reply to Emmanuele Bassi (:ebassi) from comment #47)
> ...
> 2d9081d1 wayland/gl: Ensure we use the 3.2 core profile
> cc45e828 x11/gl: Ensure we use the 3.2 core profile

For records, I have updated GDK-Win32 backend to do likewise, to ensure that we use the 3.2 core profile, in commit 60195ab.

With blessings.
Comment 49 Alexander Larsson 2015-02-12 11:58:02 UTC
Pavol: Does these changes satisfy your needs?
Comment 50 Pavol Klačanský 2015-02-12 12:18:01 UTC
Yes, thank you. Few questions:

* Why do we keep default profile flag?
* Can you update the documentation please?
Comment 51 Emmanuele Bassi (:ebassi) 2015-02-12 12:29:02 UTC
(In reply to Pavol Klačanský from comment #50)
> Yes, thank you. Few questions:
> 
> * Why do we keep default profile flag?

the profile flag is for future expansion; see bug 743746 for implementing a GLES profile on systems that support both desktop GL and GLES. I'm currently removing the profile argument from the gdk_window_create_gl_context() API, though, and moving it to be a property like the debug bit, the forward compatibility bit, and the version.

> * Can you update the documentation please?

could you please file a new bug with the documentation that should be updated? thanks!
Comment 52 Pavol Klačanský 2015-02-12 13:15:23 UTC
Thank you, bug report is here #744394.