feat: support selection foreground being cell foreground

[dal-liu]

This resolves #2685.

Changes

• Created SelectionColor tagged union that can take a Color, cell-foreground, or cell-background
• Used the new union to implement the feature for Metal and OpenGL
• Removed the use of invert_selection_fg_bg during rendering as suggested in the issue

Demo

macOS:

https://github.com/user-attachments/assets/b5b2db83-bb62-4929-8e3c-870a1e1a7a5c

Ubuntu:

https://github.com/user-attachments/assets/259dd707-d9d9-4590-8f3c-a67ccdef34bc

Any feedback would be helpful, I'm sure there's a lot of room for improvement here.

[kjmph]

I like this patch. A couple of minor things, when there are inverted cells, this patch shows the original foreground, rather than the inverted foreground. If you compare to Terminal.app, it will always show the inverted foreground when selecting inverted cells. Additionally, very oddly, the theme's cursor-text seems to not be honored with this patch, unless this is a main vs 1.0.1 change?

If any example code could help, here is my personal patch (from the 1.0.1 tag) that demonstrates equivalence with Terminal.app selection & cursor behavior, with the exception that I have a custom theme with no selection-foreground specified. I also made a change to cursor rendering so that the cell-foreground is used. Can we support cell-foreground with cursor-text as well?

diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig
index b37f440f..a2ef6c16 100644
--- a/src/renderer/Metal.zig
+++ b/src/renderer/Metal.zig
@@ -2435,8 +2435,17 @@ fn rebuildCells(
                 if (selected and !self.config.invert_selection_fg_bg) {
                     // If we don't have invert selection fg/bg set
                     // then we just use the selection foreground if
-                    // set, otherwise the default bg color.
-                    break :fg self.config.selection_foreground orelse self.background_color orelse self.default_background_color;
+                    // set, otherwise the existing fg color.
+                    if (style.flags.inverse) {
+                        break :fg self.config.selection_foreground orelse bg_style orelse self.background_color orelse self.default_background_color;
+                    } else {
+                        break :fg self.config.selection_foreground orelse fg_style;
+                    }
+                    // Except, all themes set selection_foreground! We
+                    // want a theme that doesn't set
+                    // selection_foreground. We could always force
+                    // fg_style, but that isn't a general solution
+                    //break :fg fg_style;
                 }
 
                 // Whether we need to use the bg color as our fg color:
@@ -2671,8 +2680,14 @@ fn rebuildCells(
                 break :blk sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color;
             } else if (self.config.cursor_text) |txt|
                 txt
-            else
-                self.background_color orelse self.default_background_color;
+            else base: {
+                const sty = screen.cursor.page_pin.style(screen.cursor.page_cell);
+                if (sty.flags.inverse) {
+                    break :base sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color;
+                } else {
+                    break :base sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color;
+                }
+            };
 
             self.uniforms.cursor_color = .{
                 uniform_color.r,

[dal-liu]

@kjmph Thanks for catching the inverted cell issue, it should hopefully be fixed now. I've also made cursor-text and cursor-color accept cell-foreground and cell-background the same way that the selection colors do.

[kjmph]

Haha, great improvement! Except for two things. 1) If the cursor-color is cell-foreground then it wipes out the cell's text. If cursor-color is cell-background then it can't be seen because it blends in. I think it wise to remove the support from cursor-color since it will lead people astray into odd configuration territory. 2) The cursor foreground is wiped out with inverted cells. I don't understand why that happens with your PR, unless it exists in the upstream main branch? I'm personally working on the 1.0.1 tag.

Here is an image showing this patch (and cursor-text = cell-foreground) with inverted cells:

Here is an image showing the patch I shared against 1.0.1 with inverted cells:

I'll give a test against main.

[kjmph]

Oh!!! The cursor wiping out the foreground is a problem upstream, it isn't a regression with this patch. I bet most people allow the shell integration for cursors and because it is a bar, don't notice the color mishap. Will have to open a ticket unless you want to fix it in this patch too?

[dal-liu]

To me, changing cursor-color to accept cell colors makes sense since we should have more control over the colors, plus it was suggested in the original issue as well. That being said, if a 3rd opinion disagrees then I'll gladly revert the changes, so I will leave it for now.

