GNOME Bugzilla – Bug 707723
Add support for GTK+3 to gktglarea
Last modified: 2016-01-04 10:21:30 UTC
Created attachment 254399 [details] configureac.patch Please consider the following patch(es), which add support to GTK+3 to GtkGLArea. They add the following : - support to "--with-gtk=X" parameter for configure script. Default is 2, can be 3. - conditional code support using #ifdefs (may be dirty, looking for advice on this topic). Please note that there is some API breakage due to the "GtkPixmap" object being replaced by "GtkPixbuf". It may be unavoidable ; writing a wrapper class is possible, but does seem bad practice and confusing to me. - the "simple" examples is migrated. If this bug is considered, I will migrate all examples, and fix remaining holes in migrated code. Here's how to test with GTK+3 : patch -p0 < configureac.patch patch -p0 < gtkglarea3.patch patch -P0 < examples-simple.patch ./configure --with-gtk=3 make (fails on some examples) make install cd examples ./simple Example should display "GTK+3.X.X" and a GL triangle.
Created attachment 254400 [details] [review] examples-simple.patch
Created attachment 254401 [details] [review] gtkglarea3.patch
Review of attachment 254401 [details] [review]: Can't say anything about the functionality, but gave the code a quick glance and poked at some issues for you to look at. The big wonder for me is why bother with the code shredding #ifdefs? It doesn't appear GtkGlArea is being actively maintained by anyone, so if you're going to do the work, you're probably going to end up being the maintainer as well... do you really want to maintain GTK2 compatibility for the library looking forward? ::: gtkgl/gtkglarea.h @@ -23,2 +23,4 @@ #include <gdk/gdk.h> #include <gtkgl/gdkgl.h> + +#if defined __GTK2__ This is pointless, just use #include <gtk/gtk.h> for both. GTK 2 allows for single header includes. @@ +190,1 @@ if (GTK_OBJECT_CLASS (parent_class)->destroy) The "destroy" vfunc was moved to GtkWidgetClass since it would have been harder to remove (and was basically the only thing keeping GtkObjectClass around). The code should chain up to it instead. @@ +18,3 @@ */ +// Stray comment.
(In reply to comment #3) > Do you really want to maintain GTK2 compatibility for the library looking forward? As for now, yes. > This is pointless, just use #include <gtk/gtk.h> for both. GTK 2 allows for > single header includes. > The "destroy" vfunc was moved to GtkWidgetClass since it would have been harder to remove (and was basically the only thing keeping GtkObjectClass around). The code should chain up to it instead. Many thanks for the tips ! Applied :-)
Created attachment 255532 [details] [review] gtkglarea3.patch Sorry for the delay, have been busy. Look at what we have here : a patch covering all source code + examples ! Here's how it looks like now : Win32 (GTK+ 3.6.4) : http://www.tarnyko.net/repo/gtkglarea3-win32.png Linux (GTK+ 3.4.2) : http://www.tarnyko.net/repo/gtkglarea3-linux.png The default "configure" script behaviour has been changed to look for GTK+3 by default, since it is now the most popular version. GTK+2 must be invoked explicitly. Win32 implementation is basically complete ; I mean it works just as well as before. Linux implementaton gave me a hard time ; GdkX11Drawables have been deprecated in favor of Cairo, came up with a fix (glpixmap sample) ; however for GdkFonts which have been replaced by PangoFonts (gdkfont sample), I need some functions which aren't in the backend anymore (http://harmattan-dev.nokia.com/docs/platform-api-reference/xml/daily-docs/pango/pango-X-Fonts-and-Rendering.html). Will look at it, but not immediately. Please consider the new patch.
Hi, any news on this ? Thanks.
Hi. Sorry for the delay. New job. I will try to fit this in this week.
"New job". Me too :-). No worries, do this when you have time.
These changes have been accepted in to Debian, it seems. I'm behind the 8 ball here. I have done a quick review of the code. It seems like it would reduce the margin for error if we defined a macro, for instance GTKGL_WIDGET_CLASS which specifies the class to use based on whether __GTK2__ or __GTK3__ have been defined. We would then use that macro throughout the code where we alternate between GtkWidget and GtkObject So instead of == BEGIN == #if defined __GTK2__ static void gtk_gl_area_destroy (GtkObject *object); #elif defined __GTK3__ static void gtk_gl_area_destroy (GtkWidget *object); #endif == END == we would instead use == BEGIN == #if defined __GTK2__ #define GTKGL_WIDGET_CLASS GtkObject #elif defined __GTK3__ #define GTKGL_WIDGET_CLASS GtkWidget #endif static void gtk_gl_area_destroy (GTKGL_WIDGET_CLASS *object); == END == I really prefer to keep the conditionals to a minimum. It reduces the duplicated code and thereby the margin for human error. I am also a little concerned that there is no code path for the case where neither __GTK2__ or __GTK3__ are defined. Say we have a legacy system running libraries prior to gtk+ v2, or say this code sticks around long enough that we move on to gtk+ v4... the code in its current state doesn't look like it would work in those cases. Would you be willing and able to make the above adjustments to your patch? Thanks for your hard work! C.J.
(In reply to comment #9) > I have done a quick review of the code. It seems like it would reduce the > margin for error if we defined a macro, for instance GTKGL_WIDGET_CLASS which > specifies the class to use based on whether __GTK2__ or __GTK3__ have been > defined. We would then use that macro throughout the code where we alternate > between GtkWidget and GtkObject It makes sense -and looks definitively nicer. Will do this. > I am also a little concerned that there is no code path for the case where > neither __GTK2__ or __GTK3__ are defined. Say we have a legacy system running > libraries prior to gtk+ v2, or say this code sticks around long enough that we > move on to gtk+ v4... the code in its current state doesn't look like it would > work in those cases. Well, currently the configure script looks for "gtk+-2.0" or "gtk+-3.0" ; looking for "gtk+-$value.0" may be possible, but have to check AC syntax. If it is possible, I think we should default to the most recent implementation (i.e. __GTK3__), as it clearly won't work with very old 1.x, but it *may* work with 4.x or higher. Configure script will say "can't find supported version, trying $VERSION" and "$VERSION" will be __GTK3__ for now. We could change it later. > Would you be willing and able to make the above adjustments to your patch? Sure ! Will work on this and let you know.
Created attachment 275303 [details] [review] gtkglarea3.patch Hi, As promised, here is a new version of the patch, which uses some #defines in the headers so the lib code is more readable. The examples still have #ifdefs, since most of their code is not wrappable. Additionally, it now targets the latest release of GtkGLArea (2.1.0) which happened a few days ago (https://mail.gnome.org/archives/gtk-devel-list/2014-April/msg00015.html). To try it : patch -p0 < gtkglarea3.patch autoreconf --install ./configure --with-gtk=3 (still defaults to GTK+2)
Hi, thanks a lot for your work on this. Sorry we duplicate work here, but Ive merged the GTK+3 port Ive done in master now. Nevertheless some bits are missing if you are interesed on adding them (mainly the GdkFont related and pixman related api, as they are gone in GTK+3)
GTK+ 3.16 will have OpenGL support by default, and a GtkGLArea widget which supersedes the GtkGLArea library. I think this bug can be closed.
The code in this patch, like the original gtkglext GTK2 widget it is based on, does support win32 (it can run on OSX too in GTK2). AFAICT, the code in GTK+ 3.16 and later does not. I don't think this should be closed as RESOLVED FIXED.
(In reply to Antoine Martin from comment #14) > The code in this patch, like the original gtkglext GTK2 widget it is based > on, does support win32 (it can run on OSX too in GTK2). > AFAICT, the code in GTK+ 3.16 and later does not. Windows is supported. OSX currently isn't because of Cairo bugs on Retina displays. Those bugs are in the process of being fixed - and after that, GdkGLContext drawing will work on OSX as well. > I don't think this should be closed as RESOLVED FIXED. It can be marked as "OBSOLETE": even if GtkGLArea (the library) gets renamed to avoid symbols collisions with GTK+ >= 3.16, using the old approach of "create a native window and hope that disabling double buffering will end up drawing something" is not something GTK+ developers are interested in supporting any more. It's unportable, wildly inefficient as it disables various optimizations, and it also prevents using things like overlaid and transparent widgets. As far as anybody is concerned, porting any external library dealing with OpenGL, based on assumptions targeting GTK+ 2.x, to GTK+ 3.x is a fool's errand. If you're using GTK+ 3.x and OpenGL you should depend on GTK+ >= 3.16 and use the OpenGL support inside GTK+ itself.