Skip to content

Text control should not redraw indefinitely #3260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
basilevs wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Text control should not redraw indefinitely #3260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 2 additions & 27 deletions bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/widgets/Display.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ enum APPEARANCE {
Control currentControl, trackingControl, tooltipControl, ignoreFocusControl;
Widget tooltipTarget;

NSMutableArray isPainting, needsDisplay, needsDisplayInRect, runLoopModes;
NSMutableArray isPainting, runLoopModes;

NSDictionary markedAttributes;

Expand Down Expand Up @@ -3994,7 +3994,6 @@ public boolean readAndDispatch () {
events = true;
application.sendEvent(event);
}
events |= runPaint ();
events |= runDeferredEvents ();
if (!events) {
events = isDisposed () || runAsyncMessages (false);
Expand Down Expand Up @@ -4182,11 +4181,9 @@ void releaseDisplay () {
if (screenWindow != null) screenWindow.release();
screenWindow = null;

if (needsDisplay != null) needsDisplay.release();
if (needsDisplayInRect != null) needsDisplayInRect.release();
if (isPainting != null) isPainting.release();
if (runLoopModes != null) runLoopModes.release();
needsDisplay = needsDisplayInRect = isPainting = runLoopModes = null;
isPainting = runLoopModes = null;

modalShells = null;
modalDialog = null;
Expand Down Expand Up @@ -4464,28 +4461,6 @@ NSArray runLoopModes() {
return runLoopModes;
}

boolean runPaint () {
if (needsDisplay == null && needsDisplayInRect == null) return false;
if (needsDisplay != null) {
long count = needsDisplay.count();
for (int i = 0; i < count; i++) {
OS.objc_msgSend(needsDisplay.objectAtIndex(i).id, OS.sel_setNeedsDisplay_, true);
}
needsDisplay.release();
needsDisplay = null;
}
if (needsDisplayInRect != null) {
long count = needsDisplayInRect.count();
for (int i = 0; i < count; i+=2) {
NSValue value = new NSValue(needsDisplayInRect.objectAtIndex(i+1));
OS.objc_msgSend(needsDisplayInRect.objectAtIndex(i).id, OS.sel_setNeedsDisplayInRect_, value.rectValue());
}
needsDisplayInRect.release();
needsDisplayInRect = null;
}
return true;
}

boolean runPopups () {
if (popups == null) return false;
boolean result = false;
Expand Down
21 changes: 0 additions & 21 deletions bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/widgets/Widget.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2135,21 +2135,11 @@ boolean setMarkedText_selectedRange (long id, long sel, long string, long range)

void setNeedsDisplay (long id, long sel, boolean flag) {
if (flag && !isDrawing()) return;
NSView view = new NSView(id);
/*
* Since macOS 14 the clipsToBounds property of NSView has to be set to true
* See https://developer.apple.com/documentation/macos-release-notes/appkit-release-notes-for-macos-14
*/
OS.objc_msgSend(id, OS.sel_setClipsToBounds_, true);
Copy link
Copy Markdown
Contributor Author

@basilevs basilevs May 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed CTabFolder update by restoring setClipsToBounds. I will keep trying to setup field testing and create a test covering CTabFolder regression scenario.

if (flag && display.isPainting.containsObject(view)) {
NSMutableArray needsDisplay = display.needsDisplay;
if (needsDisplay == null) {
needsDisplay = (NSMutableArray)new NSMutableArray().alloc();
display.needsDisplay = needsDisplay = needsDisplay.initWithCapacity(12);
}
needsDisplay.addObject(view);
return;
}
objc_super super_struct = new objc_super();
super_struct.receiver = id;
super_struct.super_class = OS.objc_msgSend(id, OS.sel_superclass);
Expand All @@ -2160,22 +2150,11 @@ void setNeedsDisplayInRect (long id, long sel, long arg0) {
if (!isDrawing()) return;
NSRect rect = new NSRect();
OS.memmove(rect, arg0, NSRect.sizeof);
NSView view = new NSView(id);
/*
* Since macOS 14 the clipsToBounds property of NSView has to be set to true
* See https://developer.apple.com/documentation/macos-release-notes/appkit-release-notes-for-macos-14
*/
OS.objc_msgSend(id, OS.sel_setClipsToBounds_, true);
if (display.isPainting.containsObject(view)) {
NSMutableArray needsDisplayInRect = display.needsDisplayInRect;
if (needsDisplayInRect == null) {
needsDisplayInRect = (NSMutableArray)new NSMutableArray().alloc();
display.needsDisplayInRect = needsDisplayInRect = needsDisplayInRect.initWithCapacity(12);
}
needsDisplayInRect.addObject(view);
needsDisplayInRect.addObject(NSValue.valueWithRect(rect));
return;
}
objc_super super_struct = new objc_super();
super_struct.receiver = id;
super_struct.super_class = OS.objc_msgSend(id, OS.sel_superclass);
Expand Down
169 changes: 169 additions & 0 deletions ....swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_Text.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,23 @@
*******************************************************************************/
package org.eclipse.swt.tests.junit;

import static java.lang.System.currentTimeMillis;
import static java.lang.System.nanoTime;
import static org.eclipse.swt.tests.junit.SwtTestUtil.JENKINS_DETECT_ENV_VAR;
import static org.eclipse.swt.tests.junit.SwtTestUtil.JENKINS_DETECT_REGEX;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assumptions.assumeFalse;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

import java.lang.management.ManagementFactory;

import org.eclipse.swt.SWT;
import org.eclipse.swt.events.ModifyListener;
import org.eclipse.swt.events.PaintListener;
import org.eclipse.swt.events.SegmentListener;
import org.eclipse.swt.events.SelectionEvent;
import org.eclipse.swt.events.SelectionListener;
Expand All @@ -30,12 +38,14 @@
import org.eclipse.swt.graphics.Font;
import org.eclipse.swt.graphics.FontData;
import org.eclipse.swt.graphics.Point;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Event;
import org.eclipse.swt.widgets.Group;
import org.eclipse.swt.widgets.Text;
import org.eclipse.swt.widgets.Widget;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledIfEnvironmentVariable;
Expand Down Expand Up @@ -1376,6 +1386,77 @@ public void test_showSelection() {
text.showSelection();
}

// Originally reported as https://github.com/eclipse-platform/eclipse.platform.ui/issues/3920
@Test
public void test_finiteRedrawCancelButtonWithBackground() {
if ( text != null ) text.dispose();
// Style constants are causing
// org.eclipse.swt.widgets.Text.drawInteriorWithFrame_inView_searchfield(long, long, NSRect, long)
// to call
// org.eclipse.swt.internal.cocoa.NSControl.stringValue()
// which schedules redraw
text = new Text(shell, SWT.SEARCH | SWT.ICON_CANCEL);
// Background prevents early exit from drawInteriorWithFrame_inView_searchfield(long, long, NSRect, long)
text.setBackground(shell.getDisplay().getSystemColor(SWT.COLOR_RED));
setWidget(text);
shell.setLayout(new FillLayout());
text.requestLayout();
shell.open();
text.forceFocus();
testIdleAtVariousLength();
}

@Test
public void test_finiteRedrawCancelButton() {
if ( text != null ) text.dispose();
// Style constants are causing
// org.eclipse.swt.widgets.Text.drawInteriorWithFrame_inView_searchfield(long, long, NSRect, long)
// to call
// org.eclipse.swt.internal.cocoa.NSControl.stringValue()
// which schedules redraw
text = new Text(shell, SWT.SEARCH | SWT.ICON_CANCEL);
setWidget(text);
shell.setLayout(new FillLayout());
text.requestLayout();
shell.open();
text.forceFocus();
testIdleAtVariousLength();
}

@Test
public void test_finiteRedrawCancelButtonA() {
if ( text != null ) text.dispose();
// Style constants are causing
// org.eclipse.swt.widgets.Text.drawInteriorWithFrame_inView_searchfield(long, long, NSRect, long)
// to call
// org.eclipse.swt.internal.cocoa.NSControl.stringValue()
// which schedules redraw
text = new Text(shell, SWT.SEARCH | SWT.ICON_CANCEL);
setWidget(text);
shell.setLayout(new FillLayout());
text.requestLayout();
shell.open();
text.forceFocus();
text.setText("A");
// SwtTestUtil.processEvents(100000, () -> false);
waitUntilIdle();
assertIdle();

}


@Test
public void test_finiteRedraw() {
if ( text != null ) text.dispose();
text = new Text(shell, SWT.NONE);
setWidget(text);
shell.setLayout(new FillLayout());
text.requestLayout();
shell.open();
text.forceFocus();
testIdleAtVariousLength();
}

/* custom */
Text text;
String delimiterString;
Expand Down Expand Up @@ -1558,6 +1639,7 @@ private void doSegmentsTest (boolean isListening) throws InterruptedException {
@Test
@Tag("gtk4-todo")
@DisabledIfEnvironmentVariable(named = JENKINS_DETECT_ENV_VAR, matches = JENKINS_DETECT_REGEX, disabledReason = "Display.post tests don't run reliably on Jenkins - see https://github.com/eclipse-platform/eclipse.platform.swt/issues/2571")
@Disabled("https://github.com/eclipse-platform/eclipse.platform.swt/issues/2571") // fails on MacOS too
public void test_backspaceAndDelete() throws InterruptedException {
shell.open();
text.setSize(10, 50);
Expand Down Expand Up @@ -1639,4 +1721,91 @@ private void pasteFromClipboard(Text text) throws InterruptedException {
SwtTestUtil.processEvents(1000, () -> !oldText.equals(text.getText()));
}

private void testIdleAtVariousLength() {
waitUntilIdle();
assertIdle();
text.setText("");
waitUntilIdle();
assertIdle();
text.setText("a");
waitUntilIdle();
assertIdle();
text.setText("aaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa");
waitUntilIdle();
assertIdle();
}

private void waitUntilIdle() {
long hangTimeout = currentTimeMillis() + 1000;
long lastActive = nanoTime();
while (currentTimeMillis() < hangTimeout) {
if (shell.getDisplay().readAndDispatch()) {
lastActive = nanoTime();
} else {
// On GTK, blinking caret animation fires with 60 Hz, can't expect long idle times
if (lastActive < nanoTime() - 10_000_000) {
return;
}
Thread.yield();
}
}
fail("Unexpected system events keep coming");
}

private void assertIdle() {
assumeFalse(SwtTestUtil.isGTK4(), "GTK4 bug - 10-40% CPU in idle");
long[] paintCount = new long[] { 0 };
long wallNs, cpuNs;
PaintListener paintListener = ignored -> {
paintCount[0]++;
};
text.addPaintListener(paintListener);
try {
Display display = shell.getDisplay();
var tmx = ManagementFactory.getThreadMXBean();
assumeTrue(tmx.isThreadCpuTimeSupported() && tmx.isThreadCpuTimeEnabled(),
"Thread CPU time measurement is not available on this JVM");

final int MEASURE_MS = 2000;

// Schedule a single one-shot timer for the whole measurement window.
// When it fires it (a) sets the guard that terminates the loop, and
// (b) wakes display.sleep() on platforms that would otherwise block
// indefinitely because they generate no background events.
final boolean[] done = {false};
display.timerExec(MEASURE_MS, () -> done[0] = true);

long threadId = Thread.currentThread().threadId();
long cpuBefore = tmx.getThreadCpuTime(threadId);
long wallStart = System.nanoTime();
// Additional protection against broken event loop, happens on MacOS SDK 24
long stopGuard = currentTimeMillis() + MEASURE_MS * 2;
while (!done[0]) {
if (stopGuard < currentTimeMillis()) {
fail("Timer should fire");
}
if (!display.readAndDispatch()) {
// On GTK, blinking caret animation fires with 60 Hz
// We can't just count busy iterations
// Hence the actual CPU load measurement below
display.sleep();
}
}
wallNs = System.nanoTime() - wallStart;

cpuNs = tmx.getThreadCpuTime(threadId) - cpuBefore;
} finally {
text.removePaintListener(paintListener);
}
double cpuFraction = (double) cpuNs / wallNs;
int maxPaint =20;
double maxCPU = 0.05;
String message = "CPU usage when idle: %.1f%% < %.1f%%. Paint events: %d < %d"
.formatted(cpuFraction * 100, maxCPU * 100, paintCount[0], maxPaint);
if (SwtTestUtil.verbose) {
System.out.println(message);
}
assertTrue(cpuFraction < maxCPU && paintCount[0] < maxPaint, message);
}

}
Loading