Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-2329

QColumnView: visual bug after clicking second-level items with no children

    XMLWordPrintable

Details

    • Bug
    • Resolution: Done
    • P3: Somewhat important
    • 5.4.1
    • 4.4.1, 4.4.2, 4.4.3, 4.5.0, 4.5.1, 4.5.2, 4.5.3, 4.6.0, 4.6.1, 4.6.2, 4.6.3, 4.7.0
    • Widgets: Itemviews
    • None
    • 8920200c97f0a514397e039ba74588127163c2b9

    Description

      The issue here is that when clicking on a second-level item with no children, there is a graphical bug where all items above the currently selection are highlighted for a second, before being painted correctly.

      Steps to reproduce using attached example:

      1) Open the example and click on any item in the first column

      2) Expand the window and click on a item in the second column, A third empty column should be created

      3) Click on another item in the second column, you will notice that for a second, all items above the once selected will be highlighted.

      4) Clicking on any item in this second column and holding, will demonstrate that all items are highlighted, without a drag action.

      This is error is quite annoying when trying to select multiple items in this column. It also causes much confusion to users actually attempting to make these selections.

      Cause of the Issue:

      in qcolumnview.cpp:974 QColumnViewPrivate::_q_changeCurrentColumn
      Line 989:

      "qcolumnview.cpp:989 QColumnViewPrivate::_q_changeCurrentColumn"
      if ((columns.size() > i + 1)) 
           view->setCurrentIndex(columns.at(i+1)->rootIndex()); 
      

      sets and invalid index when the next column is empty.

      This is the situation we know to cause the bug, and because this empty index is set as current, it causes the wrong position to be set here:
      QAbstractItemView::setCurrantIndex:1010

      "QAbstractItemView::setCurrantIndex:1010"
      void QAbstractItemView::setCurrentIndex(const QModelIndex &index)
      {
          Q_D(QAbstractItemView);
          if (d->selectionModel && (!index.isValid() || d->isIndexEnabled(index))) {
              QItemSelectionModel::SelectionFlags command = selectionCommand(index, 0);
              d->selectionModel->setCurrentIndex(index, command);
              d->currentIndexSet = true;
              if ((command & QItemSelectionModel::Current) == 0)
                  d->pressedPosition = visualRect(currentIndex()).center() + d->offset();
          }
      }
      

      and then when the value of d->pressedPosition is used again here:

      "QAbstractItemView::mousePressEvent : 1659"
          if (index.isValid() && d->isIndexEnabled(index)) {
              // we disable scrollTo for mouse press so the item doesn't change position
              // when the user is interacting with it (ie. clicking on it)
              bool autoScroll = d->autoScroll;
              d->autoScroll = false;
              d->selectionModel->setCurrentIndex(index, QItemSelectionModel::NoUpdate);
              d->autoScroll = autoScroll;
              QRect rect(d->pressedPosition - offset, pos);
      

      Because of the now invalid value of d->pressedPosition, a QRect is created from (0,0) to the mousePressEvent position, whereas it should be created for a 1 pixel wide value at the mouse position for a single click on an item.

      Since this rect is from the (0,0) coordinate to the mouse position, this rectangle will include all items above the one currently selected, which is the behavior of the bug.

      This fix seems to be to preventing the situation from occurring at QAbstractItemView::setCurrantIndex:1010 by making sure that the next column is populated before setting the current item as the root item of the next column.

      A patch that attempts to do this is attached, and seems to fix the issue.

      Attachments

        1. main.cpp
          0.7 kB
        2. QColumnView.patch
          0.7 kB
        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            peppe Giuseppe D'Angelo
            janichol Andy Nichols
            Votes:
            16 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes