Re: extragear/graphics/digikam/digikam

[Johannes Wienke-3]

Hi Marcel,

this commit breaks restoring the state for searching. Tag a tree view
and collapse everything so that only the first hierarchy is visible.
Afterwards start a search maybe starting with the letter "w". After
removing the search string from the searchtextbar no everything is
collapsed that contains a w. The idea of my code was to completely
backup the state of the tree view before starting a search, collapse
everything as usual during a search, and after the search restore the
complete state + collapse to the now selected album. I would suspect,
that state saving is called now also after the first letter for the
search is entered.

Johannes

Am 14.02.2010 16:18 schrieb Marcel Wiesweg:

> SVN commit 1090062 by mwiesweg:
>
> restoreState was called very often. The problem is that queued signals
> will be delivered when sent even if the connection is removed.
> When the connection is removed from the slot of the queued signal, and
> albums are added in one event loop run, then signals will be received for each
> album even though the connection is removed after the first one.
>
> Now, the restoring code is modified to work in the rowsInserted() code directly
> and not for the full hierarchy each time. Expansion only works when children
> are available, so parents will get restored a second time.
>
>  M  +88 -106   albumtreeview.cpp  
>  M  +9 -15     albumtreeview.h  
>
>
> --- trunk/extragear/graphics/digikam/digikam/albumtreeview.cpp #1090061:1090062
> @@ -320,7 +320,7 @@
>  
>          kDebug() << "Searching finished, restoring tree view state";
>  
> -        restoreState(QModelIndex(), d->searchBackup);
> +        restoreStateForHierarchy(QModelIndex(), d->searchBackup);
>          d->searchBackup.clear();
>  
>          if (d->lastSelectedAlbum)
> @@ -519,12 +519,12 @@
>  
>      KConfigGroup configGroup = getConfigGroup();
>  
> -    //kDebug() << "Loading view state from " << configGroup.name();
> +    //kDebug() << "Loading view state from " << this << configGroup.name() << objectName();
>  
>      // extract the selection from the config
>      const QStringList selection = configGroup.readEntry(entryName(d->configSelectionEntry),
>                      QStringList());
> -//    kDebug() << "selection: " << selection;
> +    //kDebug() << "selection: " << selection;
>      foreach(const QString &key, selection)
>      {
>          bool validId;
> @@ -551,7 +551,7 @@
>  
>      // extract current index from config
>      const QString key = configGroup.readEntry(entryName(d->configCurrentIndexEntry), QString());
> -//    kDebug() << "currentIndey: " << key;
> +    //kDebug() << "currentIndex: " << key;
>      bool validId;
>      const int id = key.toInt(&validId);
>      if (validId)
> @@ -559,33 +559,22 @@
>          d->statesByAlbumId[id].currentIndex = true;
>      }
>  
> -//    for (QMap<int, Digikam::State>::iterator it = d->statesByAlbumId.begin(); it
> -//                    != d->statesByAlbumId.end(); ++it)
> -//    {
> -//        kDebug() << "id = " << it.key() << ": recovered state (selected = "
> -//                 << it.value().selected << ", expanded = "
> -//                 << it.value().expanded << ", currentIndex = "
> -//                 << it.value().currentIndex << ")";
> -//    }
> -
> -
> -    // initial restore run, for everything already loaded
> -//    kDebug() << "initial restore run with " << model()->rowCount() << " rows";
> -    for (int i = 0; i < model()->rowCount(); ++i)
> +    /*
> +    for (QMap<int, Digikam::State>::iterator it = d->statesByAlbumId.begin(); it
> +        != d->statesByAlbumId.end(); ++it)
>      {
> -        const QModelIndex index = model()->index(i, 0);
> -        restoreState(index, d->statesByAlbumId);
> +        kDebug() << "id = " << it.key() << ": recovered state (selected = "
> +        << it.value().selected << ", expanded = "
> +        << it.value().expanded << ", currentIndex = "
> +        << it.value().currentIndex << ")";
>      }
> +    */
>  
> -    // if there are still untreated entries that need to be restored, used the
> -    // model's signal to handle them
> -    if (!d->statesByAlbumId.empty())
> -    {
> -        // and the watch the model for new items added
> -        connect(model(), SIGNAL(rowsInserted(QModelIndex, int, int)),
> -                         SLOT(slotFixRowsInserted(QModelIndex, int, int)), Qt::QueuedConnection);
> -    }
>  
> +    // initial restore run, for everything already loaded
> +    //kDebug() << "initial restore run with " << model()->rowCount() << " rows";
> +    restoreStateForHierarchy(QModelIndex(), d->statesByAlbumId);
> +
>      // also restore the sorting order
>      sortByColumn(configGroup.readEntry(entryName(d->configSortColumnEntry), 0),
>                   (Qt::SortOrder) configGroup.readEntry(entryName(d->configSortOrderEntry), (int) Qt::AscendingOrder));
> @@ -599,26 +588,36 @@
>  
>  }
>  
> -void AbstractAlbumTreeView::restoreState(const QModelIndex &index, QMap<int, Digikam::State> &stateStore)
> +void AbstractAlbumTreeView::restoreStateForHierarchy(const QModelIndex &index, QMap<int, Digikam::State> &stateStore)
>  {
> +    restoreState(index, stateStore);
> +    // do a recursive call of the state restoration
> +    for (int i = 0; i < model()->rowCount(index); ++i)
> +    {
> +        const QModelIndex child = model()->index(i, 0, index);
> +        restoreStateForHierarchy(child, stateStore);
> +    }
> +}
>  
> +void AbstractAlbumTreeView::restoreState(const QModelIndex &index, QMap<int, Digikam::State> &stateStore)
> +{
>      Album *album = albumFilterModel()->albumForIndex(index);
> -    if (album)
> +    if (album && stateStore.contains(album->id()))
>      {
>  
> -        Digikam::State state = stateStore[album->id()];
> +        Digikam::State state = stateStore.value(album->id());
>  
> -//        kDebug() << "Trying to restore state of album " << album->title()
> -//                 << ": state(selected = " << state.selected
> -//                 << ", expanded = " << state.expanded
> -//                 << ", currentIndex = " << state.currentIndex << ")";
> -
> -        // restore selection state
> +        /*
> +        kDebug() << "Trying to restore state of album " << album->title()
> +                 << ": state(selected = " << state.selected
> +                 << ", expanded = " << state.expanded
> +                 << ", currentIndex = " << state.currentIndex << ")" << this;
> +        */
>          if (state.selected)
>          {
> -//            kDebug() << "Selecting" << album->title();
> -            selectionModel()->select(index, QItemSelectionModel::Select
> -                            | QItemSelectionModel::Rows);
> +            //kDebug() << "Selecting" << album->title();
> +            selectionModel()->select(index, QItemSelectionModel::SelectCurrent
> +                                          | QItemSelectionModel::Rows);
>          }
>  
>          // restore expansion state but ensure that the root album is always
> @@ -635,44 +634,48 @@
>          // restore the current index
>          if (state.currentIndex)
>          {
> -//            kDebug() << "Setting current index" << album->title();
> -            setCurrentIndex(index);
> +            //kDebug() << "Setting current index" << album->title();
> +            selectionModel()->setCurrentIndex(index, QItemSelectionModel::SelectCurrent
> +                                                   | QItemSelectionModel::Rows);
>          }
> -
> -        // remove this state so that we don't get in trouble later in case the
> -        // same album id is reused again
> -        stateStore.remove(album->id());
> -
>      }
> +}
>  
> -    // do a recursive call of the state restoration
> -    for (int i = 0; i < model()->rowCount(index); ++i)
> +void AbstractAlbumTreeView::rowsInserted(const QModelIndex &parent, int start, int end)
> +{
> +    QTreeView::rowsInserted(parent, start, end);
> +
> +    if (!d->statesByAlbumId.isEmpty())
>      {
> -        const QModelIndex child = model()->index(i, 0, index);
> -        restoreState(child, stateStore);
> -    }
> +        //kDebug() << "slot rowInserted called with index = " << index
> +        //         << ", start = " << start << ", end = " << end << "remaining ids" << d->statesByAlbumId.keys();
>  
> +        // Restore state for parent a second time - expansion can only be restored if there are children
> +        restoreState(parent, d->statesByAlbumId);
> +
> +        for (int i = start; i <= end; ++i)
> +        {
> +            const QModelIndex child = model()->index(i, 0, parent);
> +            restoreState(child, d->statesByAlbumId);
> +        }
> +    }
>  }
>  
> -void AbstractAlbumTreeView::slotFixRowsInserted(const QModelIndex &index, int start, int end)
> +void AbstractAlbumTreeView::rowsAboutToBeRemoved(const QModelIndex& parent, int start, int end)
>  {
> +    QTreeView::rowsAboutToBeRemoved(parent, start, end);
>  
> -//    kDebug() << "slot rowInserted called with index = " << index
> -//             << ", start = " << start << ", end = " << end;
> -
> -    for (int i = start; i <= end; ++i)
> +    // Clean up map if album id is reused for a new album
> +    if (!d->statesByAlbumId.isEmpty())
>      {
> -        const QModelIndex child = model()->index(i, 0, index);
> -        restoreState(child, d->statesByAlbumId);
> +        for (int i = start; i <= end; ++i)
> +        {
> +            const QModelIndex child = model()->index(i, 0, parent);
> +            Album *album = albumModel()->albumForIndex(child);
> +            if (album)
> +                d->statesByAlbumId.remove(album->id());
> +        }
>      }
> -
> -    if (d->statesByAlbumId.empty())
> -    {
> -        // disconnect if not needed anymore
> -        disconnect(model(), SIGNAL(rowsInserted(QModelIndex, int, int)),
> -                   this, SLOT(slotFixRowsInserted(QModelIndex, int, int)));
> -    }
> -
>  }
>  
>  void AbstractAlbumTreeView::adaptColumnsToContent()
> @@ -883,9 +886,6 @@
>  {
>      AbstractAlbumTreeView::setAlbumFilterModel(filterModel);
>  
> -    connect(m_albumFilterModel, SIGNAL(rowsInserted(const QModelIndex &, int, int)),
> -             this, SLOT(slotRowsInserted(const QModelIndex &, int, int)));
> -
>      // Initialize expanded/collapsed showCount state
>      updateShowCountState(QModelIndex(), true);
>  }
> @@ -920,8 +920,9 @@
>      static_cast<AbstractCountingAlbumModel*>(m_albumModel)->setShowCount(AlbumSettings::instance()->getShowFolderTreeViewItemsCount());
>  }
>  
> -void AbstractCountingAlbumTreeView::slotRowsInserted(const QModelIndex& parent, int start, int end)
> +void AbstractCountingAlbumTreeView::rowsInserted(const QModelIndex& parent, int start, int end)
>  {
> +    AbstractAlbumTreeView::rowsInserted(parent, start, end);
>      // initialize showCount state when items are added
>      for (int i=start; i<=end; ++i)
>          updateShowCountState(m_albumFilterModel->index(i, 0, parent), false);
> @@ -1030,25 +1031,26 @@
>      }
>  
>      // initially sync with the albums that are already in the model
> -    for (int i = 0; i < checkableModel()->rowCount(); ++i)
> -    {
> -        const QModelIndex index = checkableModel()->index(i, 0);
> -        restoreCheckState(index);
> -    }
> +    restoreCheckStateForHierarchy(QModelIndex());
> +}
>  
> -    // wait for missing albums in the background
> -    if (!d->checkedAlbumIds.empty())
> +void AbstractCheckableAlbumTreeView::rowsInserted(const QModelIndex& parent, int start, int end)
> +{
> +    AbstractCountingAlbumTreeView::rowsInserted(parent, start, end);
> +    if (!d->checkedAlbumIds.isEmpty())
>      {
> -        // and the watch the model for new items added
> -        connect(checkableModel(), SIGNAL(rowsInserted(QModelIndex, int, int)),
> -                this,  SLOT(slotRowsAddedCheckState(QModelIndex, int, int)), Qt::QueuedConnection);
> +        for (int i = start; i <= end; ++i)
> +        {
> +            const QModelIndex child = checkableModel()->index(i, 0, parent);
> +            restoreCheckState(child);
> +        }
>      }
> -
>  }
>  
> -void AbstractCheckableAlbumTreeView::slotRowsAddedCheckState(QModelIndex index, int start, int end)
> +void AbstractCheckableAlbumTreeView::restoreCheckStateForHierarchy(const QModelIndex &index)
>  {
> -    for (int i = start; i <= end; ++i)
> +    // recurse children
> +    for (int i = 0; i < checkableModel()->rowCount(index); ++i)
>      {
>          const QModelIndex child = checkableModel()->index(i, 0, index);
>          restoreCheckState(child);
> @@ -1057,32 +1059,12 @@
>  
>  void AbstractCheckableAlbumTreeView::restoreCheckState(const QModelIndex &index)
>  {
> -
>      Album *album = checkableModel()->albumForIndex(index);
> -    if (album)
> +    if (album && d->checkedAlbumIds.contains(album->id()))
>      {
> -
> -        if (d->checkedAlbumIds.contains(album->id()))
> -        {
> -            checkableModel()->setCheckState(album, Qt::Checked);
> -            d->checkedAlbumIds.removeOne(album->id());
> -            if (d->checkedAlbumIds.empty())
> -            {
> -                // disconnect if not needed anymore
> -                disconnect(model(), SIGNAL(rowsInserted(QModelIndex, int, int)),
> -                           this, SLOT(slotRowsAddedCheckState(QModelIndex, int, int)));
> -            }
> -        }
> -
> +        checkableModel()->setCheckState(album, Qt::Checked);
> +        d->checkedAlbumIds.removeOne(album->id());
>      }
> -
> -    // recurse children
> -    for (int i = 0; i < checkableModel()->rowCount(index); ++i)
> -    {
> -        const QModelIndex child = checkableModel()->index(i, 0, index);
> -        restoreCheckState(child);
> -    }
> -
>  }
>  
>  void AbstractCheckableAlbumTreeView::doSaveState()
> --- trunk/extragear/graphics/digikam/digikam/albumtreeview.h #1090061:1090062
> @@ -233,6 +233,8 @@
>      bool checkExpandedState(const QModelIndex& index);
>      void mousePressEvent(QMouseEvent *e);
>  
> +    void rowsInserted(const QModelIndex &index, int start, int end);
> +    void rowsAboutToBeRemoved(const QModelIndex& parent, int start, int end);
>      void startDrag(Qt::DropActions supportedActions);
>      void dragEnterEvent(QDragEnterEvent *e);
>      void dragMoveEvent(QDragMoveEvent *e);
> @@ -263,6 +265,10 @@
>       * @param index index to start restoring
>       * @param stateStore states indexed by album id
>       */
> +    void restoreStateForHierarchy(const QModelIndex &index, QMap<int, Digikam::State> &stateStore);
> +    /**
> +     * Restore the state for this index.
> +     */
>      void restoreState(const QModelIndex &index, QMap<int, Digikam::State> &stateStore);
>  
>      /**
> @@ -275,16 +281,6 @@
>  private Q_SLOTS:
>  
>      /**
> -     * Used for asynchronous restoring of the tree view state if the contents of
> -     * the model are received after the view has been created.
> -     *
> -     * @param index parent index of the inserted data
> -     * @param start start row of new data under the parent index
> -     * @param end end row of new data under the parent index
> -     */
> -    void slotFixRowsInserted(const QModelIndex &index, int start, int end);
> -
> -    /**
>       * Adapts the columns in between the given model indices to the content
>       * size. This can be connected to dataChanged.
>       *
> @@ -333,13 +329,13 @@
>  protected:
>  
>      void setAlbumFilterModel(AlbumFilterModel *filterModel);
> +    virtual void rowsInserted(const QModelIndex& parent, int start, int end);
>  
>  private Q_SLOTS:
>  
>      void slotCollapsed(const QModelIndex& index);
>      void slotExpanded(const QModelIndex& index);
>      void slotSetShowCount();
> -    void slotRowsInserted(const QModelIndex& parent, int start, int end);
>      void updateShowCountState(const QModelIndex& index, bool recurse);
>  
>  private:
> @@ -391,13 +387,11 @@
>  protected:
>  
>      virtual void middleButtonPressed(Album *a);
> +    virtual void rowsInserted(const QModelIndex& parent, int start, int end);
>  
> -private Q_SLOTS:
> -
> -    void slotRowsAddedCheckState(QModelIndex index, int start, int end);
> -
>  private:
>  
> +    void restoreCheckStateForHierarchy(const QModelIndex &index);
>      void restoreCheckState(const QModelIndex &index);
>  
>      AbstractCheckableAlbumTreeViewPriv *d;

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Johannes Wienke-3]

Am 14.02.2010 17:39 schrieb Johannes Wienke:
> this commit breaks restoring the state for searching. Tag a tree view
> and collapse everything so that only the first hierarchy is visible.
> Afterwards start a search maybe starting with the letter "w". After
> removing the search string from the searchtextbar no everything is
> collapsed that contains a w.

Oh, should be "now everything is expanded"...


_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Marcel Wiesweg]

> Hi Marcel,
>
> this commit breaks restoring the state for searching. Tag a tree view
> and collapse everything so that only the first hierarchy is visible.
> Afterwards start a search maybe starting with the letter "w". After
> removing the search string from the searchtextbar no everything is
> collapsed that contains a w. The idea of my code was to completely
> backup the state of the tree view before starting a search, collapse
> everything as usual during a search, and after the search restore the
> complete state + collapse to the now selected album. I would suspect,
> that state saving is called now also after the first letter for the
> search is entered.

Should be fixed now. I hope this has no further side effects.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel