<br><br><div class="gmail_quote">On Mon, Jul 12, 2010 at 8:54 AM, Andreas Pakulat <span dir="ltr"><<a href="mailto:apaku@gmx.de">apaku@gmx.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div><div></div><div class="h5">On 12.07.10 04:46:26, Aleix Pol wrote:<br>
> hi,<br>
> This patch solves the problem we have when we try to refresh the<br>
> projectmodel from another thread, it makes the method call to be triggered<br>
> whenever the model thread runs the event loop. I know it's not a clean<br>
> implementation (that's why I'm sending it here).<br>
><br>
> I'm sending a patch so that you can try how the reload feels after that<br>
> change. I'm not commiting right away because I understand there's drawbacks<br>
> and I suppose you will want to give your opinion. I think we could give it a<br>
> try though.<br>
<br>
</div></div>I can't say too much about the cmake-part of the patch.<br>
<br>
Apart from that,<br>
<br>
> diff --git a/project/projectmodel.cpp b/project/projectmodel.cpp<br>
> index 8700e11..ac7a662 100644<br>
> --- a/project/projectmodel.cpp<br>
> +++ b/project/projectmodel.cpp<br>
> @@ -108,7 +109,6 @@ public:<br>
>      KUrl m_url;<br>
>  };<br>
><br>
> -<br>
>  ProjectBaseItem::ProjectBaseItem( IProject* project, const QString &name, ProjectBaseItem *parent )<br>
>          : d_ptr(new ProjectBaseItemPrivate)<br>
>  {<br>
> @@ -116,7 +116,16 @@ ProjectBaseItem::ProjectBaseItem( IProject* project, const QString &name, Projec<br>
>      d->project = project;<br>
>      d->text = name;<br>
>      if( parent ) {<br>
> -        parent->appendRow( this );<br>
> +        qDebug() << "---" << parent->model();<br>
> +        if(!parent->model() /*|| QThread::currentThread()==parent->model()->thread()->currentThread()*/) {<br>
> +            qDebug() << "A";<br>
> +            parent->appendRow(this);<br>
> +        } else {<br>
> +            qDebug() << "B";<br>
> +            bool ret=QMetaObject::invokeMethod(parent->model(), "appendRow", Qt::QueuedConnection,<br>
> +                                               Q_ARG(void*, parent), Q_ARG(void*, this));<br>
> +            Q_ASSERT(ret);<br>
> +        }<br>
<br>
This looks wrong, there shouldn't be a difference based on wether an<br>
item is in the model or not. The check should be wether<br>
QThread::currentThread == qApp->thread(), i.e. wether the function is<br>
called from the main thread or not.<br></blockquote><div>Yes, we want the ::currentThread==thread thing. I have it working locally it's not a problem.</div><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<br>
And last but not least: This doesn't make the model threadsafe at all,<br>
so IMHO this shouldn't be done as its confusing that one part makes an<br>
attempt to be threadsafe, but all others are still broken in that<br>
regard.<br></blockquote><div>Hm...</div><div>Well, other options I can see are: </div><div>- To move this code to the CMake manager, but this will make its code to change quite a lot</div><div>- Let the CMake tree as we had before</div>
<div>- make the rest of operations thread-friendly (not sure what would this look like :/)</div><div>- any other option I couldn't figure out.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<br>
> +void ProjectModel::appendRow(void* parent, void* item)<br>
> +{<br>
> +    static_cast<ProjectBaseItem*>(parent)->appendRow(static_cast<ProjectBaseItem*>(item));<br>
> +}<br>
<br>
Why using void* here? It should be fine to use the actual types no? <br></blockquote><div>It was 4am and Q_DECLARE_METATYPE was giving me problems. yes it should be possible</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

<br>
Andreas<br>
<font color="#888888"><br>
--<br>
You need no longer worry about the future.  This time tomorrow you'll be dead.<br>
</font><div><div></div><div class="h5"><br>
--<br>
KDevelop-devel mailing list<br>
<a href="mailto:KDevelop-devel@kdevelop.org">KDevelop-devel@kdevelop.org</a><br>
<a href="https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel" target="_blank">https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel</a><br>
</div></div></blockquote></div><br>