Skip to content

Text control should not redraw indefinitely#3260

Draft
basilevs wants to merge 1 commit intoeclipse-platform:masterfrom
basilevs:issue_3920_inifinite_redraw
Draft

Text control should not redraw indefinitely#3260
basilevs wants to merge 1 commit intoeclipse-platform:masterfrom
basilevs:issue_3920_inifinite_redraw

Conversation

@basilevs
Copy link
Copy Markdown
Contributor

@basilevs basilevs commented Apr 22, 2026

In some scenarios, Text control schedules redraws on every event loop iteration.

Thread [main] (Suspended (breakpoint at line 2150 in Widget))	
	Text(Widget).setNeedsDisplay(long, long, boolean) line: 2150	
	Display.windowProc(long, long, long) line: 6476	
	OS.objc_msgSend(long, long) line: not available [native method]	
	SWTSearchField(NSControl).stringValue() line: 122	
	Text.drawInteriorWithFrame_inView_searchfield(long, long, NSRect, long) line: 751	
	Text.drawInteriorWithFrame_inView(long, long, NSRect, long) line: 697	
	Display.windowProc(long, long, long, long) line: 6661	
	OS.objc_msgSendSuper(objc_super, long, NSRect) line: not available [native method]	
	Text(Widget).drawRect(long, long, NSRect) line: 813	
	Text.drawRect(long, long, NSRect) line: 759	
	Display.windowProc(long, long, long) line: 6246	
	OS.objc_msgSendSuper(objc_super, long, long, long, long, boolean) line: not available [native method]	
	Display.applicationNextEventMatchingMask(long, long, long, long, long, long) line: 5569	
	Display.applicationProc(long, long, long, long, long, long) line: 5970	
	OS.objc_msgSend(long, long, long, long, long, boolean) line: not available [native method]	
	NSApplication.nextEventMatchingMask(long, NSDate, NSString, boolean) line: 92	
	Display.readAndDispatch() line: 3992	
      .....

See eclipse-platform/eclipse.platform.ui#3920

This is a JUnit test reproducing the problem.

Affected patforms:

  • MacOS SDK 14.0 aarch64 - high CPU use
  • MacOS SDK 14.4 aarch64 - high CPU use
  • MacOS SDK 24.4 aarch64 - Display.asyncExec() never executes
  • GTK4

Temporary enabled verbose logging to investigate GTK failures.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 22, 2026

Test Results

  182 files  ± 0    182 suites  ±0   26m 8s ⏱️ - 2m 25s
4 726 tests + 3  4 703 ✅ + 3   23 💤 ±0  0 ❌ ±0 
6 830 runs  +18  6 659 ✅ +12  171 💤 +6  0 ❌ ±0 

Results for commit f6d4858. ± Comparison against base commit a5126b0.

♻️ This comment has been updated with latest results.

@basilevs
Copy link
Copy Markdown
Contributor Author

basilevs commented Apr 22, 2026

I'm surprised the test does not fail in CI. Is UI context really set up there?

Other Mac OS users, could you please run it in your environment to evaluate whether the original problem is specific to my setup?

OS: Mac OS X, v.26.4.1, aarch64 / cocoa
Java vendor: Homebrew
Java runtime version: 25.0.2
Java version: 25.0.2

In any case, this PR is ready to merge.

@basilevs basilevs force-pushed the issue_3920_inifinite_redraw branch from 4a59a5f to b12270f Compare April 22, 2026 18:56
vogella added a commit to vogella/eclipse.platform.swt that referenced this pull request Apr 22, 2026
On cocoa, Text.drawInteriorWithFrame_inView_searchfield queried the
search field text via NSControl.stringValue to decide whether to draw
the cancel icon. That call schedules a setNeedsDisplay: on the view
from inside the paint pass. Widget.setNeedsDisplay then enqueues the
view in display.needsDisplay, which is flushed after paint, marking
the view dirty again and causing readAndDispatch() to spin forever.

Read the text through NSCell.stringValue instead, which bypasses the
control-level side effect. Also add the JUnit regression test from
PR eclipse-platform#3260 reproducing the loop.

Fixes eclipse-platform/eclipse.platform.ui#3920

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
vogella added a commit to vogella/eclipse.platform.swt that referenced this pull request Apr 22, 2026
…gger

Picks up the more reliable reproducer from PR eclipse-platform#3260
(shell.setLayout + requestLayout + shell.open) and adds temporary
instrumentation so CI logs reveal which native call in
drawInteriorWithFrame_inView_searchfield posts the re-entrant
setNeedsDisplay that drives the loop.

