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

Gamma Correction #2125

Open
mitchellh opened this issue Aug 20, 2024 · 27 comments
Open

Gamma Correction #2125

mitchellh opened this issue Aug 20, 2024 · 27 comments
Labels

Comments

@mitchellh
Copy link
Contributor

While Ghostty supports specifying the drawable color space at the time of writing this, we don't do proper gamma correction from our linear RGB values that are used everywhere else (bg, fg, palette, SGR styles, etc.).

The result is that all specified color is a little bit off compared to native GUI toolkits (see #2122 for an example of text between Ghostty and iTerm/TextEdit), some colors can't be represented at all, and then there's an aesthetic element to it all.

We should perform gamma correction and also make the gamma configurable by users.

Resources:

@qwerasd205
Copy link
Collaborator

qwerasd205 commented Oct 15, 2024

I'm taking a look at this (on macOS), but not seeing any color difference between specified colors (either palette colors or RGB) and the pixel color which is produced. I know that colors are wrong with the GLFW apprt for some reason, but in the swift apprt colors are produced exactly as expected - or at least, they match Kitty's colors, there are some strange cases where colors are off by 1 in one or two channels, e.g. 160, 0, 0 is rendered as 160, 0, 1 but I tested in Kitty and it does the same thing so I think that's just due to round-trip color space conversion in the screenshot I'm using to check.

Does this problem only present when configuring window-colorspace to display-p3? (It seems like doing so causes the swift apprt's colors to be incorrect in the same was as GLFW)

@qwerasd205
Copy link
Collaborator

I was taking a look at the specifics of text rendering and remembered this issue. The referenced issue #2122 compares to iTerm2-- iTerm2 operates in the default generic RGB space (which means P3 on displays that support that, which includes any modern mac), including for colors specified by SGR. There's an optional field for colorspace ID in the truecolor SGRs (38/48/58) which we actually fail to parse correctly to ignore it in Ghostty (we should fix that! printf "\e[48:2:0:255:0:255mUh oh! I should be magenta not green!\n"), but its behavior is not supported by xterm or any terminal that I know of. As it stands I don't know if there's consensus on what color space terminals should be operating in- I haven't looked deeply in to it though.

tl;dr:

  • iTerm2 is rendering truecolor SGR selections in the generic RGB color space which means colors will be brighter/more saturated on high depth monitors and not match sRGB content.
  • We have a bug in how we parse SGR 38/48/58, printf "\e[48:2:0:255:0:255mUh oh! I should be magenta not green!\n", we fail to parse and ignore the optional color space identifier. (No terminal I know of does anything with it, but we need to discard it.)

As for the actual title of this issue: Gamma Correction

The only place this comes in to play for us, I believe, is in our alpha blending, which I believe may be technically incorrect, giving us anti-aliasing that doesn't look quite right depending on the foreground/background.

This page (sadly only available through archives) explains and demonstrates the issue, and also links to another page on "alpha correction" which may allow us to fix the alpha values without having to actually do gamma correction. That is, if we investigate and find that this is necessary in the first place; I haven't checked the pixels to see yet.

@lilyball
Copy link

I put some thoughts into colorspace and truecolor stuff in #2665, though while writing that I was under the impression that everyone else agreed that truecolor was sRGB.

@slice
Copy link

slice commented Dec 4, 2024

Apologies if this is unrelated/already known, but I figured to mention that font dilation (what Apple seemingly inconsistently refers to as "LCD font smoothing" or simply "font smoothing", and what Ghostty refers to as font-thicken) can depend on factors such as foreground fill color in order to account for gamma during compositing as well as perceptual differences. (This is all to my understanding.)

In this screenshot, the left c is the result of asking Core Text to draw the glyph with the relevant Core Graphics context set to use pure black as the fill color. The right is the same, but with pure white. Blending between these two with this formula has led to (IMO) savory results, but it's probably not literally what Core Graphics is doing under the hood.

It might also be that the background color is factored in when dilation is in play, because I disassembled Xcode to find that it calls a private function named CGContextSetFontSmoothingBackgroundColor. Haven't checked if this actually does anything, though.

@qwerasd205
Copy link
Collaborator

That is a notable observation, and not something that I was aware of, at least. Blending between the two would require rendering twice as many things to the atlas when thickening is enabled though which is a little bit upsetting.

@IGN-Styly
Copy link

this seems related to ghostty issue with transparency on linux where the colors seems to have their gamma amplified. while the bg opacity lowers.

@tbcrawford
Copy link

I believe my comment is related here, but please let me know if this deserves a separate issue. Hopefully this input is helpful.

Here's an example of the differences in a cyan color between Ghostty and iTerm in the p3 color space. The cyan color should be #88C0D0. I captured the colors using the Digital Color Meter macOS application set to "Display in P3".

There are other color differences between these two pictures like the background color and white text colors, but hopefully the cyan color differences help provide a very clear visual difference.

Ghostty

Rendered as #87AFAF with window-colorspace set to display-p3.

It's also worth noting that the text color on the highlighted line is #3B4252, but should just be #000000.

Image

iTerm

Rendered properly as #88C0D0. The text color is rendered as #000000 here (which is expected as well).

Image

mitchellh added a commit that referenced this issue Dec 31, 2024
Fix for a little parsing issue I took note of
[here](#2125 (comment)).

The disparity in behavior between `ghostty@main` and `xterm` can be seen
with this reproduction script:
```sh
printf "\e[0m\nForeground:\n";
printf "\e[38:2:0:255:0mGreen\n";
printf "\e[38;2;0;255;0mGreen\n";
printf "\e[38:2:0:255:0:255mMagenta\n";
printf "\e[38;2;0;255;0;255mGreen\n";

printf "\e[0m\nBackground:\n";
printf "\e[48:2:0:255:0mGreen\n";
printf "\e[48;2;0;255;0mGreen\n";
printf "\e[48:2:0:255:0:255mMagenta\n";
printf "\e[48;2;0;255;0;255mGreen\n";

printf "\e[0m\nUnderline:\n";
printf "\e[58:2:0:255:0m\e[4mGreen\n";
printf "\e[58;2;0;255;0m\e[4mGreen\n";
printf "\e[58:2:0:255:0:255m\e[4mMagenta\n";
printf "\e[58;2;0;255;0;255m\e[4mGreen\n";

printf "\e[0m\n";
```

### Outputs:
|`xterm`|`ghostty@main`|this PR|
|-|-|-|
|<img width="85" alt="image"
src="https://github.com/user-attachments/assets/a0aacff2-2103-4fff-9160-5e663d8a70a2"
/>|<img width="110" alt="image"
src="https://github.com/user-attachments/assets/0ad12e67-3f2c-46f3-b0ee-9230032d188a"
/>|<img width="110" alt="image"
src="https://github.com/user-attachments/assets/7477e3cf-7d27-419e-986b-8df581e52398"
/>|
@qwerasd205
Copy link
Collaborator

I've just went ahead and confirmed that our alpha blending is pixel-identical to TextEdit on macOS (with font-thicken = false, on a high dpi display, sRGB color space). Our rendering is also 100% pixel-identical to Terminal.app other than our line height calculations being different and Terminal having a default setting which slightly increases letter spacing (we're identical when this is reset to 1.0 instead of 1.004). So I would say that our alpha blending with CoreText is "correct for macOS". Idk how things compare on linux- certainly the platform is less uniform so there's not necessarily a "correct" as there is for macOS, but comparing to some respectable linux apps may be a good idea.

@net
Copy link

net commented Dec 31, 2024

@qwerasd205 IIRC this issue is for font-thicken = true. See #2122 (comment):

  • With font-thicken=false, Ghostty, iTerm2, TextEdit have identical rendering down to the individual pixel. I inspected each pixel of a letter and the colors fully match and pixel dimensions/locations fully match.
  • With font-thicken=true, things get messier and Ghostty looks different. Colors differ slightly and pixel locations differ slightly.

...

It's darker because of gamma correction (or rather, a lack of it). Even though we set the colorspace of our drawable surfaces, we are not performing gamma correction yet on the RGB color values set manually by users or TUI programs (i.e. background, foreground, the full color palette, etc.). I jammed in some jank gamma correction multipliers into the OpenGL shader and was able to get very close values to macOS. I'm pretty sure this is the issue.

@mitchellh
Copy link
Contributor Author

Note that second quote is me and I could be wrong. I did notice there were pixel level differences. There are good comments in here since then that may note its unrelated to gamma and actually related to font-thicken being affected by background color when rasterizing.

@qwerasd205
Copy link
Collaborator

If the source of the issue were incorrect gamma correction during blending then it would affect things whether font-thicken was enabled or not.

@qwerasd205
Copy link
Collaborator

And I'm pretty sure the remaining differences between us and iTerm2 are accounted for by the colorspace difference and the difference in how CoreText applies "font smoothing" depending on the foreground (and possibly background) color.

@AVGVSTVS96
Copy link

AVGVSTVS96 commented Jan 2, 2025

Maybe this is related to gamma correction, I'm experiencing slightly faded colors in Ghostty compared to other terminals, its consistent with #3470 (comment)

Ghostty:
Image
WezTerm:
Image
Edit: Interestingly using the neovim terminal in Ghostty shows colors which appear brighter but slightly less saturated:
Image

@net
Copy link

net commented Jan 2, 2025

@AVGVSTVS96 add window-colorspace = display-p3 to your config.

@atiladefreitas
Copy link

Is there any approach to linux distros? I'm on Hyprland on Manjaro and facing this same issue. window-colorspace is only for mac.

@AVGVSTVS96
Copy link

@AVGVSTVS96 add window-colorspace = display-p3 to your config.

This doesn't make a difference for me, thanks for the suggestion.

@axlroden
Copy link

axlroden commented Jan 6, 2025

Mac:
ghostty left, wezterm right:
Image

Ghostty is clearly darker.
Exact same font/color scheme and bg/fg colors.

display-p3 is on.

palette = 0=#363636
palette = 1=#FF0883
palette = 2=#83FF08
palette = 3=#FF8308
palette = 4=#0883FF
palette = 5=#8308FF
palette = 6=#08FF83
palette = 7=#B6B6B6

palette = 8=#424242
palette = 9=#FF1E8E
palette = 10=#8EFF1E
palette = 11=#FF8E1E
palette = 12=#1E8EFF
palette = 13=#8E1EFF
palette = 14=#1EFF8E
palette = 15=#C2C2C2

background = #0D1926
foreground = #B4E1FD

@aljoscha
Copy link

aljoscha commented Jan 6, 2025

I opened a draft PR that might fix the issue of bright-on-dark text looking slightly darker on Ghostty as well, see ☝ (#4686). If you're willing to build from source/my branch you could give it a try.

@qwerasd205
Copy link
Collaborator

I did not compare with sufficiently saturated colors to detect the difference. I did a more thorough comparison and INDEED we do not blend "correctly" to match typical CoreText rendering.

Ghostty Image
Terminal.app Image
Diff Image

The issue can be seen most strongly around the edges of text where the fg color and bg color have a large difference in 1 or more channels (such as red/green, magenta/green and cyan/red)

@axlroden
Copy link

axlroden commented Jan 7, 2025

Yea its much harder to see on screenshots, but the diff helps :)

@aljoscha
Copy link

aljoscha commented Jan 7, 2025

@qwerasd205 While looking into things for my PR (#4686) I did notice that Terminal.app also doesn't do alpha blending in linear RGB space, so what some people might call correct alpha blending. It's mostly visible in those artifacts/color fringes around fonts when using complementary colors, say green on pink. Maybe that's what you were referring to with "correct"... 😅

@aljoscha
Copy link

aljoscha commented Jan 7, 2025

@qwerasd205 I did a comparison of font rendering with browsers as well, though it's not as neatly laid out as your screenshots. It's at the bottom of my PR description (#4686). Copied here for easier comparison:

Image

We have browsers up top (Firefox, Safari, Chrome), of which I'd say Firefox does not blend in linear space but Safari and Chrome do. And I'd say Safari looks best to my eyes. In the bottom row we have Ghostty main and Terminal, which blend in sRGB space (without gamma correction), and then Kitty and this branch on the bottom right which do blend in linear space (with gamma correction). Here it also really helps to click through to the screenshot and zoom in. And yes, my eyes do hurt slightly now... 😅

@mitchellh
Copy link
Contributor Author

Resolved for macOS with #4913. I think we should make a new issue to track the now much more focused task of porting this work to the OpenGL renderer.

mitchellh added a commit that referenced this issue Jan 13, 2025
This PR addresses #2125 for the Metal renderer. Both options are
available: "Apple-style" blending where colors are blended in a wide
gamut color space, which reduces but does not eliminate artifacts; and
linear blending where colors are blended in linear RGB.

Because this doesn't add support for linear blending on Linux, I don't
know whether the issue should be closed or not.

### List of changes in no particular order
- We now set the layer's color space in the renderer not in the apprt
- We always set the layer to Display P3 color spaces
- If the user hasn't configured their `window-colorspace` to
`display-p3` then terminal colors are automatically converted from sRGB
to the corresponding Display P3 color in the shader
- Background color is not set with the clear color anymore, instead we
explicitly set all bg cell colors since this is needed for minimum
contrast to not break with dark text on the default bg color (try it
out, it forces it fully white right now), and we just draw the
background as a part of the bg cells shader. Note: We may want to move
the main background color to be the `backgroundColor` property on the
`CAMetalLayer`, because this should fix the flash of transparency during
startup (#4516) and the weirdness at the edge of the window when
resizing. I didn't make that a part of this PR because it requires
further changes and my changes are already pretty significant, but I can
make it a follow-up.
- Added a config option for changing alpha blending between "native"
blending, where colors are just blended directly in sRGB (or Display P3)
and linear blending, where colors are blended in linear space.
- Added a config option for an experimental technique that I think works
pretty well which compensates for the perceptual thinning and thickening
of dark and light text respectively when using linear blending.
- Custom shaders can now be hot reloaded with config reloads.
- Fixed a bug that was revealed when I changed how we handle
backgrounds, page widths weren't being set while cloning the screen.

### Main takeaways
Color blending now matches nearly identically to Apple apps like
Terminal.app and TextEdit, not *quite* identical in worst case
scenarios, off by the tiniest bit, because the default color space is
*slightly* different than Display P3.

Linear alpha blending is now available for mac users who prefer more
accurate color reproduction, and personally I think it looks very nice
with the alpha correction turned on, I will be daily driving that
configuration.

### Future work
- Handle primary background color with `CALayer.backgroundColor` instead
of in shader, to avoid issues around edges when resizing.
- Parse color space info directly from ICC profiles and compute the
color conversion matrix dynamically, and pass it as a uniform to the
shaders.
- Port linear blending option to OpenGL.
- Maybe support wide gamut images (right now all images are assumed to
be sRGB).
@AVGVSTVS96
Copy link

AVGVSTVS96 commented Jan 14, 2025

Resolved for macOS with #4913. I think we should make a new issue to track the now much more focused task of porting this work to the OpenGL renderer.

For me it looks a little better but still not quite right (using nightly - d1fd22a)

Ghostty:
Image

Wezterm:
Image

EDIT: Fixed after updating Ghostty to latest

@qwerasd205
Copy link
Collaborator

qwerasd205 commented Jan 14, 2025

For me it looks a little better but still not quite right (using nightly - d1fd22a)

You have different background colors configured there. The background color in Wezterm is a few shades darker than the background in Ghostty. Specifically, Ghostty: #24232D, Wezterm: #191A26. This will of course affect how the text looks. Configure both terminals the same and they will look a lot more similar.

@kkanungo17
Copy link

Patrick Walton had reverse engineered the stem darkening metric Apple uses, for use in Pathfinder. Maybe this can be used as a base for stem darkening glyphs after gamma correction: https://x.com/pcwalton/status/918991457532354560

@AVGVSTVS96
Copy link

For me it looks a little better but still not quite right (using nightly - d1fd22a)

You have different background colors configured there. The background color in Wezterm is a few shades darker than the background in Ghostty. Specifically, Ghostty: #24232D, Wezterm: #191A26. This will of course affect how the text looks. Configure both terminals the same and they will look a lot more similar.

Good catch! I updated Ghostty today and the issue is gone, colors look indistinguishable now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests