Switching shapes and tools: is it broken?

[Dag Andersen]

Mandag 20 juni 2011 22:37:10 skrev jaham at gmx.net:
> On Monday 20 June 2011 15:31:28 Dag Andersen wrote:
> > Mandag 20 juni 2011 15:20:27 skrev du:
> > > On 20.06.2011 13:02, Dag Andersen wrote:
> > > > Mandag 20 juni 2011 11:37:13 skrev du:
> > > >> On 20.06.2011 10:34, Dag Andersen wrote:
> > > >>> Before the weekend I fixed a crash in ArtisticTextShape and, as Jan
> > > >>> Hambrect pointed out, I unvittingly removed the functionallity to
> > > >>> go direct between shapes. So I thought I'd find out how this
> > > >>> *should* be implemented before I tried to put it back, but I
> > > >>> can't!
> > > >>> I can find code that afaics is supposed to do it, but it is never
> > > >>> executed. It may be that it is just me being confused so I need to
> > > >>> run this by somebody that is more familiar with the code than I am.
> > > >>> 
> > > >>> There is two "types" of switches that can occure:
> > > >>> 1) Switch between shapes with different tools.
> > > >>> E.g. TextTool ->   ArtisticTextTool.
> > > >>> 2) Switch between shapes using the same tool.
> > > >>> E.g. TextTool ->   TextTool
> > > >> 
> > > >> 3. Switch between shapes using a layer docker like in Karbon or
> > > >> Stage.
> > > > 
> > > > Hmm, how is this different from 2?
> > > 
> > > The tool itself is not involved in changing the shape selection.
> > > 
> > > >> This was the use case targeted in the artistic text tool. So the
> > > >> user could just click on another artistic text shape from the layer
> > > >> docker and have the tool use that for further editing.
> > > > 
> > > > I opened karbon, added an artistic text shape, added a new layer and
> > > > added a new artistic text shape to that layer. Is this the case you
> > > > are thinking of?
> > > 
> > > No, rather you have two artistic text shapes. While editing the first
> > > one with the artistic text tool, you click on the second one inside the
> > > layer docker. The clicked shape gets selected, the first shape gets
> > > deselected. The tool gets a signal that the shape selection changed and
> > > checks if the artistic text shape it is currently editing is still
> > > selected. It notices that it is not selected anymore so it checks if
> > > any other artistic text shape is selected. Yes there is one, so the
> > > tool changes its internal state to editing of that newly selected
> > > shape.
> > 
> > Yes, just found out what you meant.
> > I still maintain that the shape should not need to listen to the seletion
> > changed signal, all switching should be handled by KoToolManager via
> > activate()/deactivate().
> > If this is left to the shapes *all* shapes have to implement the same
> > code...
> 
> I am not sure that switching tools on each selection change is the right
> thing to do. That would mean that you are losing the tool state each time
> the selection changes.
Well, the tool is only deactivated, not deleted so you can certainly save any 
states.
But: I think I would expect to get back to the state I was in last time I 
edited a certain *shape*, so imo any states should be saved to the shape.
Also, in your case, think of this: You have 1 text shape (ts) and 2 artistic 
text shapes (as).
Now navigating as->as will give different result than as->ts->as.
As a user I think I'd be confused.

I got a bit deeper into the code, and I realized that the primary criteria for 
tool activation is which shapes are selected. Hence, any tool activation 
should be triggered by selectionChanged(). Secondary criteria is if a tool is 
active and if it is the interaction tool or not.

So I put this logic into KoToolManager::selectionChanged(), removed switching 
from currentLayerChanged() and KoToolProxy, and suddenly things started to 
work pretty well.
I certainly think this is the way to go. If you think you need to be able to 
switch shapes while your tool stays active (use case?), I think we need to add 
a swicthShapes method to the tool, and enhance the switching logic in 
KoToolManager::selectionChanged().
-- 
Mvh.
Dag Andersen