After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 728633 - Improvements to the file-raw plugin
Improvements to the file-raw plugin
Status: RESOLVED FIXED
Product: GIMP
Classification: Other
Component: Plugins
2.8.10
Other All
: Normal enhancement
: 2.10
Assigned To: GIMP Bugs
GIMP Bugs
Depends on:
Blocks:
 
 
Reported: 2014-04-21 01:35 UTC by Björn Kautler
Modified: 2016-12-30 21:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gimp-2-8: 0001-Register-data-as-save-extension-and-raw-as-load-and-.patch (1.17 KB, patch)
2014-04-21 01:36 UTC, Björn Kautler
committed Details | Review
gimp-2-8: 0002-Export-the-image-before-saving-as-raw-image-data.patch (1.77 KB, patch)
2014-04-21 01:36 UTC, Björn Kautler
committed Details | Review
gimp-2-8: 0003-Use-GtkBuilder-for-raw-image-data-export-dialog.patch (16.65 KB, patch)
2014-04-21 01:37 UTC, Björn Kautler
rejected Details | Review
gimp-2-8: 0004-Save-and-restore-last-selected-values-and-provide-se.patch (22.48 KB, patch)
2014-04-21 01:37 UTC, Björn Kautler
rejected Details | Review
gimp-2-8: 0005-Allow-non-interactive-and-last-vals-call-of-raw-imag.patch (1.86 KB, patch)
2014-04-21 01:37 UTC, Björn Kautler
rejected Details | Review
master: 0001-Register-data-as-save-extension-and-raw-as-load-and-.patch (1.19 KB, patch)
2014-04-21 21:59 UTC, Björn Kautler
committed Details | Review
master: 0002-Export-the-image-before-saving-as-raw-image-data.patch (1.81 KB, patch)
2014-04-21 21:59 UTC, Björn Kautler
committed Details | Review
master: 0003-Use-GtkBuilder-for-raw-image-data-export-dialog.patch (17.66 KB, patch)
2014-04-21 22:00 UTC, Björn Kautler
committed Details | Review
master: 0004-Save-and-restore-last-selected-values-and-provide-se.patch (22.37 KB, patch)
2014-04-21 22:01 UTC, Björn Kautler
committed Details | Review
master: 0005-Allow-non-interactive-and-last-vals-call-of-raw-imag.patch (1.88 KB, patch)
2014-04-21 22:01 UTC, Björn Kautler
committed Details | Review

Description Björn Kautler 2014-04-21 01:35:39 UTC
Here is a patch series that makes some improvements to the 2.8.10 version of the file-raw plugin.

Besides the changed *_PROC names caused by the rename to file-raw-data, this patch should also be compatible with the master version. But I'm not setup with the latest dev versions of the dependencies and thus cannot test it there.

The patch series:

- Adds "data" - which is already a load extension - as save extension and adds "raw" as load and save extension as it is also a common suffix for such files
- Exports the image before saving, to get layers merged and so on. Currently only the top layer is saved (similar to PNG plugin)
- Uses GtkBuilder to construct the export dialog (similar to PNG plugin)
- Makes the plugin remember last used export settings  (similar to PNG plugin)
- Teaches the plugin some export defaults that can also be modified by the user (similar to PNG plugin)
- Makes the plugin able to be invoked non-interactively and with last used values [Ctrl+E] (similar to PNG plugin)
Comment 1 Björn Kautler 2014-04-21 01:36:13 UTC
Created attachment 274783 [details] [review]
gimp-2-8: 0001-Register-data-as-save-extension-and-raw-as-load-and-.patch
Comment 2 Björn Kautler 2014-04-21 01:36:38 UTC
Created attachment 274784 [details] [review]
gimp-2-8: 0002-Export-the-image-before-saving-as-raw-image-data.patch
Comment 3 Björn Kautler 2014-04-21 01:37:01 UTC
Created attachment 274785 [details] [review]
gimp-2-8: 0003-Use-GtkBuilder-for-raw-image-data-export-dialog.patch
Comment 4 Björn Kautler 2014-04-21 01:37:19 UTC
Created attachment 274786 [details] [review]
gimp-2-8: 0004-Save-and-restore-last-selected-values-and-provide-se.patch
Comment 5 Björn Kautler 2014-04-21 01:37:37 UTC
Created attachment 274787 [details] [review]
gimp-2-8: 0005-Allow-non-interactive-and-last-vals-call-of-raw-imag.patch
Comment 6 Jehan 2014-04-21 10:40:26 UTC
Hi Björn,

