Locking branch ready for testing

T Hall tahall256 at gmail.com
Sun Jul 8 22:14:24 UTC 2012


On Sun, 8 Jul 2012 22:02:25 +0200
Sven Langkamp <sven.langkamp at gmail.com> wrote:

> On Sun, Jul 8, 2012 at 4:39 PM, T Hall <tahall256 at gmail.com> wrote:
> 
> > Hi Sven,
> >
> > I've just compiled your branch and given it a quick test. These are
> > my observations so far:
> >
> > > - tools can be changed even if the active layer is
> > > locked/invisible, no longer grayed out (unless it's a shape layer)
> > Seems to work fine.
> > > - cursor/brush outline is still shown when the layer is
> > > locked/invisible
> > Works fine.
> > > - selections can be edited even if the current layer is locked,
> > > unless there is a locked local selection
> > Selection tools work. However, there are a couple of functions
> > (Deselect, Reselect and, Invert Selection), which aren't allowed on
> > locked layers (Greyed-out in menu, shortcuts do nothing). Since
> > these functions only edit selections, and don't affect the layer, I
> > think they should be allowed when the selected layer is locked.
> >
> 
> Fixed, now just depends on the selection state.
Invert Selection is still greyed-out for locked layers. In theory, it
should work on any selection. It is available when the layer is not
locked, so it's definitely a locking issue. (I also think it *should*
work when nothing is selected. Invert nothing == select all. I fail to
see any advantage to blocking this possibility)
> 
> 
> > > - if the user tries to paint on a locked layer, Krita will show a
> > > small notification
> > Works fine. However, no notification is shown if the user tried to
> > edit the via a shortcut. e.g. pressing backspace to fill the layer
> > with the foreground colour (the application behaves correctly, in
> > that it doesn't allow the layer to be filled, it's only the
> > notification that's missing). I think it would be nice if the
> > notification appeared for such actions. (I haven't looked at how
> > locking a layer disables/blocks shortcuts/actions like this, so
> > don't know how difficult it would be to implement this)
> >
> > Additionally:
> > Items from the "Layer" menu (Mirror Horizontally, Mirror
> > Vertically, Shear Layer, Scale Layer, Rotate..., Convert Layer Type)
> > are still available for use on locked layers. Most of these are
> > destructive (not revertable except via undo), so definitely
> > shouldn't be allowed on locked layers.
> >
> 
> This is kind of a problem. In KDE it usual that if the action is
> disabled menu entry and shortcut are disabled. So we could either
> have a notification or disable the menu entry. Not sure what to do
> there.
I guess we'll have to manage without notifications for
disallowed shortcuts. If the user is using the menus, the item being
greyed-out at least tells the user that the action is not permitted (as
opposed to the various possibilities when nothing appears to happen
when you press a shortcut: could be you pressed the wrong key, maybe it
did something but it wasn't visible/what you expected to see, etc.
etc.) Presumably if KDE is designed to not do anything for disabled
shortcuts/menu items, then any workaround would probably be rather
messy and more effort than it's worth.

In the mean time, I notice the items from the Layer menu are still
active. I really don't think they should be.
> 
> 
> 
> > The transform tool also exhibits some odd behaviour (What's
> > new :p). It allows you to make the transformation, as long as you
> > don't apply it (Seems reasonable). However, when you try to apply
> > it (and get the "Layer is locked" notification), it returns all the
> > handles to their original position.
> > 1. It appears you can't keep your transformation and either carry on
> > transforming or unlock the layer then apply it (if you really meant
> > to do the transformation, but forgot to unlock the layer first).
> > 2. Actually, if (before attempting further transformation) you
> > switch to another tool, then switch back to the transform tool,
> > then your transformation reappears as it was before you tried to
> > apply it (allowing you to carry on transforming, or unlock and
> > apply).
> >
> > I believe correct behaviour should be:
> > When you try to apply the transformation, handles remain in their
> > transformed position, allowing you to continue working with the
> > transformation (without having to switch to another tool and back).
> >
> 
> Fixed, handles remain now.
Handles remain, but it isn't possible to manipulate them further
without switching to another tool, then switching back to the transform
tool. (try to apply, then unlock and apply now works, it's only if you
want to tweak the transformation after trying to apply that it still
doesn't work)
> 
> 
> > Probably unrelated: I notice I can't make a selection when the
> > group layer is selected, be it unlocked or locked. Following the
> > logic of selection tools do not edit the selected layer, perhaps
> > they should be allowed? This could also be related to allowing the
> > transform tool to transform multiple layers at the same time by
> > operating on a group layer (feature request detailed in bug
> > 297927). Anyway, I don't think this is related to 'lockedness' or
> > specific to your branch, so I guess it's offtopic in this email.
> >
> >
> Fixed.
For a moment, I wondered if you'd fixed bug 297927, but I see we're now
back to being able to make selections with a group layer selected,
which at least means the bug is now accurately reproducible once more :)
I suspect when that bug is fixed, the group layer will need to take into
account the 'lockedness' of all it's child layers when being edited. (I
am thinking that transforming a whole group of layers should not be
allowed if one of them is locked.)
> 
> 
> > So to summarise:
> >  - Selection functions disallowed
> >  - No notifications shown for shortcuts/menu actions
> >  - Items from the Layer menu allowed to edit locked layers
> >  - Transform tool oddities
> >  - Incorrect notification for group layer locked
> >
> > Hope that helps.
> >
> 
> It did :). Thanks a lot for the detailed testing.



More information about the kimageshop mailing list