kitty
kitty copied to clipboard
kovidgoyal
SRGB correct linear gamma blending
This pull request is building on top of #3308 to blend colors in linear colorspace. It will improve font rendering and probably color accuracy when transparency is involved.
To avoid somewhat complex calculations in shaders it uses a lookup-table in a uniform for the color-pickers in the vertex-shaders for cells and borders, and the same data is used to provide the colors supplied to the shaders themselves. GL_FRAMEBUFFER_SRGB is then used to automate the conversion back to SRGB-space (if we manually converted back from linear at the end of each shader we would have to re-convert existing colors in every shader which would carry a performance penalty, this way we can mix-and match shaders in any order).
Some things I'm a bit uncertain about:
- Not sure if all the correct textures have been changed to SRGB, if some should remain RGB or not.
- Maybe there are some better/more efficient ways to provide the LUT-data to the GPU.
- How/if this will affect color on MacOS with different
macos_colorspace. I have not yet tested this.
Interesting. Sadly I am super busy at the moment so I dont have the time to review this as it deserves. I will get to it when I can.
On Fri, Aug 26, 2022 at 07:45:00AM -0700, Martin Wernstål wrote:
- Maybe there are some better/more efficient ways to provide the LUT-data to the GPU.
Regarding this point, maybe better to use a VAO. IIRC uniform buffer sizes are quite limited, depending on GPU driver/model. Or one could even use a texture.
- How/if this will affect color on MacOS with different
macos_colorspace. I have not yet tested this. You can view, comment on, or merge this pull request online at:
Hopefully some macOS users will be willing to try it out and see. Ping @luflosi, @page-down
Thanks for this improvement.
When the text color is white and the background is black, the font thickness finally approaches the rendering of macOS Cocoa App.
The fonts are rendered basically the same as the regular style without the need to configure the macos_thicken_font option.
Before this, it was like using the thin style of the font.
However, when the background color is light, such as highlighting selected text, the text becomes thin and blurry.
Unfortunately I cannot test it on wide gamut hardware devices (e.g. with Apple Display P3-500 nits color profile) at this time. Please let me know if there is anything I can do to help.
I think I am getting closer to finding the issue with why the blending works with light-on-dark but not the reverse; the rendered glyph alpha-channel needs to be converted from linear to sRGB before being applied to the foreground color. I have trouble finding correct information on how to exactly blend for font-rendering with an alpha channel in particular, and this feels a bit odd to use one alpha value in sRGB space while the rest of them are in linear space. Could possibly also be that some settings for glyph rendering needs to be adjusted instead of the shader itself, the GL_SRGB8_ALPHA8 does not touch the alpha-channel gamma from what I can tell and should only adjust the RGB components.
The top image is the current state of the PR, the one below converts the sprite texture alpha channel to sRGB before blending:
It does seem like the light-on-dark colors get a bit blurred though, so I am not convinced this is the correct solution.
Technically the blending of the font is gamma is correct in the PR with just the use of the added sRGB lookup-table for colors, premultiplied alpha blending with those colors in linear space, and GL_FRAMEBUFFER_SRGB + GL_SRGB_* textures. FreeType specifies in the documentation that the resulting bitmap is an alpha-coverage map which means that the mixing of colors in linear space to this value should produce correct images.
This is a bit problematic though since many fonts and font-rendering engines make the assumption that the result is not going to be blended in a gamma-correct way. For example Photoshop uses a gamma of 1.42, and Qt5 on X11 uses 1.7, specifically for rendering text to compensate for fonts which look too thin when rendered correctly (while the rest of the rendering is done in a gamma-correct way). For the engines that do blend in linear colorspace (like ClearType and MacOS) they use what is called stem-darkening or glyph dilation, where thinner small font-sizes have increased weight. FreeType does have support for stem-darkening, but not with TrueType fonts, only the CFF and Type 1 drivers have exposed support for it (which can be turned on using environment-variables or system settings if a suitable font is used). All of this means that many fonts will look too thin when using gamma-correct blending alone.
It is probably best to follow Photoshop and Qt5 here and make a special adjustment to the text alpha-channel using a gamma curve, and let the user configure this value so adjustments can be done depending on the used font, size, resolution, and rendering engine. The colors and rest of the blending should still be done with sRGB-correct gamma to ensure the luminance is correct.
Here is a comparison between MacOS Terminal (on a MacBook Pro 15" 2019 with Monterey), Kitty with this PR and a gamma curve of 1.7 applied to the glyph alpha, and Kitty 0.26.0:
The left one is MacOS Terminal and the middle one is Kitty with the modifications, and finally to the right we have Kitty without this PR. Note that this screenshot looks way better and sharper than it does on the actual retina screen, the varying brightness of the text in Kitty 0.26.0 becomes more obvious when it is smaller. Gamma-correct alpha-blending combined with an applied gamma curve on the glyph alpha channel seems to be getting very close to MacOS rendering of the font.
@kovidgoyal Would it be suitable to pre-compute the font gamma adjustment to the rendered alpha channel in render_alpha_mask in fonts.c? This way we can avoid having to re-compute the pow(alpha, 1.0 / font_gamma) in the shader.
@kovidgoyal Would it be suitable to pre-compute the font gamma adjustment to the rendered alpha channel in
render_alpha_maskinfonts.c? This way we can avoid having to re-compute thepow(alpha, 1.0 / font_gamma)in the shader.
Should be fine as far as I know. It's used only for rendering box
drawing chars and normal chars from fonts.
I havent had time to read the code yet, but this reminds me that there
is also rendering of emoji to consider. So you would need to precompute
gamma for those as well if you do this.
... they use what is called stem-darkening or glyph dilation, where thinner small font-sizes have increased weight. ...
Since I use both LowDPI and HiDPI monitors, I've noticed that when using the same font size, macOS will slightly increase the weight whenever the actual rendered size is too small. So when the macOS Terminal.app window is moved to a LowDPI display, the font immediately becomes more readable.
And when the macOS Terminal window is moved to HiDPI display, the font will immediately become sharp and clear, without blurring like when kitty macos_thicken_font is applied to HiDPI.
The font SF Mono Regular can also be used for tuning.
https://github.com/kovidgoyal/kitty/discussions/4485
... let the user configure this value so adjustments can be done depending on the used font, size, resolution, and rendering engine ...
It would be nice to add the ability to adjust based on DPI, not just resolution.
... this screenshot looks way better and sharper than it does on the actual retina screen ...
However, the font in the middle screenshots appears jagged, and you can notice that at the bottom of a and d.
I prefer the anti-aliased rendering of macOS Terminal.app on the left or, kitty on the right (without this PR).
For the top and bottom screenshots in the middle, it is like using a bold font when the background color is black.
I havent had time to read the code yet, but this reminds me that there is also rendering of emoji to consider. So you would need to precompute gamma for those as well if you do this.
Gamma correction for emojis is already implemented through the GL_SRGB_* texture format parameters, and in general the alpha channel should not be touched since it is already linear. It is antialiased fonts with alpha which needs this special compensation in some cases where it otherwise would look too thin.
The font
SF Mono Regularcan also be used for tuning. #4485... let the user configure this value so adjustments can be done depending on the used font, size, resolution, and rendering engine ...
It would be nice to add the ability to adjust based on DPI, not just resolution.
... this screenshot looks way better and sharper than it does on the actual retina screen ...
However, the font in the middle screenshots appears jagged, and you can notice that at the bottom of
aandd. I prefer the anti-aliased rendering of macOS Terminal.app on the left or, kitty on the right (without this PR).For the top and bottom screenshots in the middle, it is like using a bold font when the background color is black.
Thank you, that font looks to be a much better test-case for this PR. Using the settings from #4485 without macos_thicken_font and with this PR + a font alpha gamma adjustment of 1.7 I get this comparison with Terminal.app on native resolution:
It does look to be very close in terms of shape, but this PR + extra gamma ends up looking washed out. The MacOS rendered font looks like some kind of contrast-curve is used instead of a gamma curve since it is very dark despite the thin strokes. After testing some different values with a sigmoidal contrast-curve I have landed on a guess of a slope of 6 and 10% midpoint as a decent approximation (note that the alpha channel 255 means fully colored pixel):
And with a scaled resolution:
When looking closely at the same with bright text on dark background we can see that the curve probably needs some more tweaking, especially for the native resolution, if it actually is a sigmoidal contrast curve:
After not being happy with the result of the contrast function above I have iterated a bit more on this issue. So I took screenshots of both white-on-black and black-on-white with SF Mono 11pt and compared the pixel values:
The black on white screenshot data was inverted to try to see if they overlap, which they do not. Testing with different gamma-curves before inverting the black on white could move them a bit closer but I could not find a perfect match which I'm guessing means that the font rendering is not just stem-darkening and 1.0 or 2.0 gamma (which is what Skia uses depending on if font-smoothing is off or on). We can also see that it is saturating a bit in both cases.
So instead I applied a reverse srgb gamma curve to the Terminal.app screenshot and then tried to find an approximation function to transform linear alpha values to something which would somewhat match Terminal.app after gamma:
As we can see the two colorschemes differ greatly still which makes it impossible to use any kind of transformation on just the font alpha-channel and produce a result which matches Terminal.app in both cases. When using the curve-constants matching black on white approximation we still get some artifacts in white on black in native resolution even though it is greatly improved from the sigmoidal contrast function:
On Mon, Aug 29, 2022 at 09:53:52AM -0700, Martin Wernstål wrote:
@kovidgoyal Would it be suitable to pre-compute the font gamma adjustment to the rendered alpha channel in
render_alpha_maskinfonts.c? This way we can avoid having to re-compute thepow(alpha, 1.0 / font_gamma)in the shader.
Should be fine as far as I know. It's used only for rendering box drawing chars and normal chars from fonts.
I havent had time to read the code yet, but this reminds me that there is also rendering of emoji to consider. So you would need to precompute gamma for those as well if you do this.
Apologies for the long delay, I was focusing on getting kitty-tool done. Now that this is merged into master I have the time to review this PR. Could you please resolve the conflicts.
Culd you also make a separate PR with some trivial change so thatthe tests are run automatically on thi PR in the future.
I did a basic review, the code looks mostly fine (however with graphics its the end result that matters), I will post visual results that don't look good to my eyes as I test things.
The first thing I noticed is that dark-on-light text looks significantly lighter with the patch. See below, First screenshot is without patch second is with patch.
This is on Linux X11 with the Operator Mono font on a 3840x2160 display scaled by a factor of 3 (dpi set to 288).
I am fairly certain that this change will be liked by some people and hated by others and not noticed at all by many. And we will of course hear far more from the people that hate it. As such it would be nice if there was a setting to revert rendering to what it used to be. Possibly, have a LUT containing just 1/255 uniformly and have a config option that switches between the two and also doesn't use the SRGB colorspace. Or alternatively, have a set of recommended values for the contrast curve and whitepoint settings to get close to previous rendering in different scenarios.
Another idea is to have per font contrast curves, using the existing modify_font configuration machinery that override the global settings.
We will also need to deprecate the macos_thickenfont config setting and point users to the knobs that control gamma blending instead.
Another thought is maybe we need different gamma values (aka LUTs) and curves for each platform so that the out of the box rendering is closer to platform defaults. So one for macOS, one for Linux (I dont think X11 and Wayland need different gamma).
The first thing I noticed is that dark-on-light text looks significantly lighter with the patch. See below, First screenshot is without patch second is with patch.
This is the expected result when blending in linear space, it is unfortunate since it both fixes some fonts while making others look worse (or different compared to the system rendering). This article discusses the issue and some of the possible solutions: https://skia.org/docs/dev/design/raster_tragedy/ . According to this their "Gamma Hack" is not suitable for full-pixel anti-aliased fonts, this seems to match my experience when I tried to render fonts with a different gamma-curve in sRGB-linear colorspace.
I am fairly certain that this change will be liked by some people and hated by others and not noticed at all by many. And we will of course hear far more from the people that hate it. As such it would be nice if there was a setting to revert rendering to what it used to be. Possibly, have a LUT containing just 1/255 uniformly and have a config option that switches between the two and also doesn't use the SRGB colorspace. Or alternatively, have a set of recommended values for the contrast curve and whitepoint settings to get close to previous rendering in different scenarios.
I will have to go back and experiment some more and see if I can come up with a suitable way to apply an adjustment which is scaled based on the intensity of the destination/target. Seems like this is what Skia does in its "Contrast Hack" to avoid increasing contrast when the target is darker than the source (this is an issue currently with this PR, the contrast curve further thickens light-on-dark text more than what using sRGB-gamma itself already does). This should hopefully be possible to perform in the shader, making it easy to configure (but will add some computational complexity).
If that fails to produce results acceptable by most people, a configuration-setting for sRGB on/off + contrast is probably the best bet. Would require a restart of Kitty whenever this setting is changed though (unless there is some way to reinitialize the gl-context?).
On Mon, Nov 21, 2022 at 07:31:21AM -0800, Martin Wernstål wrote:
The first thing I noticed is that dark-on-light text looks significantly lighter with the patch. See below, First screenshot is without patch second is with patch.
This is the expected result when blending in linear space, it is unfortunate since it both fixes some fonts while making others look worse (or different compared to the system rendering). This article discusses the issue and some of the possible solutions: https://skia.org/docs/dev/design/raster_tragedy/ . According to this their "Gamma Hack" is not suitable for full-pixel anti-aliased fonts, this seems to match my experience when I tried to render fonts with a different gamma-curve in sRGB-linear colorspace.
Yeah, the problem with font rendering is there is no "right" answer its all expectations, systems, fonts, eyes, etc. dependent.
I am fairly certain that this change will be liked by some people and hated by others and not noticed at all by many. And we will of course hear far more from the people that hate it. As such it would be nice if there was a setting to revert rendering to what it used to be. Possibly, have a LUT containing just 1/255 uniformly and have a config option that switches between the two and also doesn't use the SRGB colorspace. Or alternatively, have a set of recommended values for the contrast curve and whitepoint settings to get close to previous rendering in different scenarios.
I will have to go back and experiment some more and see if I can come up with a suitable way to apply an adjustment which is scaled based on the intensity of the destination/target. Seems like this is what Skia does in its "Contrast Hack" to avoid increasing contrast when the target is darker than the source (this is an issue currently with this PR, the contrast curve further thickens light-on-dark text more than what using sRGB-gamma itself already does). This should hopefully be possible to perform in the shader, making it easy to configure (but will add some computational complexity).
Worth a try.
If that fails to produce results acceptable by most people, a configuration-setting for sRGB on/off + contrast is probably the best bet. Would require a restart of Kitty whenever this setting is changed though (unless there is some way to reinitialize the gl-context?).
I'm OK with it requiring a restart, while the OpenGL state can of course be changed one would have to re-load all the graphics as well I think. Too much bother for a setting that is not meant to be tweaked often anyway.
Another possible issue is how changing opengl to use SRGB colorspaces interacts with the macos_colorspace setting, is it handled transparently? Does it cause performance issues when the display colorspace is not srgb but instead apple's p3 colorspace? @pagedown since you use macs maybe you can test this?
... macos_colorspace ... is it handled transparently?
From the results, I can't find any difference, it looks like it has been treated correctly.
Does it cause performance issues when ...
Even after actually doing more calculations (for the color space), I could not feel the impact on performance.
Just a tiny update since I have been pretty busy the last few weeks: I have managed to reproduce the non-linear blending result using a shader in sRGB-linear space by looking at luminance of foreground vs background and then solving for a new alpha value.
Unfortunately the calculations used to match non-linear blending are not close to any of the "Contrast Hack" calculations used by other renderers, though it should be possible to use a boolean flag to use this calculation instead of a contrast function (hopefully the unused pow and division instructions won't be a performance problem, worst case we can probably switch the shader or OpenGL configuration depending on this configuration value).
And I am still in the process of testing some contrast-curves to see what curve and values will get close to Mac/Windows rendering.
Another update on this:
Since these commits were partially made in an experimental fashion, with some undoing work from the previous, I decided to rewrite this PR while also improving it.
- [x] sRGB textures and framebuffer.
- [x] Option to simulate old gamma-incorrect rendering in sRGB-linear space.
- [x] Option to use additional contrast curve on text-alpha to simulate stem-darkening.
- [x] Contrast values matching MacOS Terminal.
- [ ] Contrast values matching Windows Terminal.
- [x] sRGB colors picked from look-up-tables.
- [x] LUT and configuration values stored in uniforms and only set once (
set_cell_uniforms). - [ ] Test simulation of gamma-incorrect blending with two colors of similar luminance (probably unstable if luminance of foreground and background are close to the same).
WIP branch: https://github.com/m4rw3r/kitty/tree/linear-gamma-blending
Should I replace this PR with the commits from the new branch?
Do whatever you find easier. I am fine with you opening a new PR if that's easier for you, just link to this one in its text.
I've been using this (merged) PR in my own copy for a couple weeks now, and I have no complaints. I think it looks great.
