On 20.09.2017 17:39, Even Rouault wrote:
On mercredi 20 septembre 2017 17:32:27 CEST Sandro Mani wrote:
> On 19.09.2017 19:15, Even Rouault wrote:
> > On mardi 19 septembre 2017 18:30:50 CEST Sandro Mani wrote:
> > > On 11.07.2017 14:31, Even Rouault wrote:
> > > > We could potentially imagine to defer the repacking at
> > > >
> > > > leaveUpdateMode() time, actually in the close() method which
is called
> > > >
> > > > when the reference counter of the update mode drops to zero (*),
> > > >
> > > > instead of doing it at the end of each operation.
> > >
> > > To come back to this, I was looking at moving replack() to
close(), but
> > >
> > > noticed that syncToDisc() has the logic "for shapefiles, remove
spatial
> > >
> > > index files and create a new index" which calls close. And
syncToDisk is
> > >
> > > still called after every change. So should syncToDisk be also
moved to
> > >
> > > close(), or at least blocked if an updateMode is active?
> >
> > Yes that would probably make sense to block syncToDisk() while in
> > updateMode (so when mUpdateModeStackDepth > 0), but probably only if
> > enterUpdateMode() is called by QgsVectorLayer::startEditing(), and not
> > when called implicitly by doInitialActionsForEdition(), which might be
> > the case if someone directly works at the provider level
> >
> > bool QgsOgrProvider::doInitialActionsForEdition()
> >
> > {
> >
> > if ( !mValid )
> >
> > return false;
> >
> > if ( !mWriteAccess && mWriteAccessPossible && mDynamicWriteAccess )
> >
> > {
> >
> > QgsDebugMsg( "Enter update mode implictly" );
> >
> > if ( !enterUpdateMode() )
> >
> > return false;
> >
> > }
> >
> > return true;
> >
> > }
> >
> > So probably a mImplicitUpdateMode flag should be set above near the
> > QgsDebugMsg(), and the check to decide whether we can defer
> > syncToDisk() would be ( !mImplicitUpdateMode &&
mUpdateModeStackDepth > 0)
>
> Hm but will that flag never be cleared again once set? Since AFAICT a
> there is no leaveUpdateMode matching the enterUpdateMode called by
> doInitialActionsForEdition.
>
> As I understand, the approach to do changes with stable ids would be to
> first enterUpdateMode, then perform the changes, then leaveUpdateMode,
> at which point the changes are synced and the dataset repacked.
Yes
> With the
> mImplicitUpdateMode flag, if anyone worked directly at provider level
> before you come along, deferring syncToDisk would never happen since the
> mImplicitUpdateMode was set and remains set.
Yes my idea was to keep the current behaviour where repack() is done
frequently, as you cannot know when the dataset should be in a
consistant state.
But we could also decide to defer syncToDisk() at provider deletion
(would make the code simpler)
I have no idea of the existing use cases behind and if that would
break some.
Suppose something like [1] should both allow reliably deferring
syncToDisk when an updateMode was explicitly entered while keeping the
current behaviour of immediate syncToDisk for implicit update mode.
Sandro
[1]
https://github.com/manisandro/QGIS/commit/13f451bdcbb653e9d69e55c79a5ba520d3f1712e
_______________________________________________
QGIS-Developer mailing list
[email protected]
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
