Author: stippi Date: 2010-08-26 15:10:27 +0200 (Thu, 26 Aug 2010) New Revision: 38372 Changeset: http://dev.haiku-os.org/changeset/38372 Ticket: http://dev.haiku-os.org/ticket/3897 Modified: haiku/trunk/src/kits/interface/ColumnListView.cpp Log: Fixed the overly complicated computation of the height of the removed rows in OutlineView::RemoveRow(BRow* row). It also contained a bug (tracked down by Duggan in ticket #3897, thanks!) which caused it to skip the sub-tree height computation when FindParent() returns false, which it does for root items. Now the computation is simple: The subTreeHeight is the height of the row itself, if a) the row doesn't have a parent or b) the parent is visible and expanded. Then if the row being removed is expanded, we calculate the sub-tree height recursively. Removed a lot of duplicated or even trippled checks along the way and solved two easily solvable TODOs with regards to what is invalidated. Previously the entire list view was invalidated for each row being removed, even if they were scrolled out the view bounds. Modified: haiku/trunk/src/kits/interface/ColumnListView.cpp =================================================================== --- haiku/trunk/src/kits/interface/ColumnListView.cpp 2010-08-26 12:43:44 UTC (rev 38371) +++ haiku/trunk/src/kits/interface/ColumnListView.cpp 2010-08-26 13:10:27 UTC (rev 38372) @@ -4042,42 +4042,49 @@ BRow* parentRow; bool parentIsVisible; - float subTreeHeight = row->Height(); - if (FindParent(row, &parentRow, &parentIsVisible)) { - // adjust height - if (parentIsVisible && (parentRow == 0 || parentRow->fIsExpanded)) { - if (row->fIsExpanded) { - for (RecursiveOutlineIterator iterator(row->fChildList); - iterator.CurrentRow(); iterator.GoToNext()) - subTreeHeight += iterator.CurrentRow()->Height(); - } + FindParent(row, &parentRow, &parentIsVisible); + // NOTE: This could be a root row without a parent, in which case + // it is always visible, though. + + // Adjust height for the visible sub-tree that is going to be removed. + float subTreeHeight = 0.0f; + if (parentIsVisible && (parentRow == NULL || parentRow->fIsExpanded)) { + // The row itself is visible at least. + subTreeHeight = row->Height() + 1; + if (row->fIsExpanded) { + // Adjust for the height of visible sub-items as well. + // (By default, the iterator follows open branches only.) + for (RecursiveOutlineIterator iterator(row->fChildList); + iterator.CurrentRow(); iterator.GoToNext()) + subTreeHeight += iterator.CurrentRow()->Height() + 1; } + BRect invalid; + if (FindRect(row, &invalid)) { + invalid.bottom = Bounds().bottom; + if (invalid.IsValid()) + Invalidate(invalid); + } } + + fItemsHeight -= subTreeHeight; + + FixScrollBar(false); if (parentRow != NULL) { - if (parentRow->fIsExpanded) - fItemsHeight -= subTreeHeight + 1; - } else { - fItemsHeight -= subTreeHeight + 1; - } - FixScrollBar(false); - if (parentRow) parentRow->fChildList->RemoveItem(row); - else + if (parentRow->fChildList->CountItems() == 0) { + delete parentRow->fChildList; + parentRow->fChildList = 0; + // It was the last child row of the parent, which also means the + // latch disappears. + BRect parentRowRect; + if (parentIsVisible && FindRect(parentRow, &parentRowRect)) + Invalidate(parentRowRect); + } + } else fRows.RemoveItem(row); - if (parentRow != 0 && parentRow->fChildList->CountItems() == 0) { - delete parentRow->fChildList; - parentRow->fChildList = 0; - if (parentIsVisible) - Invalidate(); // xxx crude way of redrawing latch - } - - if (parentIsVisible && (parentRow == 0 || parentRow->fIsExpanded)) - Invalidate(); // xxx make me smarter. - - // Adjust focus row if necessary. - if (fFocusRow && FindRect(fFocusRow, &fFocusRowRect) == false) { + if (fFocusRow && !FindRect(fFocusRow, &fFocusRowRect)) { // focus row is in a subtree that is gone, move it up to the parent. fFocusRow = parentRow; if (fFocusRow)