From ca21626064d078867c7241d7eb39b09f763fa894 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Mon, 7 Aug 2023 23:32:27 +0200 Subject: [PATCH 1/7] Baseline: Improve selection rendering for large quantities from 270ms to 90ms --- crates/editor/src/editor.rs | 64 +++++++++++++++++++++++++++++++++- crates/editor/src/element.rs | 67 +++++++++++++----------------------- 2 files changed, 86 insertions(+), 45 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index cd5e86b91098b35cad385331d5e8f28a380d3139..8f8511deb697913c1e8e1fcebd6702177d504c34 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -90,7 +90,7 @@ use std::{ cmp::{self, Ordering, Reverse}, mem, num::NonZeroU32, - ops::{ControlFlow, Deref, DerefMut, Range}, + ops::{ControlFlow, Deref, DerefMut, Range, RangeInclusive}, path::Path, sync::Arc, time::{Duration, Instant}, @@ -7549,6 +7549,68 @@ impl Editor { results } + pub fn selected_rows( + &self, + search_range: Range, + display_snapshot: &DisplaySnapshot, + theme: &Theme, + ) -> Vec> { + let mut results = Vec::new(); + let buffer = &display_snapshot.buffer_snapshot; + let Some((color_fetcher, ranges)) = self.background_highlights + .get(&TypeId::of::()) else { + return vec![]; + }; + + let color = color_fetcher(theme); + let start_ix = match ranges.binary_search_by(|probe| { + let cmp = probe.end.cmp(&search_range.start, buffer); + if cmp.is_gt() { + Ordering::Greater + } else { + Ordering::Less + } + }) { + Ok(i) | Err(i) => i, + }; + let mut push_region = |start, end| { + if let (Some(start_display), Some(end_display)) = (start, end) { + results.push(start_display..=end_display); + } + }; + let mut start_row = None; + let mut end_row = None; + for range in &ranges[start_ix..] { + if range.start.cmp(&search_range.end, buffer).is_ge() { + break; + } + let start = range.start.to_point(buffer).row; + let end = range.end.to_point(buffer).row; + if start_row.is_none() { + assert_eq!(end_row, None); + start_row = Some(start); + end_row = Some(end); + continue; + } + if let Some(current_end) = end_row.as_mut() { + if start > *current_end + 1 { + push_region(start_row, end_row); + start_row = Some(start); + end_row = Some(end); + } else { + // Merge two hunks. + *current_end = end; + } + } else { + unreachable!(); + } + } + // We might still have a hunk that was not rendered (if there was a search hit on the last line) + push_region(start_row, end_row); + + results + } + pub fn highlight_text( &mut self, ranges: Vec>, diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 2d4b273f5ef0523f8ef8d9d9cd107c7e30bd659d..9ef95ec929a9ebb1425eed4c049f159e95c00466 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1107,8 +1107,6 @@ impl EditorElement { if layout.is_singleton && scrollbar_settings.selections { let start_anchor = Anchor::min(); let end_anchor = Anchor::max(); - let mut start_row = None; - let mut end_row = None; let color = scrollbar_theme.selections; let border = Border { width: 1., @@ -1119,54 +1117,35 @@ impl EditorElement { bottom: false, left: true, }; - let mut push_region = |start, end| { - if let (Some(start_display), Some(end_display)) = (start, end) { - let start_y = y_for_row(start_display as f32); - let mut end_y = y_for_row(end_display as f32); - if end_y - start_y < 1. { - end_y = start_y + 1.; - } - let bounds = RectF::from_points(vec2f(left, start_y), vec2f(right, end_y)); - - scene.push_quad(Quad { - bounds, - background: Some(color), - border, - corner_radius: style.thumb.corner_radius, - }) + let mut push_region = |start: u32, end: u32| { + let start_y = y_for_row(start as f32); + let mut end_y = y_for_row(end as f32); + if end_y - start_y < 1. { + end_y = start_y + 1.; } + let bounds = RectF::from_points(vec2f(left, start_y), vec2f(right, end_y)); + + scene.push_quad(Quad { + bounds, + background: Some(color), + border, + corner_radius: style.thumb.corner_radius, + }) }; - for (row, _) in &editor - .background_highlights_in_range_for::( + let start = std::time::Instant::now(); + + let background_ranges = editor + .selected_rows::( start_anchor..end_anchor, &layout.position_map.snapshot, &theme, - ) - { - let start_display = row.start; - let end_display = row.end; - - if start_row.is_none() { - assert_eq!(end_row, None); - start_row = Some(start_display.row()); - end_row = Some(end_display.row()); - continue; - } - if let Some(current_end) = end_row.as_mut() { - if start_display.row() > *current_end + 1 { - push_region(start_row, end_row); - start_row = Some(start_display.row()); - end_row = Some(end_display.row()); - } else { - // Merge two hunks. - *current_end = end_display.row(); - } - } else { - unreachable!(); - } + ); + dbg!(start.elapsed().as_millis()); + for row in background_ranges { + let start = row.start(); + let end = row.end(); + push_region(*start, *end); } - // We might still have a hunk that was not rendered (if there was a search hit on the last line) - push_region(start_row, end_row); } if layout.is_singleton && scrollbar_settings.git_diff { From fa168959764defa98ed2c7ce5173f38afb6135aa Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 8 Aug 2023 00:27:38 +0200 Subject: [PATCH 2/7] Do not query start of range if it's end is the same as the previous hunk's --- crates/editor/src/editor.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 8f8511deb697913c1e8e1fcebd6702177d504c34..c12bdc9c55c9b97a96da6452c6ca8ea9a8512098 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7557,12 +7557,11 @@ impl Editor { ) -> Vec> { let mut results = Vec::new(); let buffer = &display_snapshot.buffer_snapshot; - let Some((color_fetcher, ranges)) = self.background_highlights + let Some((_, ranges)) = self.background_highlights .get(&TypeId::of::()) else { return vec![]; }; - let color = color_fetcher(theme); let start_ix = match ranges.binary_search_by(|probe| { let cmp = probe.end.cmp(&search_range.start, buffer); if cmp.is_gt() { @@ -7584,8 +7583,14 @@ impl Editor { if range.start.cmp(&search_range.end, buffer).is_ge() { break; } - let start = range.start.to_point(buffer).row; let end = range.end.to_point(buffer).row; + if let Some(current_row) = &end_row { + if end == *current_row { + continue; + } + } + let start = range.start.to_point(buffer).row; + if start_row.is_none() { assert_eq!(end_row, None); start_row = Some(start); From 42e12213571d475b5019113b49c1a88b3f0f0a30 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 8 Aug 2023 02:17:11 +0200 Subject: [PATCH 3/7] Add upper bound limit. Remove dbg! statements --- crates/editor/src/editor.rs | 4 +++- crates/editor/src/element.rs | 4 +--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index c12bdc9c55c9b97a96da6452c6ca8ea9a8512098..682974d61fb3b2993048db2105c740617cfba04e 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7553,6 +7553,7 @@ impl Editor { &self, search_range: Range, display_snapshot: &DisplaySnapshot, + count: usize, theme: &Theme, ) -> Vec> { let mut results = Vec::new(); @@ -7572,6 +7573,7 @@ impl Editor { }) { Ok(i) | Err(i) => i, }; + let end_ix = count.min(ranges.len()); let mut push_region = |start, end| { if let (Some(start_display), Some(end_display)) = (start, end) { results.push(start_display..=end_display); @@ -7579,7 +7581,7 @@ impl Editor { }; let mut start_row = None; let mut end_row = None; - for range in &ranges[start_ix..] { + for range in &ranges[start_ix..end_ix] { if range.start.cmp(&search_range.end, buffer).is_ge() { break; } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 9ef95ec929a9ebb1425eed4c049f159e95c00466..735abbbd13f0dd834c461f3792bd4adedd1b3f9b 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1132,15 +1132,13 @@ impl EditorElement { corner_radius: style.thumb.corner_radius, }) }; - let start = std::time::Instant::now(); - let background_ranges = editor .selected_rows::( start_anchor..end_anchor, &layout.position_map.snapshot, + 50000, &theme, ); - dbg!(start.elapsed().as_millis()); for row in background_ranges { let start = row.start(); let end = row.end(); From 241d3951b8193d2ee1ba8fd177815531334da33b Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 8 Aug 2023 02:25:30 +0200 Subject: [PATCH 4/7] Remove redundant argument --- crates/editor/src/editor.rs | 1 - crates/editor/src/element.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 682974d61fb3b2993048db2105c740617cfba04e..0711e5eb024b6cfa9ea0b3f5bc6ee313d4c2cd52 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7554,7 +7554,6 @@ impl Editor { search_range: Range, display_snapshot: &DisplaySnapshot, count: usize, - theme: &Theme, ) -> Vec> { let mut results = Vec::new(); let buffer = &display_snapshot.buffer_snapshot; diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 735abbbd13f0dd834c461f3792bd4adedd1b3f9b..c3c8681bcf0d572419c144ce410bdc32b52426b5 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1137,7 +1137,6 @@ impl EditorElement { start_anchor..end_anchor, &layout.position_map.snapshot, 50000, - &theme, ); for row in background_ranges { let start = row.start(); From b0fc6da55b495dc5f43a934ee8b2d67eb1fbe62e Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 8 Aug 2023 02:37:27 +0200 Subject: [PATCH 5/7] Use display maps --- crates/editor/src/editor.rs | 21 ++++++++++++--------- crates/editor/src/element.rs | 6 +++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 0711e5eb024b6cfa9ea0b3f5bc6ee313d4c2cd52..8f4f97ad604dc15c47dbc2373495919059497dab 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7554,7 +7554,7 @@ impl Editor { search_range: Range, display_snapshot: &DisplaySnapshot, count: usize, - ) -> Vec> { + ) -> Vec> { let mut results = Vec::new(); let buffer = &display_snapshot.buffer_snapshot; let Some((_, ranges)) = self.background_highlights @@ -7573,24 +7573,27 @@ impl Editor { Ok(i) | Err(i) => i, }; let end_ix = count.min(ranges.len()); - let mut push_region = |start, end| { + let mut push_region = |start: Option, end: Option| { if let (Some(start_display), Some(end_display)) = (start, end) { - results.push(start_display..=end_display); + results.push( + start_display.to_display_point(display_snapshot) + ..=end_display.to_display_point(display_snapshot), + ); } }; - let mut start_row = None; - let mut end_row = None; + let mut start_row: Option = None; + let mut end_row: Option = None; for range in &ranges[start_ix..end_ix] { if range.start.cmp(&search_range.end, buffer).is_ge() { break; } - let end = range.end.to_point(buffer).row; + let end = range.end.to_point(buffer); if let Some(current_row) = &end_row { - if end == *current_row { + if end.row == current_row.row { continue; } } - let start = range.start.to_point(buffer).row; + let start = range.start.to_point(buffer); if start_row.is_none() { assert_eq!(end_row, None); @@ -7599,7 +7602,7 @@ impl Editor { continue; } if let Some(current_end) = end_row.as_mut() { - if start > *current_end + 1 { + if start.row > current_end.row + 1 { push_region(start_row, end_row); start_row = Some(start); end_row = Some(end); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index c3c8681bcf0d572419c144ce410bdc32b52426b5..30a9cba85c3e0bd92bc2fa0c2e121dc6e9949835 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1117,9 +1117,9 @@ impl EditorElement { bottom: false, left: true, }; - let mut push_region = |start: u32, end: u32| { - let start_y = y_for_row(start as f32); - let mut end_y = y_for_row(end as f32); + let mut push_region = |start: DisplayPoint, end: DisplayPoint| { + let start_y = y_for_row(start.row() as f32); + let mut end_y = y_for_row(end.row() as f32); if end_y - start_y < 1. { end_y = start_y + 1.; } From 371c669e003f8082896706261779231b3c2dce11 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 8 Aug 2023 02:45:15 +0200 Subject: [PATCH 6/7] Address review feedback. Rename selected_rows to background_highlight_row_ranges. Do not return any ranges if there are more than 50k results --- crates/editor/src/editor.rs | 6 ++++-- crates/editor/src/element.rs | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 8f4f97ad604dc15c47dbc2373495919059497dab..17195cb22b5de1bc60c21c8169f929e020cc78f7 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7549,7 +7549,7 @@ impl Editor { results } - pub fn selected_rows( + pub fn background_highlight_row_ranges( &self, search_range: Range, display_snapshot: &DisplaySnapshot, @@ -7616,7 +7616,9 @@ impl Editor { } // We might still have a hunk that was not rendered (if there was a search hit on the last line) push_region(start_row, end_row); - + if results.len() > count { + return vec![]; + } results } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 30a9cba85c3e0bd92bc2fa0c2e121dc6e9949835..33ebd4c7bd632d5137b6f8f565e6e7010899abe1 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -1133,7 +1133,7 @@ impl EditorElement { }) }; let background_ranges = editor - .selected_rows::( + .background_highlight_row_ranges::( start_anchor..end_anchor, &layout.position_map.snapshot, 50000, From 1aff642981fd8b6fd9f4a0b58d7ca43758485edb Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Tue, 8 Aug 2023 13:09:27 +0200 Subject: [PATCH 7/7] Do not highlgiht selections at all over the threshold --- crates/editor/src/editor.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 17195cb22b5de1bc60c21c8169f929e020cc78f7..02cd58524bb5e6b5b97a17b751d9a2304abbaf11 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -7572,7 +7572,6 @@ impl Editor { }) { Ok(i) | Err(i) => i, }; - let end_ix = count.min(ranges.len()); let mut push_region = |start: Option, end: Option| { if let (Some(start_display), Some(end_display)) = (start, end) { results.push( @@ -7583,7 +7582,10 @@ impl Editor { }; let mut start_row: Option = None; let mut end_row: Option = None; - for range in &ranges[start_ix..end_ix] { + if ranges.len() > count { + return vec![]; + } + for range in &ranges[start_ix..] { if range.start.cmp(&search_range.end, buffer).is_ge() { break; } @@ -7616,9 +7618,6 @@ impl Editor { } // We might still have a hunk that was not rendered (if there was a search hit on the last line) push_region(start_row, end_row); - if results.len() > count { - return vec![]; - } results }