GNOME Bugzilla – Bug 674160
Redesign of "Lock panel"
Last modified: 2012-11-09 10:23:51 UTC
Created attachment 212101 [details] [review] This is a small redesign of "Lock panel" for GIMP 2.8 This is a small redesign of "Lock panel" for GIMP 2.8: Added - "Lock position" toggle (translation and transformation), Changed - "Lock pixels" toggle - applies only to paint-tools and color-tools. And I rewrote algorithms for right work each lock-features with group-layer and linked layers! Will be added in the future - "Lock all" toggle. It will disable opacity, blending mode, transformation, translation and any paint functions.
Created attachment 212106 [details] Binary files - stock-lock-16.png
Created attachment 212107 [details] Binary file - stock-unlock-16.png
Review of attachment 212101 [details] [review]: Disclaimer: I'm not a GIMP developer so I cannot comment on the logic, but the patch has several style issues - just listing a few here. ::: .gitignore @@ +52,3 @@ +/.settings/ +/TODO-Layer-lock-translation + Empty lines, plus I don't understand why this is needed in this patch. ::: app/actions/layers-actions.c @@ +243,3 @@ GIMP_HELP_LAYER_LOCK_ALPHA }, + Please avoid noise in the diff by adding random newlines / changes to space vs tab. @@ +529,3 @@ gboolean lock_alpha = FALSE; gboolean can_lock_alpha = FALSE; + Same here, and in other places... ::: app/actions/layers-commands.c @@ +279,3 @@ +//Remy: +printf("run -> layers_new_last_vals_cmd_callback\n"); This is debug stuff that you want to remove first. ::: app/core/gimpgrouplayer.c @@ +501,3 @@ GList *list; + //Remy: stop translate if group has any move-locked layers! Can you remove the "//Remy: " strings, but leave comments that you consider helpful for understanding, but use /* */ syntax instead? ::: app/core/gimpitem-linked.c @@ +40,3 @@ + * Для перемещения: + * Добавить проход по вложенным слоям для слоя-группы для поиска среди них фиксированных ! + */ Please have any comments in English. I understand "word groups" and "fixed" but that's where my Russian ends. ;-) @@ +92,3 @@ + return TRUE; + } + //Remy: if pixels locked: I'll stop reviewing here, again the "//Remy: "...
(In reply to comment #3) > Review of attachment 212101 [details] [review]: > > Disclaimer: I'm not a GIMP developer so I cannot comment on the logic, but the > patch has several style issues - just listing a few here. > > ::: .gitignore > @@ +52,3 @@ > +/.settings/ > +/TODO-Layer-lock-translation > + > > Empty lines, plus I don't understand why this is needed in this patch. > > ::: app/actions/layers-actions.c > @@ +243,3 @@ > GIMP_HELP_LAYER_LOCK_ALPHA }, > > + > > Please avoid noise in the diff by adding random newlines / changes to space vs > tab. > > @@ +529,3 @@ > gboolean lock_alpha = FALSE; > gboolean can_lock_alpha = FALSE; > + > > Same here, and in other places... > > ::: app/actions/layers-commands.c > @@ +279,3 @@ > > +//Remy: > +printf("run -> layers_new_last_vals_cmd_callback\n"); > > This is debug stuff that you want to remove first. > > ::: app/core/gimpgrouplayer.c > @@ +501,3 @@ > GList *list; > > + //Remy: stop translate if group has any move-locked layers! > > Can you remove the "//Remy: " strings, but leave comments that you consider > helpful for understanding, but use /* */ syntax instead? > > ::: app/core/gimpitem-linked.c > @@ +40,3 @@ > + * Для перемещения: > + * Добавить проход по вложенным слоям для слоя-группы для поиска среди них > фиксированных ! > + */ > > Please have any comments in English. I understand "word groups" and "fixed" but > that's where my Russian ends. ;-) > > @@ +92,3 @@ > + return TRUE; > + } > + //Remy: if pixels locked: > > I'll stop reviewing here, again the "//Remy: "... Big thanks for your remarks. It's my first patch and first experience with Git and Open Sourse project as well. Now I'm trying to make a new patch that will better than first! What about comments like Remy:, Remy! and Remy? - I used they just for myself and I forgot to remove them, before build a .patch file. So, my apologies!
Created attachment 212240 [details] [review] app: small redesign of "Lock panel" for GIMP 2.8+ This is a modern "Lock Panel" with new buttons and some useful features. That patch contain following changes: Added - "Lock position" toggle button that blocks translation and transformation for layer, channel or vector. Changed - "Lock pixels" toggle button now applies only to paint-tools and color-tools. And I rewrote algorithms for right work each lock-features with group-layer and linked layers! An example for lock-position: if item is linked with one or more locked items then it will locked too. And group layer will locked if it has any locked children. Will be added in the future - "Lock all" toggle. It will disable opacity, blending mode, transformation, translation and any paint functions. Signed-off-by: remy <remyxxl@googlemail.com>
Created attachment 212241 [details] updated - stock-lock-16.png
Created attachment 212242 [details] updated - stock-unlock-16.png
These patches are quite complete I'd say, very nice work. Hacking on them right now.
Some preparation for the lock-content feature: commit 5732088b0c50741015a938e7ba7d65eeb2b18171 Author: Michael Natterer <mitch@gimp.org> Date: Thu Nov 8 20:46:18 2012 +0100 pdb: replace gimppdb-utils' "writable" boolean by a bitmask called GimpPDBItemModify that currently has one member GIMP_PDB_ITEM_CONTENT, in order to be prepared for adding the "lock position" feature from bug 674160.
Thanks for this patch, I considerable modified stuff and pushed to master. "Lock Content" still applies to transforms that actually change the pixels too. commit d4933b30526903624340ed98f97ed094b3a80b4d Author: Michael Natterer <mitch@gimp.org> Date: Fri Nov 9 11:17:25 2012 +0100 Bug 674160 - Redesign of "Lock panel" Apply and heavily modify patch from remyDev which adds "lock position" to GimpItem, similar to "lock content". Lock position disables all sorts of translation and transform, from the GUI and the PDB. Cleaned up some aspects of the lock content code as well because a second instance of similar code always shows what went wrong the first time. app/actions/drawable-actions.c | 66 +++++++++++++-------- app/actions/drawable-commands.c | 30 ++++++++++ app/actions/drawable-commands.h | 2 + app/actions/layers-actions.c | 22 +++---- app/actions/vectors-actions.c | 61 ++++++++++++-------- app/actions/vectors-commands.c | 27 +++++++++ app/actions/vectors-commands.h | 2 + app/core/core-enums.c | 4 ++ app/core/core-enums.h | 2 + app/core/gimpgrouplayer.c | 21 +++++++ app/core/gimpimage-undo-push.c | 32 +++++++++++ app/core/gimpimage-undo-push.h | 6 ++ app/core/gimpitem-linked.c | 33 +++++++++++ app/core/gimpitem-linked.h | 48 ++++++++-------- app/core/gimpitem.c | 133 ++++++++++++++++++++++++++++++++++++------- app/core/gimpitem.h | 17 ++++-- app/core/gimpitempropundo.c | 28 +++++++++ app/core/gimpitempropundo.h | 6 +- app/core/gimplayermask.c | 64 +++++++++++++-------- app/pdb/drawable-transform-cmds.c | 54 ++++++++++++------ app/pdb/gimppdb-utils.c | 10 ++++ app/pdb/internal-procs.c | 2 +- app/pdb/item-cmds.c | 114 +++++++++++++++++++++++++++++++++++++ app/pdb/item-transform-cmds.c | 27 ++++++--- app/pdb/layer-cmds.c | 62 ++++++++++++-------- app/pdb/pdb-types.h | 3 +- app/pdb/transform-tools-cmds.c | 18 ++++-- app/pdb/vectors-cmds.c | 22 +++++-- app/tools/gimpmovetool.c | 130 +++++++++++++++++++++++++++++++----------- app/tools/gimptransformtool.c | 53 ++++++++++++----- app/tools/gimpvectortool.c | 3 +- app/widgets/gimpchanneltreeview.c | 24 ++++---- app/widgets/gimpdrawabletreeview.c | 6 +- app/widgets/gimphelp-ids.h | 7 ++- app/widgets/gimpitemtreeview.c | 121 +++++++++++++++++++++++++++++++++++++-- app/widgets/gimpitemtreeview.h | 5 ++ app/widgets/gimplayertreeview.c | 29 +++++----- app/widgets/gimpvectorstreeview.c | 31 +++++----- app/xcf/xcf-load.c | 22 +++++++ app/xcf/xcf-private.h | 3 +- app/xcf/xcf-save.c | 17 ++++++ libgimp/gimp.def | 2 + libgimp/gimpitem_pdb.c | 66 +++++++++++++++++++++ libgimp/gimpitem_pdb.h | 3 + tools/pdbgen/pdb/drawable_transform.pdb | 19 +++++-- tools/pdbgen/pdb/item.pdb | 53 +++++++++++++++++ tools/pdbgen/pdb/item_transform.pdb | 12 ++-- tools/pdbgen/pdb/layer.pdb | 62 ++++++++++++-------- tools/pdbgen/pdb/transform_tools.pdb | 18 ++++-- tools/pdbgen/pdb/vectors.pdb | 22 +++++-- 50 files changed, 1291 insertions(+), 333 deletions(-)