perf(gpu): closing a document frees what the compositor holds for it (#4529) (#4772)

GpuCanvas::retain called forget_doc only for documents still in its
texture map. A document whose entry had been dropped earlier (texture
creation failed, a fresh render failed after uploading, or the canvas
outgrew its budget after Canvas Size) kept its resident pages, effect
maps and lower-stack composite after it was closed, until another
document pushed the budgets over; with no other GPU document, for good.

Compositor::retain_docs forgets every document the caller doesn't list
as open, whatever the texture map says, and retain calls it each time.
The open documents' ids, their preview ids and the adjustment proxies
are already in that list, so nothing in use is dropped.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
FinnNk 2026-10-13 17:07:27 +01:00 committed by GitHub
parent 533b9a1c62
commit 33728243fb
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 52 additions and 6 deletions

View File

@ -845,6 +845,26 @@ impl Compositor {
self.prefix.forget(doc);
}
/// [`Self::forget_doc`] for every document `keep` rejects: what the compositor holds for
/// documents that were closed, however their canvas entry went (#4529).
pub fn retain_docs(&mut self, keep: impl Fn(DocId) -> bool) {
let mut gone: Vec<DocId> =
self.residents.values().map(|r| r.doc).chain(self.fx.values().map(|e| e.doc)).chain(self.prefix.doc()).filter(|d| !keep(*d)).collect();
gone.sort_by_key(|d| d.0);
gone.dedup();
for d in gone {
self.forget_doc(d);
}
}
/// Documents the compositor holds pages, effect maps or a lower-stack composite for.
pub fn held_docs(&self) -> Vec<DocId> {
let mut docs: Vec<DocId> = self.residents.values().map(|r| r.doc).chain(self.fx.values().map(|e| e.doc)).chain(self.prefix.doc()).collect();
docs.sort_by_key(|d| d.0);
docs.dedup();
docs
}
/// GPU memory held by cached effect maps (bytes).
pub fn fx_cache_bytes(&self) -> usize {
self.fx.values().map(FxEntry::bytes).sum()

View File

@ -85,6 +85,11 @@ impl Prefix {
}
}
/// The document the cached composite belongs to, if any.
pub(crate) fn doc(&self) -> Option<photocraft_doc::DocId> {
self.context.as_ref().map(|d| d.id)
}
pub(crate) fn prepare(&mut self, doc: &Document, plan: &Plan<'_>) -> Option<Checkpoint> {
if self.budget == 0 {
return None;

View File

@ -187,6 +187,27 @@ fn check(g: &mut Gpu, doc: &Document, what: &str) {
diff_rect(g, doc, doc.bounds(), what).unwrap_or_else(|e| panic!("{e}"));
}
/// Closing a document frees what the compositor holds for it, however its canvas entry went
/// (#4529): `retain_docs` drops the resident pages, effect maps and lower-stack composite of
/// documents that aren't open, and keeps the open ones.
#[test]
fn retain_docs_frees_closed_documents() {
let Some(mut g) = gpu() else { return };
let a = base_doc(64, 48);
let mut b = base_doc(32, 32);
b.id = photocraft_doc::DocId(a.id.0 + 1);
for d in [&a, &b] {
render_to_vec(&mut g.comp, &g.device, &g.queue, d, d.bounds()).expect("render");
}
assert_eq!(g.comp.held_docs(), vec![a.id, b.id]);
g.comp.retain_docs(|d| d == b.id);
assert_eq!(g.comp.held_docs(), vec![b.id], "only the open document is left");
g.comp.retain_docs(|_| false);
assert!(g.comp.held_docs().is_empty());
assert_eq!(g.comp.composite_cache_bytes(), 0);
check(&mut g, &a, "renders again after being forgotten");
}
#[test]
fn first_frame_renders() {
// Regression: the first submission after device creation is dropped on some drivers

View File

@ -638,14 +638,14 @@ impl GpuCanvas {
let mut renderer = self.rs.renderer.write();
let Some(res) = renderer.callback_resources.get_mut::<Resources>() else { return };
if res.docs.keys().any(|k| !live.contains(k)) {
let gone: Vec<u64> = res.docs.keys().filter(|k| !live.contains(k)).copied().collect();
if let Some(c) = res.compositor.as_mut() {
for k in &gone {
c.forget_doc(photocraft_doc::DocId(*k));
}
}
res.docs.retain(|k, _| live.contains(k));
}
// Everything the compositor holds for documents that aren't open, also when their
// texture entry was dropped earlier (over budget, a failed render), which used to leave
// their pages and effect maps resident after closing (#4529).
if let Some(c) = res.compositor.as_mut() {
c.retain_docs(|d| live.contains(&d.0));
}
// LUTs and identity-transform signatures can outlive document textures. Prune both,
// or a preserved-ID reopen can skip rebuilding a missing LUT.
res.luts.retain(|k, _| live.contains(&k.0));