yenru0/zed
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(image_viewer): address review bugs and restore plain image fit to view

Browse Source
This commit is contained in:
Kushagra Kumar committed 2026-09-02 20:22:38 +05:30
1 parent 6ff96c4970
commit 15ff33bf54
2 files changed
+117 -51
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/image_viewer/src/image_viewer.rs
+64 -29
View File
@@ -181,6 +181,7 @@ impl ImageView {
previous.drop_atlas_entry(window);
} else {
previous.release(window, cx);
self.page_bounds.borrow_mut().clear();
}
}
@@ -270,10 +271,14 @@ impl ImageView {
.read(cx)
.image_metadata
.map(|m| (m.width, m.height));
self.page_bounds.borrow_mut().clear();
cx.emit(ImageViewEvent::TitleChanged);
cx.notify();
}
ImageItemEvent::ReloadNeeded => {}
ImageItemEvent::ReloadNeeded => {
self.page_bounds.borrow_mut().clear();
cx.notify();
}
}
}
@@ -375,8 +380,8 @@ impl ImageView {
fn ensure_pdf_selection_text(&mut self, cx: &mut Context<Self>) {
if let Some(ref mut sel) = self.pdf_selection {
if sel.text.is_empty() {
let pdf_info = self.image_item.read(cx).pdf_info.clone();
if let Some(pdf_info) = pdf_info {
let item = self.image_item.read(cx);
if let Some(ref pdf_info) = item.pdf_info {
sel.text = Self::collect_selection_text(
&pdf_info.pages,
sel.start_page,
@@ -394,19 +399,13 @@ impl ImageView {
if let Some(ref sel) = self.pdf_selection {
if !sel.text.is_empty() {
cx.write_to_clipboard(gpui::ClipboardItem::new_string(sel.text.clone()));
return;
}
}
if let Some(text) = self.image_item.read(cx).extract_text() {
if !text.is_empty() {
cx.write_to_clipboard(gpui::ClipboardItem::new_string(text));
}
}
}
fn select_all(&mut self, _: &SelectAll, _window: &mut Window, cx: &mut Context<Self>) {
let pdf_info = self.image_item.read(cx).pdf_info.clone();
if let Some(pdf_info) = pdf_info {
let item = self.image_item.read(cx);
if let Some(ref pdf_info) = item.pdf_info {
let cur_page = self.pdf_scroll_handle.top_item();
let target_page = self
.pdf_selection
@@ -441,12 +440,7 @@ impl ImageView {
.as_ref()
.is_some_and(|s| !s.text.is_empty());
let is_pdf = self.image_item.read(cx).is_pdf();
let has_doc_text = self
.image_item
.read(cx)
.extract_text()
.is_some_and(|t| !t.is_empty());
let can_copy = has_selection || has_doc_text;
let can_copy = has_selection;
let focus_handle = self.focus_handle.clone();
let context_menu = ContextMenu::build(window, cx, move |menu, _, _| {
@@ -496,7 +490,8 @@ impl ImageView {
}
}
// 2. Line-level hit with horizontal proximity
// 2. Line-level hit with horizontal proximity threshold
const MAX_LINE_PROXIMITY: f32 = 0.05;
let mut closest_on_line: Option<(usize, f32)> = None;
for (i, seg) in segments.iter().enumerate() {
let line_top = seg.y - seg.height * 0.2;
@@ -504,7 +499,8 @@ impl ImageView {
if norm_y >= line_top && norm_y <= line_bottom {
let seg_center_x = seg.x + seg.width * 0.5;
let dist_x = (norm_x - seg_center_x).abs();
if closest_on_line.as_ref().map_or(true, |(_, d)| dist_x < *d) {
let allowed_dist = MAX_LINE_PROXIMITY.max(seg.width * 1.5);
if dist_x <= allowed_dist && closest_on_line.as_ref().map_or(true, |(_, d)| dist_x < *d) {
closest_on_line = Some((i, dist_x));
}
}
@@ -513,13 +509,14 @@ impl ImageView {
return Some(idx);
}
// 3. Closest segment overall
// 3. Closest segment overall within strict proximity threshold (within 5% distance)
const MAX_OVERALL_DIST_SQ: f32 = 0.05 * 0.05;
let mut closest: Option<(usize, f32)> = None;
for (i, seg) in segments.iter().enumerate() {
let center_x = seg.x + seg.width * 0.5;
let center_y = seg.y + seg.height * 0.5;
let dist_sq = (norm_x - center_x).powi(2) + (norm_y - center_y).powi(2);
if closest.as_ref().map_or(true, |(_, d)| dist_sq < *d) {
if dist_sq <= MAX_OVERALL_DIST_SQ && closest.as_ref().map_or(true, |(_, d)| dist_sq < *d) {
closest = Some((i, dist_sq));
}
}
@@ -670,8 +667,8 @@ impl ImageView {
}
fn update_pdf_drag_selection(&mut self, cursor_pos: Point<Pixels>, cx: &mut Context<Self>) {
let pdf_info = self.image_item.read(cx).pdf_info.clone();
let Some(ref pdf_info) = pdf_info else { return; };
let item = self.image_item.read(cx);
let Some(ref pdf_info) = item.pdf_info else { return; };
if let Some((page_idx, seg_idx)) = self.find_page_and_segment_at(cursor_pos, pdf_info) {
if let Some(ref mut sel) = self.pdf_selection {
if sel.end_page != page_idx || sel.end_seg != seg_idx {
@@ -809,8 +806,8 @@ impl ImageView {
cx: &mut Context<Self>,
) {
let bounds = self.page_bounds.borrow().get(&page_index).copied();
let pdf_info = self.image_item.read(cx).pdf_info.clone();
let Some(pdf_info) = pdf_info else { return; };
let item = self.image_item.read(cx);
let Some(ref pdf_info) = item.pdf_info else { return; };
let Some(page) = pdf_info.pages.iter().find(|p| p.page_index == page_index) else { return; };
if click_count >= 3 {
@@ -891,8 +888,8 @@ impl ImageView {
let norm_x = ((p_x - b_ox) / b_w).clamp(0.0, 1.0);
let norm_y = ((p_y - b_oy) / b_h).clamp(0.0, 1.0);
let pdf_info = self.image_item.read(cx).pdf_info.clone();
if let Some(pdf_info) = pdf_info {
let item = self.image_item.read(cx);
if let Some(ref pdf_info) = item.pdf_info {
if let Some(page) = pdf_info.pages.iter().find(|p| p.page_index == page_index) {
if let Some(seg_idx) = Self::find_segment_at(&page.text_segments, norm_x, norm_y) {
let need_select = self.pdf_selection.as_ref().map_or(true, |s| {
@@ -1115,7 +1112,17 @@ impl Element for ImageContentElement {
let image_view = self.image_view.read(cx);
let image = image_view.image_item.read(cx).image.clone();
let zoom_level = image_view.zoom_level;
let first_layout = image_view.container_bounds.is_none();
let initial_zoom_level = first_layout
.then(|| {
image_view
.image_size
.map(|image_size| ImageView::compute_fit_to_view_zoom(bounds, image_size))
})
.flatten();
let zoom_level = initial_zoom_level.unwrap_or(image_view.zoom_level);
let pan_offset = image_view.pan_offset;
let border_color = cx.theme().colors().border;
@@ -1145,6 +1152,9 @@ impl Element for ImageContentElement {
let render_image = image.clone().use_render_image(window, cx);
this.update_displayed_image(&image, render_image, window, cx);
this.container_bounds = Some(bounds);
if let Some(initial_zoom_level) = initial_zoom_level {
this.zoom_level = initial_zoom_level;
}
});
let mut image_content = div()
@@ -1342,7 +1352,7 @@ impl Item for ImageView {
pending_image: None,
displayed_image: None,
pdf_scroll_handle: gpui::ScrollHandle::new(),
show_text_panel: self.show_text_panel,
show_text_panel: false,
text_buffer: None,
text_editor: None,
pdf_selection: self.pdf_selection.clone(),
@@ -2512,6 +2522,31 @@ mod tests {
assert!(!ImageView::is_safe_pdf_url("powershell:something"));
assert!(!ImageView::is_safe_pdf_url("ms-msdt:/id DevDiagnostic"));
}
#[test]
fn test_find_segment_at_whitespace_rejection() {
let segments = vec![
kkpdf_zed::PdfTextSegment {
text: "Hello".to_string(),
x: 0.2,
y: 0.2,
width: 0.1,
height: 0.05,
},
];
// Direct hit inside bounding box
assert_eq!(ImageView::find_segment_at(&segments, 0.25, 0.22), Some(0));
// Close horizontal proximity on same line
assert_eq!(ImageView::find_segment_at(&segments, 0.31, 0.22), Some(0));
// Far away horizontal position on same line (> allowed distance)
assert_eq!(ImageView::find_segment_at(&segments, 0.7, 0.22), None);
// Far away arbitrary whitespace (e.g. bottom margin)
assert_eq!(ImageView::find_segment_at(&segments, 0.8, 0.8), None);
}
}
mod persistence {
crates/kkpdf_zed/src/pdfium.rs
+53 -22
View File
@@ -33,6 +33,30 @@ static GLOBAL_PDFIUM: OnceLock<Arc<Mutex<SerializedPdfium>>> = OnceLock::new();
/// preventing out-of-memory denial of service attacks from malicious documents.
pub const MAX_PAGE_DIMENSION: f32 = 8192.0;
/// Computes target pixel dimensions preserving the page aspect ratio,
/// scaling down uniformly if either dimension exceeds `MAX_PAGE_DIMENSION`.
pub(crate) fn compute_target_dimensions(
page_width_pts: f32,
page_height_pts: f32,
dpi: f32,
zoom: f32,
) -> (i32, i32) {
let scale_factor = (dpi / 72.0) * zoom;
let raw_w = (page_width_pts * scale_factor).max(1.0);
let raw_h = (page_height_pts * scale_factor).max(1.0);
let clamp_scale = if raw_w > MAX_PAGE_DIMENSION || raw_h > MAX_PAGE_DIMENSION {
(MAX_PAGE_DIMENSION / raw_w).min(MAX_PAGE_DIMENSION / raw_h)
} else {
1.0
};
let target_w = (raw_w * clamp_scale).round().clamp(1.0, MAX_PAGE_DIMENSION) as i32;
let target_h = (raw_h * clamp_scale).round().clamp(1.0, MAX_PAGE_DIMENSION) as i32;
(target_w, target_h)
}
/// Thread-safe wrapper around a Pdfium instance.
#[derive(Clone)]
pub struct PdfiumEngine {
@@ -176,14 +200,12 @@ impl PdfiumEngine {
.get(page_index as u16)
.context("Requested page index out of bounds")?;
let target_width =
(page.width().value * (options.target_dpi / 72.0) * options.zoom_factor)
.round()
.clamp(1.0, MAX_PAGE_DIMENSION) as i32;
let target_height =
(page.height().value * (options.target_dpi / 72.0) * options.zoom_factor)
.round()
.clamp(1.0, MAX_PAGE_DIMENSION) as i32;
let (target_width, target_height) = compute_target_dimensions(
page.width().value,
page.height().value,
options.target_dpi,
options.zoom_factor,
);
let render_config = PdfRenderConfig::new()
.set_target_width(target_width)
@@ -236,14 +258,12 @@ impl PdfiumEngine {
let mut rendered_pages = Vec::with_capacity(total_pages);
for (idx, page) in doc.pages().iter().enumerate() {
let target_width =
(page.width().value * (options.target_dpi / 72.0) * options.zoom_factor)
.round()
.clamp(1.0, MAX_PAGE_DIMENSION) as i32;
let target_height =
(page.height().value * (options.target_dpi / 72.0) * options.zoom_factor)
.round()
.clamp(1.0, MAX_PAGE_DIMENSION) as i32;
let (target_width, target_height) = compute_target_dimensions(
page.width().value,
page.height().value,
options.target_dpi,
options.zoom_factor,
);
let render_config = PdfRenderConfig::new()
.set_target_width(target_width)
@@ -874,12 +894,12 @@ impl PdfiumEngine {
let page_w = page.width().value;
let page_h = page.height().value;
let target_width = (page_w * (options.target_dpi / 72.0) * options.zoom_factor)
.round()
.clamp(1.0, MAX_PAGE_DIMENSION) as i32;
let target_height = (page_h * (options.target_dpi / 72.0) * options.zoom_factor)
.round()
.clamp(1.0, MAX_PAGE_DIMENSION) as i32;
let (target_width, target_height) = compute_target_dimensions(
page_w,
page_h,
options.target_dpi,
options.zoom_factor,
);
let render_config = PdfRenderConfig::new()
.set_target_width(target_width)
@@ -1030,5 +1050,16 @@ mod tests {
let tiny_dim: f32 = -50.0;
let clamped_tiny = tiny_dim.clamp(1.0, MAX_PAGE_DIMENSION);
assert_eq!(clamped_tiny, 1.0);
// Verify uniform aspect ratio scaling under extreme dimensions
let (w, h) = compute_target_dimensions(10_000.0, 2_000.0, 72.0, 1.0);
assert_eq!(w, 8192);
assert_eq!(h, 1638);
assert!(((w as f32 / h as f32) - 5.0).abs() < 0.01);
let (w2, h2) = compute_target_dimensions(2_000.0, 10_000.0, 72.0, 1.0);
assert_eq!(w2, 1638);
assert_eq!(h2, 8192);
assert!(((h2 as f32 / w2 as f32) - 5.0).abs() < 0.01);
}
}