As for the cursor-text thing, it's definitely worth opening a discussion about it.

[cwrau]

Hello,

just wondering, does this allow me to configure ghostty to behave like gnome-terminal?/

https://github.com/user-attachments/assets/acb46027-9b39-4b33-b8de-70ed42671986

Left ist gnome-terminal and right is ghostty. What I specifically want, is that the blinking cursor block has the same background color as the foreground of the text and the foreground to be the inverted foreground-color of the text (at least that's what I can figure is what gnome-terminal does)

[kjmph]

@cwrau The PR looks like it could match gnome-terminal, but I don't have your precise setup. Here is the best replication I can draw:

But again, I'm using block_hollow to show this. Otherwise, the block will wipe out the foreground because it matches the inverted foreground color in my example. (As it won't invert the text color like gnome-terminal.)

I think you would want a cursor-invert-fg-bg = true corollary to still be to get the same behavior as gnome-terminal (although it will have to be blink aware). I notice my patch also has this problem, and I get away with it via a non-standard cursor color by default. I fixed it in my personal patch, which I attached the diff from the 1.0.1 tag. Note, if you wanted to try it you would have to be on a Mac, and you would need to remove selection-foreground from your theme in: Ghostty.app/Contents/Resources/ghostty/themes/<your_theme>

Then, set the config:

shell-integration-features = no-cursor
cursor-style = block
cursor-style-blink = true
cursor-invert-fg-bg = true

That should match gnome terminal in your example. See attached video:

https://github.com/user-attachments/assets/b1f85db6-6efc-4a8a-86a9-e3298b503001

Patch:

diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig
index b37f440f..db41d413 100644
--- a/src/renderer/Metal.zig
+++ b/src/renderer/Metal.zig
@@ -2435,8 +2435,17 @@ fn rebuildCells(
                 if (selected and !self.config.invert_selection_fg_bg) {
                     // If we don't have invert selection fg/bg set
                     // then we just use the selection foreground if
-                    // set, otherwise the default bg color.
-                    break :fg self.config.selection_foreground orelse self.background_color orelse self.default_background_color;
+                    // set, otherwise the existing fg color.
+                    if (style.flags.inverse) {
+                        break :fg self.config.selection_foreground orelse bg_style orelse self.background_color orelse self.default_background_color;
+                    } else {
+                        break :fg self.config.selection_foreground orelse fg_style;
+                    }
+                    // Except, all themes set selection_foreground! We
+                    // want a theme that doesn't set
+                    // selection_foreground. We could always force
+                    // fg_style, but that isn't a general solution
+                    //break :fg fg_style;
                 }
 
                 // Whether we need to use the bg color as our fg color:
@@ -2638,7 +2647,11 @@ fn rebuildCells(
         const cursor_color = self.cursor_color orelse self.default_cursor_color orelse color: {
             if (self.cursor_invert) {
                 const sty = screen.cursor.page_pin.style(screen.cursor.page_cell);
-                break :color sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color;
+                if (sty.flags.inverse) {
+                    break :color sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color;
+                } else {
+                    break :color sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color;
+                }
             } else {
                 break :color self.foreground_color orelse self.default_foreground_color;
             }
@@ -2668,11 +2681,21 @@ fn rebuildCells(
 
             const uniform_color = if (self.cursor_invert) blk: {
                 const sty = screen.cursor.page_pin.style(screen.cursor.page_cell);
-                break :blk sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color;
+                if (sty.flags.inverse) {
+                    break :blk sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color;
+                } else {
+                    break :blk sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color;
+                }
             } else if (self.config.cursor_text) |txt|
                 txt
-            else
-                self.background_color orelse self.default_background_color;
+            else base: {
+                const sty = screen.cursor.page_pin.style(screen.cursor.page_cell);
+                if (sty.flags.inverse) {
+                    break :base sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color;
+                } else {
+                    break :base sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color;
+                }
+            };
 
             self.uniforms.cursor_color = .{
                 uniform_color.r,

[kjmph]

Actually, I stand corrected, with this PR, I can get close to gnome-terminal @cwrau:

shell-integration-features = no-cursor
cursor-style = block
cursor-text = cell-background
cursor-style-blink = true
cursor-color = cell-foreground

But the foreground colors look saturated incorrectly? (see the S and the E here:)

[cwrau]

Actually, I stand corrected, with this PR, I can get close to gnome-terminal @cwrau:

shell-integration-features = no-cursor
cursor-style = block
cursor-text = cell-background
cursor-style-blink = true
cursor-color = cell-foreground

But the foreground colors look saturated incorrectly? (see the S and the E here:)

Amazing! Looking forward to this PR 😁

[kjmph]

Anything I can assist with this PR? It would be nice to stop maintaining a private patch.

[kjmph]

I will say, my personal patches are reduced after 1.1.0 was released:

diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig
index 866f9682..7561f1a7 100644
--- a/src/renderer/Metal.zig
+++ b/src/renderer/Metal.zig
@@ -2635,8 +2635,17 @@ fn rebuildCells(
                 if (selected and !self.config.invert_selection_fg_bg) {
                     // If we don't have invert selection fg/bg set
                     // then we just use the selection foreground if
-                    // set, otherwise the default bg color.
-                    break :fg self.config.selection_foreground orelse self.background_color orelse self.default_background_color;
+                    // set, otherwise the existing fg color.
+                    if (style.flags.inverse) {
+                        break :fg self.config.selection_foreground orelse bg_style orelse self.background_color orelse self.default_background_color;
+                    } else {
+                        break :fg self.config.selection_foreground orelse fg_style;
+                    }
+                    // Except, all themes set selection_foreground! We
+                    // want a theme that doesn't set
+                    // selection_foreground. We could always force
+                    // fg_style, but that isn't a general solution
+                    //break :fg fg_style;
                 }
 
                 // Whether we need to use the bg color as our fg color:
@@ -2879,8 +2888,13 @@ fn rebuildCells(
                     (sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color);
             } else if (self.config.cursor_text) |txt|
                 txt
-            else
-                self.background_color orelse self.default_background_color;
+            else base: {
+                const sty = screen.cursor.page_pin.style(screen.cursor.page_cell);
+                break :base if (sty.flags.inverse)
+                    (sty.bg(screen.cursor.page_cell, color_palette) orelse self.background_color orelse self.default_background_color)
+                else
+                    (sty.fg(color_palette, self.config.bold_is_bright) orelse self.foreground_color orelse self.default_foreground_color);
+            };
 
             self.uniforms.cursor_color = .{
                 uniform_color.r,

[mitchellh]

Apologies for the large delay on this. I'm open to this but can you rebase it on the latest renderer refactor and I'm happy to take another look.

[mitchellh]

Pushed up backwards compatibility handlers for the old fields. I'm going to address some other feedback but have to step away for now.

[kjmph]

Apologies, @mitchellh, I know this is considered closed. Yet, if cursor-text = cell-foreground is set, I can see the cursor-text looks different. I posted here: https://github.com/ghostty-org/ghostty/pull/5219#issuecomment-2620007145

The function cell_text_vertex in src/renderer/shaders/shaders.metal seems to uses linear blending for all other cells, but the cursor cell uses the Display P3 color directly. If I set it to use linear coloring then it matches all other cells. Like so:

diff --git a/src/renderer/shaders/shaders.metal b/src/renderer/shaders/shaders.metal
index 4797f89e4..4e02b6336 100644
--- a/src/renderer/shaders/shaders.metal
+++ b/src/renderer/shaders/shaders.metal
@@ -665,13 +665,13 @@ vertex CellTextVertexOut cell_text_vertex(
   // If this cell is the cursor cell, but we're not processing
   // the cursor glyph itself, then we need to change the color.
   if ((in.bools & IS_CURSOR_GLYPH) == 0 && is_cursor_pos) {
     out.color = load_color(
       uniforms.cursor_color,
       uniforms.use_display_p3,
-      false
+      true
     );
   }
 
   return out;
 }
 

Would you prefer if this was a new issue?

[00-kat]

Would you prefer if this was a new issue?

That probably would be a good idea, I'm not sure if anyone is going to see your comment.