osc: once again, fix up osd color operations

Fixes #7951

PR #7429 did a lot to improve handling of various OSC escape codes that
get/set/reset colors. However, it didn't get it _quite_ right. This PR
fixes OSC color handling to hopefully get things closer to perfect.
This commit is contained in:
Jeffrey C. Ollie
2025-07-15 15:13:42 -05:00
parent f44c24ef88
commit c96dd58401
5 changed files with 1122 additions and 199 deletions

View File

@ -857,7 +857,7 @@ pub fn handleMessage(self: *Surface, msg: Message) !void {
}, .unlocked); }, .unlocked);
}, },
.color_change => |change| { .color_change => |change| color_change: {
// Notify our apprt, but don't send a mode 2031 DSR report // Notify our apprt, but don't send a mode 2031 DSR report
// because VT sequences were used to change the color. // because VT sequences were used to change the color.
_ = try self.rt_app.performAction( _ = try self.rt_app.performAction(
@ -865,10 +865,26 @@ pub fn handleMessage(self: *Surface, msg: Message) !void {
.color_change, .color_change,
.{ .{
.kind = switch (change.kind) { .kind = switch (change.kind) {
.background => .background, .palette => |color| @enumFromInt(color),
.foreground => .foreground, .dynamic_color => |color| switch (color) {
.foreground => .background,
.background => .foreground,
.cursor => .cursor, .cursor => .cursor,
.palette => |v| @enumFromInt(v), .pointer_foreground,
.pointer_background,
.tektronix_foreground,
.tektronix_background,
.highlight_background,
.tektronix_cursor,
.highlight_foreground,
=> {
log.warn("changing dynamic color {d} ({s}) is not implemented", .{
@intFromEnum(color),
@tagName(color),
});
break :color_change;
},
},
}, },
.r = change.color.r, .r = change.color.r,
.g = change.color.g, .g = change.color.g,

View File

@ -897,14 +897,13 @@ test "osc: 112 incomplete sequence" {
const cmd = a[0].?.osc_dispatch; const cmd = a[0].?.osc_dispatch;
try testing.expect(cmd == .color_operation); try testing.expect(cmd == .color_operation);
try testing.expectEqual(cmd.color_operation.terminator, .bel); try testing.expectEqual(cmd.color_operation.terminator, .bel);
try testing.expect(cmd.color_operation.source == .reset_cursor);
try testing.expect(cmd.color_operation.operations.count() == 1); try testing.expect(cmd.color_operation.operations.count() == 1);
var it = cmd.color_operation.operations.constIterator(0); var it = cmd.color_operation.operations.constIterator(0);
{ {
const op = it.next().?; const op = it.next().?;
try testing.expect(op.* == .reset); try testing.expect(op.* == .reset);
try testing.expectEqual( try testing.expectEqual(
osc.Command.ColorOperation.Kind.cursor, osc.Command.ColorOperation.Kind{ .dynamic_color = .cursor },
op.reset, op.reset,
); );
} }

File diff suppressed because it is too large Load Diff

View File

@ -1557,7 +1557,7 @@ pub fn Stream(comptime Handler: type) type {
.color_operation => |v| { .color_operation => |v| {
if (@hasDecl(T, "handleColorOperation")) { if (@hasDecl(T, "handleColorOperation")) {
try self.handler.handleColorOperation(v.source, &v.operations, v.terminator); try self.handler.handleColorOperation(&v.operations, v.terminator);
return; return;
} else log.warn("unimplemented OSC callback: {}", .{cmd}); } else log.warn("unimplemented OSC callback: {}", .{cmd});
}, },

View File

@ -1187,7 +1187,6 @@ pub const StreamHandler = struct {
pub fn handleColorOperation( pub fn handleColorOperation(
self: *StreamHandler, self: *StreamHandler,
source: terminal.osc.Command.ColorOperation.Source,
operations: *const terminal.osc.Command.ColorOperation.List, operations: *const terminal.osc.Command.ColorOperation.List,
terminator: terminal.osc.Terminator, terminator: terminal.osc.Terminator,
) !void { ) !void {
@ -1201,21 +1200,19 @@ pub const StreamHandler = struct {
var response: std.ArrayListUnmanaged(u8) = .empty; var response: std.ArrayListUnmanaged(u8) = .empty;
const writer = response.writer(alloc); const writer = response.writer(alloc);
var report: bool = false;
try writer.print("\x1b]{}", .{source});
var it = operations.constIterator(0); var it = operations.constIterator(0);
while (it.next()) |op| { while (it.next()) |op| {
switch (op.*) { switch (op.*) {
.set => |set| { .set => |set| set: {
switch (set.kind) { switch (set.kind) {
.palette => |i| { .palette => |i| {
self.terminal.flags.dirty.palette = true; self.terminal.flags.dirty.palette = true;
self.terminal.color_palette.colors[i] = set.color; self.terminal.color_palette.colors[i] = set.color;
self.terminal.color_palette.mask.set(i); self.terminal.color_palette.mask.set(i);
}, },
.dynamic_color => |i| {
switch (i) {
.foreground => { .foreground => {
self.foreground_color = set.color; self.foreground_color = set.color;
_ = self.renderer_mailbox.push(.{ _ = self.renderer_mailbox.push(.{
@ -1234,6 +1231,22 @@ pub const StreamHandler = struct {
.cursor_color = set.color, .cursor_color = set.color,
}, .{ .forever = {} }); }, .{ .forever = {} });
}, },
.pointer_foreground,
.pointer_background,
.tektronix_foreground,
.tektronix_background,
.highlight_background,
.tektronix_cursor,
.highlight_foreground,
=> {
log.warn("setting dynamic color {} ({s}) not implemented", .{
i,
@tagName(i),
});
break :set;
},
}
},
} }
// Notify the surface of the color change // Notify the surface of the color change
@ -1243,7 +1256,7 @@ pub const StreamHandler = struct {
} }); } });
}, },
.reset => |kind| { .reset => |kind| reset: {
switch (kind) { switch (kind) {
.palette => |i| { .palette => |i| {
const mask = &self.terminal.color_palette.mask; const mask = &self.terminal.color_palette.mask;
@ -1258,6 +1271,8 @@ pub const StreamHandler = struct {
}, },
}); });
}, },
.dynamic_color => |i| {
switch (i) {
.foreground => { .foreground => {
self.foreground_color = null; self.foreground_color = null;
_ = self.renderer_mailbox.push(.{ _ = self.renderer_mailbox.push(.{
@ -1265,7 +1280,7 @@ pub const StreamHandler = struct {
}, .{ .forever = {} }); }, .{ .forever = {} });
self.surfaceMessageWriter(.{ .color_change = .{ self.surfaceMessageWriter(.{ .color_change = .{
.kind = .foreground, .kind = kind,
.color = self.default_foreground_color, .color = self.default_foreground_color,
} }); } });
}, },
@ -1276,7 +1291,7 @@ pub const StreamHandler = struct {
}, .{ .forever = {} }); }, .{ .forever = {} });
self.surfaceMessageWriter(.{ .color_change = .{ self.surfaceMessageWriter(.{ .color_change = .{
.kind = .background, .kind = kind,
.color = self.default_background_color, .color = self.default_background_color,
} }); } });
}, },
@ -1289,33 +1304,63 @@ pub const StreamHandler = struct {
if (self.default_cursor_color) |color| { if (self.default_cursor_color) |color| {
self.surfaceMessageWriter(.{ .color_change = .{ self.surfaceMessageWriter(.{ .color_change = .{
.kind = .cursor, .kind = kind,
.color = color, .color = color,
} }); } });
} }
}, },
.pointer_foreground,
.pointer_background,
.tektronix_foreground,
.tektronix_background,
.highlight_background,
.tektronix_cursor,
.highlight_foreground,
=> {
log.warn("resetting dynamic color {} ({s}) not implemented", .{
i,
@tagName(i),
});
break :reset;
},
}
},
} }
}, },
.report => |kind| report: { .report => |kind| report: {
if (self.osc_color_report_format == .none) break :report; if (self.osc_color_report_format == .none) break :report;
report = true;
const color = switch (kind) { const color = switch (kind) {
.palette => |i| self.terminal.color_palette.colors[i], .palette => |i| self.terminal.color_palette.colors[i],
.dynamic_color => |i| switch (i) {
.foreground => self.foreground_color orelse self.default_foreground_color, .foreground => self.foreground_color orelse self.default_foreground_color,
.background => self.background_color orelse self.default_background_color, .background => self.background_color orelse self.default_background_color,
.cursor => self.cursor_color orelse .cursor => self.cursor_color orelse
self.default_cursor_color orelse self.default_cursor_color orelse
self.foreground_color orelse self.foreground_color orelse
self.default_foreground_color, self.default_foreground_color,
.pointer_foreground,
.pointer_background,
.tektronix_foreground,
.tektronix_background,
.highlight_background,
.tektronix_cursor,
.highlight_foreground,
=> {
log.warn("reporting dynamic color {} ({s}) not implemented", .{
i,
@tagName(i),
});
break :report;
},
},
}; };
switch (self.osc_color_report_format) { switch (self.osc_color_report_format) {
.@"16-bit" => switch (kind) { .@"16-bit" => switch (kind) {
.palette => |i| try writer.print( .palette => |i| try writer.print(
";{d};rgb:{x:0>4}/{x:0>4}/{x:0>4}", "\x1b]4;{d};rgb:{x:0>4}/{x:0>4}/{x:0>4}",
.{ .{
i, i,
@as(u16, color.r) * 257, @as(u16, color.r) * 257,
@ -1323,9 +1368,10 @@ pub const StreamHandler = struct {
@as(u16, color.b) * 257, @as(u16, color.b) * 257,
}, },
), ),
else => try writer.print( .dynamic_color => |i| try writer.print(
";rgb:{x:0>4}/{x:0>4}/{x:0>4}", "\x1b]{};rgb:{x:0>4}/{x:0>4}/{x:0>4}",
.{ .{
i,
@as(u16, color.r) * 257, @as(u16, color.r) * 257,
@as(u16, color.g) * 257, @as(u16, color.g) * 257,
@as(u16, color.b) * 257, @as(u16, color.b) * 257,
@ -1335,7 +1381,7 @@ pub const StreamHandler = struct {
.@"8-bit" => switch (kind) { .@"8-bit" => switch (kind) {
.palette => |i| try writer.print( .palette => |i| try writer.print(
";{d};rgb:{x:0>2}/{x:0>2}/{x:0>2}", "\x1b]4;{d};rgb:{x:0>2}/{x:0>2}/{x:0>2}",
.{ .{
i, i,
@as(u16, color.r), @as(u16, color.r),
@ -1343,9 +1389,10 @@ pub const StreamHandler = struct {
@as(u16, color.b), @as(u16, color.b),
}, },
), ),
else => try writer.print( .dynamic_color => |i| try writer.print(
";rgb:{x:0>2}/{x:0>2}/{x:0>2}", "\x1b]{};rgb:{x:0>2}/{x:0>2}/{x:0>2}",
.{ .{
i,
@as(u16, color.r), @as(u16, color.r),
@as(u16, color.g), @as(u16, color.g),
@as(u16, color.b), @as(u16, color.b),
@ -1355,13 +1402,14 @@ pub const StreamHandler = struct {
.none => unreachable, .none => unreachable,
} }
try writer.writeAll(terminator.string());
}, },
} }
} }
if (report) {
// If any of the operations were reports, finalize the report if (response.items.len > 0) {
// string and send it to the terminal. // If any of the operations were reports send it to the terminal.
try writer.writeAll(terminator.string());
const msg = try termio.Message.writeReq(self.alloc, response.items); const msg = try termio.Message.writeReq(self.alloc, response.items);
self.messageWriter(msg); self.messageWriter(msg);
} }