From c56b6510268be8aff5bb1642457867ca3ce67e3f Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Thu, 8 Oct 2026 18:53:25 +0300 Subject: [PATCH] [GTK] Keep the Image of each Tree and Table cell for getImage() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getImage() mapped the cell's surface address back to an Image through the ImageList. Once the slot of a disposed Image was reused, it returned the Image of another cell, and a column created after another one was disposed could report the old column's image. TableItem and TreeItem now keep the Image of each cell in a per column array, maintained like cellFont, and getImage() returns from it. getTextBounds() and the owner draw measure and paint code ignore a disposed cell image instead of failing in getBounds(). Split from #3631. Assisted-by: multiple AI agents and layers of automated tooling 🤖 Assisted-by: Anthropic Claude Code (claude-opus-5-5) --- .../gtk/org/eclipse/swt/widgets/Table.java | 19 +++++++- .../org/eclipse/swt/widgets/TableItem.java | 22 ++++----- .../gtk/org/eclipse/swt/widgets/Tree.java | 17 ++++++- .../gtk/org/eclipse/swt/widgets/TreeItem.java | 22 ++++----- ...est_org_eclipse_swt_widgets_TableItem.java | 48 ++++++++++++++++++- ...Test_org_eclipse_swt_widgets_TreeItem.java | 48 ++++++++++++++++++- 6 files changed, 145 insertions(+), 31 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java index a5fd5dc9f16..6b77b808c0c 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java @@ -781,6 +781,14 @@ void createItem (TableColumn column, int index) { System.arraycopy (cellFont, index, temp, index+1, columnCount-index-1); item.cellFont = temp; } + Image [] cellImage = item.cellImage; + boolean keepImages = columnCount == 1 && cellImage != null && cellImage.length == columnCount; + if (cellImage != null && !keepImages) { + Image [] temp = new Image [columnCount]; + System.arraycopy (cellImage, 0, temp, 0, index); + System.arraycopy (cellImage, index, temp, index+1, columnCount-index-1); + item.cellImage = temp; + } String [] strings = item.strings; doNotModify = columnCount == 1 && strings != null && strings.length == columnCount; if (strings != null && !doNotModify) { @@ -1105,6 +1113,13 @@ void destroyItem (TableColumn column) { item.cellFont = temp; } } + Image [] cellImage = item.cellImage; + if (cellImage != null) { + Image [] temp = new Image [columnCount]; + System.arraycopy (cellImage, 0, temp, 0, index); + System.arraycopy (cellImage, index + 1, temp, index, columnCount - index); + item.cellImage = temp; + } } } if (index == 0) { @@ -2835,7 +2850,7 @@ void sendMeasureEvent (long cell, long width, long height) { GTK.gtk_cell_renderer_get_preferred_height_for_width (cell, handle, contentWidth[0], contentHeight, null); Image image = item.getImage (columnIndex); int imageWidth = 0; - if (image != null) { + if (image != null && !image.isDisposed()) { imageWidth = image.getBounds ().width; } contentWidth [0] += imageWidth; @@ -3091,7 +3106,7 @@ void rendererRender (long cell, long cr, long snapshot, long widget, long backgr ignoreSize = false; Image image = item.getImage (columnIndex); int imageWidth = 0; - if (image != null) { + if (image != null && !image.isDisposed()) { imageWidth = image.getBounds ().width; } contentX [0] -= imageWidth; diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java index b252924f7ea..d8514a837f2 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java @@ -43,6 +43,7 @@ public class TableItem extends Item { Table parent; Font font; Font[] cellFont; + Image[] cellImage; String [] strings; boolean cached, grayed, settingData; @@ -188,18 +189,7 @@ Color _getForeground (int index) { Image _getImage(int index) { int count = Math.max(1, parent.getColumnCount()); if (0 > index || index > count - 1) return null; - - long[] surfaceHandle = new long[1]; - int modelIndex = parent.columnCount == 0 ? Table.FIRST_COLUMN : parent.columns [index].modelIndex; - GTK.gtk_tree_model_get (parent.modelHandle, handle, modelIndex + Table.CELL_SURFACE, surfaceHandle, -1); - if (surfaceHandle[0] == 0) return null; - - int imageIndex = parent.imageList.indexOf(surfaceHandle[0]); - if (imageIndex == -1) { - return null; - } else { - return parent.imageList.get(imageIndex); - } + return cellImage != null ? cellImage[index] : null; } String _getText (int index) { @@ -236,6 +226,7 @@ void clear () { cached = false; font = null; cellFont = null; + cellImage = null; strings = null; } @@ -785,7 +776,7 @@ public Rectangle getTextBounds (int index) { */ Image image = _getImage(index); int imageWidth = 0; - if (image != null) { + if (image != null && !image.isDisposed()) { imageWidth = image.getBounds ().width; } if (x [0] < imageWidth) { @@ -815,6 +806,7 @@ void releaseWidget () { super.releaseWidget (); font = null; cellFont = null; + cellImage = null; strings = null; } @@ -1215,6 +1207,10 @@ public void setImage(int index, Image image) { OS.g_object_unref(pixbuf); } GTK.gtk_list_store_set (parent.modelHandle, handle, modelIndex + Table.CELL_SURFACE, surface, -1); + if (cellImage != null || image != null) { + if (cellImage == null) cellImage = new Image [count]; + cellImage [index] = image; + } cached = true; /* * Bug 465056: single column Tables have a very small initial width. 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 index 75647ac9f65..5f5a1e8bda5 100644 --- 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 @@ -969,6 +969,14 @@ void createItem (TreeColumn column, int index) { System.arraycopy (cellFont, index, temp, index+1, columnCount-index-1); item.cellFont = temp; } + Image [] cellImage = item.cellImage; + boolean keepImages = columnCount == 1 && cellImage != null && cellImage.length == columnCount; + if (cellImage != null && !keepImages) { + Image [] temp = new Image [columnCount]; + System.arraycopy (cellImage, 0, temp, 0, index); + System.arraycopy (cellImage, index, temp, index+1, columnCount-index-1); + item.cellImage = temp; + } String [] strings = item.strings; if (strings != null) { String [] temp = new String [columnCount]; @@ -1266,6 +1274,13 @@ void destroyItem (TreeColumn column) { item.cellFont = temp; } } + Image [] cellImage = item.cellImage; + if (cellImage != null) { + Image [] temp = new Image [columnCount]; + System.arraycopy (cellImage, 0, temp, 0, index); + System.arraycopy (cellImage, index + 1, temp, index, columnCount - index); + item.cellImage = temp; + } } } if (index == 0) { @@ -3372,7 +3387,7 @@ void rendererRender (long cell, long cr, long snapshot, long widget, long backgr ignoreSize = false; Image image = item.getImage (columnIndex); int imageWidth = 0; - if (image != null) { + if (image != null && !image.isDisposed()) { imageWidth = image.getBounds ().width; } // Account for the image width on GTK3, see bug 535124. 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 index bdc4de47faf..490de5719bd 100644 --- 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 @@ -43,6 +43,7 @@ public class TreeItem extends Item { Tree parent; Font font; Font[] cellFont; + Image[] cellImage; String [] strings; boolean cached, grayed, isExpanded, updated, settingData; private int cachedChildCount = -1; @@ -251,18 +252,7 @@ Color _getForeground (int index) { Image _getImage(int index) { int count = Math.max(1, parent.getColumnCount()); if (0 > index || index > count - 1) return null; - - long[] surfaceHandle = new long[1]; - int modelIndex = parent.columnCount == 0 ? Tree.FIRST_COLUMN : parent.columns[index].modelIndex; - GTK.gtk_tree_model_get (parent.modelHandle, handle, modelIndex + Tree.CELL_SURFACE, surfaceHandle, -1); - if (surfaceHandle[0] == 0) return null; - - int imageIndex = parent.imageList.indexOf(surfaceHandle[0]); - if (imageIndex == -1) { - return null; - } else { - return parent.imageList.get(imageIndex); - } + return cellImage != null ? cellImage[index] : null; } String _getText (int index) { @@ -295,6 +285,7 @@ void clear () { font = null; strings = null; cellFont = null; + cellImage = null; } /** @@ -960,7 +951,7 @@ public Rectangle getTextBounds (int index) { */ Image image = _getImage(index); int imageWidth = 0; - if (image != null) { + if (image != null && !image.isDisposed()) { imageWidth = image.getBounds ().width; } if (x [0] < imageWidth) { @@ -1046,6 +1037,7 @@ void releaseWidget () { super.releaseWidget (); font = null; cellFont = null; + cellImage = null; strings = null; } @@ -1547,6 +1539,10 @@ public void setImage(int index, Image image) { OS.g_object_unref(pixbuf); } GTK.gtk_tree_store_set(parent.modelHandle, handle, modelIndex + Tree.CELL_SURFACE, surface, -1); + if (cellImage != null || image != null) { + if (cellImage == null) cellImage = new Image [count]; + cellImage [index] = image; + } cached = true; updated = true; } diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TableItem.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TableItem.java index 7b9f6f96166..98f76c81cb5 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TableItem.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TableItem.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2025 IBM Corporation and others. + * Copyright (c) 2000, 2026 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -16,6 +16,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -685,6 +686,51 @@ public void test_setImageIndentI() { assertEquals(1, tableItem.getImageIndent()); } +@Test +public void test_setImage_newColumnDoesNotInheritImageOfDisposedColumn() { + new TableColumn(table, SWT.LEFT); + TableColumn column = new TableColumn(table, SWT.LEFT); + new TableColumn(table, SWT.LEFT); + tableItem.setImage(1, images[0]); + column.dispose(); + new TableColumn(table, SWT.LEFT); + assertNull(tableItem.getImage(2)); +} + +@Test +public void test_setImage_imagesFollowTheirColumns() { + new TableColumn(table, SWT.LEFT); + TableColumn column = new TableColumn(table, SWT.LEFT); + new TableColumn(table, SWT.LEFT); + // index 2 first, win32 copies the image of the first non-zero index into slot 0 + tableItem.setImage(2, images[1]); + tableItem.setImage(0, images[0]); + column.dispose(); + assertSame(images[0], tableItem.getImage(0)); + assertSame(images[1], tableItem.getImage(1)); + new TableColumn(table, SWT.LEFT, 0); + assertNull(tableItem.getImage(0)); + assertSame(images[0], tableItem.getImage(1)); + assertSame(images[1], tableItem.getImage(2)); +} + +@Test +public void test_getImage_disposedImageIsNotReplacedByLaterImage() { + shell.open(); + Image disposed = new Image(shell.getDisplay(), 16, 16); + tableItem.setImage(disposed); + disposed.dispose(); + new TableItem(table, SWT.NONE).setImage(images[0]); + Image later = new Image(shell.getDisplay(), 16, 16); + try { + new TableItem(table, SWT.NONE).setImage(later); + table.update(); + assertSame(disposed, tableItem.getImage()); + } finally { + later.dispose(); + } +} + @Test public void test_setText$Ljava_lang_String() { final String TestString = "test"; diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TreeItem.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TreeItem.java index e4c014d4f2e..88283bcdbad 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TreeItem.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TreeItem.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2025 IBM Corporation and others. + * Copyright (c) 2000, 2026 IBM Corporation and others. * * This program and the accompanying materials * are made available under the terms of the Eclipse Public License 2.0 @@ -17,6 +17,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -1061,6 +1062,51 @@ public void test_setImageILorg_eclipse_swt_graphics_Image() { assertThrows(IllegalArgumentException.class, () -> treeItem.setImage(0, images[0]), "No exception thrown for disposed font"); } +@Test +public void test_setImage_newColumnDoesNotInheritImageOfDisposedColumn() { + new TreeColumn(tree, SWT.LEFT); + TreeColumn column = new TreeColumn(tree, SWT.LEFT); + new TreeColumn(tree, SWT.LEFT); + treeItem.setImage(1, images[0]); + column.dispose(); + new TreeColumn(tree, SWT.LEFT); + assertNull(treeItem.getImage(2)); +} + +@Test +public void test_setImage_imagesFollowTheirColumns() { + new TreeColumn(tree, SWT.LEFT); + TreeColumn column = new TreeColumn(tree, SWT.LEFT); + new TreeColumn(tree, SWT.LEFT); + // index 2 first, win32 copies the image of the first non-zero index into slot 0 + treeItem.setImage(2, images[1]); + treeItem.setImage(0, images[0]); + column.dispose(); + assertSame(images[0], treeItem.getImage(0)); + assertSame(images[1], treeItem.getImage(1)); + new TreeColumn(tree, SWT.LEFT, 0); + assertNull(treeItem.getImage(0)); + assertSame(images[0], treeItem.getImage(1)); + assertSame(images[1], treeItem.getImage(2)); +} + +@Test +public void test_getImage_disposedImageIsNotReplacedByLaterImage() { + shell.open(); + Image disposed = new Image(shell.getDisplay(), 16, 16); + treeItem.setImage(disposed); + disposed.dispose(); + new TreeItem(tree, SWT.NONE).setImage(images[0]); + Image later = new Image(shell.getDisplay(), 16, 16); + try { + new TreeItem(tree, SWT.NONE).setImage(later); + tree.update(); + assertSame(disposed, treeItem.getImage()); + } finally { + later.dispose(); + } +} + @Test public void test_setText$Ljava_lang_String() { final String TestString = "test";