From: Martin Nordholts

As an experiment, use Glade + GtkBuilder for the save dialog in the PNG plug-in. The layout is as good as identical. Mnemonics work and strings are still translated.
---
configure.ac | 1 + plug-ins/Makefile.am | 1 + plug-ins/common/file-png.c | 202 ++++++++++++++----------------------- plug-ins/glade/.gitignore | 2 + plug-ins/glade/Makefile.am | 6 + plug-ins/glade/file-png.glade | 226 +++++++++++++++++++++++++++++++++++++++++ po-plug-ins/POTFILES.in | 1 + 7 files changed, 313 insertions(+), 126 deletions(-) create mode 100644 plug-ins/glade/.gitignore create mode 100644 plug-ins/glade/Makefile.am create mode 100644 plug-ins/glade/file-png.glade

diff --git a/configure.ac b/configure.ac index 6edf057..2d8229c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2004,6 +2004,7 @@ plug-ins/fractal-explorer/examples/Makefile plug-ins/gfig/Makefile
plug-ins/gfig/gfig-examples/Makefile plug-ins/gfig/images/Makefile
+plug-ins/glade/Makefile
plug-ins/gimpressionist/Brushes/Makefile plug-ins/gimpressionist/Makefile
plug-ins/gimpressionist/Paper/Makefile diff --git a/plug-ins/Makefile.am b/plug-ins/Makefile.am index fd62823..b045c92 100644
--- a/plug-ins/Makefile.am
+++ b/plug-ins/Makefile.am
@@ -69,6 +69,7 @@ SUBDIRS = \
flame \
fractal-explorer \
gfig \
+ glade \
gimpressionist \
gradient-flare \
help \
diff --git a/plug-ins/common/file-png.c b/plug-ins/common/file-png.c index 7f7c86f..faaf0fb 100644
--- a/plug-ins/common/file-png.c
+++ b/plug-ins/common/file-png.c
@@ -1768,21 +1768,35 @@ respin_cmap (png_structp pp,
}

+static GtkWidget *
+toggle_button_init (GtkBuilder *builder, + const gchar *name, + gboolean initial_value, + gboolean *value_pointer) +{
+ GtkWidget *toggle = NULL;
+
+ toggle = GTK_WIDGET (gtk_builder_get_object (builder, name)); + gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (toggle), initial_value); + g_signal_connect (toggle, "toggled", + G_CALLBACK (gimp_toggle_button_update), + value_pointer); +
+ return toggle;
+}
+
static gboolean
save_dialog (gint32 image_ID,
gboolean alpha)
{
PngSaveGui pg;
GtkWidget *dialog;
- GtkWidget *table;
- GtkWidget *toggle;
- GtkObject *scale;
- GtkWidget *hbox;
- GtkWidget *button;
+ GtkBuilder *builder;
+ gchar *gladefile;
GimpParasite *parasite;

+ /* Dialog init */
dialog = gimp_export_dialog_new (_("PNG"), PLUG_IN_BINARY, SAVE_PROC); -
g_signal_connect (dialog, "response", G_CALLBACK (save_dialog_response), &pg);
@@ -1790,142 +1804,78 @@ save_dialog (gint32 image_ID, G_CALLBACK (gtk_main_quit), NULL);

- table = gtk_table_new (10, 3, FALSE); - gtk_table_set_col_spacings (GTK_TABLE (table), 6); - gtk_table_set_row_spacings (GTK_TABLE (table), 6); - gtk_container_set_border_width (GTK_CONTAINER (table), 12); - gtk_box_pack_start (GTK_BOX (gimp_export_dialog_get_content_area (dialog)), - table, TRUE, TRUE, 0); - gtk_widget_show (table);
-
- pg.interlaced = toggle =
- gtk_check_button_new_with_mnemonic (_("_Interlacing (Adam7)")); - gtk_table_attach (GTK_TABLE (table), toggle, 0, 3, 0, 1, GTK_FILL, 0, 0, 0); - gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (toggle), - pngvals.interlaced); - gtk_widget_show (toggle);
-
- g_signal_connect (toggle, "toggled", - G_CALLBACK (gimp_toggle_button_update), - &pngvals.interlaced); -
- pg.bkgd = toggle =
- gtk_check_button_new_with_mnemonic (_("Save _background color")); - gtk_table_attach (GTK_TABLE (table), toggle, 0, 3, 1, 2, GTK_FILL, 0, 0, 0); - gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (toggle), pngvals.bkgd); - gtk_widget_show (toggle);
-
- g_signal_connect (toggle, "toggled", - G_CALLBACK (gimp_toggle_button_update), &pngvals.bkgd); -
- pg.gama = toggle = gtk_check_button_new_with_mnemonic (_("Save _gamma")); - gtk_table_attach (GTK_TABLE (table), toggle, 0, 3, 2, 3, GTK_FILL, 0, 0, 0); - gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (toggle), pngvals.gama); - gtk_widget_show (toggle);
-
- g_signal_connect (toggle, "toggled", - G_CALLBACK (gimp_toggle_button_update), - &pngvals.gama); -
- pg.offs = toggle =
- gtk_check_button_new_with_mnemonic (_("Save layer o_ffset")); - gtk_table_attach (GTK_TABLE (table), toggle, 0, 3, 3, 4, GTK_FILL, 0, 0, 0); - gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (toggle), pngvals.offs); - gtk_widget_show (toggle);
-
- g_signal_connect (toggle, "toggled", - G_CALLBACK (gimp_toggle_button_update), - &pngvals.offs); -
- pg.phys = toggle = gtk_check_button_new_with_mnemonic (_("Save _resolution")); - gtk_table_attach (GTK_TABLE (table), toggle, 0, 3, 4, 5, GTK_FILL, 0, 0, 0); - gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (toggle), pngvals.phys); - gtk_widget_show (toggle);
-
- g_signal_connect (toggle, "toggled", - G_CALLBACK (gimp_toggle_button_update), - &pngvals.phys); -
- pg.time = toggle =
- gtk_check_button_new_with_mnemonic (_("Save creation _time")); - gtk_table_attach (GTK_TABLE (table), toggle, 0, 3, 5, 6, GTK_FILL, 0, 0, 0); - gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (toggle), pngvals.time); - gtk_widget_show (toggle);
-
- g_signal_connect (toggle, "toggled", - G_CALLBACK (gimp_toggle_button_update), - &pngvals.time); -
- pg.comment = toggle = gtk_check_button_new_with_mnemonic (_("Save comme_nt")); - gtk_table_attach (GTK_TABLE (table), toggle, 0, 3, 6, 7, GTK_FILL, 0, 0, 0); - gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (toggle), pngvals.comment); - gtk_widget_show (toggle);
+ /* GtkBuilder init */
+ builder = gtk_builder_new ();
+ gladefile = g_build_filename (gimp_data_directory (), + "glade/file-png.glade", + NULL); + gtk_builder_add_from_file (builder, gladefile, NULL /*error*/); + g_free (gladefile);

+ /* Table */
+ gtk_box_pack_start (GTK_BOX (gimp_export_dialog_get_content_area (dialog)), + GTK_WIDGET (gtk_builder_get_object (builder, "table")), + TRUE, TRUE, 0); +
+ /* Toggles */
+ pg.interlaced = toggle_button_init (builder, "interlace", + pngvals.interlaced, + &pngvals.interlaced); + pg.bkgd = toggle_button_init (builder, "save-background-color", + pngvals.bkgd, + &pngvals.bkgd); + pg.gama = toggle_button_init (builder, "save-gamma", + pngvals.gama, + &pngvals.gama); + pg.offs = toggle_button_init (builder, "save-layer-offset", + pngvals.offs, + &pngvals.offs); + pg.phys = toggle_button_init (builder, "save-resolution", + pngvals.phys, + &pngvals.phys); + pg.time = toggle_button_init (builder, "save-creation-time", + pngvals.time, + &pngvals.time); +
+ /* Comment toggle */
parasite = gimp_image_parasite_find (image_ID, "gimp-comment"); - gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (toggle), - pngvals.comment && parasite != NULL); - gtk_widget_set_sensitive (toggle, parasite != NULL); + pg.comment =
+ toggle_button_init (builder, "save-comment", + pngvals.comment && parasite != NULL, + &pngvals.comment); + gtk_widget_set_sensitive (pg.comment, parasite != NULL); gimp_parasite_free (parasite);

