fix: address CodeRabbit review feedback on transforms, formatting, and UI

This commit is contained in:
Combiler01 committed 2026-10-31 19:20:44 +05:30
1 parent b512e3627f
commit d67e720eed
7 files changed
+90 -17

No files matched your search

+15 -7
View File
@@ -309,7 +309,12 @@ fn run_edit_page_image(doc: &mut pdfcraft_cos::Document, page: usize, index: usi
};
let added_items: Vec<(usize, pdfcraft_edit::Added)> = pdfcraft_edit::list_added(doc).into_iter().filter(|a| a.page == page).enumerate().collect();
let matched_added = if let Some(st_ref) = target_stream_obj {
let matched_added = if let (Some(st_ref), Some(target_ref)) = (target_stream_obj, img.object) {
added_items
.iter()
.find(|(_, a)| a.obj == st_ref && matches!(&a.content, AddedContent::Image(ai) if ai.image == target_ref))
.or_else(|| added_items.iter().find(|(_, a)| matches!(&a.content, AddedContent::Image(ai) if ai.image == target_ref)))
} else if let Some(st_ref) = target_stream_obj {
added_items.iter().find(|(_, a)| a.obj == st_ref)
} else if let Some(target_ref) = img.object {
added_items.iter().find(|(_, a)| matches!(&a.content, AddedContent::Image(ai) if ai.image == target_ref))
@@ -384,29 +389,32 @@ fn run_edit_page_image(doc: &mut pdfcraft_cos::Document, page: usize, index: usi
let is_perpendicular = t_display[0].abs() < 1e-5 && t_display[3].abs() < 1e-5;
if is_parallel {
if t_display[0] < 0.0 {
let (toggle_h, toggle_v) =
if ai.rotation % 2 == 1 { (t_display[3] < 0.0, t_display[0] < 0.0) } else { (t_display[0] < 0.0, t_display[3] < 0.0) };
if toggle_h {
ai.flip_h = !ai.flip_h;
}
if t_display[3] < 0.0 {
if toggle_v {
ai.flip_v = !ai.flip_v;
}
ai.rect = bbox(t_display, ai.rect);
} else if is_perpendicular {
let b_pos = t_display[1] > 0.0;
let c_pos = t_display[2] > 0.0;
let (quarters_ccw, flip_h, flip_v) = match (b_pos, c_pos) {
let (quarters_ccw, disp_flip_h, disp_flip_v) = match (b_pos, c_pos) {
(true, false) => (1, false, false),
(false, true) => (3, false, false),
(true, true) => (1, false, true),
(false, false) => (1, true, false),
};
ai.rotation = (ai.rotation + quarters_ccw) % 4;
if flip_h {
let (toggle_h, toggle_v) = if ai.rotation % 2 == 1 { (disp_flip_v, disp_flip_h) } else { (disp_flip_h, disp_flip_v) };
if toggle_h {
ai.flip_h = !ai.flip_h;
}
if flip_v {
if toggle_v {
ai.flip_v = !ai.flip_v;
}
ai.rotation = (ai.rotation + quarters_ccw) % 4;
ai.rect = bbox(t_display, ai.rect);
} else {
return Err(EditError::Edit(pdfcraft_edit::EditError::Invalid(
+35
View File
@@ -9493,6 +9493,41 @@ fn transform_on_added_image_folds_quarter_turns_and_rejects_non_axis_aligned() {
assert!(res_skew.unwrap_err().to_string().contains("non-axis-aligned"));
}
#[test]
fn test_added_image_transform_reflections_with_rotation() {
let make_png = |rgb: [u8; 3]| {
let mut png = Vec::new();
{
let mut enc = png::Encoder::new(&mut png, 2, 2);
enc.set_color(png::ColorType::Rgb);
let data = [rgb[0], rgb[1], rgb[2], rgb[0], rgb[1], rgb[2], rgb[0], rgb[1], rgb[2], rgb[0], rgb[1], rgb[2]];
enc.write_header().unwrap().write_image_data(&data).unwrap();
}
Arc::new(png)
};
let png = make_png([255, 0, 0]);
let (mut s, id) = session_with(1);
s.apply(id, Edit::AddImage { page: 0, rect: Some([100.0, 100.0, 200.0, 150.0]), name: "img.png".into(), bytes: png }).unwrap();
// Rotate by 90° CW (quarter turns CCW: rotation = 3):
s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Rotate(1) }).unwrap();
let cos = pdfcraft_cos::Document::open(s.get(id).unwrap().bytes.clone()).unwrap();
let AddedContent::Image(ai) = &pdfcraft_edit::list_added(&cos)[0].content else { panic!() };
assert_eq!(ai.rotation, 3);
assert!(!ai.flip_h);
assert!(!ai.flip_v);
// Apply a horizontal flip in display space (parallel transform with m0 = -1):
// For rotation = 3 (odd rotation), display horizontal flip corresponds to local vertical flip (flip_v):
s.apply(id, Edit::EditPageImage { page: 0, index: 0, change: ImageEdit::Transform([-1.0, 0.0, 0.0, 1.0, 300.0, 0.0]) }).unwrap();
let cos2 = pdfcraft_cos::Document::open(s.get(id).unwrap().bytes.clone()).unwrap();
let AddedContent::Image(ai2) = &pdfcraft_edit::list_added(&cos2)[0].content else { panic!() };
assert_eq!(ai2.rotation, 3);
assert!(!ai2.flip_h, "flip_h should NOT be toggled for horizontal flip when rotation is 90°");
assert!(ai2.flip_v, "flip_v should be toggled for horizontal flip when rotation is 90°");
}
/// ISO 32000-2 Table 192, 12.7.5.2.3 and Table 228 (Comb): a check box, a push button, a comb
/// field the file leaves to the viewer. The page as displayed and printed as an image shows the
/// check box's caption symbol in its text colour, the button's grey face and the comb's cells in
+2 -2
View File
@@ -372,7 +372,7 @@ impl PdfCraftApp {
if !self.is_tool_pinned(id) {
self.pinned_tools.push(id.to_string());
if let Some(g) = pdfcraft_engine::catalog::group(id) {
self.notify_fmt("Pinned {tool} to Home", &[("tool", g.label)]);
self.notify_fmt("Pinned {tool} to Home", &[("tool", tl!(g.label))]);
}
return true;
}
@@ -384,7 +384,7 @@ impl PdfCraftApp {
if let Some(pos) = self.pinned_tools.iter().position(|t| t == id) {
self.pinned_tools.remove(pos);
if let Some(g) = pdfcraft_engine::catalog::group(id) {
self.notify_fmt("Unpinned {tool} from Home", &[("tool", g.label)]);
self.notify_fmt("Unpinned {tool} from Home", &[("tool", tl!(g.label))]);
}
}
}
+6 -2
View File
@@ -310,8 +310,12 @@ impl ImageObb {
/// Resizes the OBB proportionally along its local axes using projected mouse delta,
/// locking the diagonally opposite corner as the fixed anchor point.
pub fn scale_proportional(&self, handle: ImageHandle, delta: Vec2, min_size: f32) -> Self {
let u_vec = vec2(self.u_axis.0 as f32, self.u_axis.1 as f32);
let v_vec = vec2(self.v_axis.0 as f32, self.v_axis.1 as f32);
let u_diff = self.s10 - self.s00;
let v_diff = self.s01 - self.s00;
let u_len = u_diff.length();
let v_len = v_diff.length();
let u_vec = if u_len > 1e-4 { u_diff / u_len } else { vec2(1.0, 0.0) };
let v_vec = if v_len > 1e-4 { v_diff / v_len } else { vec2(0.0, 1.0) };
let delta_u = delta.dot(u_vec);
let delta_v = delta.dot(v_vec);
let min_s = min_size.max(5.0);
+12 -3
View File
@@ -231,8 +231,9 @@ pub(crate) fn panel(
ui.add_space(4.0);
// Size and colour | bullets, numbering and alignment.
let mut size_committed = false;
row(ui, |ui| {
font_size_input(ui, &mut look.size, fonts.mixed_size);
size_committed = font_size_input(ui, &mut look.size, fonts.mixed_size);
let swatch = colour_button(ui, t, look.color);
egui::Popup::menu(&swatch).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| {
if let Some(picked) = crate::comments::swatch_grid(ui, Some(look.color)) {
@@ -351,7 +352,7 @@ pub(crate) fn panel(
number_with_presets(ui, spacing, &CHAR_SPACINGS, half, (label.id, tl!("Character spacing")));
});
out.changed = (look.clone(), extras.as_deref().copied()) != before;
out.changed = size_committed || (look.clone(), extras.as_deref().copied()) != before;
ui.add_space(6.0);
ui.separator();
out
@@ -440,7 +441,8 @@ fn colour_button(ui: &mut egui::Ui, t: &Tokens, color: [f64; 3]) -> egui::Respon
/// Commit a typed size once (Enter or leaving the field), rather than one undo step per key.
/// The bounds match AddedText validation, so the panel also accepts coloured text below 8 pt.
fn font_size_input(ui: &mut egui::Ui, size: &mut f64, mixed: bool) {
fn font_size_input(ui: &mut egui::Ui, size: &mut f64, mixed: bool) -> bool {
let mut committed = false;
ui.scope(|ui| {
ui.spacing_mut().item_spacing.x = 2.0;
let id = ui.make_persistent_id("font-size-input");
@@ -459,6 +461,9 @@ fn font_size_input(ui: &mut egui::Ui, size: &mut f64, mixed: bool) {
response.surrender_focus();
} else if response.lost_focus() || (response.has_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter))) {
if let Some(value) = typed_font_size(&text) {
if mixed || *size != value {
committed = true;
}
*size = value;
text = size.to_string();
} else {
@@ -470,6 +475,9 @@ fn font_size_input(ui: &mut egui::Ui, size: &mut f64, mixed: bool) {
egui::Popup::menu(&list).show(|ui| {
for value in SIZES {
if ui.selectable_label(*size == value, format!("{value} pt")).clicked() {
if mixed || *size != value {
committed = true;
}
*size = value;
ui.close();
}
@@ -480,6 +488,7 @@ fn font_size_input(ui: &mut egui::Ui, size: &mut f64, mixed: bool) {
}
ui.data_mut(|d| d.insert_temp(id, (size.to_bits(), text)));
});
committed
}
fn typed_font_size(text: &str) -> Option<f64> {
+1 -1
View File
@@ -1048,7 +1048,7 @@ impl crate::PdfCraftApp {
}
let command_recent = self
.last_command_frame
.is_some_and(|(f, t)| self.frame_count.saturating_sub(f) <= 10 || (inp.time > 0.0 && (inp.time - t).abs() < 0.35));
.is_some_and(|(f, t)| if inp.time > 0.0 { (inp.time - t).abs() < 0.35 } else { self.frame_count.saturating_sub(f) <= 10 });
let command_active = modifiers.command || command_recent;
if !*pressed && !self.v_pressed && command_active {
released = true;
+19 -2
View File
@@ -77,8 +77,10 @@ pub fn toggle_bullet_list(text: &mut String) -> bool {
new_text.push('\n');
}
let trimmed = line.trim_start();
let indent = &line[..line.len() - trimmed.len()];
if all_bulleted {
if let Some(rest) = trimmed.strip_prefix("• ").or_else(|| trimmed.strip_prefix("- ")) {
new_text.push_str(indent);
new_text.push_str(rest);
} else {
new_text.push_str(line);
@@ -86,8 +88,9 @@ pub fn toggle_bullet_list(text: &mut String) -> bool {
} else if trimmed.is_empty() || trimmed.starts_with("• ") {
new_text.push_str(line);
} else {
new_text.push_str(indent);
new_text.push_str("• ");
new_text.push_str(line);
new_text.push_str(trimmed);
}
}
if has_trailing_crlf {
@@ -126,12 +129,14 @@ pub fn toggle_numbered_list(text: &mut String) -> bool {
new_text.push('\n');
}
let t = line.trim_start();
let indent = &line[..line.len() - t.len()];
if all_numbered {
if let Some(dot_idx) = t.find('.')
&& dot_idx > 0
&& t[..dot_idx].chars().all(|c| c.is_ascii_digit())
&& t[dot_idx..].starts_with(". ")
{
new_text.push_str(indent);
new_text.push_str(t.get(dot_idx + 2..).unwrap_or(""));
continue;
}
@@ -140,6 +145,7 @@ pub fn toggle_numbered_list(text: &mut String) -> bool {
new_text.push_str(line);
} else {
use std::fmt::Write as _;
new_text.push_str(indent);
let _ = write!(new_text, "{num}. ");
new_text.push_str(t);
num += 1;
@@ -697,7 +703,6 @@ impl LineEditor {
self.look0 = self.look.clone();
self.extras0 = self.extras;
self.original = self.text.clone();
self.selection = None;
// The page now has this text: the box's history restarts, even when the format left the
// text as it was, so a Redo from before can't put undone typing back on top of it.
self.typing_from = None;
@@ -1717,6 +1722,12 @@ C", &picked
assert!(toggle_bullet_list(&mut text));
assert!(!is_bulleted(&text));
assert_eq!(text, "First line\nSecond line\n\nThird line");
let mut indented = " First line\n Second line".to_string();
assert!(toggle_bullet_list(&mut indented));
assert_eq!(indented, " • First line\n • Second line");
assert!(toggle_bullet_list(&mut indented));
assert_eq!(indented, " First line\n Second line");
}
#[test]
@@ -1731,6 +1742,12 @@ C", &picked
assert!(toggle_numbered_list(&mut text));
assert!(!is_numbered(&text));
assert_eq!(text, "Item A\nItem B\n\nItem C");
let mut indented = " Item A\n Item B".to_string();
assert!(toggle_numbered_list(&mut indented));
assert_eq!(indented, " 1. Item A\n 2. Item B");
assert!(toggle_numbered_list(&mut indented));
assert_eq!(indented, " Item A\n Item B");
}
#[test]