Hi,

Continuing the work on http://gui.gimp.org/index.php/Save_%2B_export_specification , here is my API proposal for common export dialog logic to be shared by file format plug-ins:

GtkWidget * gimp_export_dialog_new (const gchar *format_name, const gchar *role, const gchar *help_id);

This function creates a new dialog, and plug-ins then use

GtkWidget * gimp_export_dialog_get_container (GtkWidget *dialog);

to get a vbox that the file-format specific widgets with export options are put it.

By getting rid of all dialogs that gimp_export_image() pops up, and by porting plug-ins to this new API, we have a much nicer export workflow already. You can try it out on the export-dialog-cleanup-tmp branch in GIMP git for JPEG, PNG, TIFF and BMP:

http://git.gnome.org/cgit/gimp/log/?h=export-dialog-cleanup-tmp

One open question is how to deal with exporting specific layers to files. With the removal of dialogs in gimp_export_image() one does not have the possibility to ignore flattening/merging the image any longer, so it's not possible to export a single layer to a PNG without creating a separate image for that layer. But on the other hand, it doesn't seem unreasonable to require a user to do that if he/she wants to export a single layer. With some effort I guess we can add common logic to the export dialog to deal with single-layer exports somehow.

Btw, the -tmp suffix to the git branch name is supposed to be interpreted as that the branch is volatile and no work should be based on it.

/ Martin

On 07/14/2009 10:44 PM, Sven Neumann wrote:

This function creates a new dialog, and plug-ins then use

GtkWidget * gimp_export_dialog_get_container (GtkWidget *dialog);

to get a vbox that the file-format specific widgets with export options are put it.

I'd suggest to have gimp_export_dialog_add() instead with semantics similar to gtk_container_add().

That's a good idea, I'll change that.

/ Martin

On 07/14/2009 10:47 PM, Sven Neumann wrote:

Hi,

On Tue, 2009-07-14 at 12:44 +0200, Martin Nordholts wrote:

Continuing the work on
http://gui.gimp.org/index.php/Save_%2B_export_specification , here is my API proposal for common export dialog logic to be shared by file format plug-ins:

GtkWidget * gimp_export_dialog_new (const gchar *format_name, const gchar *role, const gchar *help_id);

That's fine, but it doesn't deal with the problem of how the file plug-in deals with the actual export. So far the gimp_export_image() functions does take care of all the nasty details. We definitely don't want to duplicate that code all over the place.

The plan is to let plug-ins still use gimp_export_image() for creating an image suitable for exporting, it's just that we will remove that function's bad habit of asking annoying questions in the process. This is slight change in API semantics, but the ABI/API is still unchanged and I don't consider this a problem.

/ Martin

On 07/14/2009 10:55 PM, Sven Neumann wrote:

Hi,

On Tue, 2009-07-14 at 22:54 +0200, Martin Nordholts wrote:

The plan is to let plug-ins still use gimp_export_image() for creating an image suitable for exporting, it's just that we will remove that function's bad habit of asking annoying questions in the process. This is slight change in API semantics, but the ABI/API is still unchanged and I don't consider this a problem.

That will break all file plug-ins that use gimp_export_image(). I am sorry, but that is totally not an option.

Exactly how will it break existing plug-ins? It will just be changed to act as if the user clicked the Export button. Take a look at the code on the branch and you'll see what I mean

/ Martin

On 07/15/2009 12:20 AM, Sven Neumann wrote:

Hi,

On Tue, 2009-07-14 at 22:59 +0200, Martin Nordholts wrote:

Exactly how will it break existing plug-ins? It will just be changed to act as if the user clicked the Export button. Take a look at the code on the branch and you'll see what I mean

There are questions to ask the user and we can't just start to skip these questions. Third-party plug-ins that use the gimp_export_image() function should continue to behave exactly as they behave now.

Hi,

Let us then add the following function to libgimp

GimpExportReturn gimp_export_prepare_image (gint32 *image_ID, gint32 *drawable_ID, GimpExportCapabilities capabilities);

that shares the implementation and semantics with gimp_export_image() except that it doesn't show any dialogs, it just goes ahead and creates a merged copy of the image if needed. By sharing the implementation with gimp_export_image() we can use well-tested code and it becomes easy to port plug-ins to the new API.

Regarding changing gimp_export_dialog_get_container() into having semantics similar to gtk_container_add(), I fear that we will have to duplicate too much of the API. If we provide _add() we need to wrap _remove() and so on. Instead of doing that I propose a name change to

GtkWidget * gimp_export_dialog_get_vbox (GtkWidget *dialog);

Again, this API has the benefit of making porting plug-ins easy. For completeness, here is the third function in the new export API, unchanged from before:

GtkWidget * gimp_export_dialog_new (const gchar *format_name, const gchar *role, const gchar *help_id);

For people with time to look at code, the git branch is updated (rewritten): http://git.gnome.org/cgit/gimp/log/?h=export-dialog-cleanup-tmp

BR, Martin

On 07/15/2009 03:24 PM, Michael Natterer wrote:

On Tue, 2009-07-14 at 22:50 +0200, Martin Nordholts wrote:

gimp_export_dialog_get_container() was not such a bad idea imho, but I would call it gimp_export_dialog_get_content_area(), because we have precedence of this API in GtkDialog.

Consistency with GTK+ is nice, but now that we mention it, we don't need gimp_export_dialog_get_content_area() at all, we can just use gtk_dialog_get_content_area()

/ Martin

On 07/15/2009 09:06 PM, Martin Nordholts wrote:

On 07/15/2009 03:24 PM, Michael Natterer wrote:

On Tue, 2009-07-14 at 22:50 +0200, Martin Nordholts wrote:

gimp_export_dialog_get_container() was not such a bad idea imho, but I would call it gimp_export_dialog_get_content_area(), because we have precedence of this API in GtkDialog.

Consistency with GTK+ is nice, but now that we mention it, we don't need gimp_export_dialog_get_content_area() at all, we can just use gtk_dialog_get_content_area()

That depends of course on if we need to put common widgets within the content area. Hmm we should perhaps add gimp_export_dialog_get_content_area() so that we in the future can do that. Until we do, it would just be a wrapper around gtk_dialog_get_content_area()

/ Martin

Hi again,

This is what we ended up in the last discussion. Anyone have some final comments or objections before I push this to master and port plug-ins to use this?

/ Martin

/** * gimp_export_prepare_image:
* @image_ID: Pointer to the image_ID. * @drawable_ID: Pointer to the drawable_ID. * @capabilities: What can the image_format do? *
* Takes an image and a drawable to be saved together with a * description of the capabilities of the image_format. If the type of * image doesn't match the capabilities of the format a copy is * created. This copy is then converted, the image_ID and drawable_ID * are changed to point to the new image and the procedure returns * GIMP_EXPORT_EXPORT. The save_plugin has to take care of deleting * the created image using gimp_image_delete() when it has saved it. *
* If no converson is done, the image_ID and drawable_ID is not * altered, GIMP_EXPORT_IGNORE is returned and the save_plugin should * try to save the original image.
*
* This function has identical semantics to gimp_export_image() except * that it doesn't ask the user any questions. *
* Returns: An enum of #GimpExportReturn describing the action. *
* Since: GIMP 2.8
**/
GimpExportReturn
gimp_export_prepare_image (gint32 *image_ID, gint32 *drawable_ID, GimpExportCapabilities capabilities)

/** * gimp_export_dialog_new:
* @format_name: The short name of the image_format (e.g. JPEG or PNG). * @role: The dialog's @role which will be set with * gtk_window_set_role(). * @help_id: The GIMP help id.
*
* Creates a new export dialog. All file plug-ins should use this * dialog to get a consistent look on the export dialogs. Use * gimp_export_dialog_get_content_area() to get a #GtkVBox to be * filled with export options. The export dialog is a wrapped * #GimpDialog.
*
* Returns: The new export dialog.
*
* Since: GIMP 2.8
**/
GtkWidget *
gimp_export_dialog_new (const gchar *format_name, const gchar *role, const gchar *help_id)

/** * gimp_export_dialog_get_content_area: * @dialog: A dialog created with gimp_export_dialog_new() *
* Returns the #GtkVBox of the passed export dialog to be filled with * export options.
*
* Returns: The #GtkVBox to fill with export options. *
* Since: GIMP 2.8
**/
GtkWidget *
gimp_export_dialog_get_content_area (GtkWidget *dialog)

On 07/18/2009 06:53 PM, Sven Neumann wrote:

Hi,

On Sat, 2009-07-18 at 15:02 +0200, Martin Nordholts wrote:

This is what we ended up in the last discussion. Anyone have some final comments or objections before I push this to master and port plug-ins to use this?

I wonder how much sense it makes to actually create a duplicate image. Most export plug-ins will want to save what's visible. So it would be better if we would use this chance to get away from the copy-image-on-export semantics and let file plug-ins save the projection instead. About a year ago I tried to change gimp-export-image so that it does not create a duplicate, but that wasn't possible w/o breaking backwards compatibility. Now would be a very good chance to get this done right.

I did considerer the possibility of using the projection directly with the API you added a while back, but I decided to go the safe and easy-to-port path instead. I don't think it is worth the effort to rewrite a lot of code that is likely to go obsolete with GEGL later anyway.

/ Martin

On 07/18/2009 09:10 PM, Sven Neumann wrote:

Hi,

The semantics of gimp_export_image() are complete crap (I wrote this code in the first place, so I have the right to say that). So let's not make this error worse by introducing a new function that has the same broken semantics. If you really need the semantics of gimp_export_image() as an interim solution, that's fine. But then we don't need to add an extra API for this. If you pass NULL as 'format_name' to gimp_export_image(), then it will not ask the user any dialogs. So it will do exactly what you proposed for gimp_export_prepare_image().

That would not be a good API, but as an interim solution it would work. So the only API I will add to libgimp then is

GtkWidget * gimp_export_dialog_new (const gchar *format_name, const gchar *role, const gchar *help_id)

GtkWidget * gimp_export_dialog_get_content_area (GtkWidget *dialog)

Unless more complications arise I will add and port to this API soon

/ Martin

On 07/19/2009 11:06 AM, Martin Nordholts wrote:

On 07/18/2009 09:10 PM, Sven Neumann wrote:

If you really need the semantics of gimp_export_image() as an interim solution, that's fine. But then we don't need to add an extra API for this. If you pass NULL as 'format_name' to gimp_export_image(), then it will not ask the user any dialogs.

That would not be a good API, but as an interim solution it would work. So the only API I will add to libgimp then is

GtkWidget * gimp_export_dialog_new (const gchar *format_name, const gchar *role,
const gchar *help_id)

GtkWidget * gimp_export_dialog_get_content_area (GtkWidget *dialog)

Unless more complications arise I will add and port to this API soon

I have now introduced and ported file plug-ins to this API in git master.

/ Martin