From kde-kimageshop Tue May 17 11:26:15 2011 From: Dmitry Kazakov Date: Tue, 17 May 2011 11:26:15 +0000 To: kde-kimageshop Subject: Re: Proposal for global selections Message-Id: X-MARC-Message: https://marc.info/?l=kde-kimageshop&m=130563161505653 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============1610525800==" --===============1610525800== Content-Type: multipart/alternative; boundary=20cf3071c8004bf64504a3770a33 --20cf3071c8004bf64504a3770a33 Content-Type: text/plain; charset=UTF-8 On Mon, May 16, 2011 at 11:21 AM, Boudewijn Rempt <> wrote: > On Sunday 15 May 2011 May, Dmitry Kazakov wrote: > > And we can't apply a visitor to it, because it is not a node. > > Well, that's not really true. We apply a visitor on a layer, so we can get the image from the layer and from the image we should get the global selection. Or we could pass the global selection to the visitor's constructor arguments. So every visitor can access the global selection. I bet they can access the global selection! ;) But they can't do anything with the selection itself. :) Just take a look at the KisCropVisitor. To perform a crop, it needs to change the offset of the paint device of the node. This is done using the KisNodeMoveCommand. The same action is needed for the global selection. But we can't apply the same command to it, because this is not a node. So we'll have to create a special command for the selection (more precisely, we'll have to just copy-and-paste some code from KisNodeMoveCommand). And the same fate is awaiting other high-level commands those will be created for nodes one day. > If they don't that's a bug. Or pass the image to the constructor of the visitor, which would be nicer than trying to get the image from the layer, since we should be able to have a layer tree without an image. I totally agree with it, but i'll write a seperate mail about it. > > KisImage will store a KisSelectionMask inside and will put this mask as a > > child to the root node. > > With root node, you mean the root group layer, right? Yes. > I'm not sure about this. I know we've tried this before and given up on it, but I don't know why anymore. But I really, really, really want to stop going back and forth between solutions. Our handling of selections has a horrible history where we refactor time and again back to an earlier stage and then forward and backward again. We nead to break that pattern. Ok, Boud. I don't like doing refactoring as well. But current way simply does not work for all the usecases. So I should be changed. In one way or another. > > This assignment will be done in two functions > > setGlobalSelection() and setRootNode(). > > And will setRootNode replace setRootLayer? He-he. Currently we have both of them. I would prefer to remove KisImage::setRootLayer() and to leave KisNodeFacade::setRoot. setRoot() wiil be just extended by the KisImage. > > UI problem: > > If the mask is added to the node graph, then it is shown in a KisLayerBox. > > So we will have to add an option to KisConfig to hide it as we do for the > > root layer. Actually, we may leave a way for a user to show it. It worked > > quite well, when i tried to add it in a hardcoded way. The only trouble i > > got was that it was impossible to toggle "Active" property of the mask. > > Everything rest was working very well. > > If we go this way, I think we should remove the option to show the root group layer from Krita. Showing the global selection on the root group layer will make all kinds of interactions possible that we haven't designed for and that will make krita more buggy, like dragging away the global selection to another layer. If we design it well, all the iterations will work as well. Actually, I've already tried it (hardcoded version) and some of them do work out-of-box =) Anyway we will have to spend some time on these iterations: either to enable them or to disable them. Actually to do both of the tasks properly it'll take the same amount of time. Btw, if the global selection is a mask, then we don't need KisImage::deselectedGlobalSelection() anymore. We can just toggle the "Active" property of the mask. Just think about it: we will not need KisSetGlobalSelection/KisDeselectGlobalSelection commands anymode. We'll use usual node commands for it. > > Why we can't do it in another way: > > Because all the visitors and most of the commands in Krita work with nodes. > > So, for example, to shift a global selection (setX(), setY() methods) we > > will have to create one more command. We will have to modify/duplicate all > > our visitors/commands to let them work with selections. > > Modify, yes -- but duplicate? Of course not. And it's by no means impossible to do. I've already written about this duplication above. > And having the global selection as a selection on the root group layer can cause other bugs as well, for instance by code that takes the global selection from the image, and uses it, but also takes the selection mask from the root group layer. The 'taking' of the selection out of the image can be easily encapsulated inside the image. > I'm not against this plan, but I'm not sure it has been thought through enough, and I'd like other people's opinions as well. > > > How it is done in other editors: > > I tried Gimp. It transforms global selection with the image. And PS7, as far > > as i remember, crops the selection alongside with the image as well. > > For crop, the selection's bounds should become the initial crop rect. I meant that the selection is cropped as well when you select the crop area smaller than the selection itself. -- Dmitry Kazakov --20cf3071c8004bf64504a3770a33 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On Mon, May 16, 2011 at 11:21 AM, Boudewijn Rempt <> wrote:
&g= t; On Sunday 15 May 2011 May, Dmitry Kazakov wrote:
> > And we can= 't apply a visitor to it, because it is not a node.
>
> Wel= l, that's not really true. We apply a visitor on a layer, so we can get= the image from the layer and from the image we should get the global selec= tion. Or we could pass the global selection to the visitor's constructo= r arguments. So every visitor can access the global selection.

