diff --git a/docs/changelog.md b/docs/changelog.md index c729d97a1..b3075b9c0 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -59,6 +59,7 @@ As features stabilize some brief notes about them will accumulate here. * Hangul text in NFD was not always correctly composed when shaping fonts. [#1573](https://github.com/wez/wezterm/issues/1573) * Avoid OOM when processing sixels with huge repeat counts [#1610](https://github.com/wez/wezterm/issues/1610) * Set the sticky bit on socket and pid files created in `XDG_RUNTIME_DIR` to avoid removal by tmpwatch [#1601](https://github.com/wez/wezterm/issues/1601) +* Shaping combining sequences like `e U+20d7` could "lose" the vector symbol if the font produced an entry with no `x_advance`. [#1617](https://github.com/wez/wezterm/issues/1617) ### 20220101-133340-7edc5b5a diff --git a/wezterm-font/src/shaper/harfbuzz.rs b/wezterm-font/src/shaper/harfbuzz.rs index dccaee001..4d2756843 100644 --- a/wezterm-font/src/shaper/harfbuzz.rs +++ b/wezterm-font/src/shaper/harfbuzz.rs @@ -463,15 +463,13 @@ impl HarfbuzzShaper { let mut remaining_cells = cluster_info.cell_width; for info in infos.iter() { - if info.x_advance == 0 { - continue; - } - // Proportional width based on relative pixel dimensions vs. other glyphs in // this same cluster let weighted_cell_width = (cluster_info.cell_width as f64 * info.x_advance as f64 / total_width) .ceil() as u8; + // Note that weighted_cell_width can legitimately compute as zero here + // for the case where a combining mark composes over another glyph let weighted_cell_width = weighted_cell_width.min(remaining_cells); remaining_cells = remaining_cells.saturating_sub(weighted_cell_width); diff --git a/wezterm-gui/src/termwindow/render.rs b/wezterm-gui/src/termwindow/render.rs index 31ebbcf29..8c22f9798 100644 --- a/wezterm-gui/src/termwindow/render.rs +++ b/wezterm-gui/src/termwindow/render.rs @@ -1921,7 +1921,11 @@ impl super::TermWindow { // Iterate each cell that comprises this glyph. There is usually // a single cell per glyph but combining characters, ligatures // and emoji can be 2 or more cells wide. - for glyph_idx in 0..info.pos.num_cells as usize { + // Conversely, a combining glyph can produce an entry that is zero + // cells wide and which combines with a neighboring glyph that we + // (will|did) emit in another iteration of the glyph_info loop + // so we must ensure that we iterate and observe that! + for glyph_idx in 0..info.pos.num_cells.max(1) as usize { let cell_idx = visual_cell_idx + glyph_idx; if cell_idx >= num_cols { @@ -2013,36 +2017,46 @@ impl super::TermWindow { // width. We only use that if we're the first cell that // comprises this glyph; if for some reason the cursor position // is in the middle of a glyph we just use a single cell. - let cursor_width = params.line.cells()[cell_idx].width() as u8; + let cursor_width = params + .line + .cells() + .get(cell_idx) + .map(|c| c.width() as u8) + // skip in weird situations, like a combining character + // "e⃗" where we have two glyphs and one of them has + // num_cells=0. + .unwrap_or(0); - let mut quad = layers[0].allocate()?; - quad.set_position( - pos_x, - pos_y, - pos_x - + if params.use_pixel_positioning { - pixel_width - } else { - cursor_width as f32 * cell_width - }, - pos_y + cell_height, - ); - quad.set_hsv(hsv); - quad.set_has_color(false); + if cursor_width > 0 { + let mut quad = layers[0].allocate()?; + quad.set_position( + pos_x, + pos_y, + pos_x + + if params.use_pixel_positioning { + pixel_width + } else { + cursor_width as f32 * cell_width + }, + pos_y + cell_height, + ); + quad.set_hsv(hsv); + quad.set_has_color(false); - quad.set_texture( - gl_state - .glyph_cache - .borrow_mut() - .cursor_sprite( - cursor_shape, - ¶ms.render_metrics, - cursor_width, - )? - .texture_coords(), - ); + quad.set_texture( + gl_state + .glyph_cache + .borrow_mut() + .cursor_sprite( + cursor_shape, + ¶ms.render_metrics, + cursor_width, + )? + .texture_coords(), + ); - quad.set_fg_color(cursor_border_color); + quad.set_fg_color(cursor_border_color); + } } }