[GTK] Change the zoom only on scale factor changes of the Shell window by vogella · Pull Request #3679 · eclipse-platform/eclipse.platform.swt

vogella · 2026-10-06T11:23:28Z

GTK notifies scale-factor on every widget, including a child that is attached to its parent inside the Control constructor. SWT took this for a DPI change, so the half-built child changed the global zoom, received SWT.ZoomChanged and laid out all shells, which sent SWT.Skin to custom widgets like StyledText before their constructor had finished and caused a NullPointerException. Now only the Shell window's notification changes the zoom and sends SWT.ZoomChanged, while image widgets still refresh their images. Regression tests cover both the early Skin event and the Shell path.

Fixes #3458

github-actions · 2026-10-06T11:34:01Z

Test Results

228 files ± 0 228 suites ±0 25m 45s ⏱️ + 1m 18s
5 035 tests + 2 5 007 ✅ +2 28 💤 ±0 0 ❌ ±0
7 492 runs +12 7 271 ✅ +6 221 💤 +6 0 ❌ ±0

Results for commit e5db454. ± Comparison against base commit 687c8a2.

♻️ This comment has been updated with latest results.

Copilot

Copilot review overview

🟢 Approval recommended

The focused fix addresses the reported construction-time event ordering and includes appropriate regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Defers GTK DPI-change layouts to prevent SWT.Skin events during widget construction.

Changes:

Schedules shell relayout asynchronously after scale-factor changes.
Adds a GTK regression test covering StyledText construction.

File	Description
`Display.java`	Defers DPI-triggered shell layouts.
`Test_org_eclipse_swt_custom_StyledText.java`	Verifies skin events occur after construction.

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

akurtakov · 2026-10-08T14:41:04Z

Some analysis follow:
Cause: notify::scale-factor is connected on every widget. When a child is attached to its parent, GTK corrects that child's scale, and SWT mistakes this for a DPI change. So the half-built child sets the global zoom and lays out all shells, and that sends the Skin events. With this PR the Skin event is no longer early, but the zoom still flips, and ZoomChanged still goes to the half-built widget.

Suggestion: only the Shell's own window notification should change the global zoom and I don't see a reason why/when non-Shells should send zoom-changed on Gtk. The per-widget hook can stay for refreshing images:

// Widget.dpiChanged: return 0;

// Shell
@OverRide
long dpiChanged (long object, long arg0) {
if (object == shellHandle) {
int scale = GTK.gtk_widget_get_scale_factor (shellHandle);
if (DPIUtil.getDeviceZoom () / 100 != scale) {
display.dpiChanged (scale);
Event event = new Event ();
event.detail = scale;
event.doit = true;
notifyListeners (SWT.ZoomChanged, event);
}
}
return super.dpiChanged (object, arg0);
}

On both GTK3 and GTK4 that should have the effect of no early Skin event and the zoom doesn't change, while real scale changes still work. That should make the async call not needed and make things even more predictable. Please try it.

vogella · 2026-10-09T06:02:12Z

Thanks, applied.

Copilot

🟡 Changes recommended

The Shell event reports a native scale factor instead of the documented zoom percentage.

1 open finding

Report zoom percentage, not GTK scale factor, in SWT.ZoomChanged · New

🧠 Review effort: Balanced

Copilot

🟡 Changes recommended

Shell DPI handling mishandles fixed autoscaling and GTK4 popover-backed child Shells.

1 open finding

Exclude popover-backed shells from global zoom updates · New

1 resolved since last review

Report zoom percentage, not GTK scale factor, in SWT.ZoomChanged

🧠 Review effort: Balanced

akurtakov · 2026-10-09T15:57:38Z

The tests are really complicated and non-generic enough. It would be nice to have them in org.eclipse.swt.tests.gtk and use Gtk specific methods directly so it's easier to understand what it does.

vogella · 2026-10-13T16:12:23Z

@akurtakov Done, please check again

github-actions · 2026-10-13T16:22:27Z

Test Results (linux)

511 files + 3 511 suites +3 34m 52s ⏱️ - 2m 57s
4 792 tests + 3 4 450 ✅ + 3 342 💤 ±0 0 ❌ ±0
14 667 runs +18 13 285 ✅ +18 1 382 💤 ±0 0 ❌ ±0

Results for commit 2ac8665. ± Comparison against base commit 59cac9e.

♻️ This comment has been updated with latest results.

Copilot

🟡 Changes recommended

The Shell compares native GTK scale against autoscaled zoom, causing repeated false zoom changes under fixed swt.autoScale.

1 open finding

Compare GTK scale factor against native zoom to prevent repeated events · New

1 resolved since last review

Exclude popover-backed shells from global zoom updates

🧠 Review effort: Balanced

+	// Only the window changes the global zoom, children report their scale while being attached
+	if (object == shellHandle && !popover) {
+		int scale = GTK.gtk_widget_get_scale_factor (shellHandle);
+		if (DPIUtil.getDeviceZoom () / 100 != scale) {


Copilot

🔵 Needs a closer look

The native scale factor is compared against autoscaled zoom, causing repeated false zoom changes with fixed swt.autoScale.

1 open finding

Compare GTK scale factor against native zoom to prevent repeated events

🧠 Review effort: Balanced

GTK notifies scale-factor on every widget, including a child that is being attached to its parent inside the Control constructor. SWT took this for a DPI change, so the half-built child changed the global zoom, received SWT.ZoomChanged and laid out all shells, which sent SWT.Skin to custom widgets like StyledText before their constructor had initialized them. Only the notification of the Shell window now changes the zoom and sends SWT.ZoomChanged, which excludes the GtkPopover of a GTK4 ON_TOP child Shell; other widgets still refresh their images. The change is detected on the native zoom, computed as at startup, so neither a fixed swt.autoScale nor an Xft DPI other than 96 turns an unchanged scale factor into a zoom change. Fixes eclipse-platform#3458 Assisted-by: multiple AI agents and layers of automated tooling 🤖

Copilot

🟢 Approval recommended

The implementation matches the Shell-only zoom contract and includes focused regression coverage.

1 open finding

Compare GTK scale factor against native zoom to prevent repeated events

🧠 Review effort: Balanced

akurtakov · 2026-10-14T16:18:19Z

This is good to go.

vogella requested a balanced review from Copilot October 6, 2026 11:33

Copilot started reviewing on behalf of vogella October 6, 2026 11:34 View session

Copilot AI reviewed Oct 6, 2026

View reviewed changes

vogella force-pushed the styledtext-skin-3458 branch from 15653c2 to 52ec5b7 Compare October 7, 2026 06:59

vogella force-pushed the styledtext-skin-3458 branch from 52ec5b7 to a795fce Compare October 9, 2026 04:52

vogella changed the title ~~[GTK] Defer the layout after a scale factor change to avoid early SWT.Skin events~~ [GTK] Change the zoom only on scale factor changes of the Shell window Oct 9, 2026

vogella force-pushed the styledtext-skin-3458 branch from a795fce to 24442c9 Compare October 9, 2026 09:40

akurtakov requested a balanced review from Copilot October 9, 2026 09:45

Copilot started reviewing on behalf of akurtakov October 9, 2026 09:45 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Shell.java Outdated

vogella force-pushed the styledtext-skin-3458 branch 2 times, most recently from 7cb0836 to d4dba30 Compare October 9, 2026 13:41

akurtakov force-pushed the styledtext-skin-3458 branch from d4dba30 to 4c1306b Compare October 9, 2026 15:38

akurtakov requested a balanced review from Copilot October 9, 2026 15:38

Copilot started reviewing on behalf of akurtakov October 9, 2026 15:39 View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Shell.java Outdated

vogella force-pushed the styledtext-skin-3458 branch 2 times, most recently from e5db454 to 2a809a2 Compare October 13, 2026 16:12

akurtakov requested a balanced review from Copilot October 13, 2026 16:34

Copilot started reviewing on behalf of akurtakov October 13, 2026 16:34 View session

Copilot AI reviewed Oct 13, 2026

View reviewed changes

vogella requested a balanced review from Copilot October 13, 2026 21:01

Copilot started reviewing on behalf of vogella October 13, 2026 21:02 View session

Copilot AI reviewed Oct 13, 2026

View reviewed changes

vogella force-pushed the styledtext-skin-3458 branch from 2a809a2 to 0bb3052 Compare October 14, 2026 13:43

vogella force-pushed the styledtext-skin-3458 branch from 0bb3052 to 2ac8665 Compare October 14, 2026 13:59

akurtakov requested a balanced review from Copilot October 14, 2026 14:30

Copilot started reviewing on behalf of akurtakov October 14, 2026 14:30 View session

Copilot AI reviewed Oct 14, 2026

View reviewed changes

akurtakov merged commit b7bcfcc into eclipse-platform:master Oct 14, 2026
18 checks passed

Conversation

vogella commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

akurtakov commented Oct 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vogella commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

akurtakov commented Oct 9, 2026

Uh oh!

vogella commented Oct 13, 2026

Uh oh!

github-actions Bot commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results (linux)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

akurtakov commented Oct 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vogella commented Oct 6, 2026 •

edited

Loading

github-actions Bot commented Oct 6, 2026 •

edited

Loading

akurtakov commented Oct 8, 2026 •

edited

Loading

github-actions Bot commented Oct 13, 2026 •

edited

Loading