Move rows in custom model

[Please_Help_me_D]

Hi,

I've implemented custom model that allows to drag and drop items within model.
The model has constant number of columns : 1
I also use QSortFilterProxyModel inherithed proxy where I only reimplement filterAcceptsRow.

The problem is that sometimes I get seg fault somewhere within endRemoveRows() function. Usually this works as follows:
I drop one item in another few times and then all of a sudden I may get a warning from proxy model: QSortFilterProxyModel: invalid inserted rows reported by source model.
After that if I try to drag/drop once again I get segfault.

Another behaviour is that after several drag and drop operations I get either all items hidden, or some of them. Invoking invalidateFilter() from proxy may help in this case but not for 100%

bool qColadaH5Model::moveItem(qColadaH5Item *parentItem,
                              qColadaH5Item *item,
                              int position)
{
  if (!item || !parentItem)
    return false;

  if (!this->canBeMoved(parentItem, item))
    return false;

  qColadaH5Item* oldParentItem = item->getParent();
  if (!oldParentItem)
    return false;

  QModelIndex parentIndex = indexFromItem(parentItem);
  if (!parentIndex.isValid())
    return false;

  int oldPosition = item->getRow();
  if (oldPosition < 0)
    return false;

  QModelIndex oldParentIndex = indexFromItem(oldParentItem);
  if (!oldParentIndex.isValid())
    return false;

  std::cout << "oldParentIndex.row: " << oldParentIndex.row() << std::endl;
  std::cout << "parentIndex.row: " << parentIndex.row() << std::endl;
  std::cout << "-----------------------------------------" << std::endl;

  // remove item without deleting it
  beginRemoveRows(oldParentIndex, oldPosition, oldPosition);
  item = oldParentItem->takeChild(item);
  endRemoveRows();

  if (!item){
    std::cout << "item not found" << std::endl;
    return false;
  }

  if (position < 0 || position > parentItem->childCount())
    position = parentItem->childCount();

  // the app crashes (when expanding) if we move item to the unfetched parent
  if (canFetchMore(parentIndex))
    fetchMore(parentIndex);

  beginInsertRows(parentIndex, position, position);
  parentItem->insertChild(item, position);
  endInsertRows();

  return true;
}

QModelIndex qColadaH5Model::index(int row, int column,
                                  const QModelIndex &parent) const {
  qColadaH5Item *parentItem = itemFromIndex(parent);
  if (!parentItem || row < 0 || column < 0 ||
      row >= parentItem->rowCount() ||
      column >= parentItem->columnCount()) {
    return QModelIndex();
  }
  return createIndex(row, column, parentItem);
}

QModelIndex qColadaH5Model::indexFromItem(qColadaH5Item *item) const {
  if (item && item->getParent()) {
      int row = item->getRow();  // get parent and search for that item, return the found position
      return createIndex(row, 0, item->getParent());
  }
  return QModelIndex();
}

and the proxy part:

bool qColadaH5ProxyModel::filterAcceptsRow(
    int source_row, const QModelIndex &source_parent) const {
  Q_D(const qColadaH5ProxyModel);

  qColadaH5Model *sm = qobject_cast<qColadaH5Model *>(sourceModel());
  if (!sm){
    qCritical() << Q_FUNC_INFO << ": Unable to get Source model";
    return false;
  }

  QModelIndex index = sm->index(source_row, 0, source_parent);
  if (!index.isValid()){
    qCritical() << Q_FUNC_INFO << ": index is invalid";
    return false;
  }

  qColadaH5Item *item = sm->itemFromIndex(index);
  if (item == nullptr)
    return false;

  return filterCheckOnly(item) && filterLinkType(item);
}

bool qColadaH5ProxyModel::filterCheckOnly(qColadaH5Item* item) const
{
  Q_D(const qColadaH5ProxyModel);
  if (!item)
    return false;

  if (d->showCheckedOnlyFlag && !item->checkState())
    return false;

  return true;
}

bool qColadaH5ProxyModel::filterLinkType(qColadaH5Item* item) const
{
  Q_D(const qColadaH5ProxyModel);
  if (!item)
    return false;

  h5gt::LinkType linkType = static_cast<h5gt::LinkType>(item->getLinkType());
  if (!d->showHardLinks && linkType == h5gt::LinkType::Hard){
    return false;
  }
  if (!d->showSoftLinks && linkType == h5gt::LinkType::Soft){
    return false;
  }
  if (!d->showExternalLinks && linkType == h5gt::LinkType::External){
    return false;
  }
  return true;
}

Most of overriden methods should work fine, and usually I don't have any problems with getting index/item from item/index.

A video demostrating the problem is on google drive

[Please_Help_me_D]

Oh, it seems that in qColadaH5Model::moveItem I should have determine parentIndex only after rows are removed i.e. after endRemoveRows(). Because there are cases when this index is changed after removing rows and thus it becomes invalid.

Probably I should have implement takeRow and insertRow instead of moveRow: I would not spend few days for searching this bug

By the way QAbstractItemModelTester is very helpful and pretty easy to use.

[Please_Help_me_D]

I just understood (using cout) that sometimes beginInsertRows(parentIndex, position, position); can't get item using my function itemFromIndex (indirectly through rowCount()):

qColadaH5Item *qColadaH5Model::itemFromIndex(const QModelIndex &index) const {
  Q_D(const qColadaH5Model);
  if (!index.isValid())
    return d->rootItem;
  if (index.model() != this)
    return nullptr;
  qColadaH5Item *parent = static_cast<qColadaH5Item *>(index.internalPointer());
  if (parent == nullptr)
    return nullptr;
  return parent->getChild(index.row());
}

[Please_Help_me_D]

Oh, it seems that in qColadaH5Model::moveItem I should have determine parentIndex only after rows are removed i.e. after endRemoveRows(). Because there are cases when this index is changed after removing rows and thus it becomes invalid.

Probably I should have implement takeRow and insertRow instead of moveRow: I would not spend few days for searching this bug

By the way QAbstractItemModelTester is very helpful and pretty easy to use.