GNOME Bugzilla – Bug 728633
Improvements to the file-raw plugin
Last modified: 2016-12-30 21:02:06 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)
Created attachment 274783 [details] [review] gimp-2-8: 0001-Register-data-as-save-extension-and-raw-as-load-and-.patch
Created attachment 274784 [details] [review] gimp-2-8: 0002-Export-the-image-before-saving-as-raw-image-data.patch
Created attachment 274785 [details] [review] gimp-2-8: 0003-Use-GtkBuilder-for-raw-image-data-export-dialog.patch
Created attachment 274786 [details] [review] gimp-2-8: 0004-Save-and-restore-last-selected-values-and-provide-se.patch
Created attachment 274787 [details] [review] gimp-2-8: 0005-Allow-non-interactive-and-last-vals-call-of-raw-imag.patch
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!
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.
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.
Nice, but I fail to see how almost the same amount of code *plus* a builder file is better than the old dialog code ;)
Created attachment 274848 [details] [review] master: 0001-Register-data-as-save-extension-and-raw-as-load-and-.patch
Created attachment 274849 [details] [review] master: 0002-Export-the-image-before-saving-as-raw-image-data.patch
Created attachment 274850 [details] [review] master: 0003-Use-GtkBuilder-for-raw-image-data-export-dialog.patch
Created attachment 274851 [details] [review] master: 0004-Save-and-restore-last-selected-values-and-provide-se.patch
Created attachment 274852 [details] [review] master: 0005-Allow-non-interactive-and-last-vals-call-of-raw-imag.patch
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. :-)
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 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 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 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 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 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 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 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.
All the remaining patches are for master, setting milestone.
The three remaining patches are available in branch bug-728633 https://git.gnome.org/browse/gimp/tree/?h=bug-728633
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.