<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/102610/">http://git.reviewboard.kde.org/r/102610/</a>
     </td>
    </tr>
   </table>
   <br />


<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Marble.</div>
<div>By Javier Becerra Elcinto.</div>


<p style="color: grey;"><i>Updated Sept. 30, 2011, 3:18 p.m.</i></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Modified according to comments from Thibaut Gridel.
Declared the add/remove functions as slots.
Added signals added(GeoDataObject *) and removed(GeoDataObject *).
</pre>
  </td>
 </tr>
</table>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">GeoDataTreeModel allows for insertion of a GeoDataDocument using the function addDocument(GeoDataDocument*). However it is not possible to modify an existing document (i.e. by adding or removing a feature) while keeping the model/view synchonised other than calling the GeoDataTreeModel::update() function, which updates the whole treemodel and demands a lot of ressources. 
Adding new addFeature and removeFeature allows the modification of the tree while keeping the view automatically updated.
GeoDataTreeModel::addDocument and GeoDataTreeModel::removeDocument are modified to call addFeature and removeFeature, minimising the code modifying the treemodel.

This patch has been proved to work in single-threaded mode, however marblewidget crashes on KDescendantsProxyModel::mapFromSource(const QModelIndex &sourceIndex) when a new feature/document is added from a thread different from main. This behaviour is independent of this patch, as it can be reproduced in unpatched marblewidget calling GeoDataTreeModel::addDocument from another thread. The test code below can be used to test this bug uncommenting the last two lines of code of main.cpp.</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing (updated)</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Files "PlacemarkInserter.h" and "main.cpp":

#ifndef PLACEMARKINSERTER_H
#define PLACEMARKINSERTER_H

//Marble
#include "GeoDataContainer.h"
#include "GeoDataPlacemark.h"
#include "MarbleWidget.h"
#include "MarbleModel.h"
#include "GeoDataTreeModel.h"

//Qt
#include <QtCore/QTimer>
#include <QtCore/QDebug>

class PlacemarkInserter:public QObject
{
    Q_OBJECT
    Marble::GeoDataContainer*folder;
    Marble::MarbleWidget *marblewidget;
    float lat;
    float lon;

    int index;
    QString prefix;
    QTimer timer;

    bool removeprev;
    Marble::GeoDataPlacemark *prev;
public:
    PlacemarkInserter(float lon0, float lat0, int interval, QString pr, Marble::GeoDataContainer* f,  Marble::MarbleWidget *mw=NULL ): lat(lat0), lon(lon0), prefix(pr), folder(f), marblewidget(mw), index(2), prev(NULL), removeprev(false)
    {
        timer.setInterval(interval);
        timer.setSingleShot(false);
        QObject::connect(&timer,SIGNAL(timeout()),this,SLOT(addNewPlacemark()));

    }
public slots:
    void setLatitude( float lat0 ) {lat=lat0;}
    void setLongitude( float lon0 ) {lon=lon0;}
    void setRemovePrevious( bool r ) {removeprev=r;}
    void setInterval( int i ) {timer.setInterval(i);}
    void start()    {     timer.start(); }
    void stop()    {     timer.stop(); }

    void addNewPlacemark()
    {
        qDebug()<<"Adding placemark "<<prefix<<index;

        Marble::GeoDataPlacemark *myplacemark=new Marble::GeoDataPlacemark;
        myplacemark->setName(QString("%1 %2").arg(prefix).arg(index++));
        myplacemark->setCoordinate( lon+index*0.01, lat,0,Marble::GeoDataPoint::Degree);

        if( marblewidget )
        {
            if( removeprev && prev )
            {
                marblewidget->model()->treeModel()->removeFeature( prev );
                delete prev;
            }
            marblewidget->model()->treeModel()->addFeature( folder, myplacemark );
            prev=myplacemark;
        }

    }

};

#include <QThread>
class PlacemarkInserterTh: public QThread
{
    float lon;
    float lat;
    int interval;
    Marble::GeoDataContainer *folder;
    Marble::MarbleWidget *marblewidget;
    QString prefix;

public:
    PlacemarkInserterTh(float lon0, float lat0,int interval0, QString pr,Marble::GeoDataContainer *f,  Marble::MarbleWidget *mw): lon(lon0),lat(lat0), interval(interval0),folder(f), marblewidget(mw), prefix(pr) {}
    void run() {  PlacemarkInserter inserter(lon,lat,interval,prefix, folder, marblewidget ); inserter.start(); this->exec();}
};




#endif // PLACEMARKINSERTER_H



//////////////////////////////////////////////7
//main.cpp

#include "PlacemarkInserter.h"

//Marble
#include "GeoDataDocument.h"
#include "GeoDataFolder.h"
#include "GeoDataPlacemark.h"
#include "MarbleWidget.h"
#include "FileViewWidget.h"

//Qt
#include <QtGui/QApplication>
#include <QDebug>
#include <QTextCodec>




int main(int argc, char** argv)
{
    QApplication app(argc,argv);


    QTextCodec::setCodecForCStrings(QTextCodec::codecForName("UTF-8"));
    QTextCodec::setCodecForTr(QTextCodec::codecForName("UTF-8"));

    //Creating a GeoDataDocument from scratch:

    Marble::GeoDataDocument *mydoc=new Marble::GeoDataDocument();

    mydoc->setFileName("mydocument.kml"); //Will identify the document internally, must be unique
    mydoc->setName("My_GeoDataDocument"); //Name for visualisation in MarbleWidget

    Marble::GeoDataFolder *myfolder=new Marble::GeoDataFolder;
    myfolder->setName("My first folder"); //(setName from base class GeoDataFeature, inherited from GeoDataContainer)

    mydoc->append(myfolder);

    Marble::MarbleWidget marblewidget;
    marblewidget.setMapThemeId("earth/openstreetmap/openstreetmap.dgml");
    marblewidget.show();

    Marble::FileViewWidget fileviewwidget;
    fileviewwidget.setMarbleWidget(&marblewidget);
    fileviewwidget.show();

          //addDocument now issues a call to addFeature
    int idx=marblewidget.model()->treeModel()->addDocument(mydoc);

    qDebug()<<"Added document"<<idx;



    PlacemarkInserter inserter0(-2.45,42.465278,250,"Test",myfolder, &marblewidget );
    inserter0.setRemovePrevious(true);  //tests new removeFeature functionality
    inserter0.start();

    PlacemarkInserter inserter1(-2.48, 42.465278 , 500 , "Test2" , mydoc , &marblewidget );
    inserter1.setRemovePrevious(false);
    inserter1.start();


    //Uncomment next two lines if you want to see marblewidget crash when the treemodel is modified from
    // a thread other than the main one. This bug was already present before the addFeature patch;
    // calling addDocument from another thread also caused the application to crash.
    //PlacemarkInserterTh crash_on_multithread(-2.55, 42.465278 , 500 , "Crash" , myfolder , &marblewidget );
    //crash_on_multithread.start();

    return app.exec();
}
</pre>
  </td>
 </tr>
</table>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>src/lib/GeoDataTreeModel.h <span style="color: grey">(eda4c53)</span></li>

 <li>src/lib/GeoDataTreeModel.cpp <span style="color: grey">(2c17cfa)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/102610/diff/" style="margin-left: 3em;">View Diff</a></p>




  </td>
 </tr>
</table>




  </div>
 </body>
</html>