mirror of
https://github.com/zed-industries/zed.git
synced 2026-08-15 20:14:37 +00:00
Fix post-taffy-bump layouting issues (#61107)
Follow-up to https://github.com/zed-industries/zed/pull/60721 Before: <img width="564" height="470" alt="before" src="https://github.com/user-attachments/assets/d825464d-f8d9-4fa1-bee3-42b5f615d4ba" /> <img width="745" height="221" alt="image" src="https://github.com/user-attachments/assets/18139bec-6778-491a-9aa5-0112bd121723" /> After: <img width="576" height="472" alt="after" src="https://github.com/user-attachments/assets/4c3c37cf-12c4-421e-98e9-cc6e40aa099d" /> <img width="812" height="275" alt="image" src="https://github.com/user-attachments/assets/407e06b4-101f-4984-8d5b-4e33895b5ae9" /> ### Bug 1 — constraint-blind text layout cache (the collapse) `TextLayout`'s measure closure is stateful (each call overwrites the shared element state used for painting), and its cache check served cached sizes to unconstrained probes regardless of the constraints they were computed under: 1. `uniform_list::measure_item` lays an item out at `(MinContent, MinContent)`; the row's `min_w_0` lets it shrink to ~0, so labels get truncated at a tiny width — and that tiny size lands in the element state. 2. The real layout probes the label's flex base size with `available_space.width = MaxContent` (taffy maps Definite → MaxContent). Truncated labels are `whitespace_nowrap`, so both `wrap_width` and `truncate_width` were `None` → the cache check passed → the poisoned tiny size became the flex basis → labels rendered at ~5 chars. **Fix**: record `truncate_width` in `TextLayoutInner` and never serve a cached layout that was computed *with* truncation to an unconstrained probe; the honest intrinsic size is recomputed instead, and the final `PerformLayout` call re-truncates against the real resolved width. ### Bug 2 — truncation width math disagrees with shaping (the ~2-char loss) After fix 1, rows got their full width, but exact-fit strings like `Cargo.toml` still lost ~2 chars: - Sizing measures via `shape_text` (real shaping with kerning); the element gets exactly `ceil(shaped_width)`. - The truncation decision (`LineWrapper::should_truncate_line_middle` etc.) sums **per-character advances** — no kerning — with zero tolerance, overestimating width for some glyph sequences. Text sitting in a box of exactly its own measured width got truncated, losing ~2 chars to fit the `…`. String-dependent (kerning pairs), hence `Cargo.toml` broke while `Cargo.lock` didn't. - Previously masked because taffy 0.10 usually skipped the final measure invocation, so the truncation path rarely ran with an exact-fit width. **Fix**: before truncating (nowrap case, `wrap_width.is_none()`), shape the untruncated text and skip truncation if the *shaped* width fits — matching CSS `text-overflow`, which only activates on actual overflow. Cheap: the shaping result is a `line_layout_cache` hit from the earlier untruncated sizing probe. ### Bug 3 — `MinContent` used instead of `MaxContent` that caused over truncation of certain labels Release Notes: - N/A
This commit is contained in:
parent
a0bb97cad5
commit
b05f40c554
3 changed files with 26 additions and 4 deletions
|
|
@ -1038,7 +1038,7 @@ impl StateInner {
|
|||
let mut rendered_focused_item = false;
|
||||
|
||||
let available_item_space = size(
|
||||
available_width.map_or(AvailableSpace::MinContent, |width| {
|
||||
available_width.map_or(AvailableSpace::MaxContent, |width| {
|
||||
AvailableSpace::Definite(width)
|
||||
}),
|
||||
AvailableSpace::MinContent,
|
||||
|
|
|
|||
|
|
@ -618,6 +618,7 @@ struct TextLayoutInner {
|
|||
lines: SmallVec<[WrappedLine; 1]>,
|
||||
line_height: Pixels,
|
||||
wrap_width: Option<Pixels>,
|
||||
truncate_width: Option<Pixels>,
|
||||
size: Option<Size<Pixels>>,
|
||||
bounds: Option<Bounds<Pixels>>,
|
||||
}
|
||||
|
|
@ -680,16 +681,20 @@ impl TextLayout {
|
|||
// 2. wrap_width matches (or both are None)
|
||||
// 3. truncate_width is None (if truncate_width is Some, we need to re-layout
|
||||
// because the previous layout may have been computed without truncation)
|
||||
// 4. the cached layout was not truncated (a truncated layout answers an
|
||||
// unconstrained probe with the truncated size, which poisons intrinsic
|
||||
// sizing with whatever width some earlier measure pass happened to use)
|
||||
if let Some(text_layout) = element_state.0.borrow().as_ref()
|
||||
&& let Some(size) = text_layout.size
|
||||
&& (wrap_width.is_none() || wrap_width == text_layout.wrap_width)
|
||||
&& truncate_width.is_none()
|
||||
&& text_layout.truncate_width.is_none()
|
||||
{
|
||||
return size;
|
||||
}
|
||||
|
||||
let mut line_wrapper = cx.text_system().line_wrapper(text_style.font(), font_size);
|
||||
let (text, runs) = if truncate_width.is_some() {
|
||||
let (text, runs) = if let Some(truncate_width) = truncate_width {
|
||||
if let Some(max_lines) = text_style.line_clamp
|
||||
&& let Some(wrap_width) = wrap_width
|
||||
{
|
||||
|
|
@ -701,10 +706,25 @@ impl TextLayout {
|
|||
&runs,
|
||||
truncate_from,
|
||||
)
|
||||
} else if let Some(unclipped) = window
|
||||
.text_system()
|
||||
.shape_text(text.clone(), font_size, &runs, None, None)
|
||||
.log_err()
|
||||
&& unclipped
|
||||
.iter()
|
||||
.all(|line| line.size(line_height).width <= truncate_width)
|
||||
{
|
||||
// The truncation decision below sums per-character advances,
|
||||
// which overestimates the shaped width (no kerning), truncating
|
||||
// text that fits exactly in its measured width. Skip truncation
|
||||
// whenever the honestly-shaped text fits; the shaping result
|
||||
// comes from the line layout cache when the same text was
|
||||
// already measured untruncated this frame.
|
||||
(text.clone(), Cow::Borrowed(&*runs))
|
||||
} else {
|
||||
line_wrapper.truncate_line(
|
||||
text.clone(),
|
||||
truncate_width.unwrap_or(Pixels::MAX),
|
||||
truncate_width,
|
||||
&truncation_affix,
|
||||
&runs,
|
||||
truncate_from,
|
||||
|
|
@ -731,6 +751,7 @@ impl TextLayout {
|
|||
len: 0,
|
||||
line_height,
|
||||
wrap_width,
|
||||
truncate_width,
|
||||
size: Some(Size::default()),
|
||||
bounds: None,
|
||||
});
|
||||
|
|
@ -749,6 +770,7 @@ impl TextLayout {
|
|||
len,
|
||||
line_height,
|
||||
wrap_width,
|
||||
truncate_width,
|
||||
size: Some(size),
|
||||
bounds: None,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -671,7 +671,7 @@ impl UniformList {
|
|||
return Size::default();
|
||||
};
|
||||
let available_space = size(
|
||||
list_width.map_or(AvailableSpace::MinContent, |width| {
|
||||
list_width.map_or(AvailableSpace::MaxContent, |width| {
|
||||
AvailableSpace::Definite(width)
|
||||
}),
|
||||
AvailableSpace::MinContent,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue