From 064e80fd64d7e84d5115c25b46688139d00cafe1 Mon Sep 17 00:00:00 2001 From: om-11-2024 Date: Tue, 18 Feb 2025 18:16:20 +0500 Subject: [PATCH] Fix for image display problems in Tree and Table #1739 Fixes image display problems (cropping, not showing) for Tree and Table (both normal and virtual variants). Also fixes #678 Fixes #678 Fixes #1739 --- .../Eclipse SWT PI/gtk/library/gtk3.c | 10 + .../Eclipse SWT PI/gtk/library/gtk3_stats.h | 1 + .../org/eclipse/swt/internal/gtk3/GTK3.java | 4 + .../gtk/org/eclipse/swt/widgets/Table.java | 163 ++++++++++++++-- .../org/eclipse/swt/widgets/TableColumn.java | 4 +- .../org/eclipse/swt/widgets/TableItem.java | 38 +--- .../gtk/org/eclipse/swt/widgets/Tree.java | 182 ++++++++++++++---- .../org/eclipse/swt/widgets/TreeColumn.java | 4 +- .../gtk/org/eclipse/swt/widgets/TreeItem.java | 55 +----- .../Issue678_JvmCrashOnTreeItemSetImage.java | 98 ++++++++++ 10 files changed, 424 insertions(+), 135 deletions(-) create mode 100644 tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Issue678_JvmCrashOnTreeItemSetImage.java diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk3.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk3.c index bf5b9cb7af1..2df02d8fce2 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk3.c +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk3.c @@ -2048,6 +2048,16 @@ JNIEXPORT void JNICALL GTK3_NATIVE(gtk_1tree_1view_1column_1cell_1get_1size) } #endif +#ifndef NO_gtk_1tree_1view_1column_1queue_1resize +JNIEXPORT void JNICALL GTK3_NATIVE(gtk_1tree_1view_1column_1queue_1resize) + (JNIEnv *env, jclass that, jlong arg0) +{ + GTK3_NATIVE_ENTER(env, that, gtk_1tree_1view_1column_1queue_1resize_FUNC); + gtk_tree_view_column_queue_resize((GtkTreeViewColumn *)arg0); + GTK3_NATIVE_EXIT(env, that, gtk_1tree_1view_1column_1queue_1resize_FUNC); +} +#endif + #ifndef NO_gtk_1tree_1view_1get_1bin_1window JNIEXPORT jlong JNICALL GTK3_NATIVE(gtk_1tree_1view_1get_1bin_1window) (JNIEnv *env, jclass that, jlong arg0) diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk3_stats.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk3_stats.h index 9eed9c0cd17..ca739c0377b 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk3_stats.h +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk3_stats.h @@ -191,6 +191,7 @@ typedef enum { gtk_1toolbar_1set_1show_1arrow_FUNC, gtk_1toolbar_1set_1style_FUNC, gtk_1tree_1view_1column_1cell_1get_1size_FUNC, + gtk_1tree_1view_1column_1queue_1resize_FUNC, gtk_1tree_1view_1get_1bin_1window_FUNC, gtk_1viewport_1set_1shadow_1type_FUNC, gtk_1widget_1add_1accelerator_FUNC, diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk3/GTK3.java b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk3/GTK3.java index 5e5c9e278aa..fe523331932 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk3/GTK3.java +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk3/GTK3.java @@ -1002,6 +1002,10 @@ public class GTK3 { * @param height cast=(gint *) */ public static final native void gtk_tree_view_column_cell_get_size(long tree_column, GdkRectangle cell_area, int[] x_offset, int[] y_offset, int[] width, int[] height); + /** + * @param tree_column cast=(GtkTreeViewColumn *) + */ + public static final native void gtk_tree_view_column_queue_resize(long tree_column); /* GdkWindow */ /** 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 d2d99021f65..3f0f924c5f5 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 @@ -15,6 +15,9 @@ package org.eclipse.swt.widgets; +import java.util.*; +import java.util.function.*; + import org.eclipse.swt.*; import org.eclipse.swt.events.*; import org.eclipse.swt.graphics.*; @@ -98,6 +101,7 @@ public class Table extends Composite { int headerHeight; boolean boundsChangedSinceLastDraw, headerVisible, wasScrolled; boolean rowActivated; + boolean showImagesForDefaultColumn; private long headerCSSProvider; @@ -232,7 +236,7 @@ long cellDataProc (long tree_column, long cell, long tree_model, long iter, long ptr [0] = 0; if (isPixbuf) { GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_PIXBUF, ptr, -1); - OS.g_object_set (cell, OS.gicon, ptr [0], 0); + OS.g_object_set (cell, OS.pixbuf, ptr [0], 0); if (ptr [0] != 0) OS.g_object_unref (ptr [0]); } else { GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_TEXT, ptr, -1); @@ -623,9 +627,10 @@ void createColumn (TableColumn column, int index) { if (columnHandle == 0) error (SWT.ERROR_NO_HANDLES); if (index == 0 && columnCount > 0) { TableColumn checkColumn = columns [0]; - createRenderers (checkColumn.handle, checkColumn.modelIndex, false, checkColumn.style); + createRenderers (checkColumn.handle, checkColumn.modelIndex, false, checkColumn.showImages, checkColumn.style); } - createRenderers (columnHandle, modelIndex, index == 0, column == null ? 0 : column.style); + createRenderers (columnHandle, modelIndex, index == 0, + column == null ? showImagesForDefaultColumn : column.showImages, column == null ? 0 : column.style); if ((style & SWT.VIRTUAL) == 0 && columnCount == 0) { GTK.gtk_tree_view_column_set_sizing (columnHandle, GTK.GTK_TREE_VIEW_COLUMN_GROW_ONLY); } else { @@ -717,9 +722,11 @@ void createItem (TableColumn column, int index) { GTK.gtk_tree_view_column_set_sizing (column.handle, GTK.GTK_TREE_VIEW_COLUMN_FIXED); GTK.gtk_tree_view_column_set_visible (column.handle, false); column.modelIndex = FIRST_COLUMN; - createRenderers (column.handle, column.modelIndex, true, column.style); + createRenderers (column.handle, column.modelIndex, true, showImagesForDefaultColumn, column.style); column.customDraw = firstCustomDraw; firstCustomDraw = false; + column.showImages = showImagesForDefaultColumn; + showImagesForDefaultColumn = false; } else { createColumn (column, index); } @@ -826,7 +833,7 @@ void createItem (TableItem item, int index) { items [index] = item; } -void createRenderers (long columnHandle, int modelIndex, boolean check, int columnStyle) { +void createRenderers (long columnHandle, int modelIndex, boolean check, boolean showImages, int columnStyle) { GTK.gtk_tree_view_column_clear (columnHandle); if ((style & SWT.CHECK) != 0 && check) { GTK.gtk_tree_view_column_pack_start (columnHandle, checkRenderer, false); @@ -842,13 +849,14 @@ void createRenderers (long columnHandle, int modelIndex, boolean check, int colu long pixbufRenderer = ownerDraw ? OS.g_object_new (display.gtk_cell_renderer_pixbuf_get_type (), 0) : GTK.gtk_cell_renderer_pixbuf_new (); if (pixbufRenderer == 0) { error (SWT.ERROR_NO_HANDLES); - } else { - // set default size this size is used for calculating the icon and text positions in a table - if (!ownerDraw) { - // Set render size to 0x0 until we actually add images, fix for - // Bug 457196 (this applies to Tables as well). - GTK.gtk_cell_renderer_set_fixed_size(pixbufRenderer, 0, 0); + } + if (!ownerDraw) { + int width = (pixbufSizeSet && showImages) ? pixbufWidth : 1; + int height = -1; + if ((style & SWT.VIRTUAL) != 0) { + height = pixbufSizeSet ? pixbufHeight : 0; } + GTK.gtk_cell_renderer_set_fixed_size(pixbufRenderer, width, height); } long textRenderer = ownerDraw ? OS.g_object_new (display.gtk_cell_renderer_text_get_type (), 0) : GTK.gtk_cell_renderer_text_new (); if (textRenderer == 0) error (SWT.ERROR_NO_HANDLES); @@ -1055,6 +1063,7 @@ void destroyItem (TableColumn column) { long columnHandle = column.handle; if (columnCount == 1) { firstCustomDraw = column.customDraw; + showImagesForDefaultColumn = column.showImages; } System.arraycopy (columns, index + 1, columns, index, --columnCount - index); columns [columnCount] = null; @@ -1104,7 +1113,8 @@ void destroyItem (TableColumn column) { } if (index == 0) { TableColumn checkColumn = columns [0]; - createRenderers (checkColumn.handle, checkColumn.modelIndex, true, checkColumn.style); + createRenderers (checkColumn.handle, checkColumn.modelIndex, true, checkColumn.showImages, + checkColumn.style); } } if (!searchEnabled ()) { @@ -2572,11 +2582,12 @@ void recreateRenderers () { OS.g_signal_connect_closure (checkRenderer, OS.toggled, display.getClosure (TOGGLED), false); } if (columnCount == 0) { - createRenderers (GTK.gtk_tree_view_get_column (handle, 0), Table.FIRST_COLUMN, true, 0); + createRenderers (GTK.gtk_tree_view_get_column (handle, 0), Table.FIRST_COLUMN, true, showImagesForDefaultColumn, + 0); } else { for (int i = 0; i < columnCount; i++) { TableColumn column = columns [i]; - createRenderers (column.handle, column.modelIndex, i == 0, column.style); + createRenderers (column.handle, column.modelIndex, i == 0, column.showImages, column.style); } } } @@ -4240,6 +4251,130 @@ void checkSetDataInProcessBeforeRemoval(int start, int end) { } } +boolean initPixbufSize (Image image) { + if (pixbufSizeSet || image == null) { + return false; + } + int iWidth, iHeight; + if (DPIUtil.useCairoAutoScale ()) { + iWidth = image.getBounds ().width; + iHeight = image.getBounds ().height; + } else { + iWidth = image.getBoundsInPixels ().width; + iHeight = image.getBoundsInPixels ().height; + } + if (iWidth <= 0 || iHeight <= 0) { + return false; + } + pixbufSizeSet = true; + pixbufHeight = iHeight; + pixbufWidth = iWidth; + /* + * Feature in GTK: a Table with the style SWT.VIRTUAL has fixed-height-mode + * enabled. This will limit the size of any cells, including renderers. In order + * to prevent images from disappearing/being cropped, we update row heights when + * the first image is set. Fix for bug 480261. + */ + if ((style & SWT.VIRTUAL) != 0) { + initFixedRowHeight (); + } + return true; +} + +private void initFixedRowHeight () { + long columnList = GTK.gtk_tree_view_get_columns (handle); + if (columnList == 0) { + return; + } + + // set fixed height for all GtkCellRendererPixbufs + for (var colItem = columnList; colItem != 0; colItem = OS.g_list_next (colItem)) { + long column = OS.g_list_data (colItem); + long renderer = getPixbufRenderer (column); + if (renderer != 0) { + GTK.gtk_cell_renderer_set_fixed_size (renderer, 1, pixbufHeight); + } + } + OS.g_list_free (columnList); + + /* + * Change fixed height of existing rows to fit new pixbuf height. + * + * Wrapped in asyncExec because when this method is invoked from 'checkData()', + * the 'row_changed' signal is blocked (but we need it unblocked to invalidate + * existing rows). + */ + display.asyncExec ( () -> { + if (!isDisposed () && modelHandle != 0) { + /* + * Reset computed 'fixed_height' to '-1' + * + * Note: current GTK3 (3.24.41) doesn't support setting 'fixed_height_mode' to + * 'true' multiple times: each call appends additional "notify::sizing" + * callbacks to columns and doesn't remove old ones. + * + * It's fine to use here, because this method is executed once per tree. + */ + OS.g_object_set (handle, OS.fixed_height_mode, false, 0); + OS.g_object_set (handle, OS.fixed_height_mode, true, 0); + + // to update height of the existing rows we need to invalidate them in gtk + // we do that by invoking 'gtk_tree_view_row_changed' on each of row + int[] value = new int[1]; + forEachChildRow (modelHandle, 0, iter -> { + GTK.gtk_tree_model_get (modelHandle, iter, CHECKED_COLUMN, value, -1); + GTK.gtk_list_store_set (modelHandle, iter, CHECKED_COLUMN, value[0], -1); + }); + } + }); +} + +private static void forEachChildRow (long modelHandle, long parentIter, LongConsumer doWithIter) { + long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); + if (GTK.gtk_tree_model_iter_children (modelHandle, iter, parentIter)) { + do { + doWithIter.accept (iter); + forEachChildRow (modelHandle, iter, doWithIter); + } while (GTK.gtk_tree_model_iter_next (modelHandle, iter)); + } + OS.g_free (iter); +} + +boolean isShowingImagesForColumn (int columnIdx) { + if (columnCount == 0) { + Objects.checkIndex (columnIdx, 1); + return showImagesForDefaultColumn; + } else { + Objects.checkIndex (columnIdx, columnCount); + return columns[columnIdx].showImages; + } +} + +boolean showImagesForColumn (int columnIdx) { + Objects.checkIndex (columnIdx, Math.max (1, columnCount)); + if (pixbufSizeSet && !isShowingImagesForColumn (columnIdx)) { + long col = GTK.gtk_tree_view_get_column (handle, columnIdx); + if (col != 0) { + long pixbufRenderer = getPixbufRenderer (col); + if (pixbufRenderer != 0) { + int height = ((style & SWT.VIRTUAL) != 0) ? pixbufHeight : -1; + GTK.gtk_cell_renderer_set_fixed_size (pixbufRenderer, pixbufWidth, height); + if (columnCount == 0) {; + showImagesForDefaultColumn = true; + } else { + columns[columnIdx].showImages = true; + } + if (!GTK.GTK4) { + // update column renderers layout to add space for image width + GTK3.gtk_tree_view_column_queue_resize (col); + } + return true; + } + } + } + return false; +} + @Override public void dispose() { super.dispose(); diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableColumn.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableColumn.java index d44c0037a3b..3deee2b018f 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableColumn.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableColumn.java @@ -45,7 +45,7 @@ public class TableColumn extends Item { long labelHandle, imageHandle, buttonHandle; Table parent; int modelIndex, lastButton, lastTime, lastX, lastWidth; - boolean customDraw, useFixedWidth; + boolean customDraw, showImages, useFixedWidth; String toolTipText; /** @@ -606,7 +606,7 @@ public void setAlignment (int alignment) { if (index == -1 || index == 0) return; style &= ~(SWT.LEFT | SWT.RIGHT | SWT.CENTER); style |= alignment & (SWT.LEFT | SWT.RIGHT | SWT.CENTER); - parent.createRenderers (handle, modelIndex, index == 0, style); + parent.createRenderers (handle, modelIndex, index == 0, showImages, style); } void setFontDescription (long font) { 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 92af154acff..c8634cfe83c 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 @@ -1201,40 +1201,12 @@ public void setImage(int index, Image image) { } surface = imageList.getSurface(imageIndex); pixbuf = ImageList.createPixbuf(surface); - } - long parentHandle = parent.handle; - long column = GTK.gtk_tree_view_get_column (parentHandle, index); - long pixbufRenderer = parent.getPixbufRenderer (column); - int [] currentWidth = new int [1]; - int [] currentHeight= new int [1]; - GTK.gtk_cell_renderer_get_fixed_size (pixbufRenderer, currentWidth, currentHeight); - if (!parent.pixbufSizeSet) { - if (image != null) { - int iWidth, iHeight; - if (DPIUtil.useCairoAutoScale()) { - iWidth = image.getBounds ().width; - iHeight = image.getBounds ().height; - } else { - iWidth = image.getBoundsInPixels ().width; - iHeight = image.getBoundsInPixels ().height; - } - if (iWidth > currentWidth [0] || iHeight > currentHeight [0]) { - GTK.gtk_cell_renderer_set_fixed_size (pixbufRenderer, iWidth, iHeight); - parent.pixbufHeight = iHeight; - parent.pixbufWidth = iWidth; - parent.pixbufSizeSet = true; - } + if (!parent.pixbufSizeSet) { + parent.initPixbufSize (image); } - } else { - /* - * We check to see if the cached value is greater than the size of the pixbufRenderer. - * If it is, then we change the size of the pixbufRenderer accordingly. - * Bug 489025: There is a corner case where the below is triggered when current(Width|Height) is -1, - * which results in icons being set to 0. Fix is to compare only positive sizes. - */ - if (parent.pixbufWidth > Math.max(currentWidth [0], 0) || parent.pixbufHeight > Math.max(currentHeight [0], 0)) { - GTK.gtk_cell_renderer_set_fixed_size (pixbufRenderer, parent.pixbufWidth, parent.pixbufHeight); + if (parent.pixbufSizeSet && !parent.isShowingImagesForColumn (index)) { + parent.showImagesForColumn (index); } } int modelIndex = parent.columnCount == 0 ? Table.FIRST_COLUMN : parent.columns [index].modelIndex; @@ -1254,7 +1226,7 @@ public void setImage(int index, Image image) { * width and see if it's larger than the maximum of the previous widths. */ if (parent.columnCount == 0) { - column = GTK.gtk_tree_view_get_column (parent.handle, index); + long column = GTK.gtk_tree_view_get_column (parent.handle, index); parent.maxWidth = Math.max(parent.maxWidth, parent.calculateWidth(column, this.handle)); } } 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 808623da98d..ada98098f18 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 @@ -15,6 +15,7 @@ import java.util.*; +import java.util.function.*; import org.eclipse.swt.*; import org.eclipse.swt.events.*; @@ -110,6 +111,7 @@ public class Tree extends Composite { Color headerBackground, headerForeground; boolean boundsChangedSinceLastDraw, wasScrolled; boolean rowActivated; + boolean showImagesForDefaultColumn; private long headerCSSProvider; @@ -313,7 +315,7 @@ long cellDataProc (long tree_column, long cell, long tree_model, long iter, long if (isPixbuf) { ptr [0] = 0; GTK.gtk_tree_model_get (tree_model, iter, modelIndex + CELL_PIXBUF, ptr, -1); - OS.g_object_set (cell, OS.gicon, ptr [0], 0); + OS.g_object_set (cell, OS.pixbuf, ptr [0], 0); if (ptr [0] != 0) OS.g_object_unref (ptr [0]); } else { ptr [0] = 0; @@ -739,13 +741,6 @@ void copyModel (long oldModel, int oldStart, long newModel, int newStart, long o } void createColumn (TreeColumn column, int index) { -/* -* Bug in ATK. For some reason, ATK segments fault if -* the GtkTreeView has a column and does not have items. -* The fix is to insert the column only when an item is -* created. -*/ - int modelIndex = FIRST_COLUMN; if (columnCount != 0) { int modelLength = GTK.gtk_tree_model_get_n_columns (modelHandle); @@ -774,9 +769,10 @@ void createColumn (TreeColumn column, int index) { if (columnHandle == 0) error (SWT.ERROR_NO_HANDLES); if (index == 0 && columnCount > 0) { TreeColumn checkColumn = columns [0]; - createRenderers (checkColumn.handle, checkColumn.modelIndex, false, checkColumn.style); + createRenderers (checkColumn.handle, checkColumn.modelIndex, false, checkColumn.showImages, checkColumn.style); } - createRenderers (columnHandle, modelIndex, index == 0, column == null ? 0 : column.style); + createRenderers (columnHandle, modelIndex, index == 0, + column == null ? showImagesForDefaultColumn : column.showImages, column == null ? 0 : column.style); if ((style & SWT.VIRTUAL) == 0 && columnCount == 0) { GTK.gtk_tree_view_column_set_sizing (columnHandle, GTK.GTK_TREE_VIEW_COLUMN_GROW_ONLY); } else { @@ -909,9 +905,11 @@ void createItem (TreeColumn column, int index) { GTK.gtk_tree_view_column_set_sizing (column.handle, GTK.GTK_TREE_VIEW_COLUMN_FIXED); GTK.gtk_tree_view_column_set_visible (column.handle, false); column.modelIndex = FIRST_COLUMN; - createRenderers (column.handle, column.modelIndex, true, column.style); + createRenderers (column.handle, column.modelIndex, true, showImagesForDefaultColumn, column.style); column.customDraw = firstCustomDraw; firstCustomDraw = false; + column.showImages = showImagesForDefaultColumn; + showImagesForDefaultColumn = false; } else { createColumn (column, index); } @@ -1032,7 +1030,7 @@ void createItem (TreeItem item, long parentIter, int index) { } } -void createRenderers (long columnHandle, int modelIndex, boolean check, int columnStyle) { +void createRenderers (long columnHandle, int modelIndex, boolean check, boolean showImages, int columnStyle) { GTK.gtk_tree_view_column_clear (columnHandle); if ((style & SWT.CHECK) != 0 && check) { GTK.gtk_tree_view_column_pack_start (columnHandle, checkRenderer, false); @@ -1054,25 +1052,14 @@ void createRenderers (long columnHandle, int modelIndex, boolean check, int colu if (pixbufRenderer == 0) { error (SWT.ERROR_NO_HANDLES); - } else { - // set default size this size is used for calculating the icon and text positions in a tree - if ((!isOwnerDrawn)) { - /* - * When SWT.VIRTUAL is specified, size the pixbuf renderer - * according to the size of the first image set. If no image - * is set, specify a size of 0x0 like for all other Tree - * styles. Fix for bug 480261. - */ - if ((style & SWT.VIRTUAL) != 0 && pixbufSizeSet) { - GTK.gtk_cell_renderer_set_fixed_size(pixbufRenderer, pixbufHeight, pixbufWidth); - } else { - /* - * For all other styles, set render size to 0x0 until we - * actually add images, fix for bugs 469277 & 476419. - */ - GTK.gtk_cell_renderer_set_fixed_size(pixbufRenderer, 0, 0); - } + } + if (!isOwnerDrawn) { + int width = (pixbufSizeSet && showImages) ? pixbufWidth : 1; + int height = -1; + if ((style & SWT.VIRTUAL) != 0) { + height = pixbufSizeSet ? pixbufHeight : 0; } + GTK.gtk_cell_renderer_set_fixed_size(pixbufRenderer, width, height); } long textRenderer = isOwnerDrawn ? OS.g_object_new (display.gtk_cell_renderer_text_get_type (), 0) : GTK.gtk_cell_renderer_text_new (); if (textRenderer == 0) error (SWT.ERROR_NO_HANDLES); @@ -1221,6 +1208,7 @@ void destroyItem (TreeColumn column) { long columnHandle = column.handle; if (columnCount == 1) { firstCustomDraw = column.customDraw; + showImagesForDefaultColumn = column.showImages; } System.arraycopy (columns, index + 1, columns, index, --columnCount - index); columns [columnCount] = null; @@ -1264,7 +1252,8 @@ void destroyItem (TreeColumn column) { TreeColumn firstColumn = columns [0]; firstColumn.style &= ~(SWT.LEFT | SWT.RIGHT | SWT.CENTER); firstColumn.style |= SWT.LEFT; - createRenderers (firstColumn.handle, firstColumn.modelIndex, true, firstColumn.style); + createRenderers (firstColumn.handle, firstColumn.modelIndex, true, firstColumn.showImages, + firstColumn.style); } } if (!searchEnabled ()) { @@ -2848,11 +2837,12 @@ void recreateRenderers () { OS.g_signal_connect_closure (checkRenderer, OS.toggled, display.getClosure (TOGGLED), false); } if (columnCount == 0) { - createRenderers (GTK.gtk_tree_view_get_column (handle, 0), Tree.FIRST_COLUMN, true, 0); + createRenderers (GTK.gtk_tree_view_get_column (handle, 0), Tree.FIRST_COLUMN, true, showImagesForDefaultColumn, + 0); } else { for (int i = 0; i < columnCount; i++) { TreeColumn column = columns [i]; - createRenderers (column.handle, column.modelIndex, i == 0, column.style); + createRenderers (column.handle, column.modelIndex, i == 0, column.showImages, column.style); } } } @@ -3914,7 +3904,7 @@ void setParentGdkResource (Control child) { void setScrollWidth (long column, TreeItem item) { if (columnCount != 0 || currentItem == item) return; int width = GTK.gtk_tree_view_column_get_fixed_width (column); - int itemWidth = calculateWidth (column, item.handle, true); + int itemWidth = calculateWidth (column, item.handle, false); if (width < itemWidth) { GTK.gtk_tree_view_column_set_fixed_width (column, itemWidth); } @@ -4324,6 +4314,130 @@ private void throwCannotRemoveItem(int i) { throw new SWTException(message); } +boolean initPixbufSize (Image image) { + if (pixbufSizeSet || image == null) { + return false; + } + int iWidth, iHeight; + if (DPIUtil.useCairoAutoScale ()) { + iWidth = image.getBounds ().width; + iHeight = image.getBounds ().height; + } else { + iWidth = image.getBoundsInPixels ().width; + iHeight = image.getBoundsInPixels ().height; + } + if (iWidth <= 0 || iHeight <= 0) { + return false; + } + pixbufSizeSet = true; + pixbufHeight = iHeight; + pixbufWidth = iWidth; + /* + * Feature in GTK: a Tree with the style SWT.VIRTUAL has fixed-height-mode + * enabled. This will limit the size of any cells, including renderers. In order + * to prevent images from disappearing/being cropped, we update row heights when + * the first image is set. Fix for bug 480261. + */ + if ((style & SWT.VIRTUAL) != 0) { + initFixedRowHeight (); + } + return true; +} + +private void initFixedRowHeight () { + long columnList = GTK.gtk_tree_view_get_columns (handle); + if (columnList == 0) { + return; + } + + // set fixed height for all GtkCellRendererPixbufs + for (var colItem = columnList; colItem != 0; colItem = OS.g_list_next (colItem)) { + long column = OS.g_list_data (colItem); + long renderer = getPixbufRenderer (column); + if (renderer != 0) { + GTK.gtk_cell_renderer_set_fixed_size (renderer, 1, pixbufHeight); + } + } + OS.g_list_free (columnList); + + /* + * Change fixed height of existing rows to fit new pixbuf height. + * + * Wrapped in asyncExec because when this method is invoked from 'checkData()', + * the 'row_changed' signal is blocked (but we need it unblocked to invalidate + * existing rows). + */ + display.asyncExec ( () -> { + if (!isDisposed () && modelHandle != 0) { + /* + * Reset computed 'fixed_height' to '-1' + * + * Note: current GTK3 (3.24.41) doesn't support setting 'fixed_height_mode' to + * 'true' multiple times: each call appends additional "notify::sizing" + * callbacks to columns and doesn't remove old ones. + * + * It's fine to use here, because this method is executed once per tree. + */ + OS.g_object_set (handle, OS.fixed_height_mode, false, 0); + OS.g_object_set (handle, OS.fixed_height_mode, true, 0); + + // to update height of the existing rows we need to invalidate them in gtk + // we do that by invoking 'gtk_tree_view_row_changed' on each of row + int[] value = new int[1]; + forEachChildRow (modelHandle, 0, iter -> { + GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, value, -1); + GTK.gtk_tree_store_set (modelHandle, iter, ID_COLUMN, value[0], -1); + }); + } + }); +} + +private static void forEachChildRow (long modelHandle, long parentIter, LongConsumer doWithIter) { + long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); + if (GTK.gtk_tree_model_iter_children (modelHandle, iter, parentIter)) { + do { + doWithIter.accept (iter); + forEachChildRow (modelHandle, iter, doWithIter); + } while (GTK.gtk_tree_model_iter_next (modelHandle, iter)); + } + OS.g_free (iter); +} + +boolean isShowingImagesForColumn (int columnIdx) { + if (columnCount == 0) { + Objects.checkIndex (columnIdx, 1); + return showImagesForDefaultColumn; + } else { + Objects.checkIndex (columnIdx, columnCount); + return columns[columnIdx].showImages; + } +} + +boolean showImagesForColumn (int columnIdx) { + Objects.checkIndex (columnIdx, Math.max (1, columnCount)); + if (pixbufSizeSet && !isShowingImagesForColumn (columnIdx)) { + long col = GTK.gtk_tree_view_get_column (handle, columnIdx); + if (col != 0) { + long pixbufRenderer = getPixbufRenderer (col); + if (pixbufRenderer != 0) { + int height = ((style & SWT.VIRTUAL) != 0) ? pixbufHeight : -1; + GTK.gtk_cell_renderer_set_fixed_size (pixbufRenderer, pixbufWidth, height); + if (columnCount == 0) {; + showImagesForDefaultColumn = true; + } else { + columns[columnIdx].showImages = true; + } + if (!GTK.GTK4) { + // update column renderers layout to add space for image width + GTK3.gtk_tree_view_column_queue_resize (col); + } + return true; + } + } + } + return false; +} + @Override public void dispose() { super.dispose(); diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeColumn.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeColumn.java index 8c9cab8bb73..afebbf1044f 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeColumn.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeColumn.java @@ -47,7 +47,7 @@ public class TreeColumn extends Item { long labelHandle, imageHandle, buttonHandle; Tree parent; int modelIndex, lastTime, lastX, lastWidth; - boolean customDraw; + boolean customDraw, showImages; String toolTipText; /** @@ -584,7 +584,7 @@ public void setAlignment (int alignment) { if (index == -1 || index == 0) return; style &= ~(SWT.LEFT | SWT.RIGHT | SWT.CENTER); style |= alignment & (SWT.LEFT | SWT.RIGHT | SWT.CENTER); - parent.createRenderers (handle, modelIndex, index == 0, style); + parent.createRenderers (handle, modelIndex, index == 0, showImages, style); } void setFontDescription (long font) { 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 15ca6fd4ec3..959b6be1293 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 @@ -1517,61 +1517,16 @@ public void setImage(int index, Image image) { } surface = imageList.getSurface(imageIndex); pixbuf = ImageList.createPixbuf(surface); - } - int modelIndex = parent.columnCount == 0 ? Tree.FIRST_COLUMN : parent.columns [index].modelIndex; - long parentHandle = parent.handle; - long column = GTK.gtk_tree_view_get_column (parentHandle, index); - long pixbufRenderer = parent.getPixbufRenderer (column); - int [] currentWidth = new int [1]; - int [] currentHeight= new int [1]; - GTK.gtk_cell_renderer_get_fixed_size (pixbufRenderer, currentWidth, currentHeight); - if (!parent.pixbufSizeSet) { - if (image != null) { - int iWidth, iHeight; - if (DPIUtil.useCairoAutoScale()) { - iWidth = image.getBounds ().width; - iHeight = image.getBounds ().height; - } else { - iWidth = image.getBoundsInPixels ().width; - iHeight = image.getBoundsInPixels ().height; - } - if (iWidth > currentWidth [0] || iHeight > currentHeight [0]) { - GTK.gtk_cell_renderer_set_fixed_size (pixbufRenderer, iWidth, iHeight); - parent.pixbufSizeSet = true; - parent.pixbufHeight = iHeight; - parent.pixbufWidth = iWidth; - /* - * Feature in GTK: a Tree with the style SWT.VIRTUAL has - * fixed-height-mode enabled. This will limit the size of - * any cells, including renderers. In order to prevent - * images from disappearing/being cropped, we re-create - * the renderers when the first image is set. Fix for - * bug 480261. - */ - if ((parent.style & SWT.VIRTUAL) != 0) { - /* - * Only re-create SWT.CHECK renderers if this is the first column. - * Otherwise check-boxes will be rendered in columns they are not - * supposed to be rendered in. See bug 513761. - */ - boolean check = modelIndex == Tree.FIRST_COLUMN && (parent.style & SWT.CHECK) != 0; - parent.createRenderers(column, modelIndex, check, parent.style); - } - } + if (!parent.pixbufSizeSet) { + parent.initPixbufSize (image); } - } else { - /* - * Bug 483112: We check to see if the cached value is greater than the size of the pixbufRenderer. - * If it is, then we change the size of the pixbufRenderer accordingly. - * Bug 489025: There is a corner case where the below is triggered when current(Width|Height) is -1, - * which results in icons being set to 0. Fix is to compare only positive sizes. - */ - if (parent.pixbufWidth > Math.max(currentWidth [0], 0) || parent.pixbufHeight > Math.max(currentHeight [0], 0)) { - GTK.gtk_cell_renderer_set_fixed_size (pixbufRenderer, parent.pixbufWidth, parent.pixbufHeight); + if (parent.pixbufSizeSet && !parent.isShowingImagesForColumn (index)) { + parent.showImagesForColumn (index); } } + int modelIndex = parent.columnCount == 0 ? Tree.FIRST_COLUMN : parent.columns [index].modelIndex; GTK.gtk_tree_store_set(parent.modelHandle, handle, modelIndex + Tree.CELL_PIXBUF, pixbuf, -1); /* * Bug 573633: gtk_tree_store_set() will reference the handle. So we unref the pixbuf here, diff --git a/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Issue678_JvmCrashOnTreeItemSetImage.java b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Issue678_JvmCrashOnTreeItemSetImage.java new file mode 100644 index 00000000000..b04f7703da4 --- /dev/null +++ b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Issue678_JvmCrashOnTreeItemSetImage.java @@ -0,0 +1,98 @@ +package org.eclipse.swt.tests.gtk.snippets; + +import org.eclipse.swt.SWT; +import org.eclipse.swt.graphics.Image; +import org.eclipse.swt.layout.FillLayout; +import org.eclipse.swt.widgets.Display; +import org.eclipse.swt.widgets.Shell; +import org.eclipse.swt.widgets.Tree; +import org.eclipse.swt.widgets.TreeItem; + +/** + * Title: app crash + *

