From 503dfae6ffe1a10f5ebdad45d6b89060553d3cae Mon Sep 17 00:00:00 2001 From: Leah Amelia Chen Date: Tue, 6 Aug 2024 15:20:14 +0800 Subject: [PATCH 1/3] renderer: exempt Powerline cells from minimum contrast requirements With a minimum contrast set, the colored glyphs that Powerline uses would sometimes be set to white or black while the surrounding background colors remain unchanged, breaking up contiguous colors on segments of the Powerline. This no longer happens with this patch as Powerline glyphs are now special-cased and exempt from the minimum contrast adjustment. --- src/renderer/Metal.zig | 1 + src/renderer/OpenGL.zig | 9 +++------ src/renderer/cell.zig | 9 ++++++--- src/renderer/metal/shaders.zig | 1 + src/renderer/opengl/CellProgram.zig | 5 +++++ src/renderer/shaders/cell.f.glsl | 2 ++ src/renderer/shaders/cell.metal | 6 ++++++ src/renderer/shaders/cell.v.glsl | 7 +++++++ 8 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/renderer/Metal.zig b/src/renderer/Metal.zig index 7df88afb9..9b0f3cd2b 100644 --- a/src/renderer/Metal.zig +++ b/src/renderer/Metal.zig @@ -2481,6 +2481,7 @@ fn updateCell( .normal => .fg, .color => .fg_color, .constrained => .fg_constrained, + .powerline => .fg_powerline, }; try self.cells.add(self.alloc, .text, .{ diff --git a/src/renderer/OpenGL.zig b/src/renderer/OpenGL.zig index fc0e18390..dd989af62 100644 --- a/src/renderer/OpenGL.zig +++ b/src/renderer/OpenGL.zig @@ -1253,11 +1253,7 @@ pub fn rebuildCells( const screen_cell = row.cells(.all)[screen.cursor.x]; const x = screen.cursor.x - @intFromBool(screen_cell.wide == .spacer_tail); for (self.cells.items[start_i..]) |cell| { - if (cell.grid_col == x and - (cell.mode == .fg or - cell.mode == .fg_color or - cell.mode == .fg_constrained)) - { + if (cell.grid_col == x and cell.mode.isFg()) { cursor_cell = cell; break; } @@ -1382,7 +1378,7 @@ pub fn rebuildCells( _ = try self.addCursor(screen, cursor_style, cursor_color); if (cursor_cell) |*cell| { - if (cell.mode == .fg or cell.mode == .fg_constrained) { + if (cell.mode.isFg() and cell.mode != .fg_color) { const cell_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; @@ -1708,6 +1704,7 @@ fn updateCell( .normal => .fg, .color => .fg_color, .constrained => .fg_constrained, + .powerline => .fg_powerline, }; try self.cells.append(self.alloc, .{ diff --git a/src/renderer/cell.zig b/src/renderer/cell.zig index 92993f660..85c23e218 100644 --- a/src/renderer/cell.zig +++ b/src/renderer/cell.zig @@ -14,6 +14,10 @@ pub const FgMode = enum { /// size. If a glyph is larger than the cell then it must be resized /// to fit. constrained, + + /// Similar to normal, but the text consists of Powerline glyphs and is + /// optionally exempt from padding color extension and minimum contrast requirements. + powerline, }; /// Returns the appropriate foreground mode for the given cell. This is @@ -45,10 +49,9 @@ pub fn fgMode( break :text .normal; } - // We exempt the Powerline range from this since they exhibit - // box-drawing behavior and should not be constrained. + // Special-case Powerline glyphs if (isPowerline(cp)) { - break :text .normal; + break :text .powerline; } // If we are at the end of the screen its definitely constrained diff --git a/src/renderer/metal/shaders.zig b/src/renderer/metal/shaders.zig index 082c5ba5b..0652a3c98 100644 --- a/src/renderer/metal/shaders.zig +++ b/src/renderer/metal/shaders.zig @@ -319,6 +319,7 @@ pub const CellText = extern struct { fg_constrained = 2, fg_color = 3, cursor = 4, + fg_powerline = 5, }; }; diff --git a/src/renderer/opengl/CellProgram.zig b/src/renderer/opengl/CellProgram.zig index 2d4438a4f..b77c3904c 100644 --- a/src/renderer/opengl/CellProgram.zig +++ b/src/renderer/opengl/CellProgram.zig @@ -53,6 +53,7 @@ pub const CellMode = enum(u8) { fg = 2, fg_constrained = 3, fg_color = 7, + fg_powerline = 15, // Non-exhaustive because masks change it _, @@ -61,6 +62,10 @@ pub const CellMode = enum(u8) { pub fn mask(self: CellMode, m: CellMode) CellMode { return @enumFromInt(@intFromEnum(self) | @intFromEnum(m)); } + + pub fn isFg(self: CellMode) bool { + return @intFromEnum(self) & @intFromEnum(@as(CellMode, .fg)) != 0; + } }; pub fn init() !CellProgram { diff --git a/src/renderer/shaders/cell.f.glsl b/src/renderer/shaders/cell.f.glsl index 7c2a55370..f9c1ce2b1 100644 --- a/src/renderer/shaders/cell.f.glsl +++ b/src/renderer/shaders/cell.f.glsl @@ -28,6 +28,7 @@ const uint MODE_BG = 1u; const uint MODE_FG = 2u; const uint MODE_FG_CONSTRAINED = 3u; const uint MODE_FG_COLOR = 7u; +const uint MODE_FG_POWERLINE = 15u; void main() { float a; @@ -39,6 +40,7 @@ void main() { case MODE_FG: case MODE_FG_CONSTRAINED: + case MODE_FG_POWERLINE: a = texture(text, glyph_tex_coords).r; vec3 premult = color.rgb * color.a; out_FragColor = vec4(premult.rgb*a, a); diff --git a/src/renderer/shaders/cell.metal b/src/renderer/shaders/cell.metal index df2f79de4..fd8d8ee3d 100644 --- a/src/renderer/shaders/cell.metal +++ b/src/renderer/shaders/cell.metal @@ -162,6 +162,7 @@ enum CellTextMode : uint8_t { MODE_TEXT_CONSTRAINED = 2u, MODE_TEXT_COLOR = 3u, MODE_TEXT_CURSOR = 4u, + MODE_TEXT_POWERLINE = 4u, }; struct CellTextVertexIn { @@ -263,6 +264,10 @@ vertex CellTextVertexOut cell_text_vertex(unsigned int vid [[vertex_id]], // If we have a minimum contrast, we need to check if we need to // change the color of the text to ensure it has enough contrast // with the background. + // We only apply this adjustment to "normal" text with MODE_TEXT, + // since we want color glyphs to appear in their original color + // and Powerline glyphs to be unaffected (else parts of the line would + // have different colors as some parts are displayed via background colors). if (uniforms.min_contrast > 1.0f && input.mode == MODE_TEXT) { float4 bg_color = float4(input.bg_color) / 255.0f; out.color = contrasted_color(uniforms.min_contrast, out.color, bg_color); @@ -288,6 +293,7 @@ fragment float4 cell_text_fragment(CellTextVertexOut in [[stage_in]], switch (in.mode) { case MODE_TEXT_CURSOR: case MODE_TEXT_CONSTRAINED: + case MODE_TEXT_POWERLINE: case MODE_TEXT: { // Normalize the texture coordinates to [0,1] float2 size = diff --git a/src/renderer/shaders/cell.v.glsl b/src/renderer/shaders/cell.v.glsl index 3c533f2ad..942b7ac44 100644 --- a/src/renderer/shaders/cell.v.glsl +++ b/src/renderer/shaders/cell.v.glsl @@ -8,6 +8,7 @@ const uint MODE_BG = 1u; const uint MODE_FG = 2u; const uint MODE_FG_CONSTRAINED = 3u; const uint MODE_FG_COLOR = 7u; +const uint MODE_FG_POWERLINE = 15u; // The grid coordinates (x, y) where x < columns and y < rows layout (location = 0) in vec2 grid_coord; @@ -198,6 +199,7 @@ void main() { case MODE_FG: case MODE_FG_CONSTRAINED: case MODE_FG_COLOR: + case MODE_FG_POWERLINE: vec2 glyph_offset_calc = glyph_offset; // The glyph_offset.y is the y bearing, a y value that when added @@ -227,6 +229,7 @@ void main() { ivec2 text_size; switch(mode) { case MODE_FG_CONSTRAINED: + case MODE_FG_POWERLINE: case MODE_FG: text_size = textureSize(text, 0); break; @@ -242,6 +245,10 @@ void main() { // If we have a minimum contrast, we need to check if we need to // change the color of the text to ensure it has enough contrast // with the background. + // We only apply this adjustment to "normal" text with MODE_FG, + // since we want color glyphs to appear in their original color + // and Powerline glyphs to be unaffected (else parts of the line would + // have different colors as some parts are displayed via background colors). vec4 color_final = color_in / 255.0; if (min_contrast > 1.0 && mode == MODE_FG) { vec4 bg_color = bg_color_in / 255.0; From 5be3098963fcceb9f048e2aa69c251c3c6b586c4 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 6 Aug 2024 10:04:56 -0700 Subject: [PATCH 2/3] comment --- src/renderer/cell.zig | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/renderer/cell.zig b/src/renderer/cell.zig index 85c23e218..064aabc87 100644 --- a/src/renderer/cell.zig +++ b/src/renderer/cell.zig @@ -49,7 +49,10 @@ pub fn fgMode( break :text .normal; } - // Special-case Powerline glyphs + // Special-case Powerline glyphs. They exhibit box drawing behavior + // and should not be constrained. They have their own special category + // though because they're used for other logic (i.e. disabling + // min contrast). if (isPowerline(cp)) { break :text .powerline; } From 247d5e4411fae4deeaa4406850dd9b162d4dc718 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 6 Aug 2024 10:07:08 -0700 Subject: [PATCH 3/3] renderer/opengl: comptime assertion to verify --- src/renderer/opengl/CellProgram.zig | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/renderer/opengl/CellProgram.zig b/src/renderer/opengl/CellProgram.zig index b77c3904c..48386362e 100644 --- a/src/renderer/opengl/CellProgram.zig +++ b/src/renderer/opengl/CellProgram.zig @@ -64,6 +64,13 @@ pub const CellMode = enum(u8) { } pub fn isFg(self: CellMode) bool { + // Since we use bit tricks below, we want to ensure the enum + // doesn't change without us looking at this logic again. + comptime { + const info = @typeInfo(CellMode).Enum; + std.debug.assert(info.fields.len == 5); + } + return @intFromEnum(self) & @intFromEnum(@as(CellMode, .fg)) != 0; } };