Fix up landed PRs from review

- #178: ToUnicode ranges looked up without indexing
- #465: frame_num gaps are concealed (bounded) even when the SPS forbids them; cut streams keep playing
- #471: control request lines up to 256 MB so picture/media inserts still fit
- #473: a merge that loses its only row or column is removed, with regression tests
- #521: AutoCorrect no longer rewrites real words (cant, wont, im)
- #654: inherit_placeholder_geometry's doc comment back on its function

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas
2026-10-14 14:31:01 -04:00
co-authored by Claude Opus 5.5
parent d498633faf
commit 8ee597216d
7 changed files with 35 additions and 19 deletions
+2 -1
View File
@@ -35,7 +35,8 @@ impl Default for Limits {
fn default() -> Self {
Self {
max_clients: 16,
max_line: 16 * 1024 * 1024,
// Picture and media inserts carry base64 payloads: room for files of about 190 MB.
max_line: 256 * 1024 * 1024,
queue: 64,
idle_timeout: Duration::from_secs(600),
write_timeout: Duration::from_secs(30),
-3
View File
@@ -368,8 +368,6 @@ pub const AUTOCORRECT: &[(&str, &str)] = &[
("dont", "don't"),
("doesnt", "doesn't"),
("didnt", "didn't"),
("cant", "can't"),
("wont", "won't"),
("isnt", "isn't"),
("embarass", "embarrass"),
("enviroment", "environment"),
@@ -379,7 +377,6 @@ pub const AUTOCORRECT: &[(&str, &str)] = &[
("goverment", "government"),
("happend", "happened"),
("i", "I"),
("im", "I'm"),
("ive", "I've"),
("immediatly", "immediately"),
("independant", "independent"),
+2 -3
View File
@@ -476,9 +476,8 @@ impl Decoder {
} else {
let max = sps.max_frame_num();
if sh.frame_num != self.prev_ref_frame_num && sh.frame_num != (self.prev_ref_frame_num + 1) % max {
// A gap is only legal when the SPS allows it (7.4.2.1.1); otherwise the stream is broken
// and filling it would only cost work, so the picture is rejected.
ensure!(sps.gaps_in_frame_num_allowed, "frame_num gap in a stream that does not allow gaps");
// A gap is only legal when the SPS allows it (7.4.2.1.1), but a cut or lossy stream has
// them anyway: those are concealed too rather than dropping the picture.
// frame_num gap (8.2.5.2): insert "non-existing" frames. Their samples are never used by
// conforming streams; copy the latest decoded frame for concealment.
let meta = Arc::new(output_meta(sps, pts, false));
+6 -5
View File
@@ -592,12 +592,13 @@ fn gap_stream(gaps: bool) -> Vec<u8> {
}
#[test]
fn frame_num_gap_is_rejected_when_the_sps_forbids_gaps() {
fn frame_num_gap_is_concealed_even_when_the_sps_forbids_gaps() {
// A cut or lossy stream: the gap is filled (bounded) instead of dropping the picture.
let mut dec = Decoder::with_threads(1);
let res = dec.decode(&gap_stream(false), 0);
let _ = dec.flush();
assert!(res.is_err() || dec.take_error().is_some(), "a forbidden gap must be reported");
assert_eq!(dec.stats().frame_num_gaps, 0);
let mut out = dec.decode(&gap_stream(false), 0).unwrap();
out.extend(dec.flush());
assert_eq!(out.len(), 2);
assert_eq!(dec.stats().frame_num_gaps, 1);
}
#[test]
+20 -2
View File
@@ -321,7 +321,8 @@ impl Merge {
} else {
self
};
(m.rs > 1 || m.gs > 1).then_some(m)
// A merge that lost its only row or column is gone, not a 0-span merge over its neighbours.
(m.rs >= 1 && m.gs >= 1 && (m.rs > 1 || m.gs > 1)).then_some(m)
}
fn delete_col(self, at: usize) -> Option<Self> {
let m = if at < self.c {
@@ -331,7 +332,8 @@ impl Merge {
} else {
self
};
(m.rs > 1 || m.gs > 1).then_some(m)
// A merge that lost its only row or column is gone, not a 0-span merge over its neighbours.
(m.rs >= 1 && m.gs >= 1 && (m.rs > 1 || m.gs > 1)).then_some(m)
}
}
@@ -552,6 +554,22 @@ mod tests {
assert_topology(&t);
}
#[test]
fn deleting_the_only_row_or_column_of_a_merge_removes_it() {
// A 1x3 merge across a row: deleting that row leaves no merge behind.
let mut t = Table::new(3, 3, 300.0, 30.0);
assert!(t.merge(1, 0, 1, 2));
assert!(t.delete_row(1));
assert!((0..t.n_rows()).all(|r| (0..t.n_cols()).all(|c| t.cell(r, c).is_some_and(|x| !x.is_covered() && x.grid_span <= 1))));
assert_topology(&t);
// A 3x1 merge down a column: deleting that column leaves no merge behind.
let mut t = Table::new(3, 3, 300.0, 30.0);
assert!(t.merge(0, 1, 2, 1));
assert!(t.delete_col(1));
assert!((0..t.n_rows()).all(|r| (0..t.n_cols()).all(|c| t.cell(r, c).is_some_and(|x| !x.is_covered() && x.row_span <= 1))));
assert_topology(&t);
}
#[test]
fn insert_delete_merge_split() {
let mut t = Table::new(3, 3, 300.0, 30.0);
+1 -1
View File
@@ -467,7 +467,7 @@ impl Exporter<'_> {
.enumerate()
.map(|(gi, g)| {
let next = run.glyphs.get(gi + 1).map_or(g.1 + run.face.advance(g.0) * run.size / run.face.upem.max(1.0), |nx| nx.1);
let range = ranges[gi].clone();
let range = ranges.get(gi).cloned().unwrap_or(text.len()..text.len());
KrillaGlyph::new(GlyphId::new(g.0), ((next - g.1) / run.size) as f32, 0.0, ((g.2 - y0) / run.size) as f32, 0.0, range, None)
})
.collect();
+4 -4
View File
@@ -745,10 +745,6 @@ pub fn color_map(cm: &El) -> Vec<(String, String)> {
cm.attrs.iter().filter(|(k, _)| !k.starts_with("xmlns")).map(|(k, v)| (k.clone(), v.clone())).collect()
}
/// Placeholders read without a geometry (an empty custom geometry, see `shapes::sp`) take the
/// geometry of the layout or master placeholder they inherit from, as PowerPoint draws them: a
/// layout's picture placeholder cut into a shape keeps that shape on the slide. Anything left
/// is a rectangle.
/// The anchor the theme's shape defaults (`a:objectDefaults/a:spDef/a:bodyPr`) give new shapes
/// text, if any.
fn theme_shape_anchor(t: &deckcraft_model::Theme) -> Option<deckcraft_model::text::Anchor> {
@@ -796,6 +792,10 @@ fn default_text_anchor(p: &mut Presentation) {
}
}
/// Placeholders read without a geometry (an empty custom geometry, see `shapes::sp`) take the
/// geometry of the layout or master placeholder they inherit from, as PowerPoint draws them: a
/// layout's picture placeholder cut into a shape keeps that shape on the slide. Anything left
/// is a rectangle.
fn inherit_placeholder_geometry(p: &mut Presentation) {
let unset = |g: &Geom| matches!(g, Geom::Custom { paths } if paths.is_empty());
// Masters first, then layouts (from their masters), then slides (from layouts and masters).