seamless project model update

[Aleix Pol]

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:
> > hi,
> > This patch solves the problem we have when we try to refresh the
> > projectmodel from another thread, it makes the method call to be
> triggered
> > whenever the model thread runs the event loop. I know it's not a clean
> > implementation (that's why I'm sending it here).
> >
> > I'm sending a patch so that you can try how the reload feels after that
> > change. I'm not commiting right away because I understand there's
> drawbacks
> > and I suppose you will want to give your opinion. I think we could give
> it a
> > try though.
>
> I can't say too much about the cmake-part of the patch.
>
> Apart from that,
>
> > diff --git a/project/projectmodel.cpp b/project/projectmodel.cpp
> > index 8700e11..ac7a662 100644
> > --- a/project/projectmodel.cpp
> > +++ b/project/projectmodel.cpp
> > @@ -108,7 +109,6 @@ public:
> >      KUrl m_url;
> >  };
> >
> > -
> >  ProjectBaseItem::ProjectBaseItem( IProject* project, const QString
> &name, ProjectBaseItem *parent )
> >          : d_ptr(new ProjectBaseItemPrivate)
> >  {
> > @@ -116,7 +116,16 @@ ProjectBaseItem::ProjectBaseItem( IProject* project,
> const QString &name, Projec
> >      d->project = project;
> >      d->text = name;
> >      if( parent ) {
> > -        parent->appendRow( this );
> > +        qDebug() << "---" << parent->model();
> > +        if(!parent->model() /*||
> QThread::currentThread()==parent->model()->thread()->currentThread()*/) {
> > +            qDebug() << "A";
> > +            parent->appendRow(this);
> > +        } else {
> > +            qDebug() << "B";
> > +            bool ret=QMetaObject::invokeMethod(parent->model(),
> "appendRow", Qt::QueuedConnection,
> > +                                               Q_ARG(void*, parent),
> Q_ARG(void*, this));
> > +            Q_ASSERT(ret);
> > +        }
>
> 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.


>
> 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 :/)
- any other option I couldn't figure out.


>
> > +void ProjectModel::appendRow(void* parent, void* item)
> > +{
> > +
>  static_cast<ProjectBaseItem*>(parent)->appendRow(static_cast<ProjectBaseItem*>(item));
> > +}
>
> Why using void* here? It should be fine to use the actual types no?
>
It was 4am and Q_DECLARE_METATYPE was giving me problems. yes it should be
possible


>
> Andreas
>
> --
> You need no longer worry about the future.  This time tomorrow you'll be
> dead.
>
> --
> KDevelop-devel mailing list
> KDevelop-devel at kdevelop.org
> https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20100712/0553510a/attachment.html>