seamless project model update

[Andreas Pakulat]

On 12.07.10 13:08:25, Aleix Pol wrote:
> On Mon, Jul 12, 2010 at 8:54 AM, Andreas Pakulat <apaku at gmx.de> wrote:
> > On 12.07.10 04:46:26, Aleix Pol wrote:
> > This looks wrong, there shouldn't be a difference based on wether an
> > item is in the model or not. The check should be wether
> > QThread::currentThread == qApp->thread(), i.e. wether the function is
> > called from the main thread or not.
> >
> Yes, we want the ::currentThread==thread thing. I have it working locally
> it's not a problem.
> 
> The main reason for the model check is that if the branch it's not on a
> model, then it's not on a view so we can still rely on putting the items
> directly under its parent.

Sure, but at the same time you don't need to go through a
queued-connection if the thread is the gui thread.

> > And last but not least: This doesn't make the model threadsafe at all,
> > so IMHO this shouldn't be done as its confusing that one part makes an
> > attempt to be threadsafe, but all others are still broken in that
> > regard.
> >
> Hm...
> Well, other options I can see are:
> - To move this code to the CMake manager, but this will make its code to
> change quite a lot
> - Let the CMake tree as we had before
> - make the rest of operations thread-friendly (not sure what would this look
> like :/)

That would be adding a lock to the item classes. However locking is a
relatively expensive operation, so this might hit the performance of the
tree for large tree's. Additionally I'm not quite sure wether adding
locking to the items would be enough, could be its also necessary for
the model itself. In that case you'd have a problem as we don't control
all of the model code...

Anyway, my main concern was about mixing thread-safe and thread-unsafe
API in the same class. I'm a bit split between having an easy-to-use API
that does things "just right" and having this thread-safety spelled out
explicitly. That is I thought about adding a namespace method
"addRowThreadSafe( parent, child )", but then it would've been necessary
to use it inside the constructor or look at all codes that create items
to make sure they're not passing in a parent item and make the
append-step explicit.

I don't really like either solution, but as Milian already said, the
code should be inside appendRow anyway (which the constructor can call
then). So I think I'm fine with documenting that the model and items are
_thread-unsafe_ and adding a small hint to appendRow that this one adds
the items in a thread-safe way.

Andreas

-- 
Beauty and harmony are as necessary to you as the very breath of life.