Re: [GtkGLExt] GObject Introspection



Hi,

Thank you for the patch!

You can find a review below. Please address the points and I'll put your
patch into my tree. For submitting your patch and writing a commit
message, please follow the guide at [1].


> === modified file 'Makefile.am'
> --- Makefile.am 2012-03-13 20:40:48 +0000
> +++ Makefile.am 2012-05-22 09:22:18 +0000
> @@ -4,7 +4,7 @@
>  SUBDIRS = gdk gtk docs examples
>  
>  ACLOCAL_AMFLAGS = -I m4macros
> -DISTCHECK_CONFIGURE_FLAGS = --enable-gtk-doc
> +DISTCHECK_CONFIGURE_FLAGS = --enable-gtk-doc --enable-introspection
>  
>  EXTRA_DIST = \
>         README.win32                    \
> 
> === modified file 'configure.ac'
> --- configure.ac        2012-03-13 20:40:48 +0000
> +++ configure.ac        2012-05-22 09:22:18 +0000
> @@ -114,6 +114,12 @@
>  AC_PATH_PROGS([PERL], [perl5 perl])
>  
>  ##################################################
> +# GObject introspection
> +##################################################
> +
> +GOBJECT_INTROSPECTION_CHECK([1.30.0])
> +
> +##################################################
>  # Checks for gtk-doc and docbook-tools
>  ##################################################
>  
> 
> === modified file 'gdk/Makefile.am'
> --- gdk/Makefile.am     2012-03-26 18:49:10 +0000
> +++ gdk/Makefile.am     2012-05-22 09:22:18 +0000
> @@ -4,6 +4,8 @@
>  SUBDIRS = $(GDKGLEXT_BACKENDS)
>  DIST_SUBDIRS = x11 win32
>  
> +CLEANFILES =
> +

Why is this empty? Either this is missing a value, or it shouldn't be
here at all. If you need to clear CLEANFILES here, put a comment above
that says why.

> 
>  EXTRA_DIST = \
>         gdkglversion.h.in       \
>         gdkglext.def
> @@ -177,5 +179,49 @@
>    gtkglenumtypes.c.template
>  
>  #
> +# Introspection
> +#
> +
> +-include $(INTROSPECTION_MAKEFILE)
> +INTROSPECTION_GIRS =
> +INTROSPECTION_SCANNER_ARGS =
> +INTROSPECTION_COMPILER_ARGS = --includedir=$(srcdir)
> +
> +if HAVE_INTROSPECTION
> +introspection_sources = \
> +       $(filter-out gdkgldebug.h gdkglglext.h, $(gdkglext_headers)) \
> +       $(gdkglext_c_sources) \
> +       $(gdkglext_built_c_sources)
> +
> +if USE_X11
> +introspection_sources += \
> +       x11/gdkglconfig-x11.c   \
> +       x11/gdkglcontext-x11.c  \
> +       x11/gdkglquery-x11.c    \
> +       x11/gdkglwindow-x11.c
> +endif # USE_X11
> +
> +GdkGLExt-1.0.gir: $(gdkglext_targetlib)
> +GdkGLExt_1_0_gir_INCLUDES = Gdk-3.0
> +GdkGLExt_1_0_gir_SCANNERFLAGS = \
> +       --warn-all \
> +       --identifier-prefix=GdkGL \
> +       --symbol-prefix=gdk_gl \
> +       --symbol-prefix=gdk
> +GdkGLExt_1_0_gir_CFLAGS = $(common_includes)
> +GdkGLExt_1_0_gir_LIBS = $(gdkglext_targetlib)
> +GdkGLExt_1_0_gir_FILES = $(introspection_sources)
> +INTROSPECTION_GIRS += GdkGLExt-1.0.gir

I don't know much about introspection, but I'd expect the version
numbers to match the major and minor version of the source code, right?
All these GdkGLExt_1.0_*s and friends should then be replaced by
GdkGLExt_@API_MJ@_@API_MI@_*.


