Usage of setCurrentNodeLocked()

[Boudewijn Rempt]

On Tuesday 21 September 2010, Dmitry Kazakov wrote:
> Hi, All!
> 
> I found that some of the calls to setCurrentNodeLocked() are done in a wrong
> way. I mean, at least in KisToolFill and KisToolGradient there is a
> possibility that the codepath will return from the event function and leave
> the node locked forever.
> 
> I do really think we should remove all these workaround "locks" and handle
> our tools in a proper way. Because these subtle locking bugs are really
> difficult to see and catch. I think it's better to delay the release if
> needed, than to release a product with random bugs.

Okay, let's go back a step or two and consider the requirements:

* if a user executes some long running action on a layer we want him to be able to continue working on another image in another window
* if a user executes some long running action on a layer we want him to be able to continue working on another layer in the same image
* long running actions should be cancelable.
* long running actions should show progress
* if an action is running on a layer, no other action should be executed on that layer

For now, I think the systemlocked mechanism is a working option, but as you say, we are doing it in the wrong way. We could have a KisNodeLocker object that's created at the start of the option on the stack and on deletion (when the action method ends) should unlock the node, instead of manually locking and unlocking. That is a relatively small change that will help a lot.

I know you would prefer a queuing mechanism, and I still sort of agree desktop Sven's objections, but it needs really careful designing since then every non-interactive action (like fill, filter, gradient, scale) needs to be encapsulated in a job and the job put on a queue.

And there would still be a clash with the paint tool, since I don't think that its actions should be mixed in the queue with the long running actions. And it might still be better usability to disallow starting a new gradient while the old one is running, for instance.

-- 
Boudewijn Rempt | http://www.valdyas.org
Ceterum censeo lapsum particulorum probae delendum esse