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

SRGB correct linear gamma blending rewrite #5969

Merged
merged 8 commits into from
Feb 8, 2023

Conversation

m4rw3r
Copy link
Contributor

@m4rw3r m4rw3r commented Feb 2, 2023

This is a rewrite of #5423, which in turn is building on #3308.

Features:

  • Color blending in shaders is now in linear sRGB space.
  • sRGB textures and framebuffer.
  • sRGB lookup-table for color-picking functions.
  • Contrast curve applied to text-alpha to account for fonts looking thinner with gamma-correct blending and improves legibility.

Additional features/improvements compared to #5423:

  • All adjustments to the text-alpha now happens in the cell_fragment.glsl shader.
  • Better contrast curve function which accounts for the difference in luminance between foreground and background.
  • Simulation of gamma-incorrect blending, can be enabled by setting text_old_gamma yes.
  • Better assignment of constant data to shader uniforms (sRGB LUT as well as configuration values for alpha-adjustments).

To get very close to MacOS rendering of fonts, use the following values:

text_gamma_adjustment 1.7
text_contrast 30

TODO:

  • sRGB textures and framebuffer.
  • Option to simulate old gamma-incorrect rendering in sRGB-linear space.
  • Option to use additional contrast curve on text-alpha to simulate stem-darkening.
  • Contrast values matching MacOS Terminal.
  • Contrast values matching Windows Terminal.
  • sRGB colors picked from look-up-tables.
  • 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).
  • Test the performance impact of increased complexity in the fragment shader.

@m4rw3r
Copy link
Contributor Author

m4rw3r commented Feb 2, 2023

Here is a quick comparison between Kitty and MacOS Terminal:

Screenshot 2023-02-02 at 14 26 53

  • SF Mono Regular 12pt
  • Base16 Tomorrow theme
  • text_contrast 30 and text_gamma_adjustment 1.7
  • "More space" (1920x1200) on a 15" 2019 MacBook Pro

@m4rw3r
Copy link
Contributor Author

m4rw3r commented Feb 3, 2023