> +
> +girdir = $(datadir)/gir-1.0
> +gir_DATA = $(INTROSPECTION_GIRS)
> +
> +typelibdir = $(libdir)/girepository-1.0
> +typelib_DATA = $(INTROSPECTION_GIRS:.gir=.typelib)
> +
> +CLEANFILES += $(gir_DATA) $(typelib_DATA)
> +endif # HAVE_INTROSPECTION
> +
> +#
>  # Extra rules
>  #
> 
> === modified file 'gtk/Makefile.am'
> --- gtk/Makefile.am     2012-03-13 20:40:48 +0000
> +++ gtk/Makefile.am     2012-05-22 09:22:18 +0000
> @@ -1,6 +1,8 @@
>  ## -*- Makefile -*-
>  ## Makefile.am for gtkglext/gtk
>  
> +CLEANFILES =
> +

Again, why?

> 
>  EXTRA_DIST = \
>         gtkglversion.h.in       \
>         gtkglext.def
> @@ -84,3 +86,38 @@
>  libgtkglext_@API_MJ@_@API_MI@_la_SOURCES = $(gtkglext_sources)
>  libgtkglext_@API_MJ@_@API_MI@_la_LDFLAGS = $(common_ldflags)
>  libgtkglext_@API_MJ@_@API_MI@_la_LIBADD = $(common_libadd)
> +
> +#
> +# Introspection
> +#
> +
> +-include $(INTROSPECTION_MAKEFILE)
> +INTROSPECTION_GIRS =
> +INTROSPECTION_SCANNER_ARGS = --add-include-path=../gdk
> +INTROSPECTION_COMPILER_ARGS = --includedir=$(srcdir)
> --includedir=../gdk
> +
> +if HAVE_INTROSPECTION
> +introspection_sources = \
> +       $(gtkglext_public_h_sources) \
> +       $(gtkglext_c_sources)
> +
> +GtkGLExt-1.0.gir: $(gtkglext_targetlib)
> +GtkGLExt_1_0_gir_INCLUDES = Gtk-3.0 GdkGLExt-1.0
> +GtkGLExt_1_0_gir_SCANNERFLAGS = \
> +       --warn-all \
> +       --identifier-prefix=GtkGL \
> +       --symbol-prefix=gtk_gl \
> +       --symbol-prefix=gtk
> +GtkGLExt_1_0_gir_CFLAGS = $(common_includes)
> +GtkGLExt_1_0_gir_LIBS = $(gtkglext_targetlib)
> +GtkGLExt_1_0_gir_FILES = $(introspection_sources)
> +INTROSPECTION_GIRS += GtkGLExt-1.0.gir

GtkGLExt_@API_MJ@_@API_MI@ ?

> 
> +
> +girdir = $(datadir)/gir-1.0
> +gir_DATA = $(INTROSPECTION_GIRS)
> +
> +typelibdir = $(libdir)/girepository-1.0
> +typelib_DATA = $(INTROSPECTION_GIRS:.gir=.typelib)
> +
> +CLEANFILES += $(gir_DATA) $(typelib_DATA)
> +endif # HAVE_INTROSPECTION
> 
> === modified file 'gtk/gtkglwidget.c'
> 

I don't like patches that touch too many unrelated files. There should
be separate patch for the C file before the build-system patch.

> 
> --- gtk/gtkglwidget.c   2012-03-22 17:42:49 +0000
> +++ gtk/gtkglwidget.c   2012-05-22 09:26:58 +0000
> @@ -235,7 +235,7 @@
>   * gtk_widget_set_gl_capability:
>   * @widget: the #GtkWidget to be used as the rendering area.
>   * @glconfig: a #GdkGLConfig.
> - * @share_list: the #GdkGLContext with which to share display lists
> and texture
> + * @share_list: (allow-none): the #GdkGLContext with which to share
> display lists and texture
>   *              objects. NULL indicates that no sharing is to take
> place.
>   * @direct: whether rendering is to be done with a direct connection
> to
>   *          the graphics system.


Best regards
Thomas

[1] https://live.gnome.org/GnomeLove/SubmittingPatches


-- 
GnuPG:          http://tdz.users.sourceforge.net/tdz.asc
Fingerprint:    16FF F599 82F8 E5AA 18C6 5220 D9DA D7D4 4EF1 DF08

jsapigen - A free glue-code generator for Mozilla SpiderMonkey. See
http://jsapigen.sourceforge.net for more information.

Attachment: signature.asc
Description: This is a digitally signed message part



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]