Hacking:Merging lisanet.de fork

From GIMP Developer Wiki
Revision as of 11:35, 4 December 2013 by Jehan (Talk | contribs)

Jump to: navigation, search

Simone Karin Lehmann maintains an OSX release, which is basically a mini-fork since it includes several patches to GIMP.

We had a quick check with Mitch and saw that many are outdated with current code. For instance some about a switch to Carbon->Cocoa platforms already in our code tree, or others obsoleted by our menu replacement. Others seem to make sense like the "color display change".

This is a working page for GIMP developers to go through the list and decide what we want to integrate and what we want to leave out.

1/ $ svn checkout svn://svn.code.sf.net/p/gimponosx/code/ gimponosx-code

2/ $ cd gimponosx-code/GimpPorts/ports/graphics/gimp2/files/

3/ Check the patches and update the below lists to keep or not. For instance, if you reviewed a patch and it is not good for us, you can leave a "[exclude]" next to it. If we want to keep it but it needs work, add "[need work]", and so on.

4/ I set up a Bugzilla report. When a patch seems good, make a `git format-patch` and upload it there for review if needed. :-) We can close this report when all the patches of the list will have been either applied in our tree or discarded. :-)

List of Patches

patch comment merge status
app-actions-dialogs-actions.c.diff Answer: Yes, change to 'About GIMP'.

The specs supersede what other programs do.

The OS X human interface guidelines, Windows human interface guidelines provide for 'About GIMP'. The GNOME human interface guidelines uses only 'About'. More human interface guidelines are listed at Wikipedia.

Peter Sikking advices in a blog posting about cross-platform development to use the platform specific specs.