Thanks for the patches. Normally though we don't add features to the current stable release, only bug fixes. This should be more suited for the master build, which will become GIMP 2.10.

Would it be possible for you to setup the development repository and format-patch against origin/master? This would speed up any reviewing process if we don't have to fix the patches to work on master.
Thanks!
Comment 7 Björn Kautler 2014-04-21 10:50:37 UTC
Well, the patches also fix a few bugs in the raw plugin. It fixes the bug
- that data is registered as load but not as save extension
- that only the first layer is exported because the exporting was forgotten before the save
- that "Export as <file>" is not working with raw image data files
- that the last used export options are not remembered correctly (it was tried in the code, but erroneous, so this is a bugfix, no enhancement. :-) )

"New features" would essentially be only the support of "raw" as extension, the refactoring to use GtkBuilder for the save dialog and the possibility to store and load default values in the save dialog.

If you insist to add this only to the master branch ( :-( ), I can reformat the patch to apply on master, as there are almost no changes to the patches needed. But this would be untested then, because I don't think I'll try to get the master build running now.
Comment 8 Jehan 2014-04-21 11:00:04 UTC
Hi,

Ok that looks like a bunch of fix indeed, finally. :-) So the bug fix parts can most likely go to the gimp-2-8 branch.
Well in any case, we would need it for master too. So we would appreciate the porting.

I guess someone (maybe me if I get some time) will have a look to review your patches, when possible. Thanks.
Comment 9 Michael Natterer 2014-04-21 16:05:44 UTC
Nice, but I fail to see how almost the same amount of code *plus* a builder
file is better than the old dialog code ;)
Comment 10 Björn Kautler 2014-04-21 21:59:40 UTC
Created attachment 274848 [details] [review]
master: 0001-Register-data-as-save-extension-and-raw-as-load-and-.patch
Comment 11 Björn Kautler 2014-04-21 21:59:59 UTC
Created attachment 274849 [details] [review]
master: 0002-Export-the-image-before-saving-as-raw-image-data.patch
Comment 12 Björn Kautler 2014-04-21 22:00:29 UTC
Created attachment 274850 [details] [review]
master: 0003-Use-GtkBuilder-for-raw-image-data-export-dialog.patch
Comment 13 Björn Kautler 2014-04-21 22:01:07 UTC
Created attachment 274851 [details] [review]
master: 0004-Save-and-restore-last-selected-values-and-provide-se.patch
Comment 14 Björn Kautler 2014-04-21 22:01:33 UTC
Created attachment 274852 [details] [review]
master: 0005-Allow-non-interactive-and-last-vals-call-of-raw-imag.patch
Comment 15 Björn Kautler 2014-04-21 22:09:42 UTC
Ok Jehan, I ported the changeset over to master and attached those patches also. I hope they compile and run cleanly. As I said, I don't have master set-up to build, but as there were only little changes to saving in file-raw-data.c I think it should work the same as on the branch.



mitch, you are right of course and I feared that such a question will come.
I guess I could start here with a declarative ui vs. programmatic ui discussion, or say something about consistency so that file-raw works the same as file-png and a developer knowing how one works also knows how the other works a. s. o.

But to be honest, beside these causes that are also valid from my POV, the main reasons were relatively selfish. I wanted to copy over and adapt some of the logic of the file-png plugin and as that used the GtkBuilder, I thought it would be easiest to bring file-raw to level and then do the copy & adapt, especially as I do very little C programming (Java mostly) and never did any Gtk stuff before, neither programmatically nor via GtkBuilder. So it was also a great opportunity to gather some knowledge about both, programmatic and declarative Gtk development. :-)
Comment 16 Jehan 2014-04-22 00:26:08 UTC
Thanks Björn. I guess I'll let Mitch decide what to do, if he has any concern about the patches, since he's the boss around here. ;-)
Comment 17 Michael Natterer 2014-04-22 20:06:09 UTC
Comment on attachment 274848 [details] [review]
master: 0001-Register-data-as-save-extension-and-raw-as-load-and-.patch

