diff --git a/crates/git_graph/src/git_graph.rs b/crates/git_graph/src/git_graph.rs index 34599ddefdac5baf62259bccf2c7ab46feb6c43f..e0175db09f1ef95fbd62d06ef71d20cfa63e4444 100644 --- a/crates/git_graph/src/git_graph.rs +++ b/crates/git_graph/src/git_graph.rs @@ -27,7 +27,6 @@ use search::{ SearchOption, SearchOptions, SearchSource, SelectNextMatch, SelectPreviousMatch, ToggleCaseSensitive, buffer_search, }; -use settings::Settings; use smallvec::{SmallVec, smallvec}; use std::{ cell::Cell, @@ -37,7 +36,6 @@ use std::{ time::{Duration, Instant}, }; use theme::AccentColors; -use theme_settings::ThemeSettings; use time::{OffsetDateTime, UtcOffset, format_description::BorrowedFormatItem}; use ui::{ ButtonLike, Chip, ColumnWidthConfig, CommonAnimationExt as _, ContextMenu, DiffStat, Divider, @@ -58,6 +56,9 @@ const LEFT_PADDING: Pixels = px(12.0); const LINE_WIDTH: Pixels = px(1.5); const RESIZE_HANDLE_WIDTH: f32 = 8.0; const COPIED_STATE_DURATION: Duration = Duration::from_secs(2); +// Extra vertical breathing room added to the UI line height when computing +// the git graph's row height, so commit dots and lines have space around them. +const ROW_VERTICAL_PADDING: Pixels = px(4.0); struct CopiedState { copied_at: Option, @@ -901,7 +902,6 @@ pub struct GitGraph { git_store: Entity, workspace: WeakEntity, context_menu: Option<(Entity, Point, Subscription)>, - row_height: Pixels, table_interaction_state: Entity, column_widths: Entity, selected_entry_idx: Option, @@ -927,10 +927,19 @@ impl GitGraph { cx.notify(); } - fn row_height(cx: &App) -> Pixels { - let settings = ThemeSettings::get_global(cx); - let font_size = settings.buffer_font_size(cx); - font_size + px(12.0) + /// Computes the height of a single commit row in the git graph. + /// + /// The returned value is snapped to the nearest physical pixel. This is + /// required so that the canvas's float math and the `uniform_list` layout + /// (which snaps to device pixels) agree on row positions; otherwise rows + /// drift apart as the user scrolls when `ui_font_size` is fractional. + fn row_height(window: &Window, _cx: &App) -> Pixels { + let rem_size = window.rem_size(); + let line_height = window.text_style().line_height_in_pixels(rem_size); + let raw = line_height + ROW_VERTICAL_PADDING; + let scale = window.scale_factor(); + + (raw * scale).round() / scale } fn graph_canvas_content_width(&self) -> Pixels { @@ -1035,12 +1044,14 @@ impl GitGraph { ], ) }); - let mut row_height = Self::row_height(cx); + let mut row_height = Self::row_height(window, cx); - cx.observe_global_in::(window, move |this, _window, cx| { - let new_row_height = Self::row_height(cx); + cx.observe_global_in::(window, move |this, window, cx| { + let new_row_height = Self::row_height(window, cx); if new_row_height != row_height { - this.row_height = new_row_height; + // The `uniform_list` powering the table caches the item size + // from its last layout; invalidate it so it re-measures with + // the new row height on the next frame. this.table_interaction_state.update(cx, |state, _cx| { state.scroll_handle.0.borrow_mut().last_item_size = None; }); @@ -1064,7 +1075,6 @@ impl GitGraph { graph_data: graph, _commit_diff_task: None, context_menu: None, - row_height, table_interaction_state, column_widths, selected_entry_idx: None, @@ -1216,7 +1226,7 @@ impl GitGraph { fn render_table_rows( &mut self, range: Range, - _window: &mut Window, + window: &mut Window, cx: &mut Context, ) -> Vec> { let repository = self.get_repository(cx); @@ -1229,7 +1239,7 @@ impl GitGraph { .map(|branch| SharedString::from(branch.name().to_string())) }); - let row_height = self.row_height; + let row_height = Self::row_height(window, cx); // We fetch data outside the visible viewport to avoid loading entries when // users scroll through the git graph @@ -2168,7 +2178,7 @@ impl GitGraph { } pub fn render_graph(&self, window: &Window, cx: &mut Context) -> impl IntoElement { - let row_height = self.row_height; + let row_height = Self::row_height(window, cx); let table_state = self.table_interaction_state.read(cx); let viewport_height = table_state .scroll_handle @@ -2176,7 +2186,7 @@ impl GitGraph { .borrow() .last_item_size .map(|size| size.item.height) - .unwrap_or(px(600.0)); + .unwrap_or(window.viewport_size().height); let loaded_commit_count = self.graph_data.commits.len(); let content_height = row_height * loaded_commit_count; @@ -2434,7 +2444,12 @@ impl GitGraph { .h_full() } - fn row_at_position(&self, position_y: Pixels, cx: &Context) -> Option { + fn row_at_position( + &self, + position_y: Pixels, + window: &Window, + cx: &Context, + ) -> Option { let canvas_bounds = self.graph_canvas_bounds.get()?; let table_state = self.table_interaction_state.read(cx); let scroll_offset_y = -table_state.scroll_offset().y; @@ -2443,7 +2458,8 @@ impl GitGraph { if local_y >= px(0.) && local_y < canvas_bounds.size.height { let absolute_y = local_y + scroll_offset_y; - let absolute_row = (absolute_y / self.row_height).floor() as usize; + let row_height = Self::row_height(window, cx); + let absolute_row = (absolute_y / row_height).floor() as usize; if absolute_row < self.graph_data.commits.len() { return Some(absolute_row); @@ -2456,10 +2472,10 @@ impl GitGraph { fn handle_graph_mouse_move( &mut self, event: &gpui::MouseMoveEvent, - _window: &mut Window, + window: &mut Window, cx: &mut Context, ) { - if let Some(row) = self.row_at_position(event.position.y, cx) { + if let Some(row) = self.row_at_position(event.position.y, window, cx) { if self.hovered_entry_idx != Some(row) { self.hovered_entry_idx = Some(row); cx.notify(); @@ -2476,7 +2492,7 @@ impl GitGraph { window: &mut Window, cx: &mut Context, ) { - if let Some(row) = self.row_at_position(event.position().y, cx) { + if let Some(row) = self.row_at_position(event.position().y, window, cx) { self.select_entry(row, ScrollStrategy::Nearest, cx); if event.click_count() >= 2 { self.open_commit_view(row, window, cx); @@ -2502,7 +2518,7 @@ impl GitGraph { AllCommitCount::Loaded(count) => count, AllCommitCount::NotLoaded => self.graph_data.commits.len(), }; - let content_height = self.row_height * commit_count; + let content_height = Self::row_height(window, cx) * commit_count; let max_vertical_scroll = (viewport_height - content_height).min(px(0.)); let new_y = (current_offset.y + delta.y).clamp(max_vertical_scroll, px(0.)); @@ -2660,7 +2676,7 @@ impl Render for GitGraph { cx, )) .child({ - let row_height = self.row_height; + let row_height = Self::row_height(window, cx); let selected_entry_idx = self.selected_entry_idx; let hovered_entry_idx = self.hovered_entry_idx; let weak_self = cx.weak_entity(); @@ -3089,12 +3105,12 @@ mod tests { use fs::FakeFs; use git::Oid; use git::repository::InitialGraphCommitData; - use gpui::TestAppContext; + use gpui::{TestAppContext, UpdateGlobal}; use project::Project; use project::git_store::{GitStoreEvent, RepositoryEvent}; use rand::prelude::*; use serde_json::json; - use settings::SettingsStore; + use settings::{SettingsStore, ThemeSettingsContent}; use smallvec::{SmallVec, smallvec}; use std::path::Path; use std::sync::{Arc, Mutex}; @@ -4163,24 +4179,27 @@ mod tests { }); cx.run_until_parked(); - git_graph.update(cx, |graph, cx| { + git_graph.update_in(cx, |graph, window, cx| { assert!( graph.graph_data.commits.len() >= 10, "graph should load dummy commits" ); - graph.row_height = px(20.0); + let row_height = GitGraph::row_height(window, cx); let origin_y = px(100.0); graph.graph_canvas_bounds.set(Some(Bounds { origin: point(px(0.0), origin_y), - size: gpui::size(px(100.0), px(1000.0)), + size: gpui::size(px(100.0), row_height * 50.0), })); + // Scroll down by half a row so the row under a position near the + // top of the canvas is row 1 rather than row 0. + let scroll_offset = row_height * 0.75; graph.table_interaction_state.update(cx, |state, _| { - state.set_scroll_offset(point(px(0.0), px(-15.0))) + state.set_scroll_offset(point(px(0.0), -scroll_offset)) }); - let pos_y = origin_y + px(10.0); - let absolute_calc_row = graph.row_at_position(pos_y, cx); + let pos_y = origin_y + row_height * 0.5; + let absolute_calc_row = graph.row_at_position(pos_y, window, cx); assert_eq!( absolute_calc_row, @@ -4189,4 +4208,93 @@ mod tests { ); }); } + + #[gpui::test] + async fn test_row_height_matches_uniform_list_item_height(cx: &mut TestAppContext) { + init_test(cx); + + cx.update(|cx| { + SettingsStore::update_global(cx, |store, cx| { + store.update_user_settings(cx, |settings| { + *settings.theme = ThemeSettingsContent { + ui_font_size: Some(12.7.into()), + ..Default::default() + } + }); + }) + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + Path::new("/project"), + serde_json::json!({ + ".git": {}, + "file.txt": "content", + }), + ) + .await; + + let mut rng = StdRng::seed_from_u64(99); + let commits = generate_random_commit_dag(&mut rng, 20, false); + fs.set_graph_commits(Path::new("/project/.git"), commits); + + let project = Project::test(fs.clone(), [Path::new("/project")], cx).await; + cx.run_until_parked(); + + let repository = project.read_with(cx, |project, cx| { + project + .active_repository(cx) + .expect("should have a repository") + }); + + let (multi_workspace, cx) = cx.add_window_view(|window, cx| { + workspace::MultiWorkspace::test_new(project.clone(), window, cx) + }); + + let workspace_weak = + multi_workspace.read_with(&*cx, |multi, _| multi.workspace().downgrade()); + + let git_graph = cx.new_window_entity(|window, cx| { + GitGraph::new( + repository.read(cx).id, + project.read(cx).git_store().clone(), + workspace_weak, + window, + cx, + ) + }); + cx.run_until_parked(); + + cx.draw( + point(px(0.), px(0.)), + gpui::size(px(1200.), px(800.)), + |_, _| git_graph.clone().into_any_element(), + ); + cx.run_until_parked(); + + git_graph.update_in(cx, |graph, window, cx| { + let commit_count = graph.graph_data.commits.len(); + assert!( + commit_count > 0, + "need at least one commit to measure item height" + ); + + let table_state = graph.table_interaction_state.read(cx); + let item_size = table_state.scroll_handle.0.borrow().last_item_size.expect( + "uniform_list should have populated last_item_size after draw(); \ + the table has not been laid out", + ); + + let measured_item_height = item_size.contents.height / commit_count as f32; + let computed_row_height = GitGraph::row_height(window, cx); + + assert_eq!( + computed_row_height, measured_item_height, + "GitGraph::row_height ({}) must exactly match the height that \ + uniform_list measured for each table row ({}). \ + A mismatch means the canvas and table rows will drift when scrolling.", + computed_row_height, measured_item_height, + ); + }); + } }