Patch in progress (scl)
app-actions-file-actions.c.diff Add "file-hide" ("<primary>H") and "file-hide-others" ("<primary><alt>H") actions. Are they common OSX actions?
app-actions-file-commands.c.diff The code (OSX specific, apparently using Cocoa headers) for the new "file-hide" and "file-hide-others" actions.
app-actions-file-commands.h.diff Headers for the new "file-hide" and "file-hide-others" actions
app-actions-Makefile.in.diff Makefile.in are generated. See the Makefile.am patch. discarded
app-actions-plug-in-actions.c.diff Change "Repeat Last" action's shortcut from "<primary>F" to "<Alt>F" and "Re-show Last" from "<primary><shift>F" to "<Alt><shift>F". Are they shortcuts which make more sense on an OSX platform maybe? No idea.
app-actions-view-actions.c.diff Changing Fullscreen actions from default F11 to "<primary>F" shortcut. Is it an OSX standard?
app-config-gimpcoreconfig.c.diff Some code relative to the save-xcf-only config added on this fork.
app-config-gimpcoreconfig.h.diff Some code relative to the save-xcf-only config added on this fork.
app-config-gimpdisplayconfig.c.diff Changing DEFAULT_MARCHING_ANTS_SPEED from 200 to 600. Is the current default too slow?
app-config-gimprc-blurbs.h.diff Blurb for the save/export changes: "Uncheck to allow all image formats to be saved via the 'File - Save' menu entry"
app-core-gimpimage.c.diff Code relative to the save/export changes.
app-core-gimpimage.h.diff Code relative to the save/export changes.
app-core-gimp-user-install.c.diff Ah! This one looks interesting. It adds some migration code for the config in ~/Library/GIMP (apparently 2.8.2) to /Library/Application Support/GIMP (since 2.8.4). Code checked and does some bad things (like moving config folder of older versions!) so I basically rewrote it fully. Patch proposed
app-dialogs-preferences-dialog.c.diff Adding a preference option for the save-xcf-only change + change to the help browser option, where the web browser is apparently always used if GIMP help browser is not installed. The save/export option is not of interest. But maybe this help browser change makes any sense?
app-file-file-open.c.diff Disable some check which looks for a "icc-profile" parasite on our image. Something to look into why she did this?
app-file-file-save.c.diff Some code related to the save/export changes.
app-gui-gui.c.diff Looks like some kind of menu grouping change. I guess someone with OSX platform knowledge will know better what it is.
app-gui-gui-unique.c.diff Looks interesting, though I guess some OSX dev would need to look closer. Apparently some desktop integration. I copy-paste an in-code comment:
/* The application delegate handles some events
 * - terminate GIMP
 * - open files when GIMP is already running by dropping on the icon or Finder's 'Open with'
 * The event handler
 * - handles open events when GIMP is launched by dropping on the icon or Finder's 'Open with'
 * Now Cocoa based.
app-gui-Makefile.in.diff Makefile.in are generated. See the Makefile.am patch. discarded
app-Makefile.am.diff commit e56344294c90e1ba97de5c134b50c4c522f0808f merged
app-Makefile.in.diff Makefile.in are generated. See the Makefile.am patch. discarded
app-menus-menus.c.diff Not sure. Apparently loading some additional menu or something?
app-widgets-gimphelp.c.diff Some change in the help browser apparently, and apparently better integration to OSX locales, or something. Copy-paste from code:
  /* on OS X there's no need for config->help_locales. The user can handle this
   * via System Preferences. Even if she wants to use a different help locale than the 
   * default language. Just add this alternative locale in System Preferences and
   * install the corresponding User manual.
app-widgets-Makefile.in.diff Makefile.in are generated. See the Makefile.am patch. discarded
configure.diff configure is generated. See the configure.ac patch. discarded
etc-gimprc.diff Add a config option to allow to save in any format.
lcms.c.diff See plug-ins-common-lcms.c.diff (duplicate). discarded
libgimpbase-gimpenv.c.diff Just does this change:

-#include <AppKit/AppKit.h> +#import <Foundation/Foundation.h> Not sure why is that.

libgimp-gimpui.c.diff Removes a signal handling on "show" for a window that is made transient. Not sure what this signal handling was supposed to do ([NSApp arrangeInFront: nil] -> pushing it to the top of window layers?).
libgimp-Makefile.in.diff Makefile.in are generated. See the Makefile.am patch. discarded
libgimpthumb-gimp-thumbnail-list.c.diff Add support for Freedesktop's Thumbnail standard to this file. We already added support to another file as of bug 646644. It seems we may have forgotten a part of the code. :-)

I opened bug 719830 for this, with a patch based on our existing core code..

patch to be reviewed
libgimpthumb-gimpthumb-utils.c.diff Just does this change:

-#include <AppKit/AppKit.h> +#import <Foundation/Foundation.h> Not sure why is that.

libgimpwidgets-gimpdialog.c.diff Position a dialog at display center as default.
libgimpwidgets-Makefile.am.diff-old.diff Same as libgimpwidgets-Makefile.am.diff with fixed tabulation. discarded
libgimpwidgets-Makefile.in.diff Makefile.in are generated. See the Makefile.am patch. discarded
Makefile.in.diff This patch tries to disable gtk-doc on a `distcheck` target. discarded
modules-display-filter-lcms.c.diff Looks interesting. If nothing else because it seems this piece of code has been forgotten and is still using Carbon on our master (at least there is a remaining Carbon.h include)!
modules-Makefile.in.diff Makefile.in are generated. See the Makefile.am patch. discarded
plug-ins-common-file-tiff-load.c.diff See comment on plug-ins-file-jpeg-jpeg.c.diff. discarded
plug-ins-common-file-tiff-save.c.diff See comment on plug-ins-file-jpeg-jpeg.c.diff. discarded
plug-ins-common-lcms.c.diff Slight changes in icc profile handling. One of them is a fallback if lcms_image_get_profile() fails or maybe if it was deleted because not RGB. When it happens, she would extract the profile with the sips command line through system() in a temporary file and loads it as the source profile. I don't know what to think about it. Does that actually mean that lcms_image_get_profile() may fail sometimes but `sips` would succeed? (in this case, why not rather patch lcms?!) Or does that mean that she wants to support some profiles other than RGB?

+ Some other logics changes. I did not review all in details.

plug-ins-common-web-browser.c.diff Interesting patch. It adds an OSX specific code to display an URI with the OSX `open` command (would be the standard way to get the user-specified web-browser on OSX), instead of using gtk_show_uri() (which itself uses glib's g_app_info_launch_default_for_uri(), which I think follows Freedesktop's MIME actions spec). This said, I'd think that if this change is really the right one, we want to port it upstream to glib, not just waste it on this plugin's code.
plug-ins-file-jpeg-jpeg.c.diff Try to load and save back metadata (saved in a temporary empty jpeg image) by running the external command exiftool through system()! I don't think we want this, first because we have a new metadata system in master, second because that's a terrible design for just so many reasons! discarded
plugins-filejpeg-jpeg.c.diff Exactly same as plug-ins-file-jpeg-jpeg.c.diff discarded
plug-ins-help-Makefile.in.diff Makefile.in are generated. See the Makefile.am patch. discarded
plug-ins-script-fu-Makefile.in.diff Makefile.in are generated. See the Makefile.am patch. discarded
po-de.po.diff Answer: Yes, 'Über GIMP' is the proper translation. for Windows and OS X, for GNOME it is 'Über'. patch in progress (scl)
save-export-conf-patch.diff Code for the save/export changes.
save-export-patch.diff I don't think anyone has actually checked the save/export patches, but there are some written specs. If this is up to date with the patch, basically it allows to "save" images into non-XCF format if and only if the image has just

one layers. The current opinion is that this feels like "unpredictable magic". It would likely feel like really inconsistent to a user (some times you can "save" to non-XCF, sometimes no).