#3692: a linked chain past 16 widths reuses the nearest; opaque pictures and budget refusals don't cost a later picture its outline

Ultrareview finding 6: boxes past a chain's 16th distinct width (compared
by exact float equality) showed no text, which resized boxes hit easily.
Widths are now told apart to a twip, and past the cap a box takes the
nearest layout already made (same direction, widest that fits, else the
narrowest). Work stays bounded at 16 story layouts.

Ultrareview finding 7 (also raised by CodeRabbit): opaque non-JPEG
pictures were decoded and charged to the per-layout pixel budget though
they give no outline, and a budget refusal was cached in LayoutCache as a
permanent "no outline". The header (read via into_decoder, PNG tRNS
included) now says whether a picture can be transparent before anything is
charged; Alphas::outline reports OverBudget distinctly from Ok(None), and
Ctx::wrap_outline (via picture_outline) doesn't keep a refusal in
LayoutCache::outlines. Within one layout a refused picture is remembered
so its header isn't re-read.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Martin Ball
2026-10-25 21:01:51 -04:00
committed by Dylan M. Taylor
co-authored by Claude Opus 5.5
parent ed78539488
commit 32e4900bc5
5 changed files with 156 additions and 46 deletions
+35 -17
View File
@@ -5,6 +5,7 @@
//! A [`Contour`] is built once per placed object; each line then asks it which stretches of
//! the line's band it takes ([`Contour::occupied`]), a walk over its edges.
use std::collections::HashSet;
use std::hash::{Hash, Hasher};
use std::sync::Arc;
@@ -187,12 +188,19 @@ impl Alpha {
}
}
/// A picture whose transparency would take its layout past [`MAX_DECODED_PIXELS`]: it wraps
/// as its rectangle in this layout only.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub(crate) struct OverBudget;
/// Pictures' transparency decoded during one layout. A picture shown with many crops (each an
/// outline of its own) is decoded once, and decoding stops at [`MAX_DECODED_PIXELS`].
#[derive(Default)]
pub(crate) struct Alphas {
/// The picture decoded last: its bytes (address and length) and transparency (`None`: none).
last: Option<((usize, usize), Option<Arc<Alpha>>)>,
/// Pictures refused for the budget (bytes' address and length), not looked at again.
refused: HashSet<(usize, usize)>,
/// Pixels decoded so far.
pub(crate) decoded: u64,
}
@@ -200,40 +208,50 @@ pub(crate) struct Alphas {
impl Alphas {
/// The outline of picture `bytes`'s opaque pixels within `crop` (left, top, right, bottom
/// fractions), in unit coordinates of the cropped picture. `None` for pictures without
/// transparency (or that don't decode, or past the budget): their rectangle is their outline.
pub(crate) fn outline(&mut self, bytes: &[u8], crop: [f32; 4]) -> Option<Vec<(f32, f32)>> {
/// transparency (or that don't decode), and [`OverBudget`] past the budget: their rectangle
/// is their outline.
pub(crate) fn outline(&mut self, bytes: &[u8], crop: [f32; 4]) -> Result<Option<Vec<(f32, f32)>>, OverBudget> {
let key = (bytes.as_ptr() as usize, bytes.len());
let alpha = match &self.last {
Some((k, alpha)) if *k == key => alpha.clone(),
_ if self.refused.contains(&key) => return Err(OverBudget),
_ => {
let alpha = decode_alpha(bytes, &mut self.decoded).map(Arc::new);
let alpha = decode_alpha(bytes, &mut self.decoded).inspect_err(|_| {
self.refused.insert(key);
})?;
let alpha = alpha.map(Arc::new);
self.last = Some((key, alpha.clone()));
alpha
}
};
alpha_in_crop(&*alpha?, crop)
Ok(alpha.and_then(|a| alpha_in_crop(&a, crop)))
}
}
/// The transparency of picture `bytes`, charging its pixels to `decoded`. `None` for pictures
/// without transparency, that don't decode, or that would take `decoded` past
/// [`MAX_DECODED_PIXELS`].
fn decode_alpha(bytes: &[u8], decoded: &mut u64) -> Option<Alpha> {
let pic = wordcraft_geom::picture::probe(bytes).ok()?;
/// without transparency or that don't decode; [`OverBudget`] for one that would take `decoded`
/// past [`MAX_DECODED_PIXELS`]. Pictures whose header says they're opaque cost nothing.
fn decode_alpha(bytes: &[u8], decoded: &mut u64) -> Result<Option<Alpha>, OverBudget> {
use image::ImageDecoder;
let Ok(pic) = wordcraft_geom::picture::probe(bytes) else { return Ok(None) };
// Formats that can't be transparent aren't decoded.
if pic.format == image::ImageFormat::Jpeg || pic.width > MAX_PICTURE_SIDE || pic.height > MAX_PICTURE_SIDE {
return None;
return Ok(None);
}
// Only the header is read here (a PNG's `tRNS` counts as transparency).
let has_alpha = image::ImageReader::with_format(std::io::Cursor::new(bytes), pic.format)
.into_decoder()
.ok()
.is_some_and(|d| d.color_type().has_alpha());
if !has_alpha {
return Ok(None);
}
let pixels = u64::from(pic.width) * u64::from(pic.height);
let total = decoded.checked_add(pixels).filter(|t| *t <= MAX_DECODED_PIXELS)?;
*decoded = total;
let img = pic.decode(wordcraft_geom::MAX_DECODED_BYTES).ok()?;
if !img.color().has_alpha() {
return None;
}
*decoded = decoded.checked_add(pixels).filter(|t| *t <= MAX_DECODED_PIXELS).ok_or(OverBudget)?;
let Ok(img) = pic.decode(wordcraft_geom::MAX_DECODED_BYTES) else { return Ok(None) };
let img = img.into_rgba8();
let (w, h) = img.dimensions();
Some(Alpha { w, h, a: img.pixels().map(|p| p.0[3]).collect() })
Ok(Some(Alpha { w, h, a: img.pixels().map(|p| p.0[3]).collect() }))
}
/// The outline of `img`'s opaque pixels within `crop`: see [`Alphas::outline`].
@@ -273,7 +291,7 @@ mod tests {
use super::*;
fn picture_outline(bytes: &[u8], crop: [f32; 4]) -> Option<Vec<(f32, f32)>> {
Alphas::default().outline(bytes, crop)
Alphas::default().outline(bytes, crop).ok().flatten()
}
fn diamond(through: bool) -> Contour {
+25 -10
View File
@@ -535,7 +535,7 @@ pub struct LayoutCache {
measures: HashMap<Key, (f32, f32)>,
/// Outlines of pictures' opaque pixels (Tight and Through wrapping), by media key, the
/// media's bytes (address and length) and crop.
outlines: HashMap<(String, usize, usize, [u32; 4]), Option<Arc<Vec<(f32, f32)>>>>,
outlines: Outlines,
env: u64,
used: HashSet<Key>,
/// The STYLEREF style index of the last pass and what it was built from: the body blocks
@@ -793,6 +793,29 @@ fn with_pin<T>(ctx: &mut Ctx, id: u32, sect: &SectionProps, number: u32, f: impl
(out, behind, front)
}
/// Picture outlines by media name, bytes (address and length) and crop ([`LayoutCache`]).
type Outlines = HashMap<(String, usize, usize, [u32; 4]), Option<Arc<Vec<(f32, f32)>>>>;
/// Picture `bytes`'s outline within `crop`, kept in `outlines` under `key`. One refused for this
/// layout's decoding budget wraps as its rectangle and isn't kept: a later layout may decode it.
fn picture_outline(
outlines: &mut Outlines,
alphas: &mut contour::Alphas,
key: (String, usize, usize, [u32; 4]),
bytes: &[u8],
crop: [f32; 4],
) -> Option<Arc<Vec<(f32, f32)>>> {
if let Some(o) = outlines.get(&key) {
return o.clone();
}
let outline = alphas.outline(bytes, crop).ok()?.map(Arc::new);
if outlines.len() >= 256 {
outlines.clear();
}
outlines.insert(key, outline.clone());
outline
}
impl Ctx<'_> {
/// The outline Tight and Through wrapping follow around `o` (see [`contour::wrap_outline`]);
/// pictures' are worked out once per picture and crop.
@@ -801,15 +824,7 @@ impl Ctx<'_> {
contour::wrap_outline(o, |media, crop| {
let bytes = doc.media.get(media)?;
let key = (media.to_string(), Arc::as_ptr(bytes) as usize, bytes.len(), crop.map(f32::to_bits));
if let Some(o) = cache.outlines.get(&key) {
return o.clone();
}
if cache.outlines.len() >= 256 {
cache.outlines.clear();
}
let outline = alphas.outline(bytes, crop).map(Arc::new);
cache.outlines.insert(key, outline.clone());
outline
picture_outline(&mut cache.outlines, alphas, key, bytes, crop)
})
}
+80 -10
View File
@@ -38,7 +38,8 @@ fn text_box(d: &mut Document, para: usize, text: &str, w: f32, h: f32) -> u32 {
#[test]
fn a_linked_chain_lays_its_story_out_at_a_bounded_number_of_widths() {
// Forty linked boxes, each a line deep and each a different width, over a long story: every
// width would lay the whole story out again. Past the cap, the chain's later boxes show nothing.
// width would lay the whole story out again. Past the cap, later boxes reuse the nearest
// width already laid out, so every box still shows its share of the story.
let n = 40;
let mut d = Document::from_text(&vec!["p"; n].join("\n"));
let story = "Words that run on and on through every box of the chain. ".repeat(200);
@@ -47,10 +48,42 @@ fn a_linked_chain_lays_its_story_out_at_a_bounded_number_of_widths() {
d.parts.get_mut(&w[0]).unwrap().body.next = Some(w[1]);
}
let l = lay(&d);
let shown =
l.pages.iter().flat_map(|p| &p.items).filter(|it| matches!(it, Placed::Lines { story: StoryRef::Part(s), .. } if *s == ids[0])).count();
assert!(shown > 1, "the first boxes still show the story: {shown}");
assert!(shown <= textbox::MAX_FLOW_WIDTHS, "{shown} boxes laid the story out");
let lines: Vec<&Placed> =
l.pages.iter().flat_map(|p| &p.items).filter(|it| matches!(it, Placed::Lines { story: StoryRef::Part(s), .. } if *s == ids[0])).collect();
assert_eq!(lines.len(), n, "every box shows the story");
// Each width the story was laid out at is its own paragraph layout.
let mut widths: Vec<usize> =
lines.iter().filter_map(|it| if let Placed::Lines { para, .. } = it { Some(Arc::as_ptr(para) as usize) } else { None }).collect();
widths.sort_unstable();
widths.dedup();
assert!(widths.len() <= textbox::MAX_FLOW_WIDTHS, "{} widths laid the story out", widths.len());
}
#[test]
fn boxes_a_hair_apart_in_width_share_a_layout() {
// Resized boxes differ by fractions of a point; those within a twentieth of a point of each
// other share one layout of the story.
let n = 30;
let mut d = Document::from_text(&vec!["p"; n].join("\n"));
let story = "Words that run on and on through every box of the chain. ".repeat(100);
let ids: Vec<u32> = (0..n).map(|i| text_box(&mut d, i, if i == 0 { &story } else { "" }, 150.0 + 0.0005 * i as f32, 28.0)).collect();
for w in ids.windows(2) {
d.parts.get_mut(&w[0]).unwrap().body.next = Some(w[1]);
}
let l = lay(&d);
let mut widths: Vec<usize> = l
.pages
.iter()
.flat_map(|p| &p.items)
.filter_map(|it| match it {
Placed::Lines { story: StoryRef::Part(s), para, .. } if *s == ids[0] => Some(Arc::as_ptr(para) as usize),
_ => None,
})
.collect();
assert_eq!(widths.len(), n, "every box shows the story");
widths.sort_unstable();
widths.dedup();
assert_eq!(widths.len(), 1, "one layout for every box");
}
#[test]
@@ -116,7 +149,7 @@ fn a_picture_cropped_many_ways_is_decoded_once() {
let mut alphas = contour::Alphas::default();
for i in 0..300 {
let f = i as f32 / 1000.0;
assert!(alphas.outline(&png, [f, f, 0.0, 0.0]).is_some(), "crop {i} follows the disc");
assert!(alphas.outline(&png, [f, f, 0.0, 0.0]).is_ok_and(|o| o.is_some()), "crop {i} follows the disc");
}
assert_eq!(alphas.decoded, 64 * 64, "one decode for every crop");
// A crop's outline is the one the picture alone gives.
@@ -129,15 +162,52 @@ fn decoding_pictures_stops_at_the_layouts_budget() {
let (a, b) = (disc_png(64), disc_png(32));
let mut alphas = contour::Alphas::default();
alphas.decoded = contour::MAX_DECODED_PIXELS - 64 * 64 + 1;
// Too many pixels left to decode: the picture wraps as its rectangle.
assert!(alphas.outline(&a, [0.0; 4]).is_none());
// Too many pixels left to decode: the picture wraps as its rectangle, and says why.
assert_eq!(alphas.outline(&a, [0.0; 4]), Err(contour::OverBudget));
// A smaller one still fits.
assert!(alphas.outline(&b, [0.0; 4]).is_some());
assert!(alphas.outline(&b, [0.0; 4]).is_ok_and(|o| o.is_some()));
assert!(alphas.decoded <= contour::MAX_DECODED_PIXELS);
// Pictures alternating past the budget aren't decoded again.
for _ in 0..10 {
assert!(alphas.outline(&a, [0.0; 4]).is_none());
assert_eq!(alphas.outline(&a, [0.0; 4]), Err(contour::OverBudget));
let _ = alphas.outline(&b, [0.0; 4]);
}
assert!(alphas.decoded <= contour::MAX_DECODED_PIXELS);
}
/// A `size` × `size` PNG with no transparency.
fn opaque_png(size: u32) -> Vec<u8> {
let img = image::RgbImage::from_pixel(size, size, image::Rgb([0, 0, 0]));
let mut buf = Vec::new();
image::DynamicImage::from(img).write_to(&mut std::io::Cursor::new(&mut buf), image::ImageFormat::Png).unwrap();
buf
}
#[test]
fn opaque_pictures_take_none_of_the_decoding_budget() {
// Pictures whose header says they have no transparency have no outline to find: decoding
// them would only spend the budget a transparent picture after them needs.
let (opaque, disc) = (opaque_png(64), disc_png(64));
let mut alphas = contour::Alphas::default();
alphas.decoded = contour::MAX_DECODED_PIXELS - 64 * 64;
for _ in 0..3 {
assert_eq!(alphas.outline(&opaque, [0.0; 4]), Ok(None));
}
assert!(alphas.outline(&disc, [0.0; 4]).is_ok_and(|o| o.is_some()), "the transparent picture still has its outline");
assert_eq!(alphas.decoded, contour::MAX_DECODED_PIXELS);
}
#[test]
fn a_picture_refused_for_the_budget_is_not_cached_across_layouts() {
// One layout ran out of budget before this picture; the next has a budget of its own.
let disc = disc_png(64);
let mut outlines = HashMap::new();
let key = ("p".to_string(), 1, 2, [0; 4]);
let mut spent = contour::Alphas::default();
spent.decoded = contour::MAX_DECODED_PIXELS;
assert!(picture_outline(&mut outlines, &mut spent, key.clone(), &disc, [0.0; 4]).is_none());
assert!(outlines.is_empty(), "the refusal isn't kept");
let mut fresh = contour::Alphas::default();
assert!(picture_outline(&mut outlines, &mut fresh, key.clone(), &disc, [0.0; 4]).is_some());
assert!(outlines.get(&key).is_some_and(|o| o.is_some()), "the outline is kept");
}
+14 -7
View File
@@ -210,10 +210,16 @@ struct Cut {
/// Most line lengths (and text directions) a chain's story is laid out at. Each lays the whole
/// story out again and is kept while the chain is split, so a chain of boxes that each differ in
/// width would multiply its story's layout work and memory; boxes needing another one show no
/// text. Far above a real chain's few box sizes.
/// width would multiply its story's layout work and memory; boxes needing another one take the
/// nearest already laid out. Far above a real chain's few box sizes.
pub(crate) const MAX_FLOW_WIDTHS: usize = 16;
/// Line lengths are told apart to a twentieth of a point (a twip): resized boxes a hair apart
/// share one layout.
fn flow_key(len: f32, vert: TextVert) -> (i32, TextVert) {
((len * 20.0).round() as i32, vert)
}
/// Split the text of `chain`'s first box over its boxes.
fn flow(ctx: &mut Ctx, chain: &[u32], depth: usize) -> Vec<Vec<Placed>> {
let Some(&head) = chain.first() else { return Vec::new() };
@@ -227,7 +233,7 @@ fn flow(ctx: &mut Ctx, chain: &[u32], depth: usize) -> Vec<Vec<Placed>> {
// time), sharing a layout with the boxes before it of the same length.
let snap = (ctx.counters.clone(), ctx.eq_count);
let mut after = None;
let mut layouts: Vec<((u32, TextVert), Vec<Placed>)> = Vec::new();
let mut layouts: Vec<((i32, TextVert), Vec<Placed>)> = Vec::new();
let mut out = Vec::with_capacity(chain.len());
// Where the next box's text starts: `Some(None)` at the beginning, `None` when all is shown.
let mut start: Option<Option<Cut>> = Some(None);
@@ -239,13 +245,14 @@ fn flow(ctx: &mut Ctx, chain: &[u32], depth: usize) -> Vec<Vec<Placed>> {
let body = ctx.doc.parts.get(id).map(|p| p.body).unwrap_or_default();
let (w, h) = sizes.get(id).copied().unwrap_or((0.0, 0.0));
let (len, across) = frame(&body, w, h);
let key = (len.to_bits(), body.vert);
let key = flow_key(len, body.vert);
let items = match layouts.iter().find(|(k, _)| *k == key) {
Some((_, items)) => items.clone(),
// Past the cap the text stops here: this box and the rest show none.
// Past the cap the box takes the nearest layout: the same direction first, then the
// widest line length that fits it, else the narrowest.
None if layouts.len() >= MAX_FLOW_WIDTHS => {
out.push(Vec::new());
continue;
let near = layouts.iter().min_by_key(|((k, v), _)| (*v != key.1, *k > key.0, k.abs_diff(key.0)));
near.map(|(_, items)| items.clone()).unwrap_or_default()
}
None => {
(ctx.counters, ctx.eq_count) = (snap.0.clone(), snap.1);
+2 -2
View File
@@ -67,8 +67,8 @@ Exact line spacing keeps the run font baseline around tall equations; automatic
| Shape fills: gradients and transparency | 🟡 | #1500: linear (angle, `scaled`) and circular gradients and transparent fills drawn on screen, PNG and PDF (axial/radial shadings), soft edges fading the real fill; rectangular and shape-following path gradients drawn circular, `tileRect`/`flip` not drawn, outline gradients drawn in their middle colour, `rotWithShape="0"` drawn as turning with the shape | 2–4 |
| Text wrap: square, top-and-bottom, behind, in front | ✅ | 2026-10-06; beside an object, a line's first word that doesn't fit the strip goes on whole to the row's next strip or the next row, instead of being split (#1376, `layout::tests::a_word_too_wide_for_the_strip_beside_a_float_moves_on_whole`; inferred from a reporter's comparison, not yet measured in Word) a paragraph running on into the next column or page is broken again there around that column's shapes, its own floating objects left where they are (#1374). Text in an earlier column doesn't yet wrap around a shape anchored in a later one (#1158) | #884: an object dragged on a multi-column page anchors to a paragraph in the column it lands in, so that column's text wraps around it. Text laid out before the anchor (an earlier column) doesn't yet wrap around an object anchored later on the page, nor do lines a paragraph continues into the next column (broken once, in the column it starts in) |
| Text wrap: square, top-and-bottom, behind, in front | ✅ | 2026-10-06; beside an object, a line's first word that doesn't fit the strip goes on whole to the row's next strip or the next row, instead of being split (#1376, `layout::tests::a_word_too_wide_for_the_strip_beside_a_float_moves_on_whole`; inferred from a reporter's comparison, not yet measured in Word) a paragraph running on into the next column or page is broken again there around that column's shapes, its own floating objects left where they are (#1374). Text in an earlier column doesn't yet wrap around a shape anchored in a later one (#1158) | #884: an object dragged on a multi-column page anchors to a paragraph in the column it lands in, so that column's text wraps around it. Paragraphs above an object's anchor wrap around it when it is placed higher up the page (the page is laid out again with it known from the start, #2048; when that wrap pushes the anchor onto the next page, the first page keeps the room and the object goes with its anchor, and a layout that still hasn't settled moves the object down to its paragraph rather than over text); text in an earlier column beside it doesn't yet, nor do lines a paragraph continues into the next column (broken once, in the column it starts in) |
| Text wrap: tight, through (contour, wrap polygon) | 🟡 | #354: lines follow the wrap polygon, or the outline worked out from the object (shape geometry; a picture's opaque pixels as a convex hull; otherwise the rectangle), turned and flipped with it, kept the left/right distance from text; Through also uses gaps inside the outline. Wrap side (`wrapText`: both sides, left only, right only, largest side per line) applies to square, tight and through wraps, turned objects included; picture outlines are convex; a picture is decoded once for all its crops, and at most 2^28 pixels per layout are decoded (later pictures wrap as rectangles, #3692); not compared page-for-page with Word | 1–2 |
| Text boxes, overflow, linked text boxes | ✅ | #46; linked text boxes (#369): the text runs on through each box in turn at that box's line length, overflow past the last box hidden; Align Text top / middle / bottom; the box's own internal margins, and a box set to Resize shape to fit text shows all its text (#743); a chain's story is laid out at no more than 16 different box widths, later boxes needing another show nothing (#3692) | — |
| Text wrap: tight, through (contour, wrap polygon) | 🟡 | #354: lines follow the wrap polygon, or the outline worked out from the object (shape geometry; a picture's opaque pixels as a convex hull; otherwise the rectangle), turned and flipped with it, kept the left/right distance from text; Through also uses gaps inside the outline. Wrap side (`wrapText`: both sides, left only, right only, largest side per line) applies to square, tight and through wraps, turned objects included; picture outlines are convex; a picture is decoded once for all its crops, pictures whose header says they are opaque are not decoded, and at most 2^28 pixels per layout are decoded (later pictures wrap as rectangles in that layout, #3692); not compared page-for-page with Word | 1–2 |
| Text boxes, overflow, linked text boxes | ✅ | #46; linked text boxes (#369): the text runs on through each box in turn at that box's line length, overflow past the last box hidden; Align Text top / middle / bottom; the box's own internal margins, and a box set to Resize shape to fit text shows all its text (#743); a chain's story is laid out at no more than 16 box widths (told apart to a twip), later boxes needing another taking the nearest one (#3692) | — |
| Rotated objects and text | 🟡 | #332: pictures, shapes, charts and groups drawn turned and flipped about their centre (screen and PDF); square wrap and inline lines keep clear of the rotated bounds; hit testing on the turned shape. Text in a rotated text box stays upright | 2–3 |
| Group connector rerouting | 🟡 | Glued ends keep their existing connection-site path; free ends, missing targets and invalid sites retain their displayed rotated and flipped position when the connector frame is rebuilt, including under nonuniform group scaling. Model and engine regressions cover all connector kinds, both ends, rotations/flips, unit/uniform/nonuniform scale, idempotent rerouting, degenerate dimensions without nonfinite writes, and fallback spans beyond the 4000 pt display-size cap without endpoint movement or repeated shrinkage. Non-invertible display scales and unrepresentable fallback spans leave connectors needing fallback endpoints unchanged; fully resolved connectors still reroute. Invalid child extents use the renderer's unit-scale fallback. Routing around obstacles remains missing; no new native Word comparison | — |
| Page borders (from page edge or text), page colour, watermark | ✅ | | — |