Applied-Software/ghostty
5
0
Fork 0
mirror of https://github.com/ghostty-org/ghostty.git synced 2025-08-02 14:57:31 +03:00
Code Issues Packages Projects Releases Wiki Activity

renderer: disable window-padding-color=extend in certain scenarios

Browse Source 
There are scenarios where this configuration looks bad. This commit
introduces some heuristics to prevent it. Here are the heuristics:

  * Extension is always enabled on alt screen.
  * Extension is disabled if a row contains any default bg color. The
    thinking is that in this scenario, using the default bg color looks
    just fine.
  * Extension is disabled if a row is marked as a prompt (using semantic
    prompt sequences). The thinking here is that prompts often contain
    perfect fit glyphs such as Powerline glyphs and those look bad when
    extended.

This introduces some CPU cost to the extension feature but it should be
minimal and respects dirty tracking. This is unfortunate but the feature
makes many terminal scenarios look much better and the performance cost
is minimal so I believe it is worth it.

Further heuristics are likely warranted but this should be a good
starting set.
This commit is contained in:
Mitchell Hashimoto
2024-08-03 21:35:56 -07:00
parent 2d418171df
commit ea551990eb
8 changed files with 189 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
pkg/opengl/Program.zig
View File

@ -104,6 +104,7 @@ pub fn setUniform(
// Perform the correct call depending on the type of the value.
switch (@TypeOf(value)) {
bool => glad.context.Uniform1i.?(loc, if (value) 1 else 0),
comptime_int => glad.context.Uniform1i.?(loc, value),
f32 => glad.context.Uniform1f.?(loc, value),
@Vector(2, f32) => glad.context.Uniform2f.?(loc, value[0], value[1]),

11
src/config/Config.zig
View File

@ -674,6 +674,17 @@ keybind: Keybinds = .{},
/// * `background` - The background color specified in `background`.
/// * `extend` - Extend the background color of the nearest grid cell.
///
/// The "extend" value will be disabled in certain scenarios. On primary
/// screen applications (i.e. not something like Neovim), the color will not
/// be extended vertically if any of the following are true:
///
/// * The nearest row has any cells that have the default backgroudn color.
/// The thinking is that in this case, the default background color looks
/// fine as a padding color.
/// * The nearest row is a prompt row (requires shell integration). The
/// thinking here is that prompts often contain powerline glyphs that
/// do not look good extended.
///
/// The default value is "extend". This allows for smooth resizing of a
/// terminal grid without having visible empty areas around the edge. The edge
/// cells may appear slightly larger due to the extension.

26
src/renderer/Metal.zig
View File

@ -625,6 +625,8 @@ pub fn init(alloc: Allocator, options: renderer.Options) !Metal {
.cell_size = undefined,
.grid_size = undefined,
.grid_padding = undefined,
.padding_extend_top = true,
.padding_extend_bottom = true,
.min_contrast = options.config.min_contrast,
.cursor_pos = .{ std.math.maxInt(u16), std.math.maxInt(u16) },
.cursor_color = undefined,
@ -872,6 +874,7 @@ pub fn updateFrame(
const Critical = struct {
bg: terminal.color.RGB,
screen: terminal.Screen,
screen_type: terminal.ScreenType,
mouse: renderer.State.Mouse,
preedit: ?renderer.State.Preedit,
cursor_style: ?renderer.CursorStyle,
@ -1001,6 +1004,7 @@ pub fn updateFrame(
break :critical .{
.bg = self.background_color,
.screen = screen_copy,
.screen_type = state.terminal.active_screen,
.mouse = state.mouse,
.preedit = preedit,
.cursor_style = cursor_style,
@ -1018,6 +1022,7 @@ pub fn updateFrame(
try self.rebuildCells(
critical.full_rebuild,
&critical.screen,
critical.screen_type,
critical.mouse,
critical.preedit,
critical.cursor_style,
@ -1988,6 +1993,8 @@ pub fn setScreenSize(
@floatFromInt(blank.bottom),
@floatFromInt(blank.left),
},
.padding_extend_top = old.padding_extend_top,
.padding_extend_bottom = old.padding_extend_bottom,
.min_contrast = old.min_contrast,
.cursor_pos = old.cursor_pos,
.cursor_color = old.cursor_color,
@ -2085,6 +2092,7 @@ fn rebuildCells(
self: *Metal,
rebuild: bool,
screen: *terminal.Screen,
screen_type: terminal.ScreenType,
mouse: renderer.State.Mouse,
preedit: ?renderer.State.Preedit,
cursor_style_: ?renderer.CursorStyle,
@ -2126,8 +2134,14 @@ fn rebuildCells(
};
} else null;
if (rebuild) {
// If we are doing a full rebuild, then we clear the entire cell buffer.
if (rebuild) self.cells.reset();
self.cells.reset();
// We also reset our padding extension depending on the screen type
self.uniforms.padding_extend_top = screen_type == .alternate;
self.uniforms.padding_extend_bottom = screen_type == .alternate;
}
// Go row-by-row to build the cells. We go row by row because we do
// font shaping by row. In the future, we will also do dirty tracking
@ -2159,6 +2173,16 @@ fn rebuildCells(
break :sel sel.containedRow(screen, pin) orelse null;
};
// On primary screen, we still apply vertical padding extension
// under certain conditions we feel are safe. This helps make some
// scenarios look better while avoiding scenarios we know do NOT look
// good.
if (y == 0 and screen_type == .primary) {
self.uniforms.padding_extend_top = !row.neverExtendBg();
} else if (y == self.cells.size.rows - 1 and screen_type == .primary) {
self.uniforms.padding_extend_bottom = !row.neverExtendBg();
}
// Split our row into runs and shape each one.
var iter = self.font_shaper.runIterator(
self.font_grid,

110
src/renderer/OpenGL.zig
View File

@ -53,12 +53,19 @@ grid_metrics: font.face.Metrics,
/// Current screen size dimensions for this grid. This is set on the first
/// resize event, and is not immediately available.
screen_size: ?renderer.ScreenSize,
grid_size: renderer.GridSize,
/// The current set of cells to render. Each set of cells goes into
/// a separate shader call.
cells_bg: std.ArrayListUnmanaged(CellProgram.Cell),
cells: std.ArrayListUnmanaged(CellProgram.Cell),
/// The last viewport that we based our rebuild off of. If this changes,
/// then we do a full rebuild of the cells. The pointer values in this pin
/// are NOT SAFE to read because they may be modified, freed, etc from the
/// termio thread. We treat the pointers as integers for comparison only.
cells_viewport: ?terminal.Pin = null,
/// The size of the cells list that was sent to the GPU. This is used
/// to detect when the cells array was reallocated/resized and handle that
/// accordingly.
@ -120,6 +127,10 @@ draw_mutex: DrawMutex = drawMutexZero,
/// terminal is in reversed mode.
draw_background: terminal.color.RGB,
/// Whether we're doing padding extension for vertical sides.
padding_extend_top: bool = true,
padding_extend_bottom: bool = true,
/// The images that we may render.
images: ImageMap = .{},
image_placements: ImagePlacementList = .{},
@ -394,6 +405,7 @@ pub fn init(alloc: Allocator, options: renderer.Options) !OpenGL {
.cells = .{},
.grid_metrics = grid.metrics,
.screen_size = null,
.grid_size = .{},
.gl_state = gl_state,
.font_grid = grid,
.font_shaper = shaper,
@ -669,6 +681,12 @@ pub fn setFontGrid(self: *OpenGL, grid: *font.SharedGrid) void {
self.font_shaper_cache.deinit(self.alloc);
self.font_shaper_cache = font_shaper_cache;
// Update our grid size if we have a screen size. If we don't, its okay
// because this will get set when we get the screen size set.
if (self.screen_size) |size| {
self.grid_size = self.gridSize(size);
}
// Defer our GPU updates
self.deferred_font_size = .{ .metrics = grid.metrics };
}
@ -684,8 +702,10 @@ pub fn updateFrame(
// Data we extract out of the critical area.
const Critical = struct {
full_rebuild: bool,
gl_bg: terminal.color.RGB,
screen: terminal.Screen,
screen_type: terminal.ScreenType,
mouse: renderer.State.Mouse,
preedit: ?renderer.State.Preedit,
cursor_style: ?renderer.CursorStyle,
@ -715,6 +735,9 @@ pub fn updateFrame(
self.foreground_color = bg;
}
// Get the viewport pin so that we can compare it to the current.
const viewport_pin = state.terminal.screen.pages.pin(.{ .viewport = .{} }).?;
// We used to share terminal state, but we've since learned through
// analysis that it is faster to copy the terminal state than to
// hold the lock wile rebuilding GPU cells.
@ -758,9 +781,55 @@ pub fn updateFrame(
try self.prepKittyGraphics(state.terminal);
}
// If we have any terminal dirty flags set then we need to rebuild
// the entire screen. This can be optimized in the future.
const full_rebuild: bool = rebuild: {
{
const Int = @typeInfo(terminal.Terminal.Dirty).Struct.backing_integer.?;
const v: Int = @bitCast(state.terminal.flags.dirty);
if (v > 0) break :rebuild true;
}
{
const Int = @typeInfo(terminal.Screen.Dirty).Struct.backing_integer.?;
const v: Int = @bitCast(state.terminal.screen.dirty);
if (v > 0) break :rebuild true;
}
// If our viewport changed then we need to rebuild the entire
// screen because it means we scrolled. If we have no previous
// viewport then we must rebuild.
const prev_viewport = self.cells_viewport orelse break :rebuild true;
if (!prev_viewport.eql(viewport_pin)) break :rebuild true;
break :rebuild false;
};
// Reset the dirty flags in the terminal and screen. We assume
// that our rebuild will be successful since so we optimize for
// success and reset while we hold the lock. This is much easier
// than coordinating row by row or as changes are persisted.
state.terminal.flags.dirty = .{};
state.terminal.screen.dirty = .{};
{
var it = state.terminal.screen.pages.pageIterator(
.right_down,
.{ .screen = .{} },
null,
);
while (it.next()) |chunk| {
var dirty_set = chunk.page.data.dirtyBitSet();
dirty_set.unsetAll();
}
}
// Update our viewport pin for dirty tracking
self.cells_viewport = viewport_pin;
break :critical .{
.full_rebuild = full_rebuild,
.gl_bg = self.background_color,
.screen = screen_copy,
.screen_type = state.terminal.active_screen,
.mouse = state.mouse,
.preedit = preedit,
.cursor_style = cursor_style,
@ -782,7 +851,9 @@ pub fn updateFrame(
// Build our GPU cells
try self.rebuildCells(
critical.full_rebuild,
&critical.screen,
critical.screen_type,
critical.mouse,
critical.preedit,
critical.cursor_style,
@ -1089,7 +1160,9 @@ fn prepKittyImage(
/// the renderer will do this when it needs more memory space.
pub fn rebuildCells(
self: *OpenGL,
rebuild: bool,
screen: *terminal.Screen,
screen_type: terminal.ScreenType,
mouse: renderer.State.Mouse,
preedit: ?renderer.State.Preedit,
cursor_style_: ?renderer.CursorStyle,
@ -1135,6 +1208,12 @@ pub fn rebuildCells(
// remains visible.
var cursor_cell: ?CellProgram.Cell = null;
if (rebuild) {
// We also reset our padding extension depending on the screen type
self.padding_extend_top = screen_type == .alternate;
self.padding_extend_bottom = screen_type == .alternate;
}
// Build each cell
var row_it = screen.pages.rowIterator(.right_down, .{ .viewport = .{} }, null);
var y: terminal.size.CellCountInt = 0;
@ -1185,6 +1264,16 @@ pub fn rebuildCells(
}
};
// On primary screen, we still apply vertical padding extension
// under certain conditions we feel are safe. This helps make some
// scenarios look better while avoiding scenarios we know do NOT look
// good.
if (y == 0 and screen_type == .primary) {
self.padding_extend_top = !row.neverExtendBg();
} else if (y == self.grid_size.rows - 1 and screen_type == .primary) {
self.padding_extend_bottom = !row.neverExtendBg();
}
// Split our row into runs and shape each one.
var iter = self.font_shaper.runIterator(
self.font_grid,
@ -1787,13 +1876,11 @@ pub fn setScreenSize(
// Store our screen size
self.screen_size = dim;
self.padding.explicit = pad;
// Recalculate the rows/columns.
const grid_size = self.gridSize(dim);
self.grid_size = self.gridSize(dim);
log.debug("screen size screen={} grid={} cell={} padding={}", .{
dim,
grid_size,
self.grid_size,
renderer.CellSize{
.width = self.grid_metrics.cell_width,
.height = self.grid_metrics.cell_height,
@ -1996,6 +2083,21 @@ fn drawCellProgram(
self.deferred_config = null;
}
// Apply our padding extension fields
{
const program = gl_state.cell_program;
const bind = try program.program.use();
defer bind.unbind();
try program.program.setUniform(
"padding_vertical_top",
self.padding_extend_top,
);
try program.program.setUniform(
"padding_vertical_bottom",
self.padding_extend_bottom,
);
}
// Draw background images first
try self.drawImages(
gl_state,

4
src/renderer/metal/shaders.zig
View File

@ -124,6 +124,10 @@ pub const Uniforms = extern struct {
/// top, right, bottom, left.
grid_padding: [4]f32 align(16),
/// True if vertical padding gets the extended color of the nearest row.
padding_extend_top: bool align(1),
padding_extend_bottom: bool align(1),
/// The minimum contrast ratio for text. The contrast ratio is calculated
/// according to the WCAG 2.0 spec.
min_contrast: f32 align(4),

11
src/renderer/shaders/cell.metal
View File

@ -5,6 +5,8 @@ struct Uniforms {
float2 cell_size;
ushort2 grid_size;
float4 grid_padding;
bool padding_extend_top;
bool padding_extend_bottom;
float min_contrast;
ushort2 cursor_pos;
uchar4 cursor_color;
@ -101,11 +103,14 @@ vertex CellBgVertexOut cell_bg_vertex(unsigned int vid [[vertex_id]],
cell_size_scaled.x = cell_size_scaled.x * input.cell_width;
// If we're at the edge of the grid, we add our padding to the background
// to extend it. Note: grid_padding is top/right/bottom/left.
if (input.grid_pos.y == 0) {
// to extend it. Note: grid_padding is top/right/bottom/left. We always
// extend horiziontally because there is no downside but there are various
// heuristics to disable vertical extension.
if (input.grid_pos.y == 0 && uniforms.padding_extend_top) {
cell_pos.y -= uniforms.grid_padding.r;
cell_size_scaled.y += uniforms.grid_padding.r;
} else if (input.grid_pos.y == uniforms.grid_size.y - 1) {
} else if (input.grid_pos.y == uniforms.grid_size.y - 1 &&
uniforms.padding_extend_bottom) {
cell_size_scaled.y += uniforms.grid_padding.b;
}
if (input.grid_pos.x == 0) {

6
src/renderer/shaders/cell.v.glsl
View File

@ -57,6 +57,8 @@ uniform sampler2D text_color;
uniform vec2 cell_size;
uniform vec2 grid_size;
uniform vec4 grid_padding;
uniform bool padding_vertical_top;
uniform bool padding_vertical_bottom;
uniform mat4 projection;
uniform float min_contrast;
@ -171,10 +173,10 @@ void main() {
case MODE_BG:
// If we're at the edge of the grid, we add our padding to the background
// to extend it. Note: grid_padding is top/right/bottom/left.
if (grid_coord.y == 0) {
if (grid_coord.y == 0 && padding_vertical_top) {
cell_pos.y -= grid_padding.r;
cell_size_scaled.y += grid_padding.r;
} else if (grid_coord.y == grid_size.y - 1) {
} else if (grid_coord.y == grid_size.y - 1 && padding_vertical_bottom) {
cell_size_scaled.y += grid_padding.b;
}
if (grid_coord.x == 0) {

29
src/terminal/PageList.zig
View File

@ -3254,6 +3254,35 @@ pub const Pin = struct {
set.set(self.y);
}
/// Returns true if the row of this pin should never have its background
/// color extended for filling padding space in the renderer. This is
/// a set of heuristics that help making our padding look better.
pub fn neverExtendBg(self: Pin) bool {
// Any semantic prompts should not have their background extended
// because prompts often contain special formatting (such as
// powerline) that looks bad when extended.
const rac = self.rowAndCell();
switch (rac.row.semantic_prompt) {
.prompt, .prompt_continuation, .input => return true,
.unknown, .command => {},
}
for (self.cells(.all)) |*cell| {
// If any cell has a default background color then we don't
// extend because the default background color probably looks
// good enough as an extension.
switch (cell.content_tag) {
.bg_color_palette, .bg_color_rgb => {},
.codepoint, .codepoint_grapheme => {
const s = self.style(cell);
if (s.bg_color == .none) return true;
},
}
}
return false;
}
/// Iterators. These are the same as PageList iterator funcs but operate
/// on pins rather than points. This is MUCH more efficient than calling
/// pointFromPin and building up the iterator from points.