Hi all,

We have discussed the possibility to use Vala as a code generator. What follows is a set of patches so we all can see what that would look like. Any objections to me pushing these commits?

I want a helper class in the implementation of single-window mode session management, and this is in my opinion a perfect example of when to use Vala: a small utility class at the bottom of the architecture with only GLib as a dependency.

These patches do not add any additional tarball build time dependencies, valac is only required when building from git.

/ Martin

From: Martin Nordholts

Look for valac and prepare app/core for .vala source files. valac is only needed when building from git, in tarballs the generated .c and .h files are distributed.
---
app/core/Makefile.am | 37 +++++++++++++++++++++++++++++++++++-- configure.ac | 3 +++
2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/app/core/Makefile.am b/app/core/Makefile.am index 3a8ee12..3f0985c 100644
--- a/app/core/Makefile.am
+++ b/app/core/Makefile.am
@@ -384,13 +384,21 @@ libappcore_a_sources = \ gimpviewable.c \
gimpviewable.h

+libappcore_a_built_sources_vala = \ + $(NULL)
+
+libappcore_a_extra_sources_vala = \ + $(NULL)
+
libappcore_a_built_sources = \
core-enums.c \
gimpmarshal.c \
- gimpmarshal.h
+ gimpmarshal.h \
+ $(libappcore_a_built_sources_vala)
libappcore_a_extra_sources = \
- gimpmarshal.list
+ gimpmarshal.list \
+ $(libappcore_a_extra_sources_vala)
libappcore_a_SOURCES = $(libappcore_a_built_sources) $(libappcore_a_sources)
@@ -429,3 +437,28 @@ core-enums.c: $(srcdir)/core-enums.h $(GIMP_MKENUMS) $(srcdir)/core-enums.h > xgen-cec \ && cp xgen-cec $(@F) \
&& rm -f xgen-cec
+
+#
+# Vala helpers
+#
+
+.vala.c:
+ $(VALAC_CHECK_AVAILABILITY)
+ $(VALA_V)$(VALAC) --compile --ccode --output=$@ $< + @touch $@ # See Bug 649084
+
+.vala.h:
+ $(VALAC_CHECK_AVAILABILITY)
+ $(VALA_V)$(VALAC) --compile --header=$@ $< + @touch $@ # See Bug 649084
+
+$(libappcore_a_built_sources_vala): Makefile.am $(VALAC) +
+# Valac is not needed when building tarballs, but we should still fail +# properly when valac is missing
+VALAC_CHECK_AVAILABILITY = @if test -z "$(VALAC)"; then echo "$, AC_PROG_CC
AM_PROG_CC_C_O

+# Determine a Vala compiler to use. Not needed when building tarballs. +AM_PROG_VALAC(0.9.3)
+
# Initialize libtool
AM_DISABLE_STATIC
AC_LIBTOOL_WIN32_DLL

From: Martin Nordholts

Add a GimpIdTable utility class, written in Vala. ---
app/core/.gitignore | 2 +
app/core/Makefile.am | 5 +- app/core/gimp.c | 15 +++--- app/core/gimp.h | 7 +-- app/core/gimpidtable.vala | 106 +++++++++++++++++++++++++++++++++++++++++++++ app/core/gimpimage.c | 22 ++------- app/core/gimpitem.c | 31 ++++--------- 7 files changed, 135 insertions(+), 53 deletions(-) create mode 100644 app/core/gimpidtable.vala

diff --git a/app/core/.gitignore b/app/core/.gitignore index 7827fe0..ba81322 100644
--- a/app/core/.gitignore
+++ b/app/core/.gitignore
@@ -3,6 +3,8 @@
/.libs
/Makefile
/Makefile.in
+/gimpidtable.c
+/gimpidtable.h
/gimpmarshal.c
/gimpmarshal.h
/libappcore.a
diff --git a/app/core/Makefile.am b/app/core/Makefile.am index 3f0985c..393fe27 100644
--- a/app/core/Makefile.am
+++ b/app/core/Makefile.am
@@ -385,10 +385,11 @@ libappcore_a_sources = \ gimpviewable.h

libappcore_a_built_sources_vala = \ - $(NULL)
+ gimpidtable.c \
+ gimpidtable.h

libappcore_a_extra_sources_vala = \ - $(NULL)
+ gimpidtable.vala

libappcore_a_built_sources = \
core-enums.c \
diff --git a/app/core/gimp.c b/app/core/gimp.c index 434d1a7..7c19da7 100644
--- a/app/core/gimp.c
+++ b/app/core/gimp.c
@@ -62,6 +62,7 @@
#include "gimpdocumentlist.h"
#include "gimpgradient-load.h"
#include "gimpgradient.h"
+#include "gimpidtable.h"
#include "gimpimage.h"
#include "gimpimagefile.h"
#include "gimplist.h"
@@ -218,13 +219,11 @@ gimp_init (Gimp *gimp) gimp->images = gimp_list_new_weak (GIMP_TYPE_IMAGE, FALSE); gimp_object_set_static_name (GIMP_OBJECT (gimp->images), "images");
- gimp->next_image_ID = 1;
gimp->next_guide_ID = 1;
gimp->next_sample_point_ID = 1;
- gimp->image_table = g_hash_table_new (g_direct_hash, NULL); + gimp->image_table = gimp_id_table_new ();
- gimp->next_item_ID = 1;
- gimp->item_table = g_hash_table_new (g_direct_hash, NULL); + gimp->item_table = gimp_id_table_new ();
gimp->displays = g_object_new (GIMP_TYPE_LIST, "children-type", GIMP_TYPE_OBJECT, @@ -417,13 +416,13 @@ gimp_finalize (GObject *object)
if (gimp->item_table)
{
- g_hash_table_destroy (gimp->item_table); + g_object_unref (gimp->item_table); gimp->item_table = NULL;
}

if (gimp->image_table)
{
- g_hash_table_destroy (gimp->image_table); + g_object_unref (gimp->image_table); gimp->image_table = NULL;
}

@@ -492,8 +491,8 @@ gimp_get_memsize (GimpObject *object, memsize += gimp_object_get_memsize (GIMP_OBJECT (gimp->plug_in_manager), gui_size);
- memsize += gimp_g_hash_table_get_memsize (gimp->image_table, 0); - memsize += gimp_g_hash_table_get_memsize (gimp->item_table, 0); + memsize += gimp_id_table_get_memsize (gimp->image_table, 0); + memsize += gimp_id_table_get_memsize (gimp->item_table, 0);
memsize += gimp_object_get_memsize (GIMP_OBJECT (gimp->displays), gui_size);
diff --git a/app/core/gimp.h b/app/core/gimp.h index 6658c19..7cb624a 100644
--- a/app/core/gimp.h
+++ b/app/core/gimp.h
@@ -21,6 +21,7 @@

#include "gimpobject.h"
#include "gimp-gui.h"
+#include "gimpidtable.h" /* See Bug 648973 */

#define GIMP_TYPE_GIMP (gimp_get_type ()) @@ -74,13 +75,11 @@ struct _Gimp
GimpPlugInManager *plug_in_manager;
GimpContainer *images;
- gint next_image_ID; guint32 next_guide_ID; guint32 next_sample_point_ID; - GHashTable *image_table; + GimpIdTable *image_table;
- gint next_item_ID; - GHashTable *item_table; + GimpIdTable *item_table;
GimpContainer *displays; gint next_display_ID; diff --git a/app/core/gimpidtable.vala b/app/core/gimpidtable.vala new file mode 100644
index 0000000..3986a5a
--- /dev/null
+++ b/app/core/gimpidtable.vala
@@ -0,0 +1,106 @@
+/* GIMP - The GNU Image Manipulation Program + * Copyright (C) 1995 Spencer Kimball and Peter Mattis + *
+ * gimpidtable.vala
+ * Copyright (C) 2011 Martin Nordholts + *
+ * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + *
+ * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + *
+ * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */
+
+/**
+ * Contains a hash table with "int -> void *" mappings. The id for an + * object either automatically becomes the lowest unused id or is + * assigned manually.
+ */
+public class GimpIdTable : GLib.Object +{
+ private static int START_ID = 1;
+ private static int END_ID = int.MAX; +
+ private int next_id = START_ID;
+
+ private GLib.HashTable id_table = + new GLib.HashTable(GLib.direct_hash, null); +
+ /**
+ * Insert an object and assign it the lowest available id. + *
+ * Returns: The assigned id.
+ */
+ public int insert (void *data)
+ {
+ int new_id = next_id; // Must initialize, Bug 574352 +
+ do
+ {
+ new_id = next_id++;
+
+ if (next_id == END_ID)
+ next_id = START_ID;
+ }
+ while (lookup (new_id) != null); +
+ return insert_with_id (new_id, data); + }
+
+ /**
+ * Insert an object with the given id. + *
+ * Returns: The assigned id if it wasn't used, -1 otherwise. + */
+ public int insert_with_id (int id, void *data) + {
+ if (lookup (id) != null)
+ return -1;
+
+ id_table.insert(id, data);
+
+ return id;
+ }
+
+ /**
+ * Wrapper around GLib.HashTable.replace() + */
+ public void replace (int id, void *data) + {
+ id_table.replace (id, data);
+ }
+
+ /**
+ * Wrapper around GLib.HashTable.lookup() + */
+ public void *lookup (int id)
+ {
+ return id_table.lookup (id);
+ }
+
+ /**
+ * Wrapper around GLib.HashTable.remove() + */
+ public bool remove (int id)
+ {
+ return id_table.remove (id);
+ }
+
+ /**
+ * GimpObject::get_memsize() implementation. + */
+ public int64 get_memsize (int64 data_size) + {
+ /* Copy of gimp_g_hash_table_get_memsize() */ + return (2 * sizeof (int) +
+ 5 * sizeof (void *) +
+ id_table.size () * (3 * sizeof (void *) + data_size)); + }
+}
diff --git a/app/core/gimpimage.c b/app/core/gimpimage.c index 01d02cd..f156f2d 100644
--- a/app/core/gimpimage.c
+++ b/app/core/gimpimage.c
@@ -42,6 +42,7 @@
#include "gimpgrid.h"
#include "gimperror.h"
#include "gimpguide.h"
+#include "gimpidtable.h"
#include "gimpimage.h"
#include "gimpimage-colorhash.h"
#include "gimpimage-colormap.h"
@@ -749,19 +750,8 @@ gimp_image_constructed (GObject *object)
config = image->gimp->config;

- do
- {
- private->ID = image->gimp->next_image_ID++; -
- if (image->gimp->next_image_ID == G_MAXINT) - image->gimp->next_image_ID = 1; - }
- while (g_hash_table_lookup (image->gimp->image_table, - GINT_TO_POINTER (private->ID))); -
- g_hash_table_insert (image->gimp->image_table, - GINT_TO_POINTER (private->ID), - image);
+ private->ID = gimp_id_table_insert (image->gimp->image_table, + G_OBJECT (image));
template = config->default_image;
@@ -997,8 +987,7 @@ gimp_image_finalize (GObject *object)
if (image->gimp && image->gimp->image_table) {
- g_hash_table_remove (image->gimp->image_table, - GINT_TO_POINTER (private->ID)); + gimp_id_table_remove (image->gimp->image_table, private->ID); image->gimp = NULL;
}

@@ -1502,8 +1491,7 @@ gimp_image_get_by_ID (Gimp *gimp, if (gimp->image_table == NULL)
return NULL;

- return (GimpImage *) g_hash_table_lookup (gimp->image_table, - GINT_TO_POINTER (image_id)); + return (GimpImage *) gimp_id_table_lookup (gimp->image_table, image_id); }

void
diff --git a/app/core/gimpitem.c b/app/core/gimpitem.c index 856e962..6ed7167 100644
--- a/app/core/gimpitem.c
+++ b/app/core/gimpitem.c
@@ -337,8 +337,7 @@ gimp_item_finalize (GObject *object)
if (private->image && private->image->gimp) {
- g_hash_table_remove (private->image->gimp->item_table, - GINT_TO_POINTER (private->ID)); + gimp_id_table_remove (private->image->gimp->item_table, private->ID); private->image = NULL;
}

@@ -1600,8 +1599,7 @@ gimp_item_get_by_ID (Gimp *gimp, if (gimp->item_table == NULL)
return NULL;

- return (GimpItem *) g_hash_table_lookup (gimp->item_table, - GINT_TO_POINTER (item_id)); + return (GimpItem *) gimp_id_table_lookup (gimp->item_table, item_id); }

GimpTattoo
@@ -1649,19 +1647,8 @@ gimp_item_set_image (GimpItem *item,
if (private->ID == 0)
{
- do
- {
- private->ID = image->gimp->next_item_ID++; -
- if (image->gimp->next_item_ID == G_MAXINT) - image->gimp->next_item_ID = 1; - }
- while (g_hash_table_lookup (image->gimp->item_table, - GINT_TO_POINTER (private->ID))); -
- g_hash_table_insert (image->gimp->item_table, - GINT_TO_POINTER (private->ID), - item);
+ private->ID = gimp_id_table_insert (image->gimp->item_table, + G_OBJECT (item));
g_object_notify (G_OBJECT (item), "id"); }
@@ -1715,13 +1702,13 @@ gimp_item_replace_item (GimpItem *item, gimp_object_set_name (GIMP_OBJECT (item), gimp_object_get_name (replace));
if (private->ID)
- g_hash_table_remove (gimp_item_get_image (item)->gimp->item_table, - GINT_TO_POINTER (gimp_item_get_ID (item))); + gimp_id_table_remove (gimp_item_get_image (item)->gimp->item_table, + gimp_item_get_ID (item));
private->ID = gimp_item_get_ID (replace); - g_hash_table_replace (gimp_item_get_image (item)->gimp->item_table, - GINT_TO_POINTER (gimp_item_get_ID (item)), - item);
+ gimp_id_table_replace (gimp_item_get_image (item)->gimp->item_table, + gimp_item_get_ID (item), + G_OBJECT (item));
/* Set image before tatoo so that the explicitly set tatoo overrides * the one implicitly set when setting the image