From 445eb502ec4d576a35c91b9e420e066e03d7a506 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 14 Nov 2026 03:29:47 +0000 Subject: [PATCH] Render: a PDF layer variant keeps its identity while anything holds it The image cache keys decodes by the bytes' address, so a variant rebuilt as a new Arc misses the decodes it already has. With decodes in the background, more variants on screen than the variant cache kept would decode and re-render without end. Variants are now looked up weakly, with the 64 most recent held strongly. Based on #2051 by texjer. --- crates/render/src/pdf_layers.rs | 93 +++++++++++++++++++++++++++++---- 1 file changed, 82 insertions(+), 11 deletions(-) diff --git a/crates/render/src/pdf_layers.rs b/crates/render/src/pdf_layers.rs index f428b522e..5579de80d 100644 --- a/crates/render/src/pdf_layers.rs +++ b/crates/render/src/pdf_layers.rs @@ -2,8 +2,8 @@ //! file with some layers off (an appended catalog whose default configuration turns them off, //! which the PDF renderer honours). -use std::collections::HashMap; -use std::sync::{Arc, Mutex}; +use std::collections::{HashMap, VecDeque}; +use std::sync::{Arc, Mutex, Weak}; use craft_pdf::syntax::Pdf; use craft_pdf::syntax::object::{Array, Dict, MaybeRef, Name, ObjRef, Object, ObjectIdentifier}; @@ -237,21 +237,43 @@ pub fn pdf_hide_layers(bytes: &[u8], hidden: &[String]) -> Option> { Some(out) } -/// Shared copies of layer variants (by source address and hidden set). +/// Shared copies of layer variants (by source address and hidden set). A variant keeps its +/// identity (the same `Arc`) for as long as anything holds it: the image cache keys decodes by +/// the bytes' address, so a variant rebuilt as a new `Arc` would miss decodes it already has and +/// be decoded again, every frame once there are more variants on screen than are kept here. pub(crate) fn layered(data: &Arc>, hidden: &[String]) -> Option>> { type Key = (usize, Vec); - static CACHE: Mutex>, Option>>)>>> = Mutex::new(None); + /// Variants by key: the source (whose liveness says the address still means that file) and + /// the variant (`None`: the file couldn't be rewritten). Both weak; `KEEP` holds recent ones. + type Known = HashMap>, Option>>)>; + static CACHE: Mutex>, Option>>)>)>> = Mutex::new(None); + /// How many recent variants are kept alive here (each is a copy of its file). + const KEEP: usize = 64; let key = (Arc::as_ptr(data) as usize, hidden.to_vec()); let mut c = CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - let map = c.get_or_insert_with(HashMap::new); - if let Some(v) = map.get(&key) { - return v.1.clone(); - } - if map.len() > 64 { - map.clear(); + let (known, keep) = c.get_or_insert_with(Default::default); + if let Some((source, variant)) = known.get(&key) + && source.upgrade().is_some_and(|s| Arc::ptr_eq(&s, data)) + { + // `None`: this file couldn't be rewritten with these layers off. + if let Some(v) = variant.as_ref()?.upgrade() { + return Some(v); + } } let v = pdf_hide_layers(data, hidden).map(Arc::new); - map.insert(key, (data.clone(), v.clone())); + known.insert(key.clone(), (Arc::downgrade(data), v.as_ref().map(Arc::downgrade))); + keep.push_back((key, data.clone(), v.clone())); + while keep.len() > KEEP { + keep.pop_front(); + } + // Forget variants nothing holds any more, and those of files that are gone. + if known.len() > 4 * KEEP { + known.retain(|_, (source, variant)| source.strong_count() > 0 && variant.as_ref().is_none_or(|v| v.strong_count() > 0)); + // Failures are cheap to find again; a hostile run of hidden sets doesn't grow this. + if known.len() > 4 * KEEP { + known.retain(|_, (_, variant)| variant.is_some()); + } + } v } @@ -260,6 +282,55 @@ mod tests { use super::*; use craft_pdf::syntax::reader::{Reader, ReaderExt}; + /// A one-page PDF with two layers, Blue and Red. + fn two_layer_pdf() -> Vec { + let content = "/OC /a BDC 0 0 1 rg 0 0 50 100 re f EMC /OC /b BDC 1 0 0 rg 50 0 50 100 re f EMC"; + let objs = [ + "<>>>>>".to_string(), + "<>".to_string(), + "<>>>/Contents 4 0 R>>".to_string(), + format!("<>\nstream\n{content}\nendstream", content.len()), + "<>".to_string(), + "<>".to_string(), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offs = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offs.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{}\nendobj\n", i + 1, o).as_bytes()); + } + let x = out.len(); + let mut t = format!("xref\n0 {}\n0000000000 65535 f\r\n", objs.len() + 1); + for o in offs { + t.push_str(&format!("{o:010} 00000 n\r\n")); + } + t.push_str(&format!("trailer\n<>\nstartxref\n{x}\n%%EOF\n", objs.len() + 1)); + out.extend_from_slice(t.as_bytes()); + out + } + + /// A variant something still holds comes back as the same `Arc` however many others were made + /// since, so decodes keyed by its address keep matching; one nothing holds is made again. + #[test] + fn a_held_layer_variant_keeps_its_identity_past_the_cache() { + let data = Arc::new(two_layer_pdf()); + let blue = vec!["Blue".to_string()]; + let first = layered(&data, &blue).expect("Blue can be hidden"); + assert!(Arc::ptr_eq(&first, &layered(&data, &blue).unwrap())); + // More variants than are kept, each with a set of its own. + for i in 0..200 { + let _ = layered(&data, &["Red".to_string(), format!("unknown {i}")]); + } + assert!(Arc::ptr_eq(&first, &layered(&data, &blue).unwrap()), "still the same variant"); + let red = vec!["Red".to_string()]; + let weak = Arc::downgrade(&layered(&data, &red).unwrap()); + for i in 0..200 { + let _ = layered(&data, &["Blue".to_string(), format!("other {i}")]); + } + assert_eq!(weak.strong_count(), 0, "a variant nothing holds is let go"); + assert!(layered(&data, &red).is_some(), "and made again when asked for"); + } + #[test] fn trailer_lookup_does_not_take_guard_values_from_xref_comments() { for guard in ["/Encrypt 12 0 R", "/XRefStm 123"] {