Skip to content

Commit

Permalink
Revert "text cursor glyph renders at native cell size"
Browse files Browse the repository at this point in the history 
This reverts commit 2c95b98.

More people were vocally and sometimes toxic about this change
than were in favor of it.

I'm just going to revert it and leave the original issue open.

refs: #2882
  • Loading branch information
@wez
wez committed Feb 3, 2024
1 parent 91749f7 commit 5046fc2
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 82 deletions.
2 changes: 2 additions & 0 deletions docs/changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ As features stabilize some brief notes about them will accumulate here.
`NO_HINTING` when the dpi is >= 100, otherwise `DEFAULT`. #4902
* `wezterm -e` will now wait for the spawned program to terminate before
it will itself terminate. Thanks to @vimpostor! #4535 #4523
* Reverted the text cursor cell dimension change due to overwhelming and
sometimes toxic feedback. #2882
#### New
* We now show the Lua version in the debug overlay. Thanks to @bbkane! #4943
* `wezterm start --new-tab` and `wezterm connect --new-tab` to request a new
Expand Down
79 changes: 8 additions & 71 deletions wezterm-gui/src/customglyph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,17 +245,10 @@ pub enum PolyStyle {
}

impl PolyStyle {
fn apply(
self,
width: f32,
paint: &Paint,
path: &Path,
pixmap: &mut PixmapMut,
transform: Transform,
) {
fn apply(self, width: f32, paint: &Paint, path: &Path, pixmap: &mut PixmapMut) {
match self {
PolyStyle::Fill => {
pixmap.fill_path(path, paint, FillRule::Winding, transform, None);
pixmap.fill_path(path, paint, FillRule::Winding, Transform::identity(), None);
}

PolyStyle::OutlineThin | PolyStyle::Outline | PolyStyle::OutlineHeavy => {
Expand All @@ -266,7 +259,7 @@ impl PolyStyle {
} else if self == PolyStyle::OutlineThin {
stroke.width = 1.2;
}
pixmap.stroke_path(path, paint, &stroke, transform, None);
pixmap.stroke_path(path, paint, &stroke, Transform::identity(), None);
}
}
}
Expand Down Expand Up @@ -3734,7 +3727,6 @@ impl GlyphCache {
polys: &[Poly],
buffer: &mut Image,
aa: PolyAA,
transform: Transform,
) {
let (width, height) = buffer.image_dimensions();
let mut pixmap =
Expand Down Expand Up @@ -3762,13 +3754,7 @@ impl GlyphCache {
item.to_skia(width, height, metrics.underline_height as f32, &mut pb);
}
let path = pb.finish().expect("poly path to be valid");
style.apply(
metrics.underline_height as f32,
&paint,
&path,
&mut pixmap,
transform,
);
style.apply(metrics.underline_height as f32, &paint, &path, &mut pixmap);
}
}

Expand All @@ -3782,9 +3768,6 @@ impl GlyphCache {
return Ok(sprite.clone());
}

let x_scale = metrics.native_cell_size.width as f32 / metrics.cell_size.width as f32;
let y_scale = metrics.native_cell_size.height as f32 / metrics.cell_size.height as f32;

let mut metrics = metrics.scale_cell_width(width as f64);
if let Some(d) = &self.fonts.config().cursor_thickness {
metrics.underline_height = d.evaluate_as_pixels(DimensionContext {
Expand All @@ -3794,12 +3777,6 @@ impl GlyphCache {
}) as isize;
}

let thickness_scale = y_scale.max(x_scale);
let thickness = ((metrics.underline_height as f32) / thickness_scale).ceil() as isize;
let x_translate = thickness - metrics.underline_height;

metrics.underline_height = thickness;

let mut buffer = Image::new(
metrics.cell_size.width as usize,
metrics.cell_size.height as usize,
Expand All @@ -3808,40 +3785,10 @@ impl GlyphCache {
let cell_rect = Rect::new(Point::new(0, 0), metrics.cell_size);
buffer.clear_rect(cell_rect, black);

let transform = if metrics.cell_size != metrics.native_cell_size {
Transform::from_scale(x_scale, y_scale).post_translate(
// No scaled X translation because cell_width just adds blank space
// into the right of the bitmap without adjusting any x-coords.
// We just need to compensate for the line thickness scaling so
// that we don't clip it out the left side of the pixmap
x_translate as f32,
// Y translation to parallel the centering effect of line_height
(metrics.native_cell_size.height as f32 - metrics.cell_size.height as f32) / -2.,
)
} else {
Transform::identity()
};

match shape {
None => {}
Some(CursorShape::Default) => {
self.draw_polys(
&metrics,
&[Poly {
path: &[
PolyCommand::MoveTo(BlockCoord::Zero, BlockCoord::Zero),
PolyCommand::LineTo(BlockCoord::One, BlockCoord::Zero),
PolyCommand::LineTo(BlockCoord::One, BlockCoord::One),
PolyCommand::LineTo(BlockCoord::Zero, BlockCoord::One),
PolyCommand::LineTo(BlockCoord::Zero, BlockCoord::Zero),
],
intensity: BlockAlpha::Full,
style: PolyStyle::Fill,
}],
&mut buffer,
PolyAA::MoarPixels,
transform,
);
buffer.clear_rect(cell_rect, SrgbaPixel::rgba(0xff, 0xff, 0xff, 0xff));
}
Some(CursorShape::BlinkingBlock | CursorShape::SteadyBlock) => {
self.draw_polys(
Expand All @@ -3853,18 +3800,12 @@ impl GlyphCache {
PolyCommand::LineTo(BlockCoord::One, BlockCoord::One),
PolyCommand::LineTo(BlockCoord::Zero, BlockCoord::One),
PolyCommand::LineTo(BlockCoord::Zero, BlockCoord::Zero),
// An extra, seemingly redundant path segment here is
// needed to workaround tiny-skia leaving a single blank
// pixel in the top left corner when the glyph metrics
// are scaled by cell_width and/or line_height
PolyCommand::LineTo(BlockCoord::One, BlockCoord::Zero),
],
intensity: BlockAlpha::Full,
style: PolyStyle::OutlineHeavy,
}],
&mut buffer,
PolyAA::MoarPixels,
transform,
PolyAA::AntiAlias,
);
}
Some(CursorShape::BlinkingBar | CursorShape::SteadyBar) => {
Expand All @@ -3879,8 +3820,7 @@ impl GlyphCache {
style: PolyStyle::OutlineHeavy,
}],
&mut buffer,
PolyAA::MoarPixels,
transform,
PolyAA::AntiAlias,
);
}
Some(CursorShape::BlinkingUnderline | CursorShape::SteadyUnderline) => {
Expand All @@ -3895,8 +3835,7 @@ impl GlyphCache {
style: PolyStyle::OutlineHeavy,
}],
&mut buffer,
PolyAA::MoarPixels,
transform,
PolyAA::AntiAlias,
);
}
}
Expand All @@ -3923,7 +3862,6 @@ impl GlyphCache {
underline_height: *underline_height,
strike_row: 0,
cell_size: cell_size.clone(),
native_cell_size: render_metrics.native_cell_size,
},
_ => render_metrics.clone(),
};
Expand Down Expand Up @@ -4099,7 +4037,6 @@ impl GlyphCache {
} else {
PolyAA::MoarPixels
},
Transform::identity(),
);
}
}
Expand Down
12 changes: 1 addition & 11 deletions wezterm-gui/src/utilsprites.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ pub struct RenderMetrics {
pub underline_height: IntPixelLength,
pub strike_row: IntPixelLength,
pub cell_size: Size,
pub native_cell_size: Size,
}

impl RenderMetrics {
Expand All @@ -34,15 +33,13 @@ impl RenderMetrics {
let descender_plus_two =
(2 * underline_height + descender_row).min(cell_height as isize - underline_height);
let strike_row = descender_row / 2;
let cell_size = Size::new(cell_width as isize, cell_height as isize);

Self {
descender: metrics.descender,
descender_row,
descender_plus_two,
strike_row,
cell_size,
native_cell_size: cell_size,
cell_size: Size::new(cell_width as isize, cell_height as isize),
underline_height,
}
}
Expand All @@ -62,7 +59,6 @@ impl RenderMetrics {
underline_height: self.underline_height,
strike_row: self.strike_row,
cell_size: size,
native_cell_size: self.native_cell_size,
}
}

Expand All @@ -77,11 +73,6 @@ impl RenderMetrics {
.default_font_metrics()
.context("failed to get font metrics!?")?;

let native_cell_size = Size::new(
metrics.cell_width.get() as isize,
metrics.cell_height.get() as isize,
);

let line_height = fonts.config().line_height;
let cell_width = fonts.config().cell_width;

Expand Down Expand Up @@ -140,7 +131,6 @@ impl RenderMetrics {
strike_row,
cell_size: Size::new(cell_width as isize, cell_height as isize),
underline_height,
native_cell_size,
})
}
}
Expand Down

0 comments on commit 5046fc2

Please sign in to comment.