On 06.07.2017 11:00, Sandro Mani wrote:


On 06.07.2017 10:57, Even Rouault wrote:

On jeudi 6 juillet 2017 10:45:21 CEST Sandro Mani wrote:

> On 05.07.2017 14:56, Sandro Mani wrote:

> >> That could potentially be done using the current OGR API. Basically

> >> in pseudo code, to execute before executing REPACK

> >>

> >> OGR_L_ResetReading(layer)

> >>

> >> char** ignored_fields = CSLAddString(NULL, "OGR_GEOMETRY" );

> >>

> >> OGR_L_SetIgnoredFields( layer, ignored_fields); // for performance.

> >>

> >> CSLDestroy(ignored_fields);

> >>

> >> OGR_L_SetAttributeFilter( layer, NULL )

> >>

> >> OGR_L_SetSpatialFilter( layer, NULL )

> >>

> >> std::map<GIntBig, GIntBig> mapOldIdToNewId;

> >>

> >> GIntBig newId = 0;

> >>

> >> while( feature = OGR_L_GetNextFeature(layer) )

> >>

> >> {

> >>

> >> mapOldIdToNewId[OGR_F_GetFID(feature)] = newId;

> >>

> >> newId ++;

> >>

> >> OGR_F_destroy(feature);

> >>

> >> }

> >>

> >> OGR_L_SetIgnoredFields( layer, NULL );

> >

> > Ok cool, that sounds like a plan - I'll give it a shot.

>

> Hmm so while on the provider side it works well, for the geometry

> checker it is turning out to be pretty hard to deal with the changing

> feature ids (just to cite one example: error fixed by merging geometry

> of feature 10 with that of 11, results in {deleted: 10, updated: 11},

> but after the feature id adjustment this would read {deleted: 10,

> updated: 10}, meaning one would need to keep track that deleted: 10

> refers to the old featureid). Not saying that it isn't doable, but the

> complexitiy of properly handling the feature id changes is non-trivial.

>

> So, other suggestion: any objections if I add a method to

> QgsVectorDataProvider to temporarily freeze repacking? I could also add

> a notification informing the user that the shapefile should not be used

> in other applications while repacking is frozen.

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)

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.

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.

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?

Sandro

-------

#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;
}



_______________________________________________
QGIS-Developer mailing list
QGIS-Developer@lists.osgeo.org
List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer

Reply via email to