Looking at the differences between Windows Terminal text-rendering and Kitty on Linux we can clearly see that Windows renders text a bit differently (compare to the graphs for MacOS Terminal vs Kitty on Mac here #5423 (comment)):

image

The approxmiation is still decent with the following values:

text_gamma_adjustment 2.1
text_contrast 18

image

@m4rw3r
Copy link
Contributor Author

m4rw3r commented Feb 5, 2023

I have now done some crude performance-measurements. I first attempted to use the CPU-profiler to time the invocation of the shader-program, but it seems like the sample-frequency is too low for that to be anywhere close to accurate since the duration of draw_cells is too short. So I added logging to draw_cells timing the execution and then summarized the total time spent in it during a cat of a large log-file. I then ran this 10 times for each version of the fragment shader, with everything else being identical:

  • Old shader: 346 ms with a standard deviation of 24.438
  • New shader: 324 ms with a standard deviation of 27.738

The difference could be explained by the low sample-size, or the fact that some shader-parameters are unused by the old fragment shader. Or the very slight refactoring I made to it improved performance somehow despite then adding more operations to it? At least for my machine there does not seem to be any reduction in performance with the changes to the shader.

Details:

  • Measuring the total time spent in draw_cells using clock()
  • Set configuration values to repaint_delay and input_delay to 0 and sync_to_monitor no
  • Catting a 1.1 GB log-file, total run time per cat was ~1 min
  • Only kitty as well as the kitty-under-test was running
  • Size of the tested window was approx 211x76 lines and 1700x1400 px
  • CPU: AMD R9 3950X
  • GPU: AMD RX560 4GB
  • Monitor: 3440x1440px @ 60Hz
  • Display Server: Wayland
  • Display Driver: amdgpu

@m4rw3r
Copy link
Contributor Author

m4rw3r commented Feb 5, 2023

Here is a comparison between the simulated old gamma-incorrect blending, the gamma-correct blending, and the gamma-correct blending with contrast (MacOS-like settings):

image

@kovidgoyal
Copy link
Owner

Cool I will review it when I have a moment.

@ewhac
Copy link

ewhac commented Feb 8, 2023

Been using this updated PR for a few days now, and I think it looks lovely. I'm looking forward to it landing in mainline.

@kovidgoyal
Copy link
Owner

LGTM. I have merged, changing it to use only a single option to control it and also to default to values that look close to native. (i.e. 1.7 and 30 on macOS and 1.0 and 0 on Linux). If these values need adjustment do let me know. My eyes arent really good enough to tell anymore.

@kovidgoyal kovidgoyal merged commit b5b070a into kovidgoyal:master Feb 8, 2023
@AprilArcus
Copy link

Incredible work everyone. Very excited for this!

kovidgoyal referenced this pull request Feb 10, 2023
…" rendering. Also use only a single option to control it.
@ronjouch
Copy link
Contributor

ronjouch commented Feb 11, 2023

@m4rw3r as suggested by Kovid in 4dfd4d4#commitcomment-100176618 , I'm reporting here that text_composition_strategy's ramp behaves strangely differently in light vs. dark themes, which might be undesirable for people (like me) who switch between light/dark themes depending on time of day & ambient light.

Said differently, although the behavior seems to be acknowledged by documentation saying ...

Dark text on light backgrounds receives the full impact of the curve while light text on dark backgrounds is affected very little.

... a user switching between light & dark themes will have to adjust text_composition_strategy at the same time as changing from/to dark to/from light, to preserve thickness preferences 😕. This feels undesirable, as you wouldn't expect to be required to change option A when you change option B! Written from a user perspective: "I want my font thick|thin, independently of the theme dark|light-ness"!

Here's kitty with exaggerated-for-demo extremely light 0.01 0 settings. See how it has a dramatic effect in light themes, but not so much in a dark theme:

  • Theme Tango Light:
    screenshot_light_2023-02-10-16:48:48
  • Theme kanagawabones:
    screenshot_kitty-dark_2023-02-10-16:48:33

Whereas if going to crazy-thick settings 5 100, the setting's impact is similar in both dark vs. light themes:

  • Theme Tango Light:
    screenshot_l_2023-02-10-16:55:20
  • Theme kanagawabones:
    screenshot_d_2023-02-10-16:55:11

To come back to my use case: I wasn't trying to go to extreme scenarios like the ones I'm presenting for demos. I had tweaked thickness for my fav light theme, and when the night came and I switched to my fav dark theme, I was like "wait, why is this so different?". I fiddled, and almost thought the setting was broken for dark themes, given how little effect it has compared to with light themes.

→ Do you agree the inconsistency feels like something that's undesirable to people switching between dark & light themes? Am I missing something?

@m4rw3r
Copy link
Contributor Author

m4rw3r commented Feb 11, 2023

The contrast function is supposed to make it easier to achieve the same perceived thickness between dark-on-light and light-on-dark text. The main issue is that darker foregrounds gets affected more negatively in terms of thickness with respect to gamma, which is why the adjustment has to be different for the two cases (and we cannot skip gamma since that will make text appear unevenly thick even at the same thickness and color). If it was possible to use the exact same adjustment, without having to account for luminance of background/foreground, then that would be preferable but from all my experiments the contrast or stem-darkening algorithms always increase the thickness of dark-on-light a lot more than light-on-dark (some even leave the light-on-dark completely untouched).

The contrast adjustment currently works like this:

  1. Take the difference in luminance between background and foreground and scale it down to the range 0.0-1.0, where 0.0 is a white-on-black and 1.0 is black-on-white. This step is what is the source of the different behavior for the contrast function with dark-on-light and light-on-dark themes.
  2. Apply an extra gamma adjustment on the alpha-channel of the text (configurable).
  3. Blend the original alpha channel value with the extra gamma-adjusted alpha-channel, scaled by the luminance difference.
  4. Multiply the result with a linear contrast value (configurable).

In one extreme both user-configurable parameters result in visible change, but with a light-on-dark theme we only get the linear contrast. Ie. the first configuration value matters for dark-on-light and gradually is applied less and does almost nothing for light on dark.

So in your case I think I would go about it like this: first set the values to 1.0 0 and use a dark theme. Then adjust the linear contrast value (the second parameter) until it looks good, after that switch to a light theme and start adjusting the gamma-curve contrast instead to reach the same perceived thickness as the dark theme.

I did not want to add too many parameters to this feature initially, since it is very easy to get text to look bad, but there might be additional values which need to be exposed in the configuration. Or maybe I need to work on documenting the values and how they behave.

@AprilArcus
Copy link

Is macos_thicken_font still needed to match native macOS font rendering after this change?

@m4rw3r
Copy link
Contributor Author

m4rw3r commented Feb 11, 2023

Is macos_thicken_font still needed to match native macOS font rendering after this change?

I did not use macos_thicken_font when fitting the contrast-curve to MacOS Terminal. So the configuration-values which are the default for text_composition_strategy platform on MacOS should match the MacOS Terminal without any extra settings enabled.

I also found this added in the changelog:

"The obsolete macos_thicken_font will make the font too thick and needs to be removed manually if it is configured."

@ronjouch
Copy link
Contributor

ronjouch commented Feb 12, 2023

@m4rw3r

The contrast function is supposed [...]. The first configuration value matters for dark-on-light and gradually is applied less and does almost nothing for light on dark.

Thanks for the explanations!

So in your case I think I would go about it like this: first set the values to 1.0 0 and use a dark theme. Then adjust the linear contrast value (the second parameter) until it looks good, after that switch to a light theme and start adjusting the gamma-curve contrast instead to reach the same perceived thickness as the dark theme.

Fantastic, that's exactly what I was looking for: a quick "HOWTO" guide/workflow to configure these values 🙂. To my taste and with my font of choice (Cascadia Mono), I ended up with 3.0 0 for same perceived thickness between my themes of choice (light=Tango Light, dark=kanagawabones).

Suggestion: integrate to the official documentation this little guide you wrote above!

I did not want to add too many parameters to this feature initially, since it is very easy to get text to look bad, but there might be additional values which need to be exposed in the configuration. Or maybe I need to work on documenting the values and how they behave.

I understand the desire to not add even more parameters, as it's already tricky to configure!

But yes I do think (see my "Suggestion" comment above) that it'd be valuable to have a bit more "dumbed down" documentation added to the docs. Even though the docs already reasonably describe the feature now, just reading it let me entirely unable to derive the "HOWTO"-style guide you provided me above. Maybe it's something that's intuitive to you (who worked on it) or to someone familiar with that kind of computation, but it's not at all for a newbie like me. So yup, the HOWTO would be an excellent addition to the docs I think.

Lame metaphor: a great car engine manual doesn't just show you a raw diagram of the pistons & cylinders. It also teaches you that to tune it, you should A. adjust knob X at 1000RPM then B. push/pull on knob Y+Z at 3000RPM.

@page-down
Copy link
Contributor

I tested the display of complex CJK font glyphs at Low and Hi-DPI.
The font strokes are thick enough and clear and sharp to get good results with normal font size.
Especially at Low-DPI, where complex shapes need to be displayed in few pixels, the strokes can still be distinguished.

However, the slash "/" appears to have some jaggedness, compared to the macOS rendering, which looks like it lacks some anti-aliasing. Some users may want to fine-tune the default values.

... first set the values to ... after that switch to ...
... reach the same perceived thickness ...

It sounds like one that can be automatically adjusted and configured, just like a digital camera auto focusing.
There are enough light/dark themes in the official themes repository, so is it possible to give some good values based on that?

@kovidgoyal
Copy link
Owner

A PR updating the docs to make this option easier to use is most
welcome.

@carlosvigil
Copy link

Does this solve the "subpixel AA" concerns? How do we use it?

@AprilArcus
Copy link

Glyph dilation on macOS may still be relevant even with gamma correct antialiasing. See this tweet from Patrick Walton:

https://twitter.com/pcwalton/status/918991457532354560

@kovidgoyal
Copy link
Owner

kovidgoyal commented Feb 27, 2023 via email

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

Successfully merging this pull request may close these issues.

7 participants