[Marble-devel] Review Request: Added addFeature and removeFeature support to GeoDataTreeModel
Javier Becerra Elcinto
javier at auva.es
Mon Sep 19 07:33:05 UTC 2011
> On Sept. 16, 2011, 12:35 p.m., Guillaume Martres wrote:
> > 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.
I have been discussing multithread support with Thibaut, and I am testing a new version of this patch where the add.../remove functions are declared as slots.
You can then make a connection with Qt connect() of type AutoConnection or QueuedConnection, and the functions will be automatically called from the right thread,
avoiding crashes. It seems to be working ok at the moment.
I have found the same problem with Writer objects, which crash when created from another thread. As a workaround, I created a wrapper class which moves the writer object
to MarbleWidtget's thread and gets connected via signal/slots (maybe we could check the writer class and add some signals/slots magic there too, but I have not tried that).
I will update this patch with the new slots declaration asap.
- Javier
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102610/#review6567
-----------------------------------------------------------
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/20110919/a478dd4c/attachment.html>
More information about the Marble-devel
mailing list