[GTK4] Caret not being drawn in StyledText

[jonahgraham]

The Caret is not being drawn on GTK4 StyledText (or it is being drawn, but it is not visible to the user).

This can be observed with Snippet163, which is one of the simplest snippets with StyledText. Snippet74 is just the caret, and the caret works there.

[jonahgraham]

In my first pass of evaluations this is a difference I see between GTK3 and GTK4 caret drawing in StyledText.

The caret needs to be drawn (on or off) every Caret.blinkRate, which causes a bunch of drawing to happen every blinkRate (I'm calling it an iteration below)

In GTK3, each iteration calls Display.windowProc 4 times:

1. Shell {Snippet 163}
2. StyledText {}
3. StyledText {} (a second time)
4. Shell {Snippet 163} (a second time)

In (2) above the windowProc calls gtk_draw, which calls StyledText.handlePaint. Which means in each iteration the StyledText is being painted once.

In GTK4, each iteration calls Display.snapshotDrawProc 3 times:

1. Shell {Snippet 163}
2. StyledText {}
3. StyledText {} (a second time)

In 2 and 3 for GTK4 the snapshotDrawProc eventually calls gtk4_draw then StyledText.handlePaint. Which means in each iteration the StyledText is being painted twice.

The first time that snapshotDrawProc calls gtk4_draw, the Canvas.blink == true, so drawCaret is also called. But the second time gtk4_draw is called, since Canvas.blink == false, the caret doesn't get drawn, leaving the caret invisible as StyledText.handlePaint has redrawn over where the caret was.

If you "skip" the second paint of styledtext the caret is visible. For example, apply this hack (that may only work with Snippet163):

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java
index 132f293b132..3cc5a29e2ac 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java      
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java      
@@ -27,6 +27,7 @@ import java.util.regex.*;
 import java.util.regex.Pattern;
 
 import org.eclipse.swt.*;
+import org.eclipse.swt.custom.*;
 import org.eclipse.swt.graphics.*;
 import org.eclipse.swt.internal.*;
 import org.eclipse.swt.internal.GDBus.*;
@@ -1720,7 +1721,11 @@ static long rendererClassInitProc (long g_class, long class_data) {
 void snapshotDrawProc(long handle, long snapshot) {
        Display display = getCurrent ();
        Widget widget = display.getWidget (handle);
-       if (widget != null) widget.snapshotToDraw(handle, snapshot);
+       boolean skip = false;
+       if (GTK4.gtk_widget_get_first_child(handle) == 0 && widget instanceof StyledText) {
+               skip = true;
+       }
+       if (widget != null && !skip) widget.snapshotToDraw(handle, snapshot);
        long child = GTK4.gtk_widget_get_first_child(handle);
        // Propagate the snapshot down the widget tree
        while (child != 0) {

So it seems to me regardless of anything else, it is an error that StyledText is being painted twice with the same(ish) contents.

Note that it is not painting styledtext the same in the two draws. The first draw seems a little too far to the left.

First paint of styledtext:

Second paint in the iteration:

(the above screenshots were done with all the text.setStyleRange commented out in Snippet163)

[jonahgraham]

If you change the gtk_widget_get_first_child(handle) == 0 to gtk_widget_get_first_child(handle) != 0 then the other draw is the only one done. But if the second draw is the only one done, the caret is drawn in a light grey colour instead of black. This seems to be down to this change c051490 - always doing the cairo_set_source_rgb means the caret draws black in whichever paint it happens in.

Not advocating for reverting c051490 - just observing for now.

[jonahgraham]

The correct one to "skip" is the first one.

From earlier comment, updated:

In GTK4, each iteration calls Display.snapshotDrawProc 3 times:

1. Shell {Snippet 163} (not interested in this one yet)
2. StyledText {} snapshotDrawProc is called with handle value matching Control.fixedHandle here, which resolves to StyledText (from calling display.getWidget)
3. StyledText {} snapshotDrawProc is called with handle value matching Widget.handle here, which also resolves to StyledText (from calling display.getWidget)

IIUC fixedHandle is the extra handle/gtk widget to add the needed padding. So when we paint the StyledText on it (step 2) then the styledtext is painted a little too far to the left.

We should only be painting styledtext when snapshotDrawProc is being called on the "real" styledtext handle (step 3).

I suspect the issue is around Control.hookPaintSignals, but perhaps it is on snapshotDrawProc itself. Maybe if snapshotDrawProc is called on fixedHandle it needs to change what is painted, and then snapshot the child so it gets painted too.

@akurtakov can you review my findings so far and let me know if I am down a rabbit hole or perhaps on the right path?

[akurtakov]

I'm entirely convinced it's related to the snapshot. fixedHandle is the custom widget of swt to allow fixed sizing but I don't understand the comment about painted a little too far to the left.

[jonahgraham]

I'm entirely convinced it's related to the snapshot

Great - I will keep doing down this path.

the comment about painted a little too far to the left.

If you place a breakpoint on Widget.snapshotToDraw right after bounds is created, you will see it is hit three times per iteration.

1. Shell (don't really care atm)
2. StyledText for fixedHandle: bounds = Rectangle {0, 0, 309, 21}
3. StyledText for handle: bounds = Rectangle {0, 0, 307, 19}

This is because fixedHandle lives 1 pixel up and to the left (and is 2 pixels wider and taller) than handle.

Therefore, when Paint is called for fixedHandle it is painted 1 pixel to the left of where it is painted for handle.

So you can see that in the first paint, the 0 is touching the shell edge:

And in the second paint there is the (Expected by me) 1 pixel gap.

[jonahgraham]

This is because fixedHandle lives 1 pixel up and to the left (and is 2 pixels wider and taller) than handle.

That one pixel comes from Snippet 163 constructiong StyledText with SWT.BORDER. Using SWT.H_SCROLL | SWT.V_SCROLL instead means no border. Using SWT.NONE means no fixedHandle is created, and the caret works.

[jonahgraham]

the caret is drawn in a light grey colour instead of black

This is happening because cairo_set_source_rgb is being skipped here:

eclipse.platform.swt/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Canvas.java

Lines 227 to 230 in a0d6ffc

	if (!GTK.GTK4) {
	Cairo.cairo_set_source_rgb(cairo, 1, 1, 1);
	}
	Cairo.cairo_set_operator(cairo, Cairo.CAIRO_OPERATOR_DIFFERENCE);

This was introduced in c051490 to make Snippet74 work. With this part of the change the Snippet74 does indeed draw the caret in black, as expected. But this is happening because Snippet74 never draws anything else (with cairo) on those pixels. Therefore cairo thinks what is drawn already is pixels of value 0,0,0. With the cairo_set_source_rgb(cairo, 1, 1, 1) + CAIRO_OPERATOR_DIFFERENCE the resulting pixels are 1,1,1 (aka white).

If you change Snippet74 to draw the caret on a Canvas that is explicitly filled in to the background color with Cairo then the operator difference does the expected thing. Here is workaround for Snippet74:

$ git diff -- 'examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet74.java'
diff --git a/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet74.java b/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet74.java
index a824dfe3238..92a3a6adc7b 100644
--- a/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet74.java
+++ b/examples/org.eclipse.swt.snippets/src/org/eclipse/swt/snippets/Snippet74.java
@@ -28,6 +28,10 @@ public static void main (String [] args) {
        Display display = new Display ();
        Shell shell = new Shell (display);
        shell.setText("Snippet 74");
+       shell.addPaintListener(e -> {
+               e.gc.setBackground(display.getSystemColor(SWT.COLOR_WIDGET_BACKGROUND));
+               e.gc.fillRectangle(e.x, e.y, e.width, e.height);
+       });
        Caret caret = new Caret (shell, SWT.NONE);
        caret.setBounds (10, 10, 2, 32);
        shell.open ();

Of course, changing all the snippets/examples/user code to explicitly require drawing backgrounds is a bad idea. My best guess as to why this isn't a problem on GTK3 is in GTK3 everything is Cairo already so when we go to draw the caret the cairo we are using knows what the "real" colour at those pixels are.

I am still investigating where the correct place to put this fill background is for GTK4. But I will split that out of this issue as the problem is not Caret specific, as this snippet demonstrates the problem:

public class Snippet999 {

public static void main (String [] args) {
	Display display = new Display ();
	Shell shell = new Shell (display);
	shell.setText("Snippet 999");
	shell.setLayout(new RowLayout());
	Button button = new Button(shell, SWT.TOGGLE);
	button.setText("Toggle");
	Canvas canvas = new Canvas(shell, SWT.NONE);
	canvas.setSize(100, 100);
	canvas.addPaintListener(e -> {
		if (button.getSelection()) {
			e.gc.setXORMode(true);
			e.gc.setBackground(display.getSystemColor(SWT.COLOR_WHITE));
			e.gc.fillRectangle(10, 10, 5, 20);
		}
	});
	button.addListener(SWT.Selection, e -> {
		canvas.redraw();
	});
	shell.pack();
	shell.open ();
	while (!shell.isDisposed ()) {
		if (!display.readAndDispatch ()) display.sleep ();
	}
	display.dispose ();
}
}

[jonahgraham]

In GTK4, each iteration calls Display.snapshotDrawProc 3 times:

1. Shell {Snippet 163} (not interested in this one yet)
2. StyledText {} snapshotDrawProc is called with handle value matching Control.fixedHandle here, which resolves to StyledText (from calling display.getWidget)
3. StyledText {} snapshotDrawProc is called with handle value matching Widget.handle here, which also resolves to StyledText (from calling display.getWidget)

My conclusion is that we need to skip gtk4_draw if we are asked to paint fixedHandle. The fixedHandle isn't in the correct place, and the real handle will get gtk4_draw later.

There are two ways to do this:

1. Override snapshotToDraw in Control and skip fixedHandle draws.
2. Change paintHandle() to return the correct control so that fixedHandle is not returned.

I am going to provide a PR for each and then in review plus with more testing we can decide what to do.

Edit: I don't think 2 can work, because in the cases where painting starts at the shell the fixedHandle will still be asked to contribute to the snapshot, so we have to skip it.