[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-kimageshop
Subject: Re: Locking branch ready for testing
From: Sven Langkamp <sven.langkamp () gmail ! com>
Date: 2012-07-10 13:01:09
Message-ID: CAAmsBfkNx_RCXo20vr-16kdswNcuktr9mivLdHpEohmsJXzyPA () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Mon, Jul 9, 2012 at 10:25 PM, Sven Langkamp <sven.langkamp@gmail.com>wrote:
> On Mon, Jul 9, 2012 at 12:14 AM, T Hall <tahall256@gmail.com> wrote:
>
>> On Sun, 8 Jul 2012 22:02:25 +0200
>> Sven Langkamp <sven.langkamp@gmail.com> wrote:
>>
>> > On Sun, Jul 8, 2012 at 4:39 PM, T Hall <tahall256@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)
>>
>
> Fixed all the selection actions. The layer actions still are showing the
> old behavior, but that's not a regression that comes from the branch.
>
> We need someone to go through all of them and then set the correct
> behavior for each. It's quite easy so would be ideal for a junior job.
>
>> >
>> >
>> > > > - 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)
>>
>
> Works here.
>
>
>> >
>> >
>> > > 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.
>>
>>
>
Branch is now merged to master.
[Attachment #5 (text/html)]
<div class="gmail_quote">On Mon, Jul 9, 2012 at 10:25 PM, Sven Langkamp <span \
dir="ltr"><<a href="mailto:sven.langkamp@gmail.com" \
target="_blank">sven.langkamp@gmail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> <div class="gmail_quote"><div><div class="h5">On Mon, Jul 9, \
2012 at 12:14 AM, T Hall <span dir="ltr"><<a href="mailto:tahall256@gmail.com" \
target="_blank">tahall256@gmail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">
<div>On Sun, 8 Jul 2012 22:02:25 +0200<br>
Sven Langkamp <<a href="mailto:sven.langkamp@gmail.com" \
target="_blank">sven.langkamp@gmail.com</a>> wrote:<br> <br>
> On Sun, Jul 8, 2012 at 4:39 PM, T Hall <<a href="mailto:tahall256@gmail.com" \
target="_blank">tahall256@gmail.com</a>> wrote:<br> ><br>
> > Hi Sven,<br>
> ><br>
> > I've just compiled your branch and given it a quick test. These are<br>
> > my observations so far:<br>
> ><br>
> > > - tools can be changed even if the active layer is<br>
> > > locked/invisible, no longer grayed out (unless it's a shape \
layer)<br> > > Seems to work fine.<br>
> > > - cursor/brush outline is still shown when the layer is<br>
> > > locked/invisible<br>
> > Works fine.<br>
> > > - selections can be edited even if the current layer is locked,<br>
> > > unless there is a locked local selection<br>
> > Selection tools work. However, there are a couple of functions<br>
> > (Deselect, Reselect and, Invert Selection), which aren't allowed on<br>
> > locked layers (Greyed-out in menu, shortcuts do nothing). Since<br>
> > these functions only edit selections, and don't affect the layer, I<br>
> > think they should be allowed when the selected layer is locked.<br>
> ><br>
><br>
> Fixed, now just depends on the selection state.<br>
</div>Invert Selection is still greyed-out for locked layers. In theory, it<br>
should work on any selection. It is available when the layer is not<br>
locked, so it's definitely a locking issue. (I also think it *should*<br>
work when nothing is selected. Invert nothing == select all. I fail to<br>
see any advantage to blocking this \
possibility)<br></blockquote></div></div><div><br>Fixed all the selection actions. \
The layer actions still are showing the old behavior, but that's not a regression \
that comes from the branch.<br>
<br>
We need someone to go through all of them and then set the correct
behavior for each. It's quite easy so would be ideal for a junior \
job.</div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px \
0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>><br>
><br>
> > > - if the user tries to paint on a locked layer, Krita will show a<br>
> > > small notification<br>
> > Works fine. However, no notification is shown if the user tried to<br>
> > edit the via a shortcut. e.g. pressing backspace to fill the layer<br>
> > with the foreground colour (the application behaves correctly, in<br>
> > that it doesn't allow the layer to be filled, it's only the<br>
> > notification that's missing). I think it would be nice if the<br>
> > notification appeared for such actions. (I haven't looked at how<br>
> > locking a layer disables/blocks shortcuts/actions like this, so<br>
> > don't know how difficult it would be to implement this)<br>
> ><br>
> > Additionally:<br>
> > Items from the "Layer" menu (Mirror Horizontally, Mirror<br>
> > Vertically, Shear Layer, Scale Layer, Rotate..., Convert Layer Type)<br>
> > are still available for use on locked layers. Most of these are<br>
> > destructive (not revertable except via undo), so definitely<br>
> > shouldn't be allowed on locked layers.<br>
> ><br>
><br>
> This is kind of a problem. In KDE it usual that if the action is<br>
> disabled menu entry and shortcut are disabled. So we could either<br>
> have a notification or disable the menu entry. Not sure what to do<br>
> there.<br>
</div>I guess we'll have to manage without notifications for<br>
disallowed shortcuts. If the user is using the menus, the item being<br>
greyed-out at least tells the user that the action is not permitted (as<br>
opposed to the various possibilities when nothing appears to happen<br>
when you press a shortcut: could be you pressed the wrong key, maybe it<br>
did something but it wasn't visible/what you expected to see, etc.<br>
etc.) Presumably if KDE is designed to not do anything for disabled<br>
shortcuts/menu items, then any workaround would probably be rather<br>
messy and more effort than it's worth.<br>
<br>
In the mean time, I notice the items from the Layer menu are still<br>
active. I really don't think they should be.<br>
<div>><br>
><br>
><br>
> > The transform tool also exhibits some odd behaviour (What's<br>
> > new :p). It allows you to make the transformation, as long as you<br>
> > don't apply it (Seems reasonable). However, when you try to apply<br>
> > it (and get the "Layer is locked" notification), it returns all \
the<br> > > handles to their original position.<br>
> > 1. It appears you can't keep your transformation and either carry \
on<br> > > transforming or unlock the layer then apply it (if you really \
meant<br> > > to do the transformation, but forgot to unlock the layer \
first).<br> > > 2. Actually, if (before attempting further transformation) \
you<br> > > switch to another tool, then switch back to the transform tool,<br>
> > then your transformation reappears as it was before you tried to<br>
> > apply it (allowing you to carry on transforming, or unlock and<br>
> > apply).<br>
> ><br>
> > I believe correct behaviour should be:<br>
> > When you try to apply the transformation, handles remain in their<br>
> > transformed position, allowing you to continue working with the<br>
> > transformation (without having to switch to another tool and back).<br>
> ><br>
><br>
> Fixed, handles remain now.<br>
</div>Handles remain, but it isn't possible to manipulate them further<br>
without switching to another tool, then switching back to the transform<br>
tool. (try to apply, then unlock and apply now works, it's only if you<br>
want to tweak the transformation after trying to apply that it still<br>
doesn't work)<br></blockquote></div></div><div><br>Works here.<br> \
</div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px \
0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div>><br>
><br>
> > Probably unrelated: I notice I can't make a selection when the<br>
> > group layer is selected, be it unlocked or locked. Following the<br>
> > logic of selection tools do not edit the selected layer, perhaps<br>
> > they should be allowed? This could also be related to allowing the<br>
> > transform tool to transform multiple layers at the same time by<br>
> > operating on a group layer (feature request detailed in bug<br>
> > 297927). Anyway, I don't think this is related to 'lockedness' \
or<br> > > specific to your branch, so I guess it's offtopic in this \
email.<br> > ><br>
> ><br>
> Fixed.<br>
</div>For a moment, I wondered if you'd fixed bug 297927, but I see we're \
now<br> back to being able to make selections with a group layer selected,<br>
which at least means the bug is now accurately reproducible once more :)<br>
I suspect when that bug is fixed, the group layer will need to take into<br>
account the 'lockedness' of all it's child layers when being edited. \
(I<br> am thinking that transforming a whole group of layers should not be<br>
allowed if one of them is locked.)<br>
<div>><br>
><br>
> > So to summarise:<br>
> > - Selection functions disallowed<br>
> > - No notifications shown for shortcuts/menu actions<br>
> > - Items from the Layer menu allowed to edit locked layers<br>
> > - Transform tool oddities<br>
> > - Incorrect notification for group layer locked<br>
> ><br>
> > Hope that helps.<br>
> ><br>
><br>
> It did :). Thanks a lot for the detailed testing.<br>
<br>
</div></blockquote></div></div></div><br>
</blockquote></div><br>Branch is now merged to master.<br>
_______________________________________________
kimageshop mailing list
kimageshop@kde.org
https://mail.kde.org/mailman/listinfo/kimageshop
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic