<html>
<head>
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>
</p>
<br>
<div class="moz-cite-prefix">On 06.07.2017 11:00, Sandro Mani wrote:<br>
</div>
<blockquote type="cite"
cite="mid:a578492c-a193-1f6a-c24f-3091a62696dd@gmail.com">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<p>
</p>
<br>
<div class="moz-cite-prefix">On 06.07.2017 10:57, Even Rouault
wrote:<br>
</div>
<blockquote type="cite" cite="mid:3075694.1tgngYB01I@even-i700">
<meta name="qrichtext" content="1">
<style type="text/css">
p, li { white-space: pre-wrap; }
</style>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">On jeudi 6 juillet 2017 10:45:21 CEST Sandro Mani wrote:</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> On 05.07.2017 14:56, Sandro Mani wrote:</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> That could potentially be done using the current OGR API. Basically</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> in pseudo code, to execute before executing REPACK</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> OGR_L_ResetReading(layer)</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> char** ignored_fields = CSLAddString(NULL, "OGR_GEOMETRY" );</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> OGR_L_SetIgnoredFields( layer, ignored_fields); // for performance.</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> CSLDestroy(ignored_fields);</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> OGR_L_SetAttributeFilter( layer, NULL )</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> OGR_L_SetSpatialFilter( layer, NULL )</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> std::map<GIntBig, GIntBig> mapOldIdToNewId;</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> GIntBig newId = 0;</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> while( feature = OGR_L_GetNextFeature(layer) )</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> {</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> mapOldIdToNewId[OGR_F_GetFID(feature)] = newId;</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> newId ++;</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> OGR_F_destroy(feature);</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> }</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> >> OGR_L_SetIgnoredFields( layer, NULL );</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> > </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> > Ok cool, that sounds like a plan - I'll give it a shot.</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> Hmm so while on the provider side it works well, for the geometry</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> checker it is turning out to be pretty hard to deal with the changing</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> feature ids (just to cite one example: error fixed by merging geometry</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> of feature 10 with that of 11, results in {deleted: 10, updated: 11},</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> but after the feature id adjustment this would read {deleted: 10,</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> updated: 10}, meaning one would need to keep track that deleted: 10</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> refers to the old featureid). Not saying that it isn't doable, but the</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> complexitiy of properly handling the feature id changes is non-trivial.</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> So, other suggestion: any objections if I add a method to</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> QgsVectorDataProvider to temporarily freeze repacking? I could also add</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> a notification informing the user that the shapefile should not be used</p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">> in other applications while repacking is frozen.</p>
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; "> </p>
<p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">My feeling is that QgsOgrDataProvider is already complicated enough with its existing tricks. I'm not sure this temporary freeze repacking is a right move (how would you decide when you do it ? and I'm pretty sure users will not get it, or will have issues if they start an algorithm with an external tool that requires packed shapefiles)</p>
</blockquote>
It would be something I would call explicitly while the geometry
checker is active, I think it is safe to assume that users don't
expect that they can operate on the shapefile while the geometry
checker is processing it. It would not add much complexity to the
provider, just a flag that repacking is frozen, and a setter to
set that flag. When the flag is set to false, the shapefile is
repacked.<br>
<br>
</blockquote>
I did some playing around and came up with some FeatureId mapping
code which is pretty space efficient (basically requiring just
3*nDeletedFeatures entries), see below.<br>
<br>
The code however assumes that the gaps in the fid-sequence are
filled by decreasing the fids after the gap, which may be pretty
specific to the OGR/Shapefile behaviour. Hence in my view such code
should really belong into the OGR provider (or perhaps even ogr
itself) and not in the plugin. Thoughts?<br>
<br>
Sandro<br>
<br>
-------<br>
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial; word-wrap: break-word; white-space: pre-wrap;">#include <algorithm>
#include <iostream>
#include <QMap>
#include <QVector>
typedef int QgsFeatureId;
class FeatureMapping {
public:
QgsFeatureId stableToProvider(QgsFeatureId stableId) const
{
if(mDeletedStableIds.contains(stableId)){
return -1;
}
// providerId = stableId - nDeletedBeforeStableId
int nDeletedBeforeStableId = std::lower_bound(mDeletedStableIds.begin(), mDeletedStableIds.end(), stableId) - mDeletedStableIds.begin();
return stableId - nDeletedBeforeStableId;
}
QgsFeatureId providerToStable(QgsFeatureId providerId) const
{
int i = 0, n = mProviderToStableOffsetKeys.size(), offset = 0;
while(i < n && mProviderToStableOffsetKeys[i] <= providerId) {
offset += mProviderToStableOffsetValues[i++];
}
return providerId + offset;
}
void updateFeatureIdMap(const QList<QgsFeatureId> &deletedIds)
{
for(QgsFeatureId fid : deletedIds) {
mDeletedStableIds.append(providerToStable(fid));
}
std::sort(mDeletedStableIds.begin(), mDeletedStableIds.end());
int n = mProviderToStableOffsetKeys.size();
for(QgsFeatureId fid : deletedIds) {
int pos = std::lower_bound(mProviderToStableOffsetKeys.begin(), mProviderToStableOffsetKeys.end(), fid) - mProviderToStableOffsetKeys.begin();
if(pos < n && mProviderToStableOffsetKeys[pos] == fid) {
mProviderToStableOffsetValues[pos] += 1;
} else {
mProviderToStableOffsetKeys.insert(pos, fid);
mProviderToStableOffsetValues.insert(pos, 1);
++n;
}
if(pos + 1 != n && mProviderToStableOffsetKeys[pos+1] -1 == fid) {
mProviderToStableOffsetValues[pos] += mProviderToStableOffsetValues[pos+1];
mProviderToStableOffsetKeys.removeAt(pos + 1);
mProviderToStableOffsetValues.removeAt(pos + 1);
--n;
}
++pos;
while(pos < n) {
mProviderToStableOffsetKeys[pos++] -= 1;
}
}
std::cout << "offsets = {";
for(int i = 0, n = mProviderToStableOffsetKeys.size(); i < n; ++i) {
std::cout << mProviderToStableOffsetKeys[i] << ": " << mProviderToStableOffsetValues[i] << ", ";
}
std::cout << "\b\b}" << std::endl;
}
int numDeleted() const{ return mDeletedStableIds.size(); }
private:
QVector<QgsFeatureId> mProviderToStableOffsetKeys;
QVector<int> mProviderToStableOffsetValues;
QVector<QgsFeatureId> mDeletedStableIds;
};
void printMapping(const FeatureMapping& mapping) {
std::cout << "------" << std::endl;
for(int i = 0; i < 10; ++i) {
std::cout << i << " -> " << mapping.stableToProvider(i) << std::endl;
}
std::cout << "------" << std::endl;
for(int i = 0, n = 10 - mapping.numDeleted(); i < n; ++i) {
std::cout << mapping.providerToStable(i) << " <- " << i << std::endl;
}
std::cout << "------" << std::endl;
}
int main(int argc, char* argv[]){
FeatureMapping mapping;
printMapping(mapping);
while(true) {
std::cout << "> ";
std::string command;
std::getline(std::cin, command);
if(command.empty()) {
continue;
}else if(command[0] == 'q') {
break;
}else if(command[0] == 'd') {
QgsFeatureId num = std::atoi(command.substr(1).c_str());
std::cout << "* Delete: stable " << num << ", provider: " << mapping.stableToProvider(num) << std::endl;
mapping.updateFeatureIdMap({mapping.stableToProvider(num)});
printMapping(mapping);
} else {
std::cout << "Unknown command" << std::endl;
}
}
return 0;
}</pre>
<br>
<br>
</body>
</html>