patch by Rene Gollent:

* BListItems now store the top offset of the frame within the parent BListView.
* This allows binary searching the clicked item.
* ItemFrame() is now a cheap call.
* Fixed several bugs in the sorting code of BOutlineListView which lead to
  crashes of client applications.
* Implemented previously unimplemented functions in BOutlineListView.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@24092 a95241bf-73f2-0310-859d-f6bbb57e9c96

headers/os/interface/ListItem.h

@@ -18,7 +18,8 @@ class BView;
 
 class BListItem : public BArchivable {
 	public:
-							BListItem(uint32 outlineLevel = 0, bool expanded = true);
+							BListItem(uint32 outlineLevel = 0,
+								bool expanded = true);
 							BListItem(BMessage* archive);
 		virtual				~BListItem();
 
@@ -47,7 +48,7 @@ class BListItem : public BArchivable {
 
 	private:
 		friend class BOutlineListView;
-
+		friend class BListView;
 		bool				HasSubitems() const;
 
 		virtual	void		_ReservedListItem1();
@@ -58,9 +59,11 @@ class BListItem : public BArchivable {
 
 		bool				IsItemVisible() const;
 		void				SetItemVisible(bool visible);
-
+		inline float			Top() const;
+		inline float			Bottom() const;
+		void				SetTop(float top);
 	private:
-		uint32				_reserved[1];
+		float				fTop;
 		BList*				fTemporaryList;
 		float				fWidth;
 		float				fHeight;
@@ -72,6 +75,18 @@ class BListItem : public BArchivable {
 		bool				fVisible : 1;
 };
 
+inline float
+BListItem::Top(void) const 
+{
+	return fTop;
+}
+
+inline float
+BListItem::Bottom(void) const
+{
+	return (fTop + ceilf(fHeight) - 1.0);
+}
+
 #include <StringItem.h>
 	// to maintain source compatibility
 

headers/os/interface/ListView.h

@@ -43,8 +43,8 @@ class BListView : public BView, public BInvoker {
 		virtual void		MakeFocus(bool state = true);
 		virtual void		FrameResized(float newWidth, float newHeight);
 		virtual void		TargetedByScrollView(BScrollView* scroller);
-				void		ScrollTo(float x, float y);
 		virtual void		ScrollTo(BPoint where);
+		inline	void		ScrollTo(float x, float y);
 		virtual bool		AddItem(BListItem* item);
 		virtual bool		AddItem(BListItem* item, int32 atIndex);
 		virtual bool		AddList(BList* newItems);
@@ -136,7 +136,6 @@ class BListView : public BView, public BInvoker {
 		};
 
 		virtual bool		DoMiscellaneous(MiscCode code, MiscData *data);
-
 	private:
 		friend class BOutlineListView;
 
@@ -161,6 +160,7 @@ class BListView : public BView, public BInvoker {
 				bool		_TryInitiateDrag(BPoint where);
 				int32		_CalcFirstSelected(int32 after);
 				int32		_CalcLastSelected(int32 before);
+				void		_RecalcItemTops(int32 start);
 		virtual void		DrawItem(BListItem* item, BRect itemRect,
 								bool complete = false);
 	
@@ -174,12 +174,11 @@ class BListView : public BView, public BInvoker {
 		int32				fFirstSelected;
 		int32				fLastSelected;
 		int32				fAnchorIndex;
-		float				fWidth;
 		BMessage*			fSelectMessage;
 		BScrollView*		fScrollView;
 		track_data*			fTrack;
 
-		uint32				_reserved[3];
+		uint32				_reserved[4];
 };
 
 inline void

src/kits/interface/ListItem.cpp

@@ -1,5 +1,5 @@
 //------------------------------------------------------------------------------
-//	Copyright (c) 2001-2005, Haiku, Inc.
+//	Copyright (c) 2001-2008, Haiku, Inc.
 //
 //	Permission is hereby granted, free of charge, to any person obtaining a
 //	copy of this software and associated documentation files (the "Software"),
@@ -21,7 +21,8 @@
 //
 //	File Name:		ListItem.cpp
 //	Author:			Ulrich Wimboeck
-//					Marc Flerackers ([email protected])
+//				Marc Flerackers ([email protected])
+//				Rene Gollent
 //	Description:	BListItem is the base class for BListView's items,
 //					BStringItem is a subclass of BListItem which draws a string.
 //------------------------------------------------------------------------------
@@ -34,7 +35,8 @@
 
 
 BListItem::BListItem(uint32 level, bool expanded)
-	:	fWidth(0),
+	:	fTop(0.0),
+		fWidth(0),
 		fHeight(0),
 		fLevel(level),
 		fSelected(false),
@@ -48,6 +50,7 @@ BListItem::BListItem(uint32 level, bool expanded)
 
 BListItem::BListItem(BMessage *data)
 	:	BArchivable(data),
+		fTop(0.0),
 		fWidth(0),
 		fHeight(0),
 		fLevel(0),
@@ -226,6 +229,11 @@ BListItem::IsItemVisible() const
 	return fVisible;
 }
 
+void
+BListItem::SetTop(float top)
+{
+	fTop = top;
+}
 
 void
 BListItem::SetItemVisible(bool visible)

src/kits/interface/ListView.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001-2007, Haiku, Inc.
+ * Copyright (c) 2001-2008, Haiku, Inc.
  * Distributed under the terms of the MIT license.
  *
  * Authors:	
@@ -7,6 +7,7 @@
  *		Marc Flerackers ([email protected])
  *		Stephan Assmus <[email protected]>
  *		Axel Dörfler, [email protected]
+ *		Rene Gollent ([email protected])
  */
 
 
@@ -102,8 +103,6 @@ BListView::BListView(BMessage* archive)
 	fScrollView = NULL;
 	fTrack = NULL;
 
-	fWidth = Bounds().Width();
-	
 	int32 i = 0;
 	BMessage subData;
 	
@@ -464,7 +463,6 @@ BListView::MakeFocus(bool focused)
 void
 BListView::FrameResized(float width, float height)
 {
-	fWidth = Bounds().right;
 	_FixupScrollBar();
 }
 
@@ -475,15 +473,13 @@ BListView::TargetedByScrollView(BScrollView *view)
 	fScrollView = view;
 }
 
-
+// ScrollTo
 void
 BListView::ScrollTo(BPoint point)
 {
 	BView::ScrollTo(point);
-	fWidth = Bounds().right;
 }
 
-
 bool
 BListView::AddItem(BListItem *item, int32 index)
 {
@@ -502,6 +498,8 @@ BListView::AddItem(BListItem *item, int32 index)
 
 		item->Update(this, &font);
 
+		_RecalcItemTops(index);	
+		
 		_FixupScrollBar();
 		_InvalidateFrom(index);
 	}
@@ -517,13 +515,15 @@ BListView::AddItem(BListItem* item)
 		return false;
 
 	// No need to adapt selection, as this item is the last in the list
-
+	
 	if (Window()) {
 		BFont font;
 		GetFont(&font);
 
 		item->Update(this, &font);
 
+		_RecalcItemTops(CountItems() - 1);	
+
 		_FixupScrollBar();
 		InvalidateItem(CountItems() - 1);
 	}
@@ -549,12 +549,11 @@ BListView::AddList(BList* list, int32 index)
 	if (Window()) {
 		BFont font;
 		GetFont(&font);
-
+		
-		int32 i = 0;
+		for (int32 i = index; i <= (index + list->CountItems() - 1); i++) 
-		while(BListItem *item = (BListItem*)list->ItemAt(i)) {
+			ItemAt(i)->Update(this, &font);
-			item->Update(this, &font);
+		
-			i++;
+		_RecalcItemTops(index);
-		}
 		
 		_FixupScrollBar();
 		Invalidate(); // TODO
@@ -583,13 +582,15 @@ BListView::RemoveItem(int32 index)
 
 	if (!fList.RemoveItem(item))
 		return NULL;
-
+	
 	if (fFirstSelected != -1 && index < fFirstSelected)
 		fFirstSelected--;
 
 	if (fLastSelected != -1 && index < fLastSelected)
 		fLastSelected--;
-
+	
+	_RecalcItemTops(index);
+	
 	_InvalidateFrom(index);
 	_FixupScrollBar();
 
@@ -700,16 +701,24 @@ BListView::IndexOf(BListItem *item) const
 int32
 BListView::IndexOf(BPoint point) const
 {
-	float y = 0.0f;
+	int32 low = 0;
-
+	int32 high = fList.CountItems() - 1;
-	// TODO: somehow binary search, but items don't know their frame
+	int32 mid = -1;
-	for (int i = 0; i < fList.CountItems(); i++) {
+	float frameTop = -1.0;
-		y += ceilf(ItemAt(i)->Height());
+	float frameBottom = 1.0;
-
+	// binary search the list
-		if (point.y < y)
+	while (high >= low) {
-			return i;
+		mid = (low + high) / 2;
-	}
+		frameTop = ItemAt(mid)->Top();
-
+		frameBottom = ItemAt(mid)->Bottom();
+		if (point.y < frameTop) 
+			high = mid - 1;
+		else if (point.y > frameBottom) 
+			low = mid + 1;
+		else
+			return mid;
+	} 
+	
 	return -1;
 }
 
@@ -955,6 +964,7 @@ BListView::SortItems(int (*cmp)(const void *, const void *))
 	}
 
 	fList.SortItems(cmp);
+	_RecalcItemTops(0);
 	Invalidate();
 }
 
@@ -1003,7 +1013,7 @@ BListView::AttachedToWindow()
 
 	if (!Messenger().IsValid())
 		SetTarget(Window(), NULL);
-
+	
 	_FixupScrollBar();
 }
 
@@ -1018,17 +1028,16 @@ BListView::FrameMoved(BPoint new_position)
 BRect
 BListView::ItemFrame(int32 index)
 {
-	BRect frame(Bounds().left, 0, Bounds().right, -1);
+	BRect frame = Bounds();
-
+	if (index < 0 || index >= CountItems()) {
-	if (index < 0 || index >= CountItems())
+		frame.top = 0;
-		return frame;
+		frame.bottom = -1;
-
+	} else {
-	// TODO: this is very expensive, the (last) offsets could be cached
+		BListItem* item = ItemAt(index);
-	for (int32 i = 0; i <= index; i++) {
+		frame.left = 0.0;
-		frame.top = frame.bottom + 1;
+		frame.top = item->Top();
-		frame.bottom = frame.top + ceilf(ItemAt(i)->Height()) - 1;
+		frame.bottom = item->Bottom();
 	}
-
 	return frame;
 }
 
@@ -1104,7 +1113,21 @@ BListView::ResizeToPreferred()
 void
 BListView::GetPreferredSize(float *width, float *height)
 {
-	BView::GetPreferredSize(width, height);
+	int32 count = CountItems();
+
+	if (count > 0) {
+		float maxWidth = 0.0; 
+		for (int32 i = 0; i < count; i++) {
+			float itemWidth = ItemAt(i)->Width();
+			if (itemWidth > maxWidth)
+				maxWidth = itemWidth; 
+		}
+	
+		*width = maxWidth;
+		*height = ItemAt(count - 1)->Bottom();
+	} else {
+		BView::GetPreferredSize(width, height);
+	}
 }
 
 // AllAttached
@@ -1187,7 +1210,6 @@ BListView::_InitObject(list_view_type type)
 	fFirstSelected = -1;
 	fLastSelected = -1;
 	fAnchorIndex = -1;
-	fWidth = Bounds().Width();
 	fSelectMessage = NULL;
 	fScrollView = NULL;
 	fTrack = new track_data;
@@ -1206,10 +1228,11 @@ BListView::_FixupScrollBar()
 
 	BRect bounds = Bounds();
 	int32 count = CountItems();
+	
+	float itemHeight = 0.0;
 
-	float itemHeight = 0;
+	if (CountItems() > 0)
-	for (int32 i = 0; i < count; i++)
+		itemHeight = ItemAt(CountItems() - 1)->Bottom();
-		itemHeight += ceilf(ItemAt(i)->Height());
 
 	if (bounds.Height() > itemHeight) {
 		// no scrolling
@@ -1254,10 +1277,10 @@ BListView::_FontChanged()
 {
 	BFont font;
 	GetFont(&font);
-
+	for (int32 i = 0; i < CountItems(); i++) 
-	for (int i = 0; i < CountItems (); i ++)
 		ItemAt(i)->Update(this, &font);
-}
+	_RecalcItemTops(0);
+}	
 
 
 /*!
@@ -1504,7 +1527,7 @@ BListView::_SwapItems(int32 a, int32 b)
 		fAnchorIndex = b;
 	else if (fAnchorIndex == b)
 		fAnchorIndex = a;
-
+	
 	// track selection
 	// NOTE: this is only important if the selection status
 	// of both items is not the same
@@ -1525,6 +1548,8 @@ BListView::_SwapItems(int32 a, int32 b)
 	if (Window()) {
 		// NOTE: window looper is assumed to be locked!
 		if (aFrame.Height() != bFrame.Height()) {
+			ItemAt(a)->SetTop(bFrame.top);
+			ItemAt(b)->SetTop(aFrame.top);
 			// items in between shifted visually
 			Invalidate(aFrame | bFrame);
 		} else {
@@ -1558,7 +1583,9 @@ BListView::_MoveItem(int32 from, int32 to)
 		// same, the selection has still changed
 		SelectionChanged();
 	}
-
+	
+	_RecalcItemTops((to > from) ? from : to);
+	
 	// take care of invalidation
 	if (Window()) {
 		// NOTE: window looper is assumed to be locked!
@@ -1594,6 +1621,7 @@ BListView::_ReplaceItem(int32 index, BListItem *item)
 		_RescanSelection(start, end);
 		SelectionChanged();
 	}
+	_RecalcItemTops(index);
 
 	bool itemHeightChanged = frame != ItemFrame(index);
 
@@ -1647,4 +1675,18 @@ BListView::_RescanSelection(int32 from, int32 to)
 	}
 }
 
+void
+BListView::_RecalcItemTops(int32 start)
+{
+	int32 count = CountItems();
+	if ((start < 0) || (start >= count))
+		return;
+	float top = (start == 0) ? 0.0 : ItemAt(start - 1)->Bottom() + 1.0;
+
+	for (int32 i = start; i < CountItems(); i++) {
+		BListItem *item = ItemAt(i);
+		item->SetTop(top);
+		top += ceilf(item->Height());
+	}
+}
 

src/kits/interface/OutlineListView.cpp

@@ -1,15 +1,17 @@
 /*
- * Copyright 2001-2007, Haiku Inc.
+ * Copyright 2001-2008, Haiku Inc.
  * Distributed under the terms of the MIT License.
  *
  * Authors:
  *		Marc Flerackers ([email protected])
  *		Axel Dörfler, [email protected]
+ *		Rene Gollent ([email protected])
  */
 
 //! BOutlineListView represents a "nestable" list view.
 
 #include <OutlineListView.h>
+#include <Window.h>
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -35,14 +37,14 @@ quick_sort_item_array(BListItem** items, int32 first, int32 last,
 	int32 right = last;
 
 	do {
-		while (compareFunc(items[left], pivot) < 0) {
+		while ((compareFunc(items[left], pivot) < 0) && (left < last)) 
 			left++;
-		}
+		
-		while (compareFunc(items[right], pivot) > 0) {
+		while ((compareFunc(items[right], pivot) > 0) && (right > first)) 
 			right--;
-		}
+		
 
-		if (left <= right) {
+		if (left < right) {
 			// swap entries
 			BListItem* temp = items[left];
 			items[left] = items[right];
@@ -51,11 +53,14 @@ quick_sort_item_array(BListItem** items, int32 first, int32 last,
 			left++;
 			right--;
 		}
-	} while (left <= right);
+	} while (left < right);
+
+	// if our subset of the array consists of two elements, then we're done recursing
+	if (last - first <= 1)
+		return;
 
 	// At this point, the elements in the left half are all smaller
 	// than the elements in the right half
-
 	if (first < right) {
 		// sort left half
 		quick_sort_item_array(items, first, right, compareFunc);
@@ -181,7 +186,7 @@ BOutlineListView::AddUnder(BListItem* item, BListItem* superitem)
 {
 	if (superitem == NULL)
 		return AddItem(item);
-
+	
 	fFullList.AddItem(item, FullListIndexOf(superitem) + 1);
 
 	item->fLevel = superitem->OutlineLevel() + 1;
@@ -254,16 +259,20 @@ BOutlineListView::AddItem(BListItem* item, int32 fullListIndex)
 bool
 BOutlineListView::AddList(BList* newItems)
 {
-	printf("BOutlineListView::AddList Not implemented\n");
+	return AddList(newItems, FullListCountItems());
-	return false;
 }
 
 
 bool
 BOutlineListView::AddList(BList* newItems, int32 fullListIndex)
 {
-	printf("BOutlineListView::AddList Not implemented\n");
+	if ((newItems == NULL) || (newItems->CountItems() == 0))
-	return false;
+		return false;
+
+	for (int32 i = 0; i < newItems->CountItems(); i++)
+		AddItem((BListItem *)newItems->ItemAt(i), fullListIndex + i);
+
+	return true;
 }
 
 
@@ -308,7 +317,12 @@ BOutlineListView::FullListItemAt(int32 fullListIndex) const
 int32
 BOutlineListView::FullListIndexOf(BPoint point) const
 {
-	return BListView::IndexOf(point);
+	int32 index = BListView::IndexOf(point);
+	
+	if (index > 0)
+		index = FullListIndex(index);
+
+	return index;
 }
 
 
@@ -386,7 +400,7 @@ void
 BOutlineListView::FullListDoForEach(bool (*func)(BListItem* item, void* arg),
 	void* arg)
 {
-	fList.DoForEach(reinterpret_cast<bool (*)(void*, void*)>(func), arg);
+	fFullList.DoForEach(reinterpret_cast<bool (*)(void*, void*)>(func), arg);
 }
 
 
@@ -412,12 +426,12 @@ BOutlineListView::Expand(BListItem* item)
 	uint32 level = item->fLevel;
 	int32 fullIndex = FullListIndexOf(item);
 	int32 index = IndexOf(item) + 1;
+	int32 startIndex = index;
 	int32 count = FullListCountItems() - fullIndex - 1;
 	BListItem** items = (BListItem**)fFullList.Items() + fullIndex + 1;
 
 	BFont font;
 	GetFont(&font);
-
 	while (count-- > 0) {
 		item = items[0];
 		if (item->fLevel <= level)
@@ -445,7 +459,7 @@ BOutlineListView::Expand(BListItem* item)
 		} else
 			items++;
 	}
-
+	_RecalcItemTops(startIndex);
 	_FixupScrollBar();
 	Invalidate();
 }
@@ -462,6 +476,7 @@ BOutlineListView::Collapse(BListItem* item)
 	uint32 level = item->fLevel;
 	int32 fullIndex = FullListIndexOf(item);
 	int32 index = IndexOf(item);
+	int32 startIndex = index;
 	int32 max = FullListCountItems() - fullIndex - 1;
 	int32 count = 0;
 	BListItem** items = (BListItem**)fFullList.Items() + fullIndex + 1;
@@ -482,7 +497,8 @@ BOutlineListView::Collapse(BListItem* item)
 
 		items++;
 	}
-
+	
+	_RecalcItemTops(startIndex);
 	// fix selection hints
 	// TODO: revise for multi selection lists
 	if (index < fFirstSelected) {
@@ -496,7 +512,7 @@ BOutlineListView::Collapse(BListItem* item)
 			Select(index);
 		}
 	}
-
+	
 	_FixupScrollBar();
 	Invalidate();
 }
@@ -615,8 +631,9 @@ BOutlineListView::SortItemsUnder(BListItem* underItem, bool oneLevelOnly,
 		}
 
 		// only invalidate what may have changed
+		_RecalcItemTops(firstIndex);
 		BRect top = ItemFrame(firstIndex);
-		BRect bottom = ItemFrame(lastIndex);
+		BRect bottom = ItemFrame(lastIndex - 1);
 		BRect update(top.left, top.top, bottom.right, bottom.bottom);
 		Invalidate(update);
 	}
