mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-13 09:20:07 -06:00
LR-EDIT-OPTICS-PROFILE: a decode from before a lens profile change never returns to the source cache (CodeRabbit, #3073)
A job's source identity carries the photo's database profile with its content, and Session::accept keeps a loaded source only when both still match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
77e6578140
commit
0a9707efd6
@@ -99,6 +99,20 @@ impl QuickJob {
|
||||
}
|
||||
|
||||
/// What identifies a photo's pixels for caching: its content hash, else its source.
|
||||
/// Which decode of a photo a loaded source is: its file's content and, when it has one, its database lens profile
|
||||
/// ([`lens_key`]; the decode fits the starting colour through it). A job's source is kept only when this still
|
||||
/// matches when the job finishes ([`super::Session::accept`]), so a decode started before a profile change never
|
||||
/// returns to the cache after `cmd::lens_refresh` has dropped the photo's sources.
|
||||
pub fn source_identity(p: &Photo) -> Hash128 {
|
||||
let mut k = Hasher128::new();
|
||||
k.str(&content_key(p));
|
||||
let lens = lens_key(p);
|
||||
if lens != 0 {
|
||||
k.u64(lens);
|
||||
}
|
||||
k.finish()
|
||||
}
|
||||
|
||||
pub fn content_key(p: &Photo) -> String {
|
||||
match (&p.content_hash, &p.source) {
|
||||
(Some(h), _) => h.clone(),
|
||||
|
||||
@@ -47,8 +47,8 @@ impl crate::Session {
|
||||
// share a result (a view slot showing photo A would otherwise look current for photo B)
|
||||
// …and so is the file's content: a file changed on disk (Reload) renders afresh
|
||||
let content_key = content_key(&p);
|
||||
let source_key = Hasher128::new().str(&content_key).finish();
|
||||
let content = source_key.0 as u64;
|
||||
let content = Hasher128::new().str(&content_key).finish().0 as u64;
|
||||
let source_key = super::quick::source_identity(&p);
|
||||
// Temporary "before" settings must not replace the photo's cached develop hash.
|
||||
let settings_hash = if before { settings.hash64() } else { self.media.settings_hashes.get(id, &settings) };
|
||||
let salt = self.media.denoise_salt(id, &settings);
|
||||
@@ -136,7 +136,7 @@ impl crate::Session {
|
||||
Some(RenderJob {
|
||||
request_id: 0,
|
||||
cache_generation: self.media.rendered.generation(),
|
||||
source_key: Some(Hasher128::new().str(&content_key(&p)).finish()),
|
||||
source_key: Some(super::quick::source_identity(&p)),
|
||||
photo: id,
|
||||
level,
|
||||
source,
|
||||
@@ -191,7 +191,7 @@ impl crate::Session {
|
||||
Some(RenderJob {
|
||||
request_id: 0,
|
||||
cache_generation: self.media.rendered.generation(),
|
||||
source_key: Some(Hasher128::new().str(&content_key(&p)).finish()),
|
||||
source_key: Some(super::quick::source_identity(&p)),
|
||||
photo: id,
|
||||
level,
|
||||
source,
|
||||
@@ -389,7 +389,7 @@ impl crate::Session {
|
||||
/// has wrong is corrected too ([`Self::learn_as_shot_wb`]).
|
||||
pub fn accept(&mut self, r: &RenderResult) {
|
||||
if let Some(src) = &r.loaded
|
||||
&& r.source_key.is_none_or(|key| self.catalog.photo(r.photo).is_some_and(|p| Hasher128::new().str(&content_key(p)).finish() == key))
|
||||
&& r.source_key.is_none_or(|key| self.catalog.photo(r.photo).is_some_and(|p| super::quick::source_identity(p) == key))
|
||||
{
|
||||
if let Some(info) = &src.info {
|
||||
self.learn_as_shot_wb(r.photo, info);
|
||||
|
||||
@@ -88,6 +88,25 @@ fn source_from_before_content_reload_is_not_accepted() {
|
||||
s.accept(&old);
|
||||
assert_eq!(s.media.source_usage().0, 0);
|
||||
}
|
||||
/// A decode that started before the photo's database lens profile changed (a lens database arriving, looked up again
|
||||
/// by `cmd::lens_refresh`, which drops the photo's sources) never puts its source back in the cache: the source's
|
||||
/// colour was fitted through the old profile.
|
||||
#[test]
|
||||
fn source_from_before_a_lens_profile_change_is_not_accepted() {
|
||||
let mut s = session();
|
||||
s.media.file_loader = Some(Arc::new(|_, _| Ok((Rgb32f::new(8, 8), SourceInfo::default()))));
|
||||
let old = s.thumb_job(PhotoId(1), 256).unwrap().run();
|
||||
assert!(old.loaded.is_some());
|
||||
let lens = lightcraft_develop::EmbeddedLens { source: lightcraft_develop::LensSource::Database, ..Default::default() };
|
||||
s.catalog.apply(Op::SetEmbeddedLens { id: PhotoId(1), lens: Some(Box::new(lens)) }).unwrap();
|
||||
s.media.forget(PhotoId(1));
|
||||
s.accept(&old);
|
||||
assert_eq!(s.media.source_usage().0, 0, "the old decode is dropped");
|
||||
// a decode made with the new profile is kept
|
||||
let new = s.thumb_job(PhotoId(1), 256).unwrap().run();
|
||||
s.accept(&new);
|
||||
assert_ne!(s.media.source_usage().0, 0);
|
||||
}
|
||||
/// A panic while rendering (here in the decoder) used to end the pool's worker thread without
|
||||
/// an answer, so the view that asked showed "Rendering…" forever. It is the job's failure now,
|
||||
/// for renders and for stand-ins, answering the request it came from.
|
||||
|
||||
Reference in New Issue
Block a user