[GTK] Do not pass handles around if TreeItem is available. eclipse-pl…

…atform#882
basilevs · Jan 22, 2024 · 014dfc0 · 014dfc0
1 parent b1fe223
commit 014dfc0
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 16 deletions.
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java
@@ -1619,24 +1619,27 @@ public TreeItem getItem (int index) {
 	if (index < 0) error (SWT.ERROR_INVALID_RANGE);
 	if (index >= roots.size ()) error (SWT.ERROR_INVALID_RANGE);
 
-	return _getItem (null, 0, roots, index);
+	return _getItem (null, roots, index);
 }
 
-TreeItem _getItem(TreeItem parentItem, long parentIter, List<TreeItem> siblings, int index) {
+TreeItem _getItem(TreeItem parentItem, List<TreeItem> siblings, int index) {
+	long parentIter = parentItem == null ? 0 : parentItem.handle;
 	TreeItem result = siblings.get (index);
 	if (result != null) return result;
 
-	// TODO: shorter iteration when index - 1 exists
-
 	long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ());
 	try {
-		if (!GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, parentIter, index)) error (SWT.ERROR_INVALID_RANGE);
-		result = new TreeItem (this, parentItem, SWT.NONE, index, iter);
-		TreeItem existing = siblings.set (index, result);
-		if (existing != null) {
-			existing.dispose ();
-			throw new AssertionError("Out-of-bound item creation");
+		// Optimization: do not restart iteration if previous element is known
+		// Often happens when items are accessed sequentially
+		TreeItem previous = index > 1 ? siblings.get (index - 1) : null;
+		if (previous != null) {
+			C.memmove (iter, previous.handle, GTK.GtkTreeIter_sizeof ());
+			if (!GTK.gtk_tree_model_iter_next(handle, iter)) error (SWT.ERROR_INVALID_RANGE);
+		} else {
+			if (!GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, parentIter, index)) error (SWT.ERROR_INVALID_RANGE);
 		}
+		result = new TreeItem (this, parentItem, SWT.NONE, index, iter);
+		assert siblings.get (index) == result: "TreeItem shoud self-insert into sibling list";
 		return result;
 	} finally {
 		OS.g_free (iter);
@@ -1812,19 +1815,21 @@ int getItemHeightInPixels () {
  */
 public TreeItem [] getItems () {
 	checkWidget ();
-	return _getItems (null, roots, 0);
+	return _getItems (null, roots);
 }
 
-TreeItem [] _getItems (TreeItem parentItem, List<TreeItem> items, long parentIter) {
+TreeItem [] _getItems (TreeItem parentItem, List<TreeItem> items) {
+	long parentIter = parentItem == null ? 0 : parentItem.handle;
 	int i = 0;
+	if (items.indexOf (null) < 0) return items.toArray (new TreeItem [items.size ()]);
 	long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ());
 	if (iter == 0) error (SWT.ERROR_NO_HANDLES);
 	try {
 		boolean valid = GTK.gtk_tree_model_iter_children (modelHandle, iter, parentIter);
 		while (valid) {
 			if (items.size () <= i || items.get (i) == null) {
-				new TreeItem (this, parentItem, SWT.NONE, i, iter);
-				if (items.get (i) == null) throw new AssertionError ("Item constructor should update sibling list");
+				TreeItem inserted = new TreeItem (this, parentItem, SWT.NONE, i, iter);
+				assert items.get (i) == inserted : "TreeItem constructor should update sibling list";
 			}
 			i++;
 			valid = GTK.gtk_tree_model_iter_next (modelHandle, iter);

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java
@@ -859,7 +859,7 @@ public TreeItem getItem (int index) {
 	if (index < 0) error (SWT.ERROR_INVALID_RANGE);
 	if (index >= items.size()) error (SWT.ERROR_INVALID_RANGE);
 	if (!parent.checkData (this)) error (SWT.ERROR_WIDGET_DISPOSED);
-	return parent._getItem(this, handle, items, index);
+	return parent._getItem(this, items, index);
 }
 
 TreeItem _peekItem (int index) {
@@ -885,7 +885,7 @@ TreeItem _peekItem (int index) {
 public TreeItem [] getItems () {
 	checkWidget();
 	if (!parent.checkData (this)) error (SWT.ERROR_WIDGET_DISPOSED);
-	return parent._getItems (this, items, handle);
+	return parent._getItems (this, items);
 }
 
 @Override