[Marble-devel] Review Request: Added addFeature and removeFeature support to GeoDataTreeModel
Guillaume Martres
smarter at ubuntu.com
Fri Sep 16 12:35:16 UTC 2011
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102610/#review6567
-----------------------------------------------------------
Nice work!
« 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. »
I'd like to point out that this is critical for the SatellitesPlugin(code at: http://quickgit.kde.org/?p=clones%2Fmarble%2Fgmartres%2Fsocis-2011-satellites.git&a=summary ) as it regularly needs to add new placemarks. I'm not sure if there's a way to do that in the main thread as a workaround.
- Guillaume
On Sept. 14, 2011, 10:57 a.m., Javier Becerra Elcinto wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102610/
> -----------------------------------------------------------
>
> (Updated Sept. 14, 2011, 10:57 a.m.)
>
>
> Review request for Marble.
>
>
> Summary
> -------
>
> 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.
>
>
> Diffs
> -----
>
> src/lib/GeoDataTreeModel.h eda4c53
> src/lib/GeoDataTreeModel.cpp 2c17cfa
>
> Diff: http://git.reviewboard.kde.org/r/102610/diff
>
>
> Testing
> -------
>
> Files "PlacemarkInserte.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();
> }
>
>
> Thanks,
>
> Javier
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20110916/b329bd5f/attachment.html>
More information about the Marble-devel
mailing list