- Text.drawInteriorWithFrame_inView / _searchfield: print a stable
  step marker to stderr before and after every significant native
  call; update Text.DEBUG_PAINT_STEP so Widget.setNeedsDisplay can
  attribute the trigger.
- Widget.setNeedsDisplay / setNeedsDisplayInRect: when called
  re-entrantly during paint (isPainting contains view) and a paint
  step is armed, log the offending step, view id and widget class,
  plus a stack trace for the first 3 occurrences.

Temporary: to be removed once the trigger is identified.

Refs eclipse-platform/eclipse.platform.ui#3920

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@basilevs
Copy link
Copy Markdown
Contributor Author

I suspect that CI is using Java based on an incompatible MacOS SDK and that masks some test failures. There is a major difference in behavior resulting from different SDKs JVM is build against.

@basilevs basilevs force-pushed the issue_3920_inifinite_redraw branch from b12270f to 7982d1f Compare April 24, 2026 13:39
@basilevs
Copy link
Copy Markdown
Contributor Author

basilevs commented Apr 24, 2026

With @vogella help, I've managed to reproduce the problem on Mac SDK 14.0 too. It is less severe there, as Display.asyncExec() is not broken, but high CPU usage is still observed. I expect CI to fail now.

@basilevs basilevs force-pushed the issue_3920_inifinite_redraw branch from 7982d1f to bd543d3 Compare April 24, 2026 15:26
@basilevs
Copy link
Copy Markdown
Contributor Author

basilevs commented Apr 24, 2026

The test failures happens on GTK4 CI in addition to expected MacOS. @HeikoKlare could you please check if the test conceptually sound?

Display.readAndDispatch() should almost always return false once UI is drawn and settles. This is normally the case (4 events per second). When the bug is active (Text background color is set) the event rate grows to 122 events per second on fast workstation (or to 60 events per second on a slow one).

I'm not sure what's going on with GTK, if the overall idea is sound, I'll investigate.

@basilevs basilevs force-pushed the issue_3920_inifinite_redraw branch 7 times, most recently from 3a75b47 to 268aad3 Compare April 24, 2026 20:04
@basilevs
Copy link
Copy Markdown
Contributor Author

The test probably can be simplified by just ensuring that UI thread sleeps for 10 ms at least sometimes.

@Phillipus
Copy link
Copy Markdown
Contributor

Phillipus commented Apr 25, 2026

From eclipse-platform/eclipse.platform.ui#3920 (comment)

@Phillipus #3260 now fails on all SDK versions. It effectively measures CPU load in idle state. I would appreciate your review. Thanks for the help so far.

I refactored the tests into a standalone snippet.

Sometimes the plain Text control (SWT.NONE) passed the tests but other times fails on the 2nd assertion (intensity < 1e-4):

Detected 18/171610 UI event streaks/idle event loop iterations per second, intensity 0.00010. Expected: 2/100_000.

But the Cancel/Search Text control with background color failed both assertions (eventStreakCount < 50 and intensity < 1e-4):

Detected 68/161349 UI event streaks/idle event loop iterations per second, intensity 0.00042. Expected: 2/100_000.

macOS 15.7.5 with Temurin 21 Java.

@vogella
Copy link
Copy Markdown
Contributor

vogella commented Apr 25, 2026

@basilevs if my commit helps feel free to cherry pick it and push a patch to SWT

@basilevs
Copy link
Copy Markdown
Contributor Author

Sometimes the plain Text control (SWT.NONE) passed the tests but other times fails on the 2nd assertion (intensity < 1e-4):

Thanks! I saw this instability too, but I've been writing it of as me accidentally moving mouse around. Will keep an eye on it.

@basilevs
Copy link
Copy Markdown
Contributor Author

basilevs commented Apr 25, 2026

@basilevs if my commit helps feel free to cherry pick it and push a patch to SWT

Thanks, maybe later. I'm thinking about GTK4 now. It blinks cursor at 60 Hz refresh rate, breaking my idle assumptions. Disabling blinking for tests does not feel right - it moves the environment further from production.

@basilevs basilevs force-pushed the issue_3920_inifinite_redraw branch from 268aad3 to 43016ca Compare April 25, 2026 15:17
@tobiasmelcher
Copy link
Copy Markdown
Contributor

I think the high cpu load comes from call ((NSSearchField) view).stringValue() in method Text#drawInteriorWithFrame_inView_searchfield. Internally Widget#setNeedsDisplay is called which forces a redraw event. I would say that a method like stringValue() which wants to read the content of a UI widget should not trigger a redraw event.

@basilevs basilevs marked this pull request as draft April 25, 2026 18:59
@basilevs
Copy link
Copy Markdown
Contributor Author

basilevs commented Apr 25, 2026

I think the high cpu load comes from call ((NSSearchField) view).stringValue() in method Text#drawInteriorWithFrame_inView_searchfield. Internally Widget#setNeedsDisplay is called which forces a redraw event. I would say that a method like stringValue() which wants to read the content of a UI widget should not trigger a redraw event.

Unfortunately, this method delegates to MacOS and there is not much that we can do about that:

public NSString stringValue() {
	long result = OS.objc_msgSend(this.id, OS.sel_stringValue);
	return result != 0 ? new NSString(result) : null;
}

Lars Vogel has suggested a fix where the method is not called in the hotspot. That visibly solves the problem.

@basilevs basilevs force-pushed the issue_3920_inifinite_redraw branch from 43016ca to 55305c7 Compare April 25, 2026 21:24
@basilevs basilevs force-pushed the issue_3920_inifinite_redraw branch 2 times, most recently from b894089 to 4104caf Compare April 25, 2026 21:28
@Phillipus
Copy link
Copy Markdown
Contributor

Does this PR also fix the problem where the cancel "x" button has to be double-clicked instead of single-clicked to clear the text?

@basilevs
Copy link
Copy Markdown
Contributor Author

Does this PR also fix the problem where the cancel "x" button has to be double-clicked instead of single-clicked to clear the text?

No ☹️

@basilevs basilevs force-pushed the issue_3920_inifinite_redraw branch from 4104caf to 05b60df Compare April 26, 2026 12:31
In some scenarios, `Text` control redraws on every event loop iteration.
See eclipse-platform/eclipse.platform.ui#3920

Replace native text query in search-field paint with Java-side cache

Introduce a searchFieldHasText boolean on Text, refreshed at every
SWT.Modify emission point (append, cut, insert, paste/replace,
setText, setTextChars, verify-listener replacement, and
textDidChange). The helper only queries stringValue when the display
is not currently painting, so the Java-side read cannot re-arm the
loop. The paint method in drawInteriorWithFrame_inView_searchfield
consults the cache instead of calling stringValue on the cell.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Lars Vogel <Lars.Vogel@vogella.com>
@basilevs basilevs force-pushed the issue_3920_inifinite_redraw branch from 05b60df to f6d4858 Compare April 26, 2026 15:10
@basilevs
Copy link
Copy Markdown
Contributor Author

basilevs commented Apr 26, 2026

I've found that CPU waste increases once user enter some text even after the fix. Investigating again.

Thread [main] (Suspended (breakpoint at line 2150 in Widget))	
	Text(Widget).setNeedsDisplay(long, long, boolean) line: 2150	
	Display.windowProc(long, long, long) line: 6476	
	OS.objc_msgSendSuper(objc_super, long, NSRect, long) line: not available [native method]	
	Text(Widget).callSuper(long, long, NSRect, long) line: 260	
	Text(Widget).drawInteriorWithFrame_inView(long, long, NSRect, long) line: 772	
	Text.drawInteriorWithFrame_inView(long, long, NSRect, long) line: 714	
	Display.windowProc(long, long, long, long) line: 6661	
	OS.objc_msgSendSuper(objc_super, long, NSRect) line: not available [native method]	
	Text(Widget).drawRect(long, long, NSRect) line: 813	
	Text.drawRect(long, long, NSRect) line: 784	
	Display.windowProc(long, long, long) line: 6246	
	OS.objc_msgSendSuper(objc_super, long, long, long, long, boolean) line: not available [native method]	
	Display.applicationNextEventMatchingMask(long, long, long, long, long, long) line: 5569	
	Display.applicationProc(long, long, long, long, long, long) line: 5970	
	OS.objc_msgSend(long, long, long, long, long, boolean) line: not available [native method]	
	NSApplication.nextEventMatchingMask(long, NSDate, NSString, boolean) line: 92	
	Display.readAndDispatch() line: 3992	
	SwtTestUtil.processEvents(int, BooleanSupplier) line: 525	
	Test_org_eclipse_swt_widgets_Text.test_finiteRedrawCancelButton() line: 1423	
	95 collapsed frames	

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants