seamless project model update

[Milian Wolff]

On Monday 12 July 2010 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:
> > > 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<ProjectBase
> >  Item*>(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

*revive*

what happened to this? I'd still like to see the seamless update very much :)

Bye
-- 
Milian Wolff
mail at milianw.de
http://milianw.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20100807/9e03a305/attachment.sig>