<br>On Mon, May 16, 2011 at 11:21 AM, Boudewijn Rempt <> wrote:<br>> On Sunday 15 May 2011 May, Dmitry Kazakov wrote:<br>> > And we can't apply a visitor to it, because it is not a node.<br>><br>> 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. <br>
<br>I bet they can access the global selection! ;) But they can't do anything with the selection itself. :)<br><br>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.<br>
<br>> 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.<br>
<br>I totally agree with it, but i'll write a seperate mail about it.<br><br><br>> > KisImage will store a KisSelectionMask inside and will put this mask as a<br>> > child to the root node.<br>><br>> With root node, you mean the root group layer, right?<br>
<br>Yes.<br><br>> 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.<br>
<br>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.<br><br>> > This assignment will be done in two functions<br>
> > setGlobalSelection() and setRootNode().<br>><br>> And will setRootNode replace setRootLayer?<br><br>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.<br>
<br>> > UI problem:<br>> > If the mask is added to the node graph, then it is shown in a KisLayerBox.<br>> > So we will have to add an option to KisConfig to hide it as we do for the<br>> > root layer. Actually, we may leave a way for a user to show it. It worked<br>
> > quite well, when i tried to add it in a hardcoded way. The only trouble i<br>> > got was that it was impossible to toggle "Active" property of the mask.<br>> > Everything rest was working very well.<br>
><br>> 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.<br>
<br>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.<br>
<br>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.<br>
<br><br>> > Why we can't do it in another way:<br>> > Because all the visitors and most of the commands in Krita work with nodes.<br>> > So, for example, to shift a global selection (setX(), setY() methods) we<br>
> > will have to create one more command. We will have to modify/duplicate all<br>> > our visitors/commands to let them work with selections.<br>><br>> Modify, yes -- but duplicate? Of course not. And it's by no means impossible to do.<br>
<br>I've already written about this duplication above.<br><br>> 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.<br>
<br>The 'taking' of the selection out of the image can be easily encapsulated inside the image.<br><br>> 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.<br>
><br>> > How it is done in other editors:<br>> > I tried Gimp. It transforms global selection with the image. And PS7, as far<br>> > as i remember, crops the selection alongside with the image as well.<br>
><br>> For crop, the selection's bounds should become the initial crop rect.<br><br>I meant that the selection is cropped as well when you select the crop area smaller than the selection itself.<br><br><br>--<br>
Dmitry Kazakov<br><br>