From 5046fc225992db6ba2ef8812743fadfdfe4b184a Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Sat, 3 Feb 2024 11:06:03 -0700 Subject: [PATCH] Revert "text cursor glyph renders at native cell size" This reverts commit 2c95b98447ac88e0327b84c725efc7b837bdbd75. 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: https://github.com/wez/wezterm/issues/2882 --- docs/changelog.md | 2 + wezterm-gui/src/customglyph.rs | 79 ++++------------------------------ wezterm-gui/src/utilsprites.rs | 12 +----- 3 files changed, 11 insertions(+), 82 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 3d99d1d473d..96dd5b36726 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -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 diff --git a/wezterm-gui/src/customglyph.rs b/wezterm-gui/src/customglyph.rs index fb53680eac3..1b9b7699c40 100644 --- a/wezterm-gui/src/customglyph.rs +++ b/wezterm-gui/src/customglyph.rs @@ -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 => { @@ -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); } } } @@ -3734,7 +3727,6 @@ impl GlyphCache { polys: &[Poly], buffer: &mut Image, aa: PolyAA, - transform: Transform, ) { let (width, height) = buffer.image_dimensions(); let mut pixmap = @@ -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); } } @@ -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 { @@ -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, @@ -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( @@ -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) => { @@ -3879,8 +3820,7 @@ impl GlyphCache { style: PolyStyle::OutlineHeavy, }], &mut buffer, - PolyAA::MoarPixels, - transform, + PolyAA::AntiAlias, ); } Some(CursorShape::BlinkingUnderline | CursorShape::SteadyUnderline) => { @@ -3895,8 +3835,7 @@ impl GlyphCache { style: PolyStyle::OutlineHeavy, }], &mut buffer, - PolyAA::MoarPixels, - transform, + PolyAA::AntiAlias, ); } } @@ -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(), }; @@ -4099,7 +4037,6 @@ impl GlyphCache { } else { PolyAA::MoarPixels }, - Transform::identity(), ); } } diff --git a/wezterm-gui/src/utilsprites.rs b/wezterm-gui/src/utilsprites.rs index eef75e393b6..b50982d1b68 100644 --- a/wezterm-gui/src/utilsprites.rs +++ b/wezterm-gui/src/utilsprites.rs @@ -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 { @@ -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, } } @@ -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, } } @@ -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; @@ -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, }) } }