[Backport release-3_18] Bugfix gh41477 editbuffer passthrough #41793
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
nyalldawson
added
NOT FOR MERGE
|
The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check
|
github-actions
bot
added
the
stale
elpaso
removed
the
stale
src/core/vector/qgsvectorlayer.cpp
Outdated
| @@ -874,7 +874,7 @@ QgsRectangle QgsVectorLayer::extent() const | |||
| } | |||
|
|
|||
| if ( !mEditBuffer || | |||
| ( mEditBuffer->mDeletedFeatureIds.isEmpty() && mEditBuffer->mChangedGeometries.isEmpty() ) || | |||
| ( !mDataProvider->transaction() && ( mEditBuffer->deletedFeatureIds().isEmpty() && mEditBuffer->changedGeometries().isEmpty() ) ) || | |||
There was a problem hiding this comment.
It would be a bit cleaner if the !mDataProvider->transaction() checks were instead handled by either dynamic casting mEditBuffer to QgsVectorLayerEditPassthrough or by a new "isPassthrough" or "buffersChanges" method QgsVectorLayerEditBuffer which is overridden by QgsVectorLayerEditPassthrough. Then we'd have a cleaner separation between transactions and the use of QgsVectorLayerEditPassthrough (which may potentially be used in other situations in future, or by plugins...)
There was a problem hiding this comment.
I've added isPassthrough() to edit buffer for a clearer semantic
| QgsFeatureRequest request; | ||
| request.setFilterFid( mFid ); | ||
| request.setSubsetOfAttributes( {} ); | ||
| std::unique_ptr<QgsVectorLayer> layerClone( layer()->clone() ); |
There was a problem hiding this comment.
This is very expensive to call. Why are we using a clone here instead of getting the features from the data provider directly?
There was a problem hiding this comment.
We cannot use the provider directly because it will "see" what is in the current transaction and not the status of the backend storage. What we need here is to know if the geometry has changed from what is in the storage outside of the current transaction.
I can't think of another way to access the status of the storage outside of the transaction than cloning the provider but suggestions are welcome.
| request.setFilterFid( mFid ); | ||
| request.setFlags( QgsFeatureRequest::NoGeometry ); | ||
| request.setSubsetOfAttributes( QgsAttributeList() << mFieldIndex ); | ||
| std::unique_ptr<QgsVectorLayer> layerClone( layer()->clone() ); |
There was a problem hiding this comment.
Same here -- this is very expensive to call. Is there no other approach?
|
@elpaso reviewed! |
…hrough Bugfix gh41477 editbuffer passthrough
elpaso
force-pushed
the
backport-41539-to-release-3_18
branch
from
678aaa3
to
b013991
Compare
|
While working on this I discovered many more bugs that should be fixed, I'm closing this and I will continue to submit the patches on master. I will hopefully find the time to do another backport eventually. |
Backport b3dae9a from #41539