Repository navigation
[GTK] Keep the Image of each Tree and Table cell for getImage() - #3695
Merged
akurtakov merged 1 commit intoOct 9, 2026
Merged
Conversation
Contributor
Contributor
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open findings
What changed in this PR
Fixes incorrect getImage() results for GTK TreeItem/TableItem cells by storing per-column Image references (instead of reverse-mapping native surfaces through ImageList), and makes layout/paint paths ignore disposed images.
Changes:
- Store per-cell images in
TreeItem/TableItem(cellImage[]) and return from it in_getImage() - Keep
cellImage[]aligned with column insert/remove inTree/Table - Add JUnit coverage ensuring images don’t “inherit” across disposed columns and disposed images aren’t replaced
| File | Description |
|---|---|
| tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TreeItem.java | Adds regression tests around column disposal/recreation and disposed-image stability for TreeItem.getImage() |
| tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TableItem.java | Adds analogous regression tests for TableItem.getImage() |
| bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java | Introduces cellImage[], switches _getImage() to use it, and ignores disposed images in text bounds calculations |
| bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java | Maintains TreeItem.cellImage[] when columns are inserted/removed; ignores disposed images during render sizing |
| bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TableItem.java | Introduces cellImage[], switches _getImage() to use it, and ignores disposed images in text bounds calculations |
| bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java | Maintains TableItem.cellImage[] when columns are inserted/removed; ignores disposed images in measure/render sizing |
🧠 Review effort: Lite
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Copilot stopped reviewing on behalf of
akurtakov due to an error
October 8, 2026 16:34
vogella
reviewed
Oct 9, 2026
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 eclipse-platform#3631. Assisted-by: multiple AI agents and layers of automated tooling 🤖 Assisted-by: Anthropic Claude Code (claude-opus-5-5)
akurtakov
force-pushed
the
gtk-table-tree-cell-images
branch
from
October 9, 2026 06:12
846d00a to
c56b651
Compare
vogella
approved these changes
Oct 9, 2026
vogella
added a commit
to vogella/eclipse.platform.swt
that referenced
this pull request
Oct 9, 2026
GtkCellRendererPixbuf treats the "pixbuf" attribute as a scale 1 image, so at 200% zoom native Tree and Table cell images were reduced to 1x and upscaled again, while the same image drawn with GC.drawImage stayed sharp. On GTK3 at a widget scale factor above 1 the pixbuf renderer now binds its "surface" property to the CELL_SURFACE model column, a boxed cairo surface that carries the device scale, instead of "pixbuf". The binding is swapped through gtk_tree_view_column_add_attribute, so plain widgets run no Java code per row: the cell data function is still installed only for VIRTUAL, custom draw and owner draw, as before, and there it sets the surface or the pixbuf according to the same mode. The binding goes back to "pixbuf" when the control is disabled, because the surface bypasses GTK's insensitive icon effect, and when the scale factor drops to 1. Table and Tree rebind from enableWidget and from the existing scale factor notification (dpiChanged). At a scale above 1 the cell differs from the 100% rendering in ways inherent to the surface property: theme icon effects on selected or hovered rows are not applied, GTK's accessible image size is missing because it is read from the pixbuf, and the cell draws the surface of the Image itself rather than a pixbuf copy, so drawing into an Image after setImage shows up in the cell at that scale only. A snapshot copy was not added, since it would duplicate the pixels of every cell image that is shared between many rows. Disposing a column also clears its CELL_SURFACE slot, so a column that reuses the slot does not show the old image. getImage() relies on the per cell Image array from eclipse-platform#3695 rather than the surface address. The CELL_SURFACE column type is taken from the value type of the pixbuf renderer's "surface" GParamSpec and a missing type raises ERROR_NO_HANDLES instead of being cached as 0. The VIRTUAL path keeps setData = checkData (item), since binding by attribute needs no early return. Assisted-by: multiple AI agents and layers of automated tooling 🤖
vogella
added a commit
to vogella/eclipse.platform.swt
that referenced
this pull request
Oct 9, 2026
GtkCellRendererPixbuf treats the "pixbuf" attribute as a scale 1 image, so at 200% zoom native Tree and Table cell images were reduced to 1x and upscaled again, while the same image drawn with GC.drawImage stayed sharp. On GTK3 at a widget scale factor above 1 the pixbuf renderer now binds its "surface" property to the CELL_SURFACE model column, a boxed cairo surface that carries the device scale, instead of "pixbuf". The binding is swapped through gtk_tree_view_column_add_attribute, so plain widgets run no Java code per row: the cell data function is still installed only for VIRTUAL, custom draw and owner draw, as before, and there it sets the surface or the pixbuf according to the same mode. The binding goes back to "pixbuf" when the control is disabled, because the surface bypasses GTK's insensitive icon effect, and when the scale factor drops to 1. Table and Tree rebind from enableWidget and from the existing scale factor notification (dpiChanged). At a scale above 1 the cell differs from the 100% rendering in ways inherent to the surface property: theme icon effects on selected or hovered rows are not applied, GTK's accessible image size is missing because it is read from the pixbuf, and the cell draws the surface of the Image itself rather than a pixbuf copy, so drawing into an Image after setImage shows up in the cell at that scale only. A snapshot copy was not added, since it would duplicate the pixels of every cell image that is shared between many rows. Disposing a column also clears its CELL_SURFACE slot, so a column that reuses the slot does not show the old image. getImage() relies on the per cell Image array from eclipse-platform#3695 rather than the surface address. The CELL_SURFACE column type is taken from the value type of the pixbuf renderer's "surface" GParamSpec and a missing type raises ERROR_NO_HANDLES instead of being cached as 0. The VIRTUAL path keeps setData = checkData (item), since binding by attribute needs no early return. Assisted-by: multiple AI agents and layers of automated tooling 🤖
akurtakov
pushed a commit
to vogella/eclipse.platform.swt
that referenced
this pull request
Oct 9, 2026
GtkCellRendererPixbuf treats the "pixbuf" attribute as a scale 1 image, so at 200% zoom native Tree and Table cell images were reduced to 1x and upscaled again, while the same image drawn with GC.drawImage stayed sharp. On GTK3 at a widget scale factor above 1 the pixbuf renderer now binds its "surface" property to the CELL_SURFACE model column, a boxed cairo surface that carries the device scale, instead of "pixbuf". The binding is swapped through gtk_tree_view_column_add_attribute, so plain widgets run no Java code per row: the cell data function is still installed only for VIRTUAL, custom draw and owner draw, as before, and there it sets the surface or the pixbuf according to the same mode. The binding goes back to "pixbuf" when the control is disabled, because the surface bypasses GTK's insensitive icon effect, and when the scale factor drops to 1. Table and Tree rebind from enableWidget and from the existing scale factor notification (dpiChanged). At a scale above 1 the cell differs from the 100% rendering in ways inherent to the surface property: theme icon effects on selected or hovered rows are not applied, GTK's accessible image size is missing because it is read from the pixbuf, and the cell draws the surface of the Image itself rather than a pixbuf copy, so drawing into an Image after setImage shows up in the cell at that scale only. A snapshot copy was not added, since it would duplicate the pixels of every cell image that is shared between many rows. Disposing a column also clears its CELL_SURFACE slot, so a column that reuses the slot does not show the old image. getImage() relies on the per cell Image array from eclipse-platform#3695 rather than the surface address. The CELL_SURFACE column type is taken from the value type of the pixbuf renderer's "surface" GParamSpec and a missing type raises ERROR_NO_HANDLES instead of being cached as 0. The VIRTUAL path keeps setData = checkData (item), since binding by attribute needs no early return. Assisted-by: multiple AI agents and layers of automated tooling 🤖
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)