I bet they can access the global selection! ;) But they can't do an= ything with the selection itself. :)

Just take a look at the KisCrop= Visitor. To perform a crop, it needs to change the offset of the paint devi= ce of the node. This is done using the KisNodeMoveCommand. The same action = is needed for the global selection. But we can't apply the same command= to it, because this is not a node. So we'll have to create a special c= ommand for the selection (more precisely, we'll have to just copy-and-p= aste some code from KisNodeMoveCommand). And the same fate is awaiting othe= r high-level commands those will be created for nodes one day.

> If they don't that's a bug. Or pass the image to the const= ructor of the visitor, which would be nicer than trying to get the image fr= om the layer, since we should be able to have a layer tree without an image= .

I totally agree with it, but i'll write a seperate mail about it.

> > KisImage will store a KisSelectionMask inside and will = put this mask as a
> > child to the root node.
>
> Wit= h root node, you mean the root group layer, right?

Yes.

> I'm not sure about this. I know we've tried th= is before and given up on it, but I don't know why anymore. But I reall= y, really, really want to stop going back and forth between solutions. Our = handling of selections has a horrible history where we refactor time and ag= ain back to an earlier stage and then forward and backward again. We nead t= o break that pattern.

Ok, Boud. I don't like doing refactoring as well. But current way s= imply does not work for all the usecases. So I should be changed. In one wa= y or another.

> > This assignment will be done in two function= s
> > setGlobalSelection() and setRootNode().
>
> And will = setRootNode replace setRootLayer?

He-he. Currently we have both of t= hem. I would prefer to remove KisImage::setRootLayer() and to leave KisNode= Facade::setRoot. setRoot() wiil be just extended by the KisImage.

> > UI problem:
> > If the mask is added to the node gra= ph, then it is shown in a KisLayerBox.
> > So we will have to add = an option to KisConfig to hide it as we do for the
> > root layer.= Actually, we may leave a way for a user to show it. It worked
> > quite well, when i tried to add it in a hardcoded way. The only t= rouble i
> > got was that it was impossible to toggle "Active= " property of the mask.
> > Everything rest was working very = well.
>
> If we go this way, I think we should remove the option to show= the root group layer from Krita. Showing the global selection on the root = group layer will make all kinds of interactions possible that we haven'= t designed for and that will make krita more buggy, like dragging away the = global selection to another layer.

If we design it well, all the iterations will work as well. Actually, I= 've already tried it (hardcoded version) and some of them do work out-o= f-box =3D) Anyway we will have to spend some time on these iterations: eith= er to enable them or to disable them. Actually to do both of the tasks prop= erly it'll take the same amount of time.

Btw, if the global selection is a mask, then we don't need KisImage= ::deselectedGlobalSelection() anymore. We can just toggle the "Active&= quot; property of the mask. Just think about it: we will not need KisSetGlo= balSelection/KisDeselectGlobalSelection commands anymode. We'll use usu= al node commands for it.


> > Why we can't do it in another way:
> > Becau= se all the visitors and most of the commands in Krita work with nodes.
&= gt; > So, for example, to shift a global selection (setX(), setY() metho= ds) we
> > will have to create one more command. We will have to modify/dupl= icate all
> > our visitors/commands to let them work with selectio= ns.
>
> Modify, yes -- but duplicate? Of course not. And it'= ;s by no means impossible to do.

I've already written about this duplication above.

> And = having the global selection as a selection on the root group layer can caus= e other bugs as well, for instance by code that takes the global selection = from the image, and uses it, but also takes the selection mask from the roo= t group layer.

The 'taking' of the selection out of the image can be easily en= capsulated inside the image.

> I'm not against this plan, but= I'm not sure it has been thought through enough, and I'd like othe= r people's opinions as well.
>
> > How it is done in other editors:
> > I tried Gim= p. It transforms global selection with the image. And PS7, as far
> &= gt; as i remember, crops the selection alongside with the image as well. >
> For crop, the selection's bounds should become the initial= crop rect.

I meant that the selection is cropped as well when you s= elect the crop area smaller than the selection itself.


--
Dmitry Kazakov

--20cf3071c8004bf64504a3770a33-- --===============1610525800== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ kimageshop mailing list kimageshop@kde.org https://mail.kde.org/mailman/listinfo/kimageshop --===============1610525800==--