- g_signal_connect (toggle, "toggled", - G_CALLBACK (gimp_toggle_button_update), - &pngvals.comment); -
- pg.save_transp_pixels = toggle =
- gtk_check_button_new_with_mnemonic (_("Save color _values from " - "transparent pixels")); - gtk_table_attach (GTK_TABLE (table), toggle, 0, 3, 7, 8, GTK_FILL, 0, 0, 0); - gtk_toggle_button_set_active (GTK_TOGGLE_BUTTON (toggle), - alpha && pngvals.save_transp_pixels); - gtk_widget_set_sensitive (toggle, alpha); - gtk_widget_show (toggle);
-
- g_signal_connect (toggle, "toggled", - G_CALLBACK (gimp_toggle_button_update), - &pngvals.save_transp_pixels); -
- pg.compression_level = scale =
- gimp_scale_entry_new (GTK_TABLE (table), 0, 8, - _("Co_mpression level:"), - SCALE_WIDTH, 0, - pngvals.compression_level, - 0.0, 9.0, 1.0, 1.0, 0, TRUE, 0.0, 0.0, - _("Choose a high compression level " - "for small file size"), NULL); -
- g_signal_connect (scale, "value-changed", + /* Transparent pixels toggle */
+ pg.save_transp_pixels =
+ toggle_button_init (builder,
+ "save-transparent-pixels", + alpha && pngvals.save_transp_pixels, + &pngvals.save_transp_pixels); + gtk_widget_set_sensitive (pg.save_transp_pixels, alpha); +
+ /* Compression level scale */
+ pg.compression_level =
+ GTK_OBJECT (gtk_builder_get_object (builder, "compression-level")); + g_signal_connect (pg.compression_level, "value-changed", G_CALLBACK (gimp_int_adjustment_update), &pngvals.compression_level);
- hbox = gtk_hbutton_box_new ();
- gtk_box_set_spacing (GTK_BOX (hbox), 6); - gtk_button_box_set_layout (GTK_BUTTON_BOX (hbox), GTK_BUTTONBOX_START); - gtk_table_attach (GTK_TABLE (table), hbox, 0, 3, 9, 10, - GTK_FILL | GTK_EXPAND, 0, 0, 0); - gtk_widget_show (hbox);
-
- button = gtk_button_new_with_mnemonic (_("_Load Defaults")); - gtk_box_pack_start (GTK_BOX (hbox), button, FALSE, FALSE, 0); - gtk_widget_show (button);
-
- g_signal_connect_swapped (button, "clicked", + /* Load/save defaults buttons */
+ g_signal_connect_swapped (gtk_builder_get_object (builder, "load-defaults"), + "clicked", G_CALLBACK (load_gui_defaults), &pg);

- button = gtk_button_new_with_mnemonic (_("S_ave Defaults")); - gtk_box_pack_start (GTK_BOX (hbox), button, FALSE, FALSE, 0); - gtk_widget_show (button);
-
- g_signal_connect_swapped (button, "clicked", + g_signal_connect_swapped (gtk_builder_get_object (builder, "save-defaults"), + "clicked", G_CALLBACK (save_defaults), &pg);

+ /* Show dialog and run */
gtk_widget_show (dialog);
-
pg.run = FALSE;
-
gtk_main ();
-
return pg.run;
}

diff --git a/plug-ins/glade/.gitignore b/plug-ins/glade/.gitignore new file mode 100644
index 0000000..b336cc7
--- /dev/null
+++ b/plug-ins/glade/.gitignore
@@ -0,0 +1,2 @@
+/Makefile
+/Makefile.in
diff --git a/plug-ins/glade/Makefile.am b/plug-ins/glade/Makefile.am new file mode 100644
index 0000000..ce54c0d
--- /dev/null
+++ b/plug-ins/glade/Makefile.am
@@ -0,0 +1,6 @@
+gladedatadir = $(gimpdatadir)/glade +
+gladedata_DATA = \
+ file-png.glade
+
+EXTRA_DIST = $(gladedata_DATA)
diff --git a/plug-ins/glade/file-png.glade b/plug-ins/glade/file-png.glade new file mode 100644
index 0000000..0005595
--- /dev/null
+++ b/plug-ins/glade/file-png.glade
@@ -0,0 +1,226 @@
+
+
+ + +
+ 9
+
+ + 9
+
+ 1
+ 1
+
+
+ True
+ 12
+ 10
+ 3
+ 6
+ 6
+
+
+ _Interlacing (Adam7)
+ True
+ True
+ False
+ True
+ True
+
+
+ 3
+
+
+
+
+ Save _background color
+ True
+ True
+ False
+ True
+ True
+
+
+ 3
+ 1
+ 2
+
+
+
+
+ Save gamma
+ True
+ True
+ False
+ True
+
+
+ 3
+ 2
+ 3
+
+
+
+
+ Save layer o_ffset
+ True
+ True
+ False
+ True
+ True
+
+
+ 3
+ 3
+ 4
+
+
+
+
+ Save _resolution
+ True
+ True
+ False
+ True
+ True
+
+
+ 3
+ 4
+ 5
+
+
+
+
+ Save creation _time
+ True
+ True
+ False
+ True
+ True
+
+
+ 3
+ 5
+ 6
+
+
+
+
+ Save comme_nt
+ True
+ True
+ False
+ True
+ True
+
+
+ 3
+ 6
+ 7
+
+
+
+
+ Save color _values from transparent pixels + True
+ False
+ True
+ False
+ True
+ True
+
+
+ 3
+ 7
+ 8
+
+
+
+
+ True
+ 0
+ Co_mpression level:
+ True
+ compression-level-spin
+
+
+ 8
+ 9
+
+
+
+
+
+ True
+ True
+ compression-level
+ False
+
+
+ 1
+ 2
+ 8
+ 9
+
+
+
+
+ True
+ True
+ True
+ Choose a high compression level for small file size + ●
+ compression-level
+ True
+
+
+ 2
+ 3
+ 8
+ 9
+
+
+
+
+
+ True
+ 6
+ start
+
+
+ _Load Defaults
+ True
+ True
+ True
+ True
+
+
+ False
+ False
+ 0
+
+
+
+
+ S_ave Defaults
+ True
+ True
+ True
+ True
+
+
+ False
+ False
+ 1
+
+
+
+
+ 3
+ 9
+ 10
+
+
+
+
diff --git a/po-plug-ins/POTFILES.in b/po-plug-ins/POTFILES.in index 512cf96..6fefb91 100644
--- a/po-plug-ins/POTFILES.in
+++ b/po-plug-ins/POTFILES.in
@@ -207,6 +207,7 @@ plug-ins/gimpressionist/repaint.c plug-ins/gimpressionist/size.c
plug-ins/gimpressionist/sizemap.c
plug-ins/gimpressionist/utils.c
+plug-ins/glade/file-png.glade
plug-ins/gradient-flare/gradient-flare.c plug-ins/help-browser/dialog.c
plug-ins/help-browser/help-browser.c

Hi,

Right now GIMP has virtuall all of the UI imperatively constructed with code. This has a few problems:

* Tweaking the UI requires re-building the app * A lot of code duplication
* Not very modern

The patch I am replying to ports the PNG save dialog to Glade and uses GtkBuilder to construct the UI. The purpose of this change is to introduce declarative construction of UIs in GIMP to give us experience in this area. The benefits of using Glade is:

* Tweaking the UI doesn't requires re-bulding the app * Less code
* Easier to start using a script language like JavaScript in the core * We make the GIMP code base more modern * UIs can be constructed through drag-and-drop with Glade

There are a few things that should be noted:

* Translations still works * Mnemonics still works
* The tooltip is still there (although only on the spinner, not scale) * We use GtkBuilder, not the deprecated libglade * There is no need for any proprocessing, GtkBuilder reads direct XML * We don't rely on any extra tool, XML files can be written manually

And a warning: * Order of XML properties, see GtkAdjustment comment in file-png.glade

So, do people think this is a step in the right direction? Personally I do and would like to commit the patch. Opinions?

Regards, Martin

The conclusion was to go ahead so I will commit the patch soon after changing "glade" to "ui" as per Sven's comment.

In response to Akira's comment on performance:

We need to be aware of that we are introducing extra disk IO which can give us problems, especialy during startup if we have to load a lot of files. In practice I don't think this will be a problem for a long time though, but doesn't hurt to have a mitigation strategy. We could:

* Put files as static strings in the code so that we don't have to significant disk IO at all
* Merge several files together to minimize disk seeking times for many files

In response to the question on to what extent to use Glade:

We should not sacrifice good interaction design for simplicitly of writing the UI code. UIs constructed through drag-and-drop of widgets have a tendency to have bad usability, it is important that we keep this in mind so we don't fall into the same trap.

In response to Torsten's comment on custom widgets:

Yes I agree, eventually we need to support our custom widgets with Glade. This might require adaptions to our widgets and even to Glade and to GtkBuilder.

In response to Michael:

"Less code" means less C code. In this case file-png.c now has 46 lines of C code less. Also, you seemed to interpreted all my points as arguments for using Glade while many of them were just comments. For example, the bullet that we don't rely on any extra tool was just to calm down people that don't want any new build dependency. The use of Glade is optional but I except only extreme hackers to code the XML files manually :).

/ Martin

Aurimas Juška wrote:

Why not to convert XML's to C code when compiling distribution ? XML's are large and not very fast to parse. This approach would have all the benefits of XML's while developing and would not cause additional penalties for release distribution.

That's also a good option in case parsing turns out to be a big bottleneck

/ Martin