+ * + * How to run: run this class as java application. + *

+ * + * Bug description: when {@link TreeItem#setImage(Image)} is called within an + * {@link SWT#SetData} event handler for a {@link SWT#VIRTUAL} tree, then a JVM + * crash can happen because of use-after-free gtk3 renderer in + * {@code Tree.cellDataProc()}. + * + * 

+ * Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
+ * C  [libgobject-2.0.so.0+0x3b251]  g_type_check_instance_is_fundamentally_a+0x11
+ * C  [libswt-pi3-gtk-4958r2.so+0x4b609]  Java_org_eclipse_swt_internal_gtk_OS_g_1object_1set__J_3BJJ+0x4a
+ *
+ * Java frames: (J=compiled Java code, j=interpreted, Vv=VM code)
+ * J 11988  org.eclipse.swt.internal.gtk.OS.g_object_set(J[BJJ)V (0 bytes)
+ * J 10921 c1 org.eclipse.swt.widgets.Tree.cellDataProc(JJJJJ)J (486 bytes)
+ * J 10920 c1 org.eclipse.swt.widgets.Display.cellDataProc(JJJJJ)J (29 bytes)
+ * v  ~StubRoutines::call_stub
+ * J 11619  org.eclipse.swt.internal.gtk3.GTK3.gtk_main_iteration_do(Z)Z (0 bytes)
+ * J 11623 c1 org.eclipse.swt.widgets.Display.readAndDispatch()Z (88 bytes)
+ *
+ * + * Please note that the crash isn't guaranteed to happen.
+ * Why: the causes of the bug is use-after-free in native code.
+ * The app crash happens when in-between "free" and "use" the released memory is + * allocated and modified by some other native code.
+ * For example some if some pointer accessed during "use" is changed to contain + * address to inaccessible memory, then the app will crash with SIGSEGV.
+ * The problem is that the native code in-between "free" and "use" (i.e. jvm and + * 3d-party libraries) is too complex to predict. + * + * Expected results: image is shown in the tree, no crashes, no errors. + *

+ * + * GTK Version(s): 3.24.37 + */ +public class Issue678_JvmCrashOnTreeItemSetImage { + private static final int NUM_ITERATIONS = 100; + + public static void main (String[] args) { + Display display = new Display (); + Shell shell = new Shell (display); + shell.setSize (400, 300); + shell.setLayout (new FillLayout ()); + shell.open (); + Image image = new Image (display, 20, 20); + for (int i = 0; i < NUM_ITERATIONS; i++) { + Tree tree = new Tree (shell, SWT.VIRTUAL); + tree.addListener (SWT.SetData, e -> { + TreeItem item = (TreeItem) e.item; + item.setText (0, "A"); + // for some reason sleeping increases probability of crash + try { + Thread.sleep (50); + } catch (InterruptedException ex) { + throw new RuntimeException (ex); + } + item.setImage (image); // <-- this is the critical line! + }); + tree.setItemCount (1); + shell.layout (); + waitUntilIdle (); + tree.dispose (); + } + display.dispose (); + } + + private static void waitUntilIdle () { + long lastActive = System.currentTimeMillis (); + while (true) { + if (Thread.interrupted ()) { + throw new AssertionError (); + } + if (Display.getCurrent ().readAndDispatch ()) { + lastActive = System.currentTimeMillis (); + } else { + if (lastActive + 10 < System.currentTimeMillis ()) { + return; + } + Thread.yield (); + } + } + } +}