diff --git a/quadratic-client/src/app/grid/sheet/SheetCursor.ts b/quadratic-client/src/app/grid/sheet/SheetCursor.ts index 3b1f5d95f9..a923cdb0d5 100644 --- a/quadratic-client/src/app/grid/sheet/SheetCursor.ts +++ b/quadratic-client/src/app/grid/sheet/SheetCursor.ts @@ -268,7 +268,9 @@ export class SheetCursor { } if (this.multiCursor) { for (const rect of this.multiCursor) { - columns.add(rect.x); + for (let x = rect.x; x < rect.x + rect.width; x++) { + columns.add(x); + } } } columns.add(this.cursorPosition.x); @@ -283,7 +285,9 @@ export class SheetCursor { } if (this.multiCursor) { for (const rect of this.multiCursor) { - rows.add(rect.y); + for (let y = rect.y; y < rect.y + rect.height; y++) { + rows.add(y); + } } } rows.add(this.cursorPosition.y); diff --git a/quadratic-core/src/controller/active_transactions/pending_transaction.rs b/quadratic-core/src/controller/active_transactions/pending_transaction.rs index bdb54f8fe0..2858c4cd74 100644 --- a/quadratic-core/src/controller/active_transactions/pending_transaction.rs +++ b/quadratic-core/src/controller/active_transactions/pending_transaction.rs @@ -12,7 +12,7 @@ use crate::{ controller::{ execution::TransactionType, operations::operation::Operation, transaction::Transaction, }, - grid::{sheet::validations::validation::Validation, CodeCellLanguage, CodeRun, SheetId}, + grid::{sheet::validations::validation::Validation, CodeCellLanguage, CodeRun, Sheet, SheetId}, selection::Selection, Pos, SheetPos, SheetRect, }; @@ -234,6 +234,46 @@ impl PendingTransaction { dirty_hashes.extend(hashes); } + // Adds dirty hashes for all hashes from col_start to col_end (goes to sheet bounds if not provided) + pub fn add_dirty_hashes_from_sheet_columns( + &mut self, + sheet: &Sheet, + col_start: i64, + col_end: Option, + ) { + let col_end = col_end.unwrap_or(sheet.bounds(false).last_column().unwrap_or(col_start)); + let dirty_hashes = self.dirty_hashes.entry(sheet.id).or_default(); + for col in col_start..=col_end { + if let Some((start, end)) = sheet.column_bounds(col, false) { + for y in start..=end { + let mut pos = Pos { x: col, y }; + pos.to_quadrant(); + dirty_hashes.insert(pos); + } + } + } + } + + // Adds dirty hashes for all hashes from row_start to row_end (goes to sheet bounds if not provided) + pub fn add_dirty_hashes_from_sheet_rows( + &mut self, + sheet: &Sheet, + row_start: i64, + row_end: Option, + ) { + let row_end = row_end.unwrap_or(sheet.bounds(false).last_row().unwrap_or(row_start)); + let dirty_hashes = self.dirty_hashes.entry(sheet.id).or_default(); + for row in row_start..=row_end { + if let Some((start, end)) = sheet.row_bounds(row, false) { + for x in start..=end { + let mut pos = Pos { x, y: row }; + pos.to_quadrant(); + dirty_hashes.insert(pos); + } + } + } + } + /// Adds a code cell, html cell and image cell to the transaction from a CodeRun pub fn add_from_code_run(&mut self, sheet_id: SheetId, pos: Pos, code_run: &Option) { if let Some(code_run) = &code_run { @@ -288,6 +328,23 @@ impl PendingTransaction { } } } + + /// Updates the offsets modified for a column or row. + pub fn offsets_modified( + &mut self, + sheet_id: SheetId, + column: Option, + row: Option, + size: Option, + ) { + let offsets_modified = self.offsets_modified.entry(sheet_id).or_default(); + if let Some(column) = column { + offsets_modified.insert((Some(column), None), size.unwrap_or(0.0)); + } + if let Some(row) = row { + offsets_modified.insert((None, Some(row)), size.unwrap_or(0.0)); + } + } } #[cfg(test)] @@ -492,4 +549,54 @@ mod tests { assert_eq!(transaction.image_cells.len(), 1); assert_eq!(transaction.image_cells[&sheet_id].len(), 1); } + + #[test] + #[parallel] + fn test_offsets_modified() { + let mut transaction = PendingTransaction::default(); + let sheet_id = SheetId::new(); + transaction.offsets_modified(sheet_id, Some(1), None, Some(10.0)); + assert_eq!(transaction.offsets_modified.len(), 1); + assert_eq!(transaction.offsets_modified[&sheet_id].len(), 1); + assert_eq!( + transaction.offsets_modified[&sheet_id][&(Some(1), None)], + 10.0 + ); + transaction.offsets_modified(sheet_id, None, Some(1), Some(10.0)); + assert_eq!(transaction.offsets_modified[&sheet_id].len(), 2); + assert_eq!( + transaction.offsets_modified[&sheet_id][&(None, Some(1))], + 10.0 + ); + } + + #[test] + #[parallel] + fn test_add_dirty_hashes_from_sheet_columns() { + let mut sheet = Sheet::test(); + sheet.set_cell_value(Pos::new(1, 1), "A1".to_string()); + sheet.recalculate_bounds(); + + let mut transaction = PendingTransaction::default(); + transaction.add_dirty_hashes_from_sheet_columns(&sheet, 0, None); + + let dirty_hashes = transaction.dirty_hashes.get(&sheet.id).unwrap(); + assert!(dirty_hashes.contains(&Pos { x: 0, y: 0 })); + assert_eq!(dirty_hashes.len(), 1); + } + + #[test] + #[parallel] + fn test_add_dirty_hashes_from_sheet_rows() { + let mut sheet = Sheet::test(); + sheet.set_cell_value(Pos::new(1, 1), "A1".to_string()); + sheet.recalculate_bounds(); + + let mut transaction = PendingTransaction::default(); + transaction.add_dirty_hashes_from_sheet_rows(&sheet, 0, None); + + let dirty_hashes = transaction.dirty_hashes.get(&sheet.id).unwrap(); + assert!(dirty_hashes.contains(&Pos { x: 0, y: 0 })); + assert_eq!(dirty_hashes.len(), 1); + } } diff --git a/quadratic-core/src/controller/execution/execute_operation/execute_offsets.rs b/quadratic-core/src/controller/execution/execute_operation/execute_offsets.rs index b6814ef8bf..7c84dc26dc 100644 --- a/quadratic-core/src/controller/execution/execute_operation/execute_offsets.rs +++ b/quadratic-core/src/controller/execution/execute_operation/execute_offsets.rs @@ -169,9 +169,8 @@ impl GridController { }); if (cfg!(target_family = "wasm") || cfg!(test)) && !transaction.is_server() { - let offsets_modified = transaction.offsets_modified.entry(sheet_id).or_default(); row_heights.iter().for_each(|&JsRowHeight { row, height }| { - offsets_modified.insert((None, Some(row)), height); + transaction.offsets_modified(sheet_id, None, Some(row), Some(height)); }); } diff --git a/quadratic-core/src/grid/bounds.rs b/quadratic-core/src/grid/bounds.rs index 3dea91448b..f48049a2a5 100644 --- a/quadratic-core/src/grid/bounds.rs +++ b/quadratic-core/src/grid/bounds.rs @@ -92,6 +92,24 @@ impl GridBounds { GridBounds::NonEmpty(rect) => rect.extend_y(y), } } + + pub fn first_column(&self) -> Option { + self.to_bounds_rect().map(|rect| rect.x) + } + + pub fn last_column(&self) -> Option { + self.to_bounds_rect() + .map(|rect| rect.x + rect.width as i64 - 1) + } + + pub fn first_row(&self) -> Option { + self.to_bounds_rect().map(|rect| rect.y) + } + + pub fn last_row(&self) -> Option { + self.to_bounds_rect() + .map(|rect| rect.y + rect.height as i64 - 1) + } } #[cfg(test)] @@ -134,4 +152,44 @@ mod test { grid_bounds.extend_y(5); assert_eq!(grid_bounds, GridBounds::NonEmpty(Rect::new(1, 2, 3, 5))); } + + #[test] + #[parallel] + fn test_first_column() { + let grid_bounds = GridBounds::NonEmpty(Rect::new(1, 2, 3, 4)); + assert_eq!(grid_bounds.first_column(), Some(1)); + + let empty_bounds = GridBounds::Empty; + assert_eq!(empty_bounds.first_column(), None); + } + + #[test] + #[parallel] + fn test_last_column() { + let grid_bounds = GridBounds::NonEmpty(Rect::new(1, 2, 3, 4)); + assert_eq!(grid_bounds.last_column(), Some(3)); + + let empty_bounds = GridBounds::Empty; + assert_eq!(empty_bounds.last_column(), None); + } + + #[test] + #[parallel] + fn test_first_row() { + let grid_bounds = GridBounds::NonEmpty(Rect::new(1, 2, 3, 4)); + assert_eq!(grid_bounds.first_row(), Some(2)); + + let empty_bounds = GridBounds::Empty; + assert_eq!(empty_bounds.first_row(), None); + } + + #[test] + #[parallel] + fn test_last_row() { + let grid_bounds = GridBounds::NonEmpty(Rect::new(1, 2, 3, 4)); + assert_eq!(grid_bounds.last_row(), Some(4)); + + let empty_bounds = GridBounds::Empty; + assert_eq!(empty_bounds.last_row(), None); + } } diff --git a/quadratic-core/src/grid/sheet/col_row/column.rs b/quadratic-core/src/grid/sheet/col_row/column.rs index 5e9d85e890..16595e9699 100644 --- a/quadratic-core/src/grid/sheet/col_row/column.rs +++ b/quadratic-core/src/grid/sheet/col_row/column.rs @@ -1,5 +1,3 @@ -use std::collections::HashSet; - use crate::{ cell_values::CellValues, controller::{ @@ -7,7 +5,6 @@ use crate::{ operations::operation::{CopyFormats, Operation}, }, grid::{formats::Formats, Sheet}, - renderer_constants::CELL_SHEET_HEIGHT, selection::Selection, Pos, Rect, SheetPos, }; @@ -101,11 +98,7 @@ impl Sheet { } if !changed.is_empty() && !transaction.is_server() { changed.iter().for_each(|(index, size)| { - transaction - .offsets_modified - .entry(self.id) - .or_default() - .insert((Some(*index), None), *size); + transaction.offsets_modified(self.id, Some(*index), None, Some(*size)); }); } } @@ -142,7 +135,8 @@ impl Sheet { }); } - let mut updated_cols = HashSet::new(); + // mark hashes of existing columns dirty + transaction.add_dirty_hashes_from_sheet_columns(self, column, None); // remove the column's data from the sheet if let Some(c) = self.columns.get(&column) { @@ -150,7 +144,6 @@ impl Sheet { transaction.fill_cells.insert(self.id); } self.columns.remove(&column); - updated_cols.insert(column); } // remove the column's code runs from the sheet @@ -176,7 +169,6 @@ impl Sheet { transaction.fill_cells.insert(self.id); } self.formats_columns.remove(&column); - updated_cols.insert(column); } // remove the column's borders from the sheet @@ -198,7 +190,6 @@ impl Sheet { transaction.fill_cells.insert(self.id); } self.columns.insert(col - 1, column_data); - updated_cols.insert(col - 1); } } @@ -243,22 +234,11 @@ impl Sheet { for col in formats_to_update { if let Some(format) = self.formats_columns.remove(&col) { self.formats_columns.insert(col - 1, format); - updated_cols.insert(col); - updated_cols.insert(col - 1); } } - // send the value hashes that have changed to the client - let dirty_hashes = transaction.dirty_hashes.entry(self.id).or_default(); - updated_cols.iter().for_each(|col| { - if let Some((start, end)) = self.column_bounds(*col, false) { - for y in (start..=end).step_by(CELL_SHEET_HEIGHT as usize) { - let mut pos = Pos { x: *col, y }; - pos.to_quadrant(); - dirty_hashes.insert(pos); - } - } - }); + // mark hashes of new columns dirty + transaction.add_dirty_hashes_from_sheet_columns(self, column, None); self.validations.remove_column(transaction, self.id, column); } @@ -314,7 +294,8 @@ impl Sheet { }); } - let mut updated_cols = HashSet::new(); + // mark hashes of existing columns dirty + transaction.add_dirty_hashes_from_sheet_columns(self, column, None); // update the indices of all columns impacted by the insertion let mut columns_to_update = Vec::new(); @@ -328,7 +309,6 @@ impl Sheet { if let Some(mut column_data) = self.columns.remove(&col) { column_data.x += 1; self.columns.insert(col + 1, column_data); - updated_cols.insert(col + 1); } } @@ -374,7 +354,6 @@ impl Sheet { for col in formats_to_update { if let Some(format) = self.formats_columns.remove(&col) { self.formats_columns.insert(col + 1, format); - updated_cols.insert(col + 1); } } @@ -383,17 +362,8 @@ impl Sheet { transaction.sheet_borders.insert(self.id); } - // signal client to update the hashes for changed columns - let dirty_hashes = transaction.dirty_hashes.entry(self.id).or_default(); - updated_cols.iter().for_each(|col| { - if let Some((start, end)) = self.column_bounds(*col, false) { - for y in (start..=end).step_by(CELL_SHEET_HEIGHT as usize) { - let mut pos = Pos { x: *col, y }; - pos.to_quadrant(); - dirty_hashes.insert(pos); - } - } - }); + // mark hashes of new columns dirty + transaction.add_dirty_hashes_from_sheet_columns(self, column, None); self.validations.insert_column(transaction, self.id, column); @@ -402,11 +372,7 @@ impl Sheet { let changes = self.offsets.insert_column(column); if !changes.is_empty() { changes.iter().for_each(|(index, size)| { - transaction - .offsets_modified - .entry(self.id) - .or_default() - .insert((Some(*index), None), *size); + transaction.offsets_modified(self.id, Some(*index), None, Some(*size)); }); } } diff --git a/quadratic-core/src/grid/sheet/col_row/row.rs b/quadratic-core/src/grid/sheet/col_row/row.rs index 32dad00364..d263beba60 100644 --- a/quadratic-core/src/grid/sheet/col_row/row.rs +++ b/quadratic-core/src/grid/sheet/col_row/row.rs @@ -1,5 +1,3 @@ -use std::collections::HashSet; - use chrono::Utc; use crate::{ @@ -9,7 +7,6 @@ use crate::{ operations::operation::{CopyFormats, Operation}, }, grid::{formats::Formats, GridBounds, Sheet}, - renderer_constants::CELL_SHEET_WIDTH, selection::Selection, Pos, Rect, SheetPos, }; @@ -87,14 +84,13 @@ impl Sheet { } /// Removes any value at row and shifts the remaining values up by 1. - fn delete_and_shift_values(&mut self, row: i64, updated_rows: &mut HashSet) { + fn delete_and_shift_values(&mut self, row: i64) { // use the sheet bounds to determine the approximate bounds for the impacted range if let GridBounds::NonEmpty(bounds) = self.bounds(true) { for x in bounds.min.x..=bounds.max.x { if let Some(column) = self.columns.get_mut(&x) { if column.values.contains_key(&row) { column.values.remove(&row); - updated_rows.insert(row); } let mut keys_to_move: Vec = column @@ -110,8 +106,6 @@ impl Sheet { for key in keys_to_move { if let Some(value) = column.values.remove(&key) { column.values.insert(key - 1, value); - updated_rows.insert(key - 1); - updated_rows.insert(key); } } } @@ -120,55 +114,26 @@ impl Sheet { } /// Removes format at row and shifts remaining formats to the left by 1. - fn formats_remove_and_shift_up( - &mut self, - transaction: &mut PendingTransaction, - row: i64, - updated_rows: &mut HashSet, - ) { + fn formats_remove_and_shift_up(&mut self, transaction: &mut PendingTransaction, row: i64) { if let GridBounds::NonEmpty(bounds) = self.bounds(false) { for x in bounds.min.x..=bounds.max.x { if let Some(column) = self.columns.get_mut(&x) { - if column.align.remove_and_shift_left(row) { - updated_rows.insert(row); - } - if column.align.remove_and_shift_left(row) { - updated_rows.insert(row); - } - if column.vertical_align.remove_and_shift_left(row) { - updated_rows.insert(row); - } - if column.wrap.remove_and_shift_left(row) { - updated_rows.insert(row); - } - if column.numeric_format.remove_and_shift_left(row) { - updated_rows.insert(row); - } - if column.numeric_decimals.remove_and_shift_left(row) { - updated_rows.insert(row); - } - if column.numeric_commas.remove_and_shift_left(row) { - updated_rows.insert(row); - } - if column.bold.remove_and_shift_left(row) { - updated_rows.insert(row); - } - if column.italic.remove_and_shift_left(row) { - updated_rows.insert(row); - } - if column.text_color.remove_and_shift_left(row) { - updated_rows.insert(row); - } + column.align.remove_and_shift_left(row); + column.vertical_align.remove_and_shift_left(row); + column.wrap.remove_and_shift_left(row); + column.numeric_format.remove_and_shift_left(row); + column.numeric_decimals.remove_and_shift_left(row); + column.numeric_commas.remove_and_shift_left(row); + column.bold.remove_and_shift_left(row); + column.italic.remove_and_shift_left(row); + column.text_color.remove_and_shift_left(row); if column.fill_color.remove_and_shift_left(row) { - updated_rows.insert(row); transaction.fill_cells.insert(self.id); } - if column.render_size.remove_and_shift_left(row) { - updated_rows.insert(row); - } - if column.date_time.remove_and_shift_left(row) { - updated_rows.insert(row); - } + column.render_size.remove_and_shift_left(row); + column.date_time.remove_and_shift_left(row); + column.underline.remove_and_shift_left(row); + column.strike_through.remove_and_shift_left(row); } } } @@ -233,14 +198,14 @@ impl Sheet { } }); - let mut updated_rows = HashSet::new(); + // mark hashes of existing rows dirty + transaction.add_dirty_hashes_from_sheet_rows(self, row, None); // remove the row's formats from the sheet if let Some((format, _)) = self.formats_rows.remove(&row) { if format.fill_color.is_some() { transaction.fill_cells.insert(self.id); } - updated_rows.insert(row); } // remove the column's borders from the sheet @@ -249,7 +214,7 @@ impl Sheet { } // update all cells that were impacted by the deletion - self.delete_and_shift_values(row, &mut updated_rows); + self.delete_and_shift_values(row); // update the indices of all code_runs impacted by the deletion let mut code_runs_to_move = Vec::new(); @@ -284,7 +249,7 @@ impl Sheet { } // update the indices of all column-based formats impacted by the deletion - self.formats_remove_and_shift_up(transaction, row, &mut updated_rows); + self.formats_remove_and_shift_up(transaction, row); // update the indices of all row-based formats impacted by the deletion let mut formats_to_update = Vec::new(); @@ -299,22 +264,11 @@ impl Sheet { transaction.fill_cells.insert(self.id); } self.formats_rows.insert(row - 1, format); - updated_rows.insert(row); - updated_rows.insert(row - 1); } } - // send the value hashes that have changed to the client - let dirty_hashes = transaction.dirty_hashes.entry(self.id).or_default(); - updated_rows.iter().for_each(|row| { - if let Some((start, end)) = self.row_bounds(*row, false) { - for x in (start..=end).step_by(CELL_SHEET_WIDTH as usize) { - let mut pos = Pos { x, y: *row }; - pos.to_quadrant(); - dirty_hashes.insert(pos); - } - } - }); + // mark hashes of new rows dirty + transaction.add_dirty_hashes_from_sheet_rows(self, row, None); // reverse operation to create the column (this will also shift all impacted columns) transaction.reverse_operations.push(Operation::InsertRow { @@ -327,7 +281,7 @@ impl Sheet { } /// Removes any value at row and shifts the remaining values up by 1. - fn insert_and_shift_values(&mut self, row: i64, updated_rows: &mut HashSet) { + fn insert_and_shift_values(&mut self, row: i64) { // use the sheet bounds to determine the approximate bounds for the impacted range if let GridBounds::NonEmpty(bounds) = self.bounds(true) { for x in bounds.min.x..=bounds.max.x { @@ -345,8 +299,6 @@ impl Sheet { for key in keys_to_move { if let Some(value) = column.values.remove(&key) { column.values.insert(key + 1, value); - updated_rows.insert(key); - updated_rows.insert(key + 1); } } } @@ -355,46 +307,26 @@ impl Sheet { } /// Removes format at row and shifts remaining formats to the left by 1. - fn formats_insert_and_shift_down(&mut self, row: i64, updated_rows: &mut HashSet) { + fn formats_insert_and_shift_down(&mut self, row: i64, transaction: &mut PendingTransaction) { if let GridBounds::NonEmpty(bounds) = self.bounds(false) { for x in bounds.min.x..=bounds.max.x { if let Some(column) = self.columns.get_mut(&x) { - if column.align.insert_and_shift_right(row) { - updated_rows.insert(row); - } - if column.vertical_align.insert_and_shift_right(row) { - updated_rows.insert(row); - } - if column.wrap.insert_and_shift_right(row) { - updated_rows.insert(row); - } - if column.numeric_format.insert_and_shift_right(row) { - updated_rows.insert(row); - } - if column.numeric_decimals.insert_and_shift_right(row) { - updated_rows.insert(row); - } - if column.numeric_commas.insert_and_shift_right(row) { - updated_rows.insert(row); - } - if column.bold.insert_and_shift_right(row) { - updated_rows.insert(row); - } - if column.italic.insert_and_shift_right(row) { - updated_rows.insert(row); - } - if column.text_color.insert_and_shift_right(row) { - updated_rows.insert(row); - } + column.align.insert_and_shift_right(row); + column.vertical_align.insert_and_shift_right(row); + column.wrap.insert_and_shift_right(row); + column.numeric_format.insert_and_shift_right(row); + column.numeric_decimals.insert_and_shift_right(row); + column.numeric_commas.insert_and_shift_right(row); + column.bold.insert_and_shift_right(row); + column.italic.insert_and_shift_right(row); + column.text_color.insert_and_shift_right(row); if column.fill_color.insert_and_shift_right(row) { - updated_rows.insert(row); - } - if column.render_size.insert_and_shift_right(row) { - updated_rows.insert(row); - } - if column.date_time.insert_and_shift_right(row) { - updated_rows.insert(row); + transaction.fill_cells.insert(self.id); } + column.render_size.insert_and_shift_right(row); + column.date_time.insert_and_shift_right(row); + column.underline.insert_and_shift_right(row); + column.strike_through.insert_and_shift_right(row); } } } @@ -449,9 +381,10 @@ impl Sheet { }); } - let mut updated_rows = HashSet::new(); + // mark hashes of existing rows dirty + transaction.add_dirty_hashes_from_sheet_rows(self, row, None); - self.insert_and_shift_values(row, &mut updated_rows); + self.insert_and_shift_values(row); // update the indices of all code_runs impacted by the insertion let mut code_runs_to_move = Vec::new(); @@ -486,7 +419,7 @@ impl Sheet { } // update the indices of all column-based formats impacted by the deletion - self.formats_insert_and_shift_down(row, &mut updated_rows); + self.formats_insert_and_shift_down(row, transaction); // signal client to update the borders for changed columns if self.borders.insert_row(row) { @@ -504,21 +437,11 @@ impl Sheet { for row in formats_to_update { if let Some(format) = self.formats_rows.remove(&row) { self.formats_rows.insert(row + 1, format); - updated_rows.insert(row + 1); } } - // signal client to update the hashes for changed columns - let dirty_hashes = transaction.dirty_hashes.entry(self.id).or_default(); - updated_rows.iter().for_each(|row| { - if let Some((start, end)) = self.row_bounds(*row, false) { - for x in (start..=end).step_by(CELL_SHEET_WIDTH as usize) { - let mut pos = Pos { x, y: *row }; - pos.to_quadrant(); - dirty_hashes.insert(pos); - } - } - }); + // mark hashes of new rows dirty + transaction.add_dirty_hashes_from_sheet_rows(self, row, None); self.validations.insert_row(transaction, self.id, row); @@ -527,11 +450,7 @@ impl Sheet { let changes = self.offsets.insert_row(row); if !changes.is_empty() { changes.iter().for_each(|(index, size)| { - transaction - .offsets_modified - .entry(self.id) - .or_default() - .insert((None, Some(*index)), *size); + transaction.offsets_modified(self.id, None, Some(*index), Some(*size)); }); } } @@ -566,7 +485,7 @@ mod test { ], ); sheet.calculate_bounds(); - sheet.delete_and_shift_values(1, &mut HashSet::new()); + sheet.delete_and_shift_values(1); assert_eq!( sheet.cell_value(Pos { x: 1, y: 1 }), Some(CellValue::Text("E".to_string()))