GNOME Bugzilla – Bug 605017
Non-destructive layer edits
Last modified: 2009-12-20 11:00:54 UTC
Created attachment 150073 [details] [review] Initial layer scaling (non-destructive) This bug report is for working on non-destructive layer edits. It hijacks the layer groups and uses that to implement the features. The layer group's children are used as the unchanging source, then the modifications are made on the layer group's drawable so the children do not change. Edits are made by right-clicking a group layer and selecting "Edit Layer Attributes."
Created attachment 150074 [details] Header file for non-destructive functions Header file for needed for functionality. (goes in app/core)
Created attachment 150075 [details] Code for initial patch. Code for non-destructive edits.
Created attachment 150080 [details] [review] Layer non-destructive patch Non-destructive scaling and brightness/contrast. Add #define ENABLE_LAYERND to config.h, then compile.
Created attachment 150094 [details] [review] Properly formatted patch (hopefully) This is my first attempt at a properly formatted patch. Comments on approach and feasibility wanted as well as feature requests.
Review of attachment 150094 [details] [review]: A couple of more comments on coding style: 1. Preprocessor directives have their own indentation level, so all #ifdef ENABLE_LAYER_NONDESTRUCT shall not be indented along with the code it switches 2. "if ( GIMP_IS_GROUP_LAYER(layer) )" shall be "if (GIMP_IS_GROUP_LAYER (layer))" 3. Struct members and variable assignments are aligned on a per code-block basis 4. "GIMP_GROUP_LAYER(layer);" shall be "GIMP_GROUP_LAYER (layer);" 5. In my opinion (not sure about other devs) it would not be a good idea to commit this non-GEGL approach, so there is no need to put #ifdef ENABLE_LAYER_NONDESTRUCT everywhere. Even if we will accept the patch that is a bad idea, it's better to write good code from the start than bad code that can be switched out (don't get me wrong) 6. "} else {" is wrong, braces are always on their own lines 7. You are using C++-style comments, //, but they are not allowed in GIMP's core code, you should use /* */ 8. It's better to have GIMP_GROUP_LAYER_NONDESTRUCT_MODE_NONE enums instead of defines 9. Function prototypes in header files shall be aligned 10. Always typedef structs (thinking of struct nd) 11. The proper name for "struct nd" would be "GimpNonDestructive"
I guess we all agree that the approach taken by this patch is not acceptable for main-line. Thus we should close this report and continue discussion of the proper approach on the gimp-developer mailing-list. Note that closing this report as WONTFIX doesn't mean that we don't want to have non-destructive layer edits. Non-desctructive editing is clearly on our roadmap.