Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metal alpha blending fixes + color handling improvements #4913

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 0 additions & 16 deletions macos/Sources/Features/QuickTerminal/QuickTerminalController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -375,19 +375,6 @@ class QuickTerminalController: BaseTerminalController {
// Some APIs such as window blur have no effect unless the window is visible.
guard window.isVisible else { return }

// Terminals typically operate in sRGB color space and macOS defaults
// to "native" which is typically P3. There is a lot more resources
// covered in this GitHub issue: https://github.com/mitchellh/ghostty/pull/376
// Ghostty defaults to sRGB but this can be overridden.
switch (self.derivedConfig.windowColorspace) {
case "display-p3":
window.colorSpace = .displayP3
case "srgb":
fallthrough
default:
window.colorSpace = .sRGB
}

// If we have window transparency then set it transparent. Otherwise set it opaque.
if (self.derivedConfig.backgroundOpacity < 1) {
window.isOpaque = false
Expand Down Expand Up @@ -457,15 +444,13 @@ class QuickTerminalController: BaseTerminalController {
let quickTerminalAnimationDuration: Double
let quickTerminalAutoHide: Bool
let quickTerminalSpaceBehavior: QuickTerminalSpaceBehavior
let windowColorspace: String
let backgroundOpacity: Double

init() {
self.quickTerminalScreen = .main
self.quickTerminalAnimationDuration = 0.2
self.quickTerminalAutoHide = true
self.quickTerminalSpaceBehavior = .move
self.windowColorspace = ""
self.backgroundOpacity = 1.0
}

Expand All @@ -474,7 +459,6 @@ class QuickTerminalController: BaseTerminalController {
self.quickTerminalAnimationDuration = config.quickTerminalAnimationDuration
self.quickTerminalAutoHide = config.quickTerminalAutoHide
self.quickTerminalSpaceBehavior = config.quickTerminalSpaceBehavior
self.windowColorspace = config.windowColorspace
self.backgroundOpacity = config.backgroundOpacity
}
}
Expand Down
13 changes: 0 additions & 13 deletions macos/Sources/Features/Terminal/TerminalController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -366,19 +366,6 @@ class TerminalController: BaseTerminalController {
// If window decorations are disabled, remove our title
if (!config.windowDecorations) { window.styleMask.remove(.titled) }

// Terminals typically operate in sRGB color space and macOS defaults
// to "native" which is typically P3. There is a lot more resources
// covered in this GitHub issue: https://github.com/mitchellh/ghostty/pull/376
// Ghostty defaults to sRGB but this can be overridden.
switch (config.windowColorspace) {
case "display-p3":
window.colorSpace = .displayP3
case "srgb":
fallthrough
default:
window.colorSpace = .sRGB
}

// If we have only a single surface (no splits) and that surface requested
// an initial size then we set it here now.
if case let .leaf(leaf) = surfaceTree {
Expand Down
9 changes: 0 additions & 9 deletions macos/Sources/Ghostty/Ghostty.Config.swift
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,6 @@ extension Ghostty {
return v
}

var windowColorspace: String {
guard let config = self.config else { return "" }
var v: UnsafePointer<Int8>? = nil
let key = "window-colorspace"
guard ghostty_config_get(config, &v, key, UInt(key.count)) else { return "" }
guard let ptr = v else { return "" }
return String(cString: ptr)
}

var windowSaveState: String {
guard let config = self.config else { return "" }
var v: UnsafePointer<Int8>? = nil
Expand Down
63 changes: 63 additions & 0 deletions pkg/macos/graphics/color_space.zig
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,72 @@ pub const ColorSpace = opaque {
) orelse Allocator.Error.OutOfMemory;
}

pub fn createNamed(name: Name) Allocator.Error!*ColorSpace {
return @as(
?*ColorSpace,
@ptrFromInt(@intFromPtr(c.CGColorSpaceCreateWithName(name.cfstring()))),
) orelse Allocator.Error.OutOfMemory;
}

pub fn release(self: *ColorSpace) void {
c.CGColorSpaceRelease(@ptrCast(self));
}

pub const Name = enum {
/// This color space uses the DCI P3 primaries, a D65 white point, and
/// the sRGB transfer function.
displayP3,
/// The Display P3 color space with a linear transfer function and
/// extended-range values.
extendedLinearDisplayP3,
/// The sRGB colorimetry and non-linear transfer function are specified
/// in IEC 61966-2-1.
sRGB,
/// This color space has the same colorimetry as `sRGB`, but uses a
/// linear transfer function.
linearSRGB,
/// This color space has the same colorimetry as `sRGB`, but you can
/// encode component values below `0.0` and above `1.0`. Negative values
/// are encoded as the signed reflection of the original encoding
/// function, as shown in the formula below:
/// ```
/// extendedTransferFunction(x) = sign(x) * sRGBTransferFunction(abs(x))
/// ```
extendedSRGB,
/// This color space has the same colorimetry as `sRGB`; in addition,
/// you may encode component values below `0.0` and above `1.0`.
extendedLinearSRGB,
/// ...
genericGrayGamma2_2,
/// ...
linearGray,
/// This color space has the same colorimetry as `genericGrayGamma2_2`,
/// but you can encode component values below `0.0` and above `1.0`.
/// Negative values are encoded as the signed reflection of the
/// original encoding function, as shown in the formula below:
/// ```
/// extendedGrayTransferFunction(x) = sign(x) * gamma22Function(abs(x))
/// ```
extendedGray,
/// This color space has the same colorimetry as `linearGray`; in
/// addition, you may encode component values below `0.0` and above `1.0`.
extendedLinearGray,

fn cfstring(self: Name) c.CFStringRef {
return switch (self) {
.displayP3 => c.kCGColorSpaceDisplayP3,
.extendedLinearDisplayP3 => c.kCGColorSpaceExtendedLinearDisplayP3,
.sRGB => c.kCGColorSpaceSRGB,
.extendedSRGB => c.kCGColorSpaceExtendedSRGB,
.linearSRGB => c.kCGColorSpaceLinearSRGB,
.extendedLinearSRGB => c.kCGColorSpaceExtendedLinearSRGB,
.genericGrayGamma2_2 => c.kCGColorSpaceGenericGrayGamma2_2,
.extendedGray => c.kCGColorSpaceExtendedGray,
.linearGray => c.kCGColorSpaceLinearGray,
.extendedLinearGray => c.kCGColorSpaceExtendedLinearGray,
};
}
};
};

test {
Expand Down
40 changes: 40 additions & 0 deletions src/config/Config.zig
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,32 @@ const c = @cImport({
/// This is currently only supported on macOS.
@"font-thicken-strength": u8 = 255,

/// What color space to use when performing alpha blending.
///
/// This affects how text looks for different background/foreground color pairs.
///
/// Valid values:
///
/// * `native` - Perform alpha blending in the native color space for the OS.
/// On macOS this corresponds to Display P3, and on Linux it's sRGB.
///
/// * `linear` - Perform alpha blending in linear space. This will eliminate
/// the darkening artifacts around the edges of text that are very visible
/// when certain color combinations are used (e.g. red / green), but makes
/// dark text look much thinner than normal and light text much thicker.
/// This is also sometimes known as "gamma correction".
/// (Currently only supported on macOS. Has no effect on Linux.)
///
/// * `linear-corrected` - Corrects the thinning/thickening effect of linear
/// by applying a correction curve to the text alpha depending on its
/// brightness. This compensates for the thinning and makes the weight of
/// most text appear very similar to when it's blended non-linearly.
///
/// Note: This setting affects more than just text, images will also be blended
/// in the selected color space, and custom shaders will receive colors in that
/// color space as well.
@"text-blending": TextBlending = .native,

/// All of the configurations behavior adjust various metrics determined by the
/// font. The values can be integers (1, -1, etc.) or a percentage (20%, -15%,
/// etc.). In each case, the values represent the amount to change the original
Expand Down Expand Up @@ -5749,6 +5775,20 @@ pub const GraphemeWidthMethod = enum {
unicode,
};

/// See text-blending
pub const TextBlending = enum {
native,
linear,
@"linear-corrected",

pub fn isLinear(self: TextBlending) bool {
return switch (self) {
.native => false,
.linear, .@"linear-corrected" => true,
};
}
};

/// See freetype-load-flag
pub const FreetypeLoadFlags = packed struct {
// The defaults here at the time of writing this match the defaults
Expand Down
7 changes: 3 additions & 4 deletions src/font/face/coretext.zig
Original file line number Diff line number Diff line change
Expand Up @@ -343,13 +343,12 @@ pub const Face = struct {
} = if (!self.isColorGlyph(glyph_index)) .{
.color = false,
.depth = 1,
.space = try macos.graphics.ColorSpace.createDeviceGray(),
.context_opts = @intFromEnum(macos.graphics.BitmapInfo.alpha_mask) &
@intFromEnum(macos.graphics.ImageAlphaInfo.only),
.space = try macos.graphics.ColorSpace.createNamed(.linearGray),
.context_opts = @intFromEnum(macos.graphics.ImageAlphaInfo.only),
} else .{
.color = true,
.depth = 4,
.space = try macos.graphics.ColorSpace.createDeviceRGB(),
.space = try macos.graphics.ColorSpace.createNamed(.displayP3),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qwerasd205 suggestion: Should we make this ...ColorSpace.createFromName(.displayP3)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? I could understand changing it to createWithName if we wanted to match the name of the CoreGraphics function, but this really feels like splitting hairs either way. I'm not devoted to the name, but if I changed it I'd be more likely to change it to createWithName than createFromName unless there's particularly good reasoning...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@qwerasd205 The way I see it, we are creating a ColorSpace from a ColorSpace.Name called .displayP3.

From that perspective, it would logically flow into ColorSpace.createFromName(.displayP3).

I do think createWithName is also a great option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically our macos pkg is a thin layer over Carbon/AppKit APIs and we just mirror their names, so technically the correct name here is createWithName but I'm not going to lose sleep over it and don't really care.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mitchellh @qwerasd205 Im also happy if we leave it the way it is.

Resolving comment.

.context_opts = @intFromEnum(macos.graphics.BitmapInfo.byte_order_32_little) |
@intFromEnum(macos.graphics.ImageAlphaInfo.premultiplied_first),
};
Expand Down
Loading