[GTK] Render Tree and Table images crisp at HiDPI zoom

[vogella]

On GTK3 at 200% zoom, images set with TreeItem.setImage or TableItem.setImage were rendered blurry, because GtkCellRendererPixbuf treats the bound pixbuf as a scale 1 image.
At a scale factor above 1 the pixbuf renderer now binds its surface property to the CELL_SURFACE model column, which carries the device scale, so native cell images are as sharp as GC.drawImage output.
The binding is swapped with gtk_tree_view_column_add_attribute when the scale or the enabled state changes, so plain Trees and Tables run no Java code per row, and disabled controls keep GTK's dimmed icons.

[vogella]

[github-actions]

Test Results (linux)

  115 files  ±0    115 suites  ±0   13m 12s ⏱️ - 1m 24s
4 665 tests ±0  4 430 ✅ ±0  235 💤 ±0  0 ❌ ±0 
3 557 runs  ±0  3 468 ✅ ±0   89 💤 ±0  0 ❌ ±0 

Results for commit 3117e06. ± Comparison against base commit 8f4fe7a.

♻️ This comment has been updated with latest results.

[Copilot]

Copilot review overview

🟡 Changes recommended

Reused column model slots retain stale surfaces, causing removed images to reappear.

Review effort: Balanced
Findings: 2

Open (2)

• Clear CELL_SURFACE when destroying TableColumn data · New
• Clear CELL_SURFACE when destroying TreeColumn data · New

What changed in this PR

Uses scale-aware Cairo surfaces to render crisp Tree/Table images on GTK3 HiDPI displays.

Changes:

• Binds GTK3 cell renderers to stored image surfaces.
• Registers pixbuf data callbacks for all Tree/Table renderers.
• Adds the GTK surface property constant.

File	Description
Tree.java	Renders Tree images from Cairo surfaces.
Table.java	Renders Table images from Cairo surfaces.
OS.java	Defines the native surface property name.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Test Results

  188 files   -    36    188 suites   - 36   21m 11s ⏱️ - 4m 39s
5 016 tests +    6  4 988 ✅ +    6   28 💤 ± 0  0 ❌ ±0 
6 222 runs   - 1 140  6 067 ✅  - 1 101  155 💤  - 39  0 ❌ ±0 

Results for commit 35961bc. ± Comparison against base commit f87da9a.

♻️ This comment has been updated with latest results.

[Copilot]

Copilot review overview

🟡 Changes recommended

Raw surface pointers can become dangling, and GTK4 receives unnecessary per-cell callbacks.

Review effort: Balanced
Findings: 2

Open (2)

• Dangling Cairo surface pointer after disposed image · New
• Dangling Cairo surface pointer after disposed image · New

Resolved since last review (2)

• Clear CELL_SURFACE when destroying TreeColumn data
• Clear CELL_SURFACE when destroying TableColumn data

[Copilot]

Copilot review overview

🟡 Changes recommended

GTK4 receives unnecessary render callbacks, and the painting regressions use zero-sized controls.

Review effort: Balanced
Findings: 2 · 2

Open (4)

• Avoid unconditional GTK4 table cell-data callbacks · New
• Avoid unconditional GTK4 tree cell-data callbacks · New
• Set a nonzero table size to exercise cell painting · New
• Set a nonzero tree size to exercise cell painting · New

Resolved since last review (2)

• Dangling Cairo surface pointer after disposed image
• Dangling Cairo surface pointer after disposed image

[akurtakov]

There are all the details to handle here but it also fixes a nasty drawing problem:

public static void main(String[] args) {
      Display display = new Display();
      Shell shell = new Shell(display);
      shell.setText("zoom " + display.getPrimaryMonitor().getZoom() + "%");
      shell.setLayout(new RowLayout());

      // 1 device pixel black/white stripes at every zoom, any resampling makes it grey
      Image stripes = new Image(display, (ImageDataProvider) zoom -> {
              int size = 16 * zoom / 100;
              ImageData data = new ImageData(size, size, 24, new PaletteData(0xFF0000, 0xFF00, 0xFF));
              for (int y = 0; y < size; y++)
                      for (int x = 0; x < size; x++)
                              data.setPixel(x, y, x % 2 == 0 ? 0 : 0xFFFFFF);
              return data;
      });

      Tree tree = new Tree(shell, SWT.BORDER);
      new TreeItem(tree, SWT.NONE).setImage(stripes);

      Label reference = new Label(shell, SWT.NONE);
      reference.setImage(stripes);

      shell.pack();
      shell.open();
      while (!shell.isDisposed()) {
              if (!display.readAndDispatch()) display.sleep();
      }
      stripes.dispose();
      display.dispose();
}

with GDK_SCALE=2 draws a solid gray instead of striped.

[Copilot]

Copilot review overview

🟢 Approval recommended

The GTK3-specific implementation is coherent, with only minor test resource-cleanup feedback remaining.

Review effort: Balanced
Findings: 2

Open (2)

• Dispose Image in finally when waitEvent assertion fails · New
• Dispose Image in finally when waitEvent assertion fails · New

Resolved since last review (4)

• Avoid unconditional GTK4 tree cell-data callbacks
• Avoid unconditional GTK4 table cell-data callbacks
• Set a nonzero tree size to exercise cell painting
• Set a nonzero table size to exercise cell painting

[Copilot]

Copilot review overview

🟡 Changes recommended

Stale non-owning surface addresses can alias replacement surfaces and render the wrong image.

Review effort: Balanced
Findings: 2

Open (2)

• Stale surface pointer aliases replacement image after slot reuse · New
• Stale surface pointer aliases replacement image after slot reuse · New

Resolved since last review (2)

• Dispose Image in finally when waitEvent assertion fails
• Dispose Image in finally when waitEvent assertion fails

[Copilot]

Copilot review overview

🟢 Approval recommended

The rendering and ownership changes are coherent and tested; only a minor class-reference cleanup remains.

Review effort: Balanced
Findings: 1

Open (1)

• Unref class returned by g_type_class_ref · New

Resolved since last review (2)

• Stale surface pointer aliases replacement image after slot reuse
• Stale surface pointer aliases replacement image after slot reuse

[vogella]

Fixed

[vogella]

@akurtakov let me know if you have more feedback otherwise I plan to merge tomorrow.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Native GTK resource ownership and platform-specific visual behavior require final human validation.

Review effort: Balanced
Findings: None

Resolved since last review (1)

• Unref class returned by g_type_class_ref

[akurtakov]

Thanks for this, the crisp icons are a real improvement. I have a few concerns, mostly about the GTK3 path.

1. Every GTK3 Tree/Table now runs cellDataProc for every row, at any zoom.
   createRenderers now attaches the pixbuf cell data func whenever !GTK.GTK4, so plain widgets (not VIRTUAL, no custom draw) get it too. SWT only enables fixed-height-mode for VIRTUAL widgets. For the others, GtkTreeView's idle row validation (do_validate_rows → validate_row → gtk_tree_view_column_cell_set_cell_data) visits every row, not just the visible ones, and now calls into Java for each.

I measured it on a plain 10,000-row Table on GTK3 at 100%, with nothing done after shell.open(). A TableItem subclass counts the isDisposed() calls that come from cellDataProc:

	cellDataProc calls	distinct rows	visible rows
master	0	0	~16
this PR	20,039	10,000	~16

At scale 1 every one of those calls does nothing: item lookup, qdata, a scale and state query, then it falls through. So "unchanged at 100%" holds for what is drawn, but not for cost.

Now that CELL_SURFACE is a boxed CairoSurface column on GTK3, could the renderer's surface property be bound with gtk_tree_view_column_add_attribute instead? The pixbuf/surface binding would be swapped when the scale or sensitivity changes, so nothing would run in Java per row, and plain widgets would keep master's cost.

2. HiDPI rendering differs from 100% in ways beyond sharpness.

• In GTK3, -gtk-icon-effect is applied only to pixbuf/gicon sources (ensure_surface_from_pixbuf); ensure_surface_from_surface just references the surface. The INSENSITIVE check covers the disabled case, but other theme effects (e.g. highlight on selected or hovered rows) now apply at 100% but not at 200%.
• For image surfaces, ImageList.convertSurface just references image.surface. So at 200% the cell draws the Image's live surface, while at 100% it draws the pixbuf copy. An app that draws into an Image after setImage() sees the change at 200% but not at 100%.
• Setting surface replaces the renderer's image, so its pixbuf property becomes NULL. GtkImageCellAccessible reads image size only from pixbuf, so ATK clients would see no image in cells at scale > 1.

3. The stale-image mix-up is still there on GTK4.
   On GTK4, CELL_SURFACE stays G_TYPE_LONG, so getImage() still maps a raw (possibly freed) surface address back to an Image through ImageList. The new tests skip GTK4 rather than cover it. The underlying problem is that getImage() finds the Image by surface address. Keeping the Image reference per item on the Java side would fix it for both toolkits.

4. Smaller points

• cairoSurfaceType() looks the type up by the name "CairoSurface", relying on the pixbuf renderer class having registered it, and caches 0 if that fails. gtk_list_store_newv/gtk_tree_store_newv would then fail for every Table/Tree. Taking value_type from the renderer's surface pspec (g_object_class_find_property) seems more robust.
• In the VIRTUAL path, setData = checkData (item); became if (!checkData (item)) return 0;. That changes behavior on GTK4 as well: when checkData fails, the custom-draw background/font/foreground setup further down is now skipped. If that's intended, a note in the commit message would help.

[vogella]

Thanks, all addressed and force-pushed. The image is now bound via gtk_tree_view_column_add_attribute (no per-row Java calls) and falls back to the dimmed pixbuf when disabled. Items keep their Image per cell, so getImage() no longer maps surface addresses, on GTK4 too. The surface type now comes from the pspec, and the checkData change is reverted. The remaining differences at >100% are noted in the commit message. Your stripes snippet stays crisp at scale 2.

[akurtakov]

There are conflicts

[Copilot]

Copilot review overview

🟡 Changes recommended

Monitor-scale transitions retain stale surfaces, and ancestor-disabled controls keep undimmed images.

Review effort: Balanced
Findings: 2

Open (2)

• Update image binding when an ancestor disables the Table · New
• Update image binding when an ancestor disables the Tree · New

[Copilot]

🟡 Changes recommended

Runtime DPI changes reuse surfaces cached at the previous scale, leaving moved Trees and Tables blurry.

2 open findings

• Table image caches remain blurry after monitor scale changes · New
• Tree image caches remain blurry after monitor scale changes · New

2 resolved since last review

• Update image binding when an ancestor disables the Tree
• Update image binding when an ancestor disables the Table

🧠 Review effort: Balanced

---

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

[akurtakov]

I have split the getImage() part of this PR into #3695, so the bug fix can go in on its own while the HiDPI rendering is reviewed. #3695 contains:

• the per-cell cellImage array in TableItem/TreeItem and its maintenance in createItem/destroyItem,
• the six new tests,
• one addition: getTextBounds() and the owner-draw measure/paint code now skip a disposed cell image. Otherwise an app that disposes an image still in use gets an exception on every repaint.

It needs no new natives, behaves the same at every scale, and fixes getImage() returning another item's image once a disposed image's ImageList slot is reused. The Table, Tree, item and column tests (389) pass on GTK3 and GTK4. You're kept as the commit author.
I hope you don't mind this approach as that part of the PR is easy to review and will make this PR easier strictly on the rendering thus easier to review.

[Copilot]

🔵 Needs a closer look

DPI transitions rebind stale cached surfaces without regenerating them for the new zoom.

2 open findings

• Tree image caches remain blurry after monitor scale changes
• Table image caches remain blurry after monitor scale changes

🧠 Review effort: Balanced