From 31f6ce07c3ab6df645e89e611f3d24c9a0b74f44 Mon Sep 17 00:00:00 2001 From: Martin Dobias Date: Wed, 19 Dec 2012 00:28:04 +0100 Subject: [PATCH] Changed data structure for added features from list to map to allow efficient lookups --- python/core/conversions.sip | 133 ++++++++++++--------- python/core/qgsvectorlayereditbuffer.sip | 3 +- src/core/qgsvectorlayer.cpp | 2 +- src/core/qgsvectorlayereditbuffer.cpp | 34 ++---- src/core/qgsvectorlayereditbuffer.h | 5 +- src/core/qgsvectorlayerfeatureiterator.cpp | 2 +- src/core/qgsvectorlayerfeatureiterator.h | 4 +- src/core/qgsvectorlayerundocommand.cpp | 111 ++++++----------- tests/src/python/test_qgsvectorlayer.py | 6 +- 9 files changed, 135 insertions(+), 165 deletions(-) diff --git a/python/core/conversions.sip b/python/core/conversions.sip index f663ecc9292..c9ec9476190 100644 --- a/python/core/conversions.sip +++ b/python/core/conversions.sip @@ -14,7 +14,7 @@ which are not wrapped by PyQt: - QMap - QMap - QMultiMap -- QMap +- QMap - QMap - QList< QPair< QString, QList > > - QVector @@ -661,94 +661,117 @@ template }; +// +// copied from PyQt4 QMap and adapted to qint64 +// -%MappedType QMap +// QMap is implemented as a Python dictionary. +template +%MappedType QMap /DocType="dict-of-qint64-TYPE"/ { %TypeHeaderCode -#include +#include %End %ConvertFromTypeCode - // Create the list. - PyObject *d; + // Create the dictionary. + PyObject *d = PyDict_New(); - if ((d = PyDict_New()) == NULL) - return NULL; + if (!d) + return NULL; - // Set the list elements. - for (QMap::iterator it = sipCpp->begin(); it != sipCpp->end(); ++it) - { - PyObject *kobj = PyLong_FromLongLong(it.key()); - PyObject *tobj = sipConvertFromInstance( &it.value(), sipClass_QgsGeometry, sipTransferObj); + // Set the dictionary elements. + QMap::const_iterator i = sipCpp->constBegin(); - if (kobj == NULL || tobj == NULL || PyDict_SetItem(d, kobj, tobj) < 0) + while (i != sipCpp->constEnd()) { - Py_DECREF(d); + TYPE *t = new TYPE(i.value()); + + PyObject *kobj = PyLong_FromLongLong(i.key()); + //PyObject *kobj = SIPLong_FromLong(i.key()); + PyObject *tobj = sipConvertFromNewType(t, sipType_TYPE, sipTransferObj); + + if (kobj == NULL || tobj == NULL || PyDict_SetItem(d, kobj, tobj) < 0) + { + Py_DECREF(d); + + if (kobj) + { + Py_DECREF(kobj); + } + + if (tobj) + { + Py_DECREF(tobj); + } + else + { + delete t; + } + + return NULL; + } - if (kobj) - { Py_DECREF(kobj); - } - - if (tobj) - { Py_DECREF(tobj); - } - return NULL; + ++i; } - Py_DECREF(tobj); - } - - return d; + return d; %End %ConvertToTypeCode - PyObject *kobj, *tobj; + PyObject *kobj, *tobj; + SIP_SSIZE_T i = 0; - // Check the type if that is all that is required. - if (sipIsErr == NULL) - { - if (!PyDict_Check(sipPy)) - return 0; + // Check the type if that is all that is required. + if (sipIsErr == NULL) + { + if (!PyDict_Check(sipPy)) + return 0; + + while (PyDict_Next(sipPy, &i, &kobj, &tobj)) + if (!sipCanConvertToType(tobj, sipType_TYPE, SIP_NOT_NONE)) + return 0; + + return 1; + } + + QMap *qm = new QMap; - Py_ssize_t i = 0; while (PyDict_Next(sipPy, &i, &kobj, &tobj)) { - if (!sipCanConvertToInstance(tobj, sipClass_QgsGeometry, SIP_NOT_NONE)) - return 0; - } - return 1; - } + int state; + //, k = SIPLong_AsLong(kobj); + qint64 k = PyLong_AsLongLong(kobj); + TYPE *t = reinterpret_cast(sipConvertToType(tobj, sipType_TYPE, sipTransferObj, SIP_NOT_NONE, &state, sipIsErr)); - QMap *qm = new QMap; + if (*sipIsErr) + { + sipReleaseType(t, sipType_TYPE, state); - Py_ssize_t i = 0; - while (PyDict_Next(sipPy, &i, &kobj, &tobj)) - { - int state; - qint64 k = PyLong_AsLongLong(kobj); - QgsGeometry * t = reinterpret_cast(sipConvertToInstance(tobj, sipClass_QgsGeometry, sipTransferObj,SIP_NOT_NONE,&state,sipIsErr)); + delete qm; + return 0; + } - if (*sipIsErr) - { - sipReleaseInstance(t, sipClass_QgsGeometry, state); - delete qm; - return 0; + qm->insert(k, *t); + + sipReleaseType(t, sipType_TYPE, state); } - qm->insert(k, *t); - sipReleaseInstance(t, sipClass_QgsGeometry, state); - } + *sipCppPtr = qm; - *sipCppPtr = qm; - return sipGetState(sipTransferObj); + return sipGetState(sipTransferObj); %End }; + + + + %MappedType QMap { %TypeHeaderCode diff --git a/python/core/qgsvectorlayereditbuffer.sip b/python/core/qgsvectorlayereditbuffer.sip index 331e7539ad4..048482acf2d 100644 --- a/python/core/qgsvectorlayereditbuffer.sip +++ b/python/core/qgsvectorlayereditbuffer.sip @@ -1,4 +1,5 @@ +typedef QMap QgsFeatureMap; class QgsVectorLayerEditBuffer : QObject { @@ -65,7 +66,7 @@ public: /** New features which are not commited. */ - const QgsFeatureList& addedFeatures(); + const QgsFeatureMap& addedFeatures(); /** Changed attributes values which are not commited */ const QgsChangedAttributesMap& changedAttributeValues(); diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp index f053e5deecc..5257a3a0a1f 100644 --- a/src/core/qgsvectorlayer.cpp +++ b/src/core/qgsvectorlayer.cpp @@ -1584,7 +1584,7 @@ QgsRectangle QgsVectorLayer::extent() rect.combineExtentWith( &r ); } - for ( QgsFeatureList::iterator it = mEditBuffer->mAddedFeatures.begin(); it != mEditBuffer->mAddedFeatures.end(); it++ ) + for ( QgsFeatureMap::iterator it = mEditBuffer->mAddedFeatures.begin(); it != mEditBuffer->mAddedFeatures.end(); it++ ) { QgsRectangle r = it->geometry()->boundingBox(); rect.combineExtentWith( &r ); diff --git a/src/core/qgsvectorlayereditbuffer.cpp b/src/core/qgsvectorlayereditbuffer.cpp index 61b2e41a641..6b4e8418b3c 100644 --- a/src/core/qgsvectorlayereditbuffer.cpp +++ b/src/core/qgsvectorlayereditbuffer.cpp @@ -113,16 +113,7 @@ bool QgsVectorLayerEditBuffer::deleteFeature( QgsFeatureId fid ) { if ( FID_IS_NEW( fid ) ) { - bool found = false; - for ( int i = 0; i < mAddedFeatures.count(); ++i ) - { - if ( mAddedFeatures[i].id() == fid ) - { - found = true; - break; - } - } - if (!found) + if (!mAddedFeatures.contains(fid)) return false; } else // existing feature @@ -304,25 +295,22 @@ bool QgsVectorLayerEditBuffer::commitChanges(QStringList& commitErrors) { if ( cap & QgsVectorDataProvider::AddFeatures ) { - QList ids; - foreach ( const QgsFeature &f, mAddedFeatures ) - { - ids << f.id(); - } + QList ids = mAddedFeatures.keys(); + QgsFeatureList featuresToAdd = mAddedFeatures.values(); - if ( provider->addFeatures( mAddedFeatures ) ) + if ( provider->addFeatures( featuresToAdd ) ) { - commitErrors << tr( "SUCCESS: %n feature(s) added.", "added features count", mAddedFeatures.size() ); + commitErrors << tr( "SUCCESS: %n feature(s) added.", "added features count", featuresToAdd.size() ); - emit committedFeaturesAdded( L->id(), mAddedFeatures ); + emit committedFeaturesAdded( L->id(), featuresToAdd ); // notify everyone that the features with temporary ids were updated with permanent ids - for ( int i = 0; i < mAddedFeatures.size(); i++ ) + for ( int i = 0; i < featuresToAdd.count(); ++i ) { - if ( mAddedFeatures[i].id() != ids[i] ) + if ( featuresToAdd[i].id() != ids[i] ) { emit featureDeleted( ids[i] ); - emit featureAdded( mAddedFeatures[i].id() ); + emit featureAdded( featuresToAdd[i].id() ); } } @@ -448,7 +436,7 @@ void QgsVectorLayerEditBuffer::handleAttributeAdded( int index ) } // go through added features and adapt attributes - QgsFeatureList::iterator featureIt = mAddedFeatures.begin(); + QgsFeatureMap::iterator featureIt = mAddedFeatures.begin(); for ( ; featureIt != mAddedFeatures.end(); ++featureIt ) { QgsAttributes& attrs = featureIt->attributes(); @@ -471,7 +459,7 @@ void QgsVectorLayerEditBuffer::handleAttributeDeleted( int index ) } // go through added features and adapt attributes - QgsFeatureList::iterator featureIt = mAddedFeatures.begin(); + QgsFeatureMap::iterator featureIt = mAddedFeatures.begin(); for ( ; featureIt != mAddedFeatures.end(); ++featureIt ) { QgsAttributes& attrs = featureIt->attributes(); diff --git a/src/core/qgsvectorlayereditbuffer.h b/src/core/qgsvectorlayereditbuffer.h index 969382081c3..89fcabb7815 100644 --- a/src/core/qgsvectorlayereditbuffer.h +++ b/src/core/qgsvectorlayereditbuffer.h @@ -15,6 +15,7 @@ class QgsVectorLayer; typedef QList QgsAttributeList; typedef QSet QgsAttributeIds; +typedef QMap QgsFeatureMap; class CORE_EXPORT QgsVectorLayerEditBuffer : public QObject { @@ -79,7 +80,7 @@ public: /** New features which are not commited. */ - inline const QgsFeatureList& addedFeatures() { return mAddedFeatures; } + inline const QgsFeatureMap& addedFeatures() { return mAddedFeatures; } /** Changed attributes values which are not commited */ inline const QgsChangedAttributesMap& changedAttributeValues() { return mChangedAttributeValues; } @@ -161,7 +162,7 @@ protected: QgsFeatureIds mDeletedFeatureIds; /** New features which are not commited. */ - QgsFeatureList mAddedFeatures; + QgsFeatureMap mAddedFeatures; /** Changed attributes values which are not commited */ QgsChangedAttributesMap mChangedAttributeValues; diff --git a/src/core/qgsvectorlayerfeatureiterator.cpp b/src/core/qgsvectorlayerfeatureiterator.cpp index 3d8a02ff5bf..3adace632ca 100644 --- a/src/core/qgsvectorlayerfeatureiterator.cpp +++ b/src/core/qgsvectorlayerfeatureiterator.cpp @@ -430,7 +430,7 @@ bool QgsVectorLayerFeatureIterator::nextFeatureFid( QgsFeature& f ) } // added features - for ( QgsFeatureList::iterator iter = editBuffer->mAddedFeatures.begin(); iter != editBuffer->mAddedFeatures.end(); ++iter ) + for ( QgsFeatureMap::iterator iter = editBuffer->mAddedFeatures.begin(); iter != editBuffer->mAddedFeatures.end(); ++iter ) { if ( iter->id() == featureId ) { diff --git a/src/core/qgsvectorlayerfeatureiterator.h b/src/core/qgsvectorlayerfeatureiterator.h index 8b668800b15..89e34fd1825 100644 --- a/src/core/qgsvectorlayerfeatureiterator.h +++ b/src/core/qgsvectorlayerfeatureiterator.h @@ -5,6 +5,8 @@ #include +typedef QMap QgsFeatureMap; + class QgsVectorLayer; class QgsVectorJoinInfo; @@ -40,7 +42,7 @@ protected: // only related to editing QSet mFetchConsidered; QgsGeometryMap::iterator mFetchChangedGeomIt; - QgsFeatureList::iterator mFetchAddedFeaturesIt; + QgsFeatureMap::iterator mFetchAddedFeaturesIt; bool mFetchedFid; // when iterating by FID: indicator whether it has been fetched yet or not diff --git a/src/core/qgsvectorlayerundocommand.cpp b/src/core/qgsvectorlayerundocommand.cpp index ae13d02b450..2a639e1f14f 100644 --- a/src/core/qgsvectorlayerundocommand.cpp +++ b/src/core/qgsvectorlayerundocommand.cpp @@ -47,14 +47,9 @@ QgsVectorLayerUndoCommandAddFeature::QgsVectorLayerUndoCommandAddFeature( QgsVec void QgsVectorLayerUndoCommandAddFeature::undo() { - for (int i = 0; mBuffer->mAddedFeatures.count(); ++i) - { - if (mBuffer->mAddedFeatures[i].id() == mFeature.id()) - { - mBuffer->mAddedFeatures.removeAt(i); - break; - } - } + QgsFeatureMap::const_iterator it = mBuffer->mAddedFeatures.find(mFeature.id()); + Q_ASSERT( it != mBuffer->mAddedFeatures.end() ); + mBuffer->mAddedFeatures.remove(mFeature.id()); if ( mFeature.geometry() ) cache()->removeGeometry(mFeature.id()); @@ -64,7 +59,7 @@ void QgsVectorLayerUndoCommandAddFeature::undo() void QgsVectorLayerUndoCommandAddFeature::redo() { - mBuffer->mAddedFeatures.append( mFeature ); + mBuffer->mAddedFeatures.insert( mFeature.id(), mFeature ); if ( mFeature.geometry() ) cache()->cacheGeometry( mFeature.id(), *mFeature.geometry() ); @@ -81,14 +76,9 @@ QgsVectorLayerUndoCommandDeleteFeature::QgsVectorLayerUndoCommandDeleteFeature( if ( FID_IS_NEW( mFid ) ) { - for ( int i = 0; i < mBuffer->mAddedFeatures.count(); ++i ) - { - if ( mBuffer->mAddedFeatures[i].id() == fid ) - { - mOldAddedFeature = mBuffer->mAddedFeatures[i]; - break; - } - } + QgsFeatureMap::const_iterator it = mBuffer->mAddedFeatures.find( mFid ); + Q_ASSERT( it != mBuffer->mAddedFeatures.end() ); + mOldAddedFeature = it.value(); } } @@ -96,7 +86,7 @@ void QgsVectorLayerUndoCommandDeleteFeature::undo() { if ( FID_IS_NEW( mFid ) ) { - mBuffer->mAddedFeatures.append( mOldAddedFeature ); + mBuffer->mAddedFeatures.insert( mOldAddedFeature.id(), mOldAddedFeature ); } else { @@ -110,14 +100,7 @@ void QgsVectorLayerUndoCommandDeleteFeature::redo() { if ( FID_IS_NEW( mFid ) ) { - for ( int i = 0; i < mBuffer->mAddedFeatures.count(); ++i ) - { - if ( mBuffer->mAddedFeatures[i].id() == mFid ) - { - mBuffer->mAddedFeatures.removeAt(i); - break; - } - } + mBuffer->mAddedFeatures.remove( mFid ); } else { @@ -136,14 +119,9 @@ QgsVectorLayerUndoCommandChangeGeometry::QgsVectorLayerUndoCommandChangeGeometry if ( FID_IS_NEW( mFid ) ) { - for ( int i = 0; i < mBuffer->mAddedFeatures.count(); ++i ) - { - if ( mBuffer->mAddedFeatures[i].id() == mFid ) - { - mOldGeom = new QgsGeometry( *mBuffer->mAddedFeatures[i].geometry() ); - break; - } - } + QgsFeatureMap::const_iterator it = mBuffer->mAddedFeatures.find( mFid ); + Q_ASSERT( it != mBuffer->mAddedFeatures.end() ); + mOldGeom = new QgsGeometry( *it.value().geometry() ); } else { @@ -167,14 +145,10 @@ void QgsVectorLayerUndoCommandChangeGeometry::undo() if ( FID_IS_NEW( mFid ) ) { // modify added features - for ( int i = 0; i < mBuffer->mAddedFeatures.count(); ++i ) - { - if ( mBuffer->mAddedFeatures[i].id() == mFid ) - { - mBuffer->mAddedFeatures[i].setGeometry( *mOldGeom ); - break; - } - } + QgsFeatureMap::iterator it = mBuffer->mAddedFeatures.find( mFid ); + Q_ASSERT( it != mBuffer->mAddedFeatures.end() ); + it.value().setGeometry( *mOldGeom ); + cache()->cacheGeometry( mFid, *mOldGeom ); emit mBuffer->geometryChanged( mFid, *mOldGeom ); } @@ -208,14 +182,9 @@ void QgsVectorLayerUndoCommandChangeGeometry::redo() if ( FID_IS_NEW( mFid ) ) { // modify added features - for ( int i = 0; i < mBuffer->mAddedFeatures.count(); ++i ) - { - if ( mBuffer->mAddedFeatures[i].id() == mFid ) - { - mBuffer->mAddedFeatures[i].setGeometry( *mNewGeom ); - break; - } - } + QgsFeatureMap::iterator it = mBuffer->mAddedFeatures.find( mFid ); + Q_ASSERT( it != mBuffer->mAddedFeatures.end() ); + it.value().setGeometry( *mNewGeom ); } else { @@ -240,14 +209,12 @@ QgsVectorLayerUndoCommandChangeAttribute::QgsVectorLayerUndoCommandChangeAttribu if ( FID_IS_NEW( mFid ) ) { // work with added feature - for ( int i = 0; i < mBuffer->mAddedFeatures.size(); i++ ) + QgsFeatureMap::const_iterator it = mBuffer->mAddedFeatures.find( mFid ); + Q_ASSERT( it != mBuffer->mAddedFeatures.end() ); + if ( it.value().attribute( mFieldIndex ).isValid() ) { - if ( mBuffer->mAddedFeatures[i].id() == mFid && mBuffer->mAddedFeatures[i].attribute( mFieldIndex ).isValid() ) - { - mOldValue = mBuffer->mAddedFeatures[i].attribute( mFieldIndex ); - mFirstChange = false; - break; - } + mOldValue = it.value().attribute( mFieldIndex ); + mFirstChange = false; } } else @@ -268,14 +235,9 @@ void QgsVectorLayerUndoCommandChangeAttribute::undo() if ( FID_IS_NEW( mFid ) ) { // added feature - for ( int i = 0; i < mBuffer->mAddedFeatures.size(); i++ ) - { - if ( mBuffer->mAddedFeatures[i].id() == mFid ) - { - mBuffer->mAddedFeatures[i].setAttribute( mFieldIndex, mOldValue ); - break; - } - } + QgsFeatureMap::iterator it = mBuffer->mAddedFeatures.find( mFid ); + Q_ASSERT( it != mBuffer->mAddedFeatures.end() ); + it.value().setAttribute( mFieldIndex, mOldValue ); } else { @@ -310,14 +272,9 @@ void QgsVectorLayerUndoCommandChangeAttribute::redo() if ( FID_IS_NEW( mFid ) ) { // updated added feature - for ( int i = 0; i < mBuffer->mAddedFeatures.size(); i++ ) - { - if ( mBuffer->mAddedFeatures[i].id() == mFid ) - { - mBuffer->mAddedFeatures[i].setAttribute( mFieldIndex, mNewValue ); - break; - } - } + QgsFeatureMap::iterator it = mBuffer->mAddedFeatures.find( mFid ); + Q_ASSERT( it != mBuffer->mAddedFeatures.end() ); + it.value().setAttribute( mFieldIndex, mNewValue ); } else { @@ -385,9 +342,9 @@ QgsVectorLayerUndoCommandDeleteAttribute::QgsVectorLayerUndoCommandDeleteAttribu } // save values of new features - for (int i = 0; i < mBuffer->mAddedFeatures.count(); ++i) + for (QgsFeatureMap::const_iterator it = mBuffer->mAddedFeatures.begin(); it != mBuffer->mAddedFeatures.end(); ++it) { - const QgsFeature& f = mBuffer->mAddedFeatures[i]; + const QgsFeature& f = it.value(); mDeletedValues.insert(f.id(), f.attribute(mFieldIndex)); } @@ -416,9 +373,9 @@ void QgsVectorLayerUndoCommandDeleteAttribute::undo() mBuffer->handleAttributeAdded( mFieldIndex ); // update changed attributes + new features // set previously used attributes of new features - for (int i = 0; i < mBuffer->mAddedFeatures.count(); ++i) + for (QgsFeatureMap::iterator it = mBuffer->mAddedFeatures.begin(); it != mBuffer->mAddedFeatures.end(); ++it) { - QgsFeature& f = mBuffer->mAddedFeatures[i]; + QgsFeature& f = it.value(); f.setAttribute( mFieldIndex, mDeletedValues.value(f.id()) ); } // set previously used changed attributes diff --git a/tests/src/python/test_qgsvectorlayer.py b/tests/src/python/test_qgsvectorlayer.py index 48439a29a67..c7fc2ed0a1b 100644 --- a/tests/src/python/test_qgsvectorlayer.py +++ b/tests/src/python/test_qgsvectorlayer.py @@ -72,7 +72,7 @@ def dumpEditBuffer(layer): print "NO EDITING!" return print "ADDED:" - for f in editBuffer.addedFeatures(): print "%d: %s | %s" % (f.id(), formatAttributes(f.attributes()), f.geometry().exportToWkt()) + for fid,f in editBuffer.addedFeatures().iteritems(): print "%d: %s | %s" % (f.id(), formatAttributes(f.attributes()), f.geometry().exportToWkt()) print "CHANGED GEOM:" for fid,geom in editBuffer.changedGeometries().iteritems(): print "%d | %s" % (f.id(), f.geometry().exportToWkt()) @@ -136,7 +136,7 @@ class TestQgsVectorLayer(TestCase): checkBefore() layer.undoStack().redo() checkAfter() - + assert layer.commitChanges() checkAfter() @@ -232,7 +232,6 @@ class TestQgsVectorLayer(TestCase): fid = feat.id() assert layer.deleteFeature(fid) checkAfter2() - dumpEditBuffer(layer) # now try undo/redo layer.undoStack().undo() @@ -244,7 +243,6 @@ class TestQgsVectorLayer(TestCase): layer.undoStack().redo() checkAfter2() - dumpEditBuffer(layer) assert layer.commitChanges() checkAfter2()