@@ -798,6 +815,12 @@ BOutlineListView::ItemUnderAt(BListItem* underItem,
 bool
 BOutlineListView::DoMiscellaneous(MiscCode code, MiscData* data)
 {
+	if (code == B_SWAP_OP) {
+		// todo: If we do a swap and the items in question have children, we need
+		// to move the child hierarchy together with the item if we want to correctly
+		// mimic R5 behavior.
+	} 
+		
 	return BListView::DoMiscellaneous(code, data);
 }
 
@@ -915,29 +938,24 @@ BOutlineListView::_RemoveItem(BListItem* item, int32 fullIndex)
 	uint32 level = item->OutlineLevel();
 	int32 superIndex;
 	BListItem* super = _SuperitemForIndex(fullIndex, level, &superIndex);
-
+	
 	if (item->IsItemVisible()) {
 		// remove children, too
-		int32 max = FullListCountItems() - fullIndex - 1;
+		while (fullIndex + 1 < CountItems()) {
-		BListItem** items = (BListItem**)fFullList.Items() + fullIndex + 1;
+			BListItem *subItem = ItemAt(fullIndex + 1);
-
+			
-		while (max-- > 0) {
+			if (subItem->OutlineLevel() <= level)
-			BListItem* subItem = items[0];
-			if (subItem->fLevel <= level)
 				break;
 
 			if (subItem->IsItemVisible())
 				BListView::RemoveItem(subItem);
-
+			
 			fFullList.RemoveItem(fullIndex + 1);
-
-			// TODO: this might be problematic, see comment above
 			delete subItem;
 		}
-
 		BListView::RemoveItem(item);
 	}
-
+	
 	fFullList.RemoveItem(fullIndex);
 
 	if (super != NULL) {
@@ -946,7 +964,6 @@ BOutlineListView::_RemoveItem(BListItem* item, int32 fullIndex)
 		if (child == NULL || child->OutlineLevel() <= super->OutlineLevel())
 			super->fHasSubitems = false;
 	}
-
 	return item;
 }