Pushed to master, minus the "raw" extension, as discussed on IRC.

commit 3e96b96640fe4d6f69174cfbe2db7f304409e61e
Author: Björn Kautler <Bjoern@Kautler.net>
Date:   Tue Apr 15 23:26:44 2014 +0200

    Bug 728633 - Improvements to the file-raw plugin
    
    Register 'data' as save extension.

 plug-ins/common/file-raw-data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 18 Michael Natterer 2014-04-22 20:08:39 UTC
Comment on attachment 274783 [details] [review]
gimp-2-8: 0001-Register-data-as-save-extension-and-raw-as-load-and-.patch

Pushed to gimp-2-8:

commit 6c1b88d912fb73855507433fdf797642ed4646cf
Author: Björn Kautler <Bjoern@Kautler.net>
Date:   Tue Apr 15 23:26:44 2014 +0200

    Bug 728633 - Improvements to the file-raw plugin
    
    Register 'data' as save extension.

 plug-ins/common/file-raw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 19 Michael Natterer 2014-04-22 20:10:29 UTC
Comment on attachment 274784 [details] [review]
gimp-2-8: 0002-Export-the-image-before-saving-as-raw-image-data.patch

Pushed to gimp-2-8:

commit 8b62ab865be7311aa55e11759f184f54b4f50246
Author: Björn Kautler <Bjoern@Kautler.net>
Date:   Thu Apr 17 03:21:26 2014 +0200

    Bug 728633 - Improvements to the file-raw plugin
    
    Export the image before saving as raw image data.

 plug-ins/common/file-raw.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Comment 20 Michael Natterer 2014-04-22 20:12:20 UTC
Comment on attachment 274849 [details] [review]
master: 0002-Export-the-image-before-saving-as-raw-image-data.patch

Pushed to master:

commit c3f2a5a1169c13bd28a1ff252703dc67897f1fc4
Author: Björn Kautler <Bjoern@Kautler.net>
Date:   Thu Apr 17 03:21:26 2014 +0200

    Bug 728633 - Improvements to the file-raw plugin
    
    Export the image before saving as raw image data.

 plug-ins/common/file-raw-data.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
Comment 21 Michael Natterer 2014-04-22 20:26:10 UTC
Comment on attachment 274785 [details] [review]
gimp-2-8: 0003-Use-GtkBuilder-for-raw-image-data-export-dialog.patch

This refactoring is too much for stable.
Comment 22 Michael Natterer 2014-04-22 20:26:49 UTC
Comment on attachment 274786 [details] [review]
gimp-2-8: 0004-Save-and-restore-last-selected-values-and-provide-se.patch

This is a new feature, so not for stable.
Comment 23 Michael Natterer 2014-04-22 20:31:43 UTC
Comment on attachment 274787 [details] [review]
gimp-2-8: 0005-Allow-non-interactive-and-last-vals-call-of-raw-imag.patch

There is a comment about a good reason not to support non-interactive in this plug-in. Let's reject that for 2-8 and think about it for master.
Comment 24 Michael Natterer 2014-04-22 20:32:18 UTC
All the remaining patches are for master, setting milestone.
Comment 25 Michael Schumacher 2016-06-13 23:41:00 UTC
The three remaining patches are available in branch bug-728633
https://git.gnome.org/browse/gimp/tree/?h=bug-728633
Comment 26 Michael Schumacher 2016-12-30 20:57:51 UTC
commit 1ed5def8e78013c99941f69643c713f6fd12de34
Merge: 779649bdb7 ce9e7feabd
Author: Michael Schumacher <schumaml@gmx.de>
Date:   Fri Dec 30 21:41:38 2016 +0100

    Bug 728633 - Improvements to the file-raw plugin
    
    Merge branch bug-728633 to introduce the file-data-raw changes.