Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/app/src/workspace/chrome.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ impl Workspace {

/// Jump in history: negative = undo n steps, positive = redo n steps.
pub fn history_jump(&mut self, steps: i32, cx: &mut Context<Self>) {
let profile_before = self.doc.as_ref().and_then(|doc| doc.icc_profile.clone());
if let Some(doc) = &mut self.doc {
if steps < 0 {
for _ in 0..(-steps) {
Expand All @@ -220,6 +221,9 @@ impl Workspace {
}
}
}
if self.doc.as_ref().and_then(|doc| doc.icc_profile.clone()) != profile_before {
self.rebuild_color_transforms();
}
self.after_change(cx);
}

Expand Down
18 changes: 11 additions & 7 deletions crates/app/src/workspace/colormgmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ impl Workspace {
/// Assign a profile: same numbers, new interpretation.
pub fn assign_profile(&mut self, profile: schist_colormgmt::Profile, cx: &mut Context<Self>) {
if let Some(doc) = self.doc.as_mut() {
doc.icc_profile = profile.icc_bytes().map(|b| b.to_vec());
doc.dirty = true;
let mut edit = doc.begin_edit(format!("Assign {}", profile.name()));
edit.set_icc_profile(profile.icc_bytes().map(|b| b.to_vec()));
edit.commit();
doc.damage_all();
}
self.status = format!("Assigned {}", profile.name()).into();
Expand Down Expand Up @@ -112,19 +113,22 @@ impl Workspace {
tile.encode_f32(&buf);
}
}
edit.set_icc_profile(profile.icc_bytes().map(|b| b.to_vec()));
edit.commit();
doc.icc_profile = profile.icc_bytes().map(|b| b.to_vec());
self.status = format!("Converted to {}", profile.name()).into();
self.rebuild_color_transforms();
self.after_change(cx);
}

/// Toggle soft proofing against a device profile.
pub fn toggle_proof(&mut self, profile: schist_colormgmt::Profile, cx: &mut Context<Self>) {
self.color.proof = match &self.color.proof {
Some(_) => None,
None => Some(profile),
};
// Picking a different proof profile while one is active switches to
// it; picking the active profile turns proofing off. Names are not
// identities: loaded device profiles use a generic display name.
let already_proofing_this = self.color.proof.as_ref().is_some_and(|current| {
current.icc_bytes() == profile.icc_bytes() && current.name() == profile.name()
});
self.color.proof = (!already_proofing_this).then_some(profile);
self.status = if self.color.proof.is_some() {
"Proof colors on".into()
} else {
Expand Down
7 changes: 7 additions & 0 deletions crates/app/src/workspace/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ impl Workspace {
if id.starts_with("edit.paste") {
self.sync_clipboard_in(cx);
}
let profile_before = self.doc.as_ref().and_then(|doc| doc.icc_profile.clone());
let Some(doc) = self.doc.as_mut() else { return };
if let Some(command) = self.registry.command(id) {
let mut ctx = CommandCtx {
Expand All @@ -80,6 +81,12 @@ impl Workspace {
} else {
log::warn!("unknown command {id}");
}
// Undo and redo are plugin commands, so a profile restored by
// their history operation must rebuild the cached display hop.
// Damage alone only drops pixels rendered through that cache.
if self.doc.as_ref().and_then(|doc| doc.icc_profile.clone()) != profile_before {
self.rebuild_color_transforms();
}
self.after_change(cx);
}

Expand Down
25 changes: 22 additions & 3 deletions crates/codec-psd/src/writer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,26 @@ fn write_image_resources(b: &mut Buf, doc: &Document) {
continue; // written as its own section above
}
seen_resolution |= res.id == RES_RESOLUTION_INFO;
seen_icc |= res.id == RES_ICC_PROFILE;
// `doc.icc_profile` is the authority when it is set. Convert and
// Assign Profile rewrite it, while the preserved resource still
// describes the space the file arrived in; re-emitting that one
// tagged converted pixels with the profile they were converted
// away from.
//
// It goes in the preserved block's own place rather than being
// appended at the end. The reader always mirrors ICC into
// `doc.icc_profile`, so the substitution fires on every round
// trip, and moving the tag rewrote the resource section's layout
// for a file nothing had actually changed.
let mut data: &[u8] = &res.data;
if res.id == RES_ICC_PROFILE {
seen_icc = true;
// With no profile of its own the document keeps whatever the
// file arrived with, rather than losing its colour space.
if let Some(icc) = &doc.icc_profile {
data = icc;
}
}
b.bytes(b"8BIM");
b.u16(res.id);
// `name` holds the raw pascal bytes (length byte + content + pad)
Expand All @@ -152,8 +171,8 @@ fn write_image_resources(b: &mut Buf, doc: &Document) {
b.u8(0);
}
}
b.u32(res.data.len() as u32);
b.bytes(&res.data);
b.u32(data.len() as u32);
b.bytes(data);
b.pad_to(2);
}

Expand Down
102 changes: 102 additions & 0 deletions crates/codec-psd/tests/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,108 @@ fn psd_keeps_u32_lengths_for_the_same_keys() {
assert_eq!(block.data, b"preserved payload");
}

#[test]
fn a_converted_profile_replaces_the_preserved_one() {
// Convert/Assign Profile rewrite `doc.icc_profile` while the resource
// preserved from the source file still describes the old space. The
// writer re-emitted that one and suppressed the new tag, so converted
// pixels came back tagged with the profile they were converted away
// from.
let mut doc = base_doc();
doc.push_layer(solid_layer(
"l",
IntRect::from_xywh(0, 0, 8, 8),
[1, 2, 3, 255],
Depth::Eight,
));
// A file that arrived carrying one profile...
doc.preserved_resources.push(PreservedResource {
id: 0x040F,
name: Vec::new(),
data: b"OLD-PROFILE-BYTES".to_vec(),
});
// ...and was then converted to another.
doc.icc_profile = Some(b"NEW-PROFILE-BYTES".to_vec());

let back = read_psd(&write_psd(&doc).unwrap()).unwrap();
assert_eq!(
back.icc_profile.as_deref(),
Some(b"NEW-PROFILE-BYTES".as_slice()),
"the document's own profile must win"
);
let iccs = back
.preserved_resources
.iter()
.filter(|r| r.id == 0x040F)
.count();
assert!(iccs <= 1, "the stale profile must not be written alongside");
}

#[test]
fn an_untagged_document_keeps_a_preserved_profile() {
// The other direction: with no profile of its own, dropping the
// preserved resource would lose the file's colour space outright.
let mut doc = base_doc();
doc.push_layer(solid_layer(
"l",
IntRect::from_xywh(0, 0, 8, 8),
[1, 2, 3, 255],
Depth::Eight,
));
doc.preserved_resources.push(PreservedResource {
id: 0x040F,
name: Vec::new(),
data: b"ONLY-PROFILE".to_vec(),
});
doc.icc_profile = None;

let back = read_psd(&write_psd(&doc).unwrap()).unwrap();
assert_eq!(
back.icc_profile.as_deref(),
Some(b"ONLY-PROFILE".as_slice()),
"the preserved profile must survive"
);
}

#[test]
fn a_round_trip_leaves_the_resource_section_where_it_was() {
// The reader always mirrors ICC into `doc.icc_profile`, so the
// substitution above fires on every round trip. Appending the tag at
// the end instead of writing it in the preserved block's own place
// rewrote the resource layout of a file nothing had changed.
let mut doc = base_doc();
doc.push_layer(solid_layer(
"l",
IntRect::from_xywh(0, 0, 8, 8),
[1, 2, 3, 255],
Depth::Eight,
));
doc.preserved_resources.push(PreservedResource {
id: 0x040F,
name: Vec::new(),
data: b"THE-PROFILE".to_vec(),
});
doc.preserved_resources.push(PreservedResource {
id: 0x0421,
name: Vec::new(),
data: b"after-the-profile".to_vec(),
});
doc.icc_profile = Some(b"THE-PROFILE".to_vec());

let once = write_psd(&doc).unwrap();
let twice = write_psd(&read_psd(&once).unwrap()).unwrap();
assert_eq!(once, twice, "saving again moved bytes around");

let back = read_psd(&once).unwrap();
let ids: Vec<u16> = back.preserved_resources.iter().map(|r| r.id).collect();
let icc = ids.iter().position(|id| *id == 0x040F).expect("icc kept");
let after = ids
.iter()
.position(|id| *id == 0x0421)
.expect("the block after it kept");
assert!(icc < after, "the profile moved to the end: {ids:?}");
}

#[test]
fn an_adjustment_layer_made_in_schist_survives_a_save() {
// The critical one: adjustment layers created in the app carry their
Expand Down
Loading
Loading