Repository: wezterm/wezterm (language: rust)

Issue:
Fonts: Reduce default skew factor and/or permit user-specified skew factor
## Is your feature request related to a problem? Please describe.

When a font doesn't have built-in italics, wezterm (and most terminals) will approximate an italic variant by simply _skewing_ the letters. Currently, the skew factor is [fixed at 20%](https://github.com/wez/wezterm/blob/87cd9c5f6cc26809527344a9713aff19227dc243/wezterm-font/src/rasterizer/mod.rs#L25), and it doesn't look great. Here is a side by side. First image is wezterm, second is iTerm2, both using Fira Code Medium:
<img width="1091" alt="Screenshot 2023-04-18 at 01 17 59" src="https://user-images.githubusercontent.com/752802/232679455-bc754a73-8619-4d37-8dc4-0a03bbe33c41.png">
<img width="1032" alt="Screenshot 2023-04-18 at 01 18 02" src="https://user-images.githubusercontent.com/752802/232679467-7058a677-42ad-4ea5-b4a0-bc9cc820486a.png">

In fact, the letters are so skewed, that the cursor highlights portions of adjacent letters sometimes:
<img width="815" alt="Screenshot 2023-04-18 at 01 29 22" src="https://user-images.githubusercontent.com/752802/232679753-4e59dad0-dbc8-4daa-9a2c-c8eb40f00f0f.png">


### Describe the solution you'd like

Two fixes, both of which can happen independently:

1. I think the default skew factor should be reduced.  I haven't played with the values so I don't have a concrete recommendation for a number
2. In addition, make the skew factor configurable. E.g.,
```lua
config.font = wezterm.font("Fira Code", { weight="Medium", skew_factor = 0.10 })
```

### Describe alternatives you've considered

Looked at other font options. There's nothing rn that addresses this.


Discussion / hints:
I'll accept PR(s) for either/both of these
Update: I'm now thinking that there might be a bug. Looks like wezterm is skewing _in addition_ to doing italics. Here's a side by side of `factor = 0.0` and `factor = 0.2`. Will investigate further.
<img width="768" alt="Screenshot 2023-04-18 at 02 20 47" src="https://user-images.githubusercontent.com/752802/232689132-977459d4-dfb2-4eb6-b93a-55a35bfe8cc0.png">
<img width="768" alt="Screenshot 2023-04-18 at 02 21 20" src="https://user-images.githubusercontent.com/752802/232689138-89aea679-16a8-4b66-8db0-267881ff3dfd.png">

Aha, it looks like it's double-skewing. [This line](https://github.com/wez/wezterm/blob/1e8fd7a95ba84e45ebf5349b48cdb27bfac95a38/wezterm-font/src/rasterizer/freetype.rs#L319) already sets the skew to 20% for the face.

Question: what should the change look like here? Should `rasterize_glyph()` do any skewing at all, given that this transform is done for all `synthesize_italic` fonts? Lmk
Oh, good find! I think the set_transform call is probably the better one to keep, because it already has the `ParsedFont` available, which is where the skew factor would get carried through from the config.

Maybe `ParsedFont::synthesize_italic` should be `Option<f32>` rather than a `bool`, where the f32 value is the skew factor?


Fix patch:
diff --git a/wezterm-font/src/rasterizer/freetype.rs b/wezterm-font/src/rasterizer/freetype.rs
index 7934eb49627..57c0aa2315a 100644
--- a/wezterm-font/src/rasterizer/freetype.rs
+++ b/wezterm-font/src/rasterizer/freetype.rs
@@ -1,8 +1,8 @@
 use crate::parser::ParsedFont;
-use crate::rasterizer::FontRasterizer;
+use crate::rasterizer::{FontRasterizer, FAKE_ITALIC_SKEW};
 use crate::units::*;
 use crate::{ftwrap, RasterizedGlyph};
-use ::freetype::{FT_GlyphSlotRec_, FT_Glyph_Format, FT_Matrix};
+use ::freetype::{FT_GlyphSlotRec_, FT_Matrix};
 use anyhow::bail;
 use config::{DisplayPixelGeometry, FreeTypeLoadFlags, FreeTypeLoadTarget};
 use std::cell::RefCell;
@@ -14,7 +14,6 @@ pub struct FreeTypeRasterizer {
     face: RefCell<ftwrap::Face>,
     _lib: ftwrap::Library,
     synthesize_bold: bool,
-    synthesize_italic: bool,
     freetype_load_target: Option<FreeTypeLoadTarget>,
     freetype_render_target: Option<FreeTypeLoadTarget>,
     freetype_load_flags: Option<FreeTypeLoadFlags>,
@@ -63,13 +62,7 @@ impl FontRasterizer for FreeTypeRasterizer {
             mode => bail!("unhandled pixel mode: {:?}", mode),
         };
 
-        let slot = unsafe { &mut *(*face.face).glyph };
-        if self.synthesize_italic && slot.format == FT_Glyph_Format::FT_GLYPH_FORMAT_BITMAP {
-            // The source was and thus the italic transform did nothing
-            Ok(glyph.skew())
-        } else {
-            Ok(glyph)
-        }
+        Ok(glyph)
     }
 }
 
@@ -314,10 +307,10 @@ impl FreeTypeRasterizer {
 
         if parsed.synthesize_italic {
             face.set_transform(Some(FT_Matrix {
-                xx: 1 * 65536,            // scale x
-                yy: 1 * 65536,            // scale y
-                xy: (0.2 * 65536.0) as _, // skew x
-                yx: 0 * 65536,            // skey y
+                xx: 1 * 65536,                         // scale x
+                yy: 1 * 65536,                         // scale y
+                xy: (FAKE_ITALIC_SKEW * 65536.0) as _, // skew x
+                yx: 0 * 65536,                         // skew y
             }));
         }
 
@@ -326,7 +319,6 @@ impl FreeTypeRasterizer {
             face: RefCell::new(face),
             has_color,
             synthesize_bold: parsed.synthesize_bold,
-            synthesize_italic: parsed.synthesize_italic,
             freetype_load_flags: parsed.freetype_load_flags,
             freetype_load_target: parsed.freetype_load_target,
             freetype_render_target: parsed.freetype_render_target,
diff --git a/wezterm-font/src/rasterizer/mod.rs b/wezterm-font/src/rasterizer/mod.rs
index b9ab8ec3e70..c37ae6c932f 100644
--- a/wezterm-font/src/rasterizer/mod.rs
+++ b/wezterm-font/src/rasterizer/mod.rs
@@ -2,6 +2,10 @@ use crate::parser::ParsedFont;
 use crate::units::*;
 use config::FontRasterizerSelection;
 
+/// The amount, as a number in [0,1], to horizontally skew a glyph when rendering synthetic
+/// italics
+pub(crate) const FAKE_ITALIC_SKEW: f64 = 0.2;
+
 pub mod freetype;
 
 /// A bitmap representation of a glyph.
@@ -16,65 +20,6 @@ pub struct RasterizedGlyph {
     pub has_color: bool,
 }
 
-impl RasterizedGlyph {
-    /// Computes a skewed version of this glyph to produce a synthesized oblique variant
-    pub fn skew(&self) -> Self {
-        // This function is derived from code which is subject to the terms of the
-        // Mozilla Public License, v. 2.0. http://mozilla.org/MPL/2.0/
-
-        let factor = 0.2f64; // Skew factor
-        let stride = self.width * 4;
-
-        // Calculate the skewed horizontal offsets of the bottom and top of the glyph.
-        let bottom = self.bearing_y.get().round() as f64 - self.height as f64;
-        let skew_min = ((bottom + 0.5) * factor).floor();
-        let skew_max = ((self.bearing_y.get() as f64 - 0.5) * factor).ceil();
-
-        // Allocate enough extra width for the min/max skew offsets.
-        let skew_width = self.width + (skew_max - skew_min) as usize;
-        let mut skew_buffer = vec![0u8; skew_width * self.height * 4];
-        for y in 0..self.height {
-            // Calculate a skew offset at the vertical center of the current row.
-            let offset = (self.bearing_y.get() - y as f64 - 0.5) * factor - skew_min;
-            // Get a blend factor in 0..256 constant across all pixels in the row.
-            let blend = (offset.fract() * 256.0) as u32;
-            let src_row = y * stride;
-            let dest_row = (y * skew_width + offset.floor() as usize) * 4;
-            let mut prev_px = [0u32; 4];
-            for (src, dest) in self.data[src_row..src_row + stride]
-                .chunks(4)
-                .zip(skew_buffer[dest_row..dest_row + stride].chunks_mut(4))
-            {
-                let px = [src[0] as u32, src[1] as u32, src[2] as u32, src[3] as u32];
-                // Blend current pixel with previous pixel based on blend factor.
-                let next_px = [px[0] * blend, px[1] * blend, px[2] * blend, px[3] * blend];
-                dest[0] = ((((px[0] << 8) - next_px[0]) + prev_px[0] + 128) >> 8) as u8;
-                dest[1] = ((((px[1] << 8) - next_px[1]) + prev_px[1] + 128) >> 8) as u8;
-                dest[2] = ((((px[2] << 8) - next_px[2]) + prev_px[2] + 128) >> 8) as u8;
-                dest[3] = ((((px[3] << 8) - next_px[3]) + prev_px[3] + 128) >> 8) as u8;
-                // Save the remainder for blending onto the next pixel.
-                prev_px = next_px;
-            }
-            // If the skew misaligns the final pixel, write out the remainder.
-            if blend > 0 {
-                let dest = &mut skew_buffer[dest_row + stride..dest_row + stride + 4];
-                dest[0] = ((prev_px[0] + 128) >> 8) as u8;
-                dest[1] = ((prev_px[1] + 128) >> 8) as u8;
-                dest[2] = ((prev_px[2] + 128) >> 8) as u8;
-                dest[3] = ((prev_px[3] + 128) >> 8) as u8;
-            }
-        }
-        Self {
-            data: skew_buffer,
-            height: self.height,
-            width: skew_width,
-            bearing_x: self.bearing_x + PixelLength::new(skew_min),
-            bearing_y: self.bearing_y,
-            has_color: self.has_color,
-        }
-    }
-}
-
 /// Rasterizes the specified glyph index in the associated font
 /// and returns the generated bitmap
 pub trait FontRasterizer {


Test patch: