diff --git a/packages/core/src/text-buffer-view.test.ts b/packages/core/src/text-buffer-view.test.ts index 9b42b28a5..0e4de9d2d 100644 --- a/packages/core/src/text-buffer-view.test.ts +++ b/packages/core/src/text-buffer-view.test.ts @@ -637,5 +637,69 @@ describe("TextBufferView", () => { expect(result!.lineCount).toBe(4) expect(result!.maxWidth).toBe(10) }) + + it("should cache measure results for same width", () => { + const styledText = stringToStyledText("ABCDEFGHIJKLMNOPQRST") + buffer.setStyledText(styledText) + + view.setWrapMode("char") + + // First call - cache miss + const result1 = view.measureForDimensions(10, 10) + expect(result1).not.toBeNull() + expect(result1!.lineCount).toBe(2) + + // Second call with same width - should return cached result + const result2 = view.measureForDimensions(10, 10) + expect(result2).not.toBeNull() + expect(result2!.lineCount).toBe(2) + expect(result2!.maxWidth).toBe(result1!.maxWidth) + }) + + it("should invalidate cache when content changes", () => { + const styledText1 = stringToStyledText("ABCDEFGHIJ") + buffer.setStyledText(styledText1) + + view.setWrapMode("char") + + // Measure with width 5 - should be 2 lines + const result1 = view.measureForDimensions(5, 10) + expect(result1!.lineCount).toBe(2) + + // Change content to be longer + const styledText2 = stringToStyledText("ABCDEFGHIJKLMNOPQRST") + buffer.setStyledText(styledText2) + + // Same width should now return different result + const result2 = view.measureForDimensions(5, 10) + expect(result2!.lineCount).toBe(4) + }) + + it("should invalidate cache when wrap mode changes", () => { + const styledText = stringToStyledText("Hello world test string here") + buffer.setStyledText(styledText) + + view.setWrapMode("word") + const resultWord = view.measureForDimensions(10, 10) + + view.setWrapMode("char") + const resultChar = view.measureForDimensions(10, 10) + + // Word and char wrap should produce different results + expect(resultWord!.lineCount).not.toBe(resultChar!.lineCount) + }) + + it("should handle width 0 for intrinsic measurement", () => { + const styledText = stringToStyledText("Hello World") + buffer.setStyledText(styledText) + + view.setWrapMode("word") + + // Width 0 means get intrinsic width (no wrapping) + const result = view.measureForDimensions(0, 10) + expect(result).not.toBeNull() + expect(result!.lineCount).toBe(1) + expect(result!.maxWidth).toBe(11) // "Hello World" = 11 chars + }) }) }) diff --git a/packages/core/src/zig/tests/text-buffer-view_test.zig b/packages/core/src/zig/tests/text-buffer-view_test.zig index a8b155c42..eda874172 100644 --- a/packages/core/src/zig/tests/text-buffer-view_test.zig +++ b/packages/core/src/zig/tests/text-buffer-view_test.zig @@ -1,5 +1,6 @@ const std = @import("std"); const text_buffer = @import("../text-buffer.zig"); +const iter_mod = @import("../text-buffer-iterators.zig"); const text_buffer_view = @import("../text-buffer-view.zig"); const gp = @import("../grapheme.zig"); @@ -1915,6 +1916,107 @@ test "TextBufferView measureForDimensions - does not modify cache" { try std.testing.expectEqual(@as(u32, 1), actual_count); } +test "TextBufferView measureForDimensions - cache invalidates after updateVirtualLines" { + const pool = gp.initGlobalPool(std.testing.allocator); + defer gp.deinitGlobalPool(); + + var tb = try TextBuffer.init(std.testing.allocator, pool, .wcwidth); + defer tb.deinit(); + + var view = try TextBufferView.init(std.testing.allocator, tb); + defer view.deinit(); + + try tb.setText("AAAAA"); + view.setWrapMode(.char); + view.setWrapWidth(5); + + const result1 = try view.measureForDimensions(5, 10); + try std.testing.expectEqual(@as(u32, 1), result1.line_count); + try std.testing.expectEqual(@as(u32, 5), result1.max_width); + + try tb.setText("AAAAAAAAAA"); + + // This clears the dirty flag, which would cause a false cache hit + // if we keyed on dirty instead of epoch. + _ = view.getVirtualLineCount(); + + const result2 = try view.measureForDimensions(5, 10); + try std.testing.expectEqual(@as(u32, 2), result2.line_count); + try std.testing.expectEqual(@as(u32, 5), result2.max_width); +} + +test "TextBufferView measureForDimensions - width 0 uses intrinsic line widths" { + const pool = gp.initGlobalPool(std.testing.allocator); + defer gp.deinitGlobalPool(); + + var tb = try TextBuffer.init(std.testing.allocator, pool, .wcwidth); + defer tb.deinit(); + + var view = try TextBufferView.init(std.testing.allocator, tb); + defer view.deinit(); + + try tb.setText("abc\ndefghij"); + view.setWrapMode(.char); + + const result = try view.measureForDimensions(0, 24); + try std.testing.expectEqual(tb.getLineCount(), result.line_count); + try std.testing.expectEqual(iter_mod.getMaxLineWidth(&tb.rope), result.max_width); +} + +test "TextBufferView measureForDimensions - no wrap matches multi-segment line widths" { + const pool = gp.initGlobalPool(std.testing.allocator); + defer gp.deinitGlobalPool(); + + var tb = try TextBuffer.init(std.testing.allocator, pool, .wcwidth); + defer tb.deinit(); + + var view = try TextBufferView.init(std.testing.allocator, tb); + defer view.deinit(); + + try tb.setText("AAAA"); + try tb.append("BBBB"); + view.setWrapMode(.none); + + const line_info = view.getCachedLineInfo(); + var expected_max: u32 = 0; + for (line_info.widths) |w| { + expected_max = @max(expected_max, w); + } + + const result = try view.measureForDimensions(80, 24); + try std.testing.expectEqual(expected_max, result.max_width); + try std.testing.expectEqual(@as(u32, @intCast(line_info.widths.len)), result.line_count); +} + +test "TextBufferView measureForDimensions - cache invalidates on switchToBuffer" { + const pool = gp.initGlobalPool(std.testing.allocator); + defer gp.deinitGlobalPool(); + + var tb = try TextBuffer.init(std.testing.allocator, pool, .wcwidth); + defer tb.deinit(); + + var other_tb = try TextBuffer.init(std.testing.allocator, pool, .wcwidth); + defer other_tb.deinit(); + + var view = try TextBufferView.init(std.testing.allocator, tb); + defer view.deinit(); + + try tb.setText("AAAAAA"); + view.setWrapMode(.char); + + const result1 = try view.measureForDimensions(10, 10); + try std.testing.expectEqual(@as(u32, 6), result1.max_width); + + try other_tb.setText("BBBBBBBBBB"); + try std.testing.expectEqual(tb.getContentEpoch(), other_tb.getContentEpoch()); + + view.switchToBuffer(other_tb); + + const result2 = try view.measureForDimensions(10, 10); + try std.testing.expectEqual(@as(u32, 10), result2.max_width); + try std.testing.expectEqual(@as(u32, 1), result2.line_count); +} + test "TextBufferView measureForDimensions - char wrap" { const pool = gp.initGlobalPool(std.testing.allocator); defer gp.deinitGlobalPool(); diff --git a/packages/core/src/zig/tests/wrap-cache-perf_test.zig b/packages/core/src/zig/tests/wrap-cache-perf_test.zig index 08fe8e224..0e0993f0e 100644 --- a/packages/core/src/zig/tests/wrap-cache-perf_test.zig +++ b/packages/core/src/zig/tests/wrap-cache-perf_test.zig @@ -49,7 +49,7 @@ test "word wrap complexity - width changes are O(n)" { _ = view.getVirtualLineCount(); for (widths, 0..) |width, i| { - times[i] = measureMedianViewUpdate(view, width, 3); + times[i] = measureMedianViewUpdate(view, width, 5); } var min_time: u64 = std.math.maxInt(u64); @@ -61,8 +61,10 @@ test "word wrap complexity - width changes are O(n)" { const ratio = @as(f64, @floatFromInt(max_time)) / @as(f64, @floatFromInt(min_time)); - // All times should be roughly similar (within 3x) since text size is constant - try std.testing.expect(ratio < 3.0); + // All times should be roughly similar since text size is constant. + // Using 5x threshold to accommodate CI runner variability while still + // catching O(n^2) regressions (which would show 10-100x differences). + try std.testing.expect(ratio < 5.0); } test "word wrap - virtual line count correctness" { diff --git a/packages/core/src/zig/text-buffer-view.zig b/packages/core/src/zig/text-buffer-view.zig index cf8ff0b43..6f20f5bf8 100644 --- a/packages/core/src/zig/text-buffer-view.zig +++ b/packages/core/src/zig/text-buffer-view.zig @@ -124,9 +124,22 @@ pub const UnifiedTextBufferView = struct { cached_line_vline_counts: std.ArrayListUnmanaged(u32), global_allocator: Allocator, virtual_lines_arena: *std.heap.ArenaAllocator, + + /// Persistent arena for measureForDimensions. Each call resets it with + /// retain_capacity to avoid mmap/munmap churn during streaming. + measure_arena: std.heap.ArenaAllocator, tab_indicator: ?u32, tab_indicator_color: ?RGBA, + // Measurement cache for Yoga layout. Keyed by (buffer, epoch, width, wrap_mode). + // Using epoch instead of dirty flag prevents stale returns when unrelated + // code paths clear dirty (e.g., updateVirtualLines). + cached_measure_width: ?u32, + cached_measure_wrap_mode: WrapMode, + cached_measure_result: ?MeasureResult, + cached_measure_epoch: u64, + cached_measure_buffer: ?*UnifiedTextBuffer, + pub fn init(global_allocator: Allocator, text_buffer: *UnifiedTextBuffer) TextBufferViewError!*Self { const self = global_allocator.create(Self) catch return TextBufferViewError.OutOfMemory; errdefer global_allocator.destroy(self); @@ -156,8 +169,14 @@ pub const UnifiedTextBufferView = struct { .cached_line_vline_counts = .{}, .global_allocator = global_allocator, .virtual_lines_arena = virtual_lines_internal_arena, + .measure_arena = std.heap.ArenaAllocator.init(global_allocator), .tab_indicator = null, .tab_indicator_color = null, + .cached_measure_width = null, + .cached_measure_wrap_mode = .none, + .cached_measure_result = null, + .cached_measure_epoch = 0, + .cached_measure_buffer = null, }; return self; @@ -170,6 +189,7 @@ pub const UnifiedTextBufferView = struct { self.original_text_buffer.unregisterView(self.view_id); self.virtual_lines_arena.deinit(); self.global_allocator.destroy(self.virtual_lines_arena); + self.measure_arena.deinit(); self.global_allocator.destroy(self); } @@ -690,14 +710,47 @@ pub const UnifiedTextBufferView = struct { /// Measure dimensions for given width/height WITHOUT modifying virtual lines cache /// This is useful for Yoga measure functions that need to know dimensions without committing changes - /// Special case: width=0 means "measure intrinsic/max-content width" (no wrapping) - pub fn measureForDimensions(self: *const Self, width: u32, height: u32) TextBufferViewError!MeasureResult { + /// Special case: width=0 or wrap_mode=.none means "measure intrinsic/max-content width" (no wrapping) + pub fn measureForDimensions(self: *Self, width: u32, height: u32) TextBufferViewError!MeasureResult { _ = height; // Height is for future use, currently only width affects layout - // Create temporary arena for measurement - var measure_arena = std.heap.ArenaAllocator.init(self.global_allocator); - defer measure_arena.deinit(); - const measure_allocator = measure_arena.allocator(); + const epoch = self.text_buffer.getContentEpoch(); + if (self.cached_measure_result) |result| { + if (self.cached_measure_epoch == epoch and self.cached_measure_buffer == self.text_buffer) { + if (self.cached_measure_width) |cached_width| { + if (cached_width == width and self.cached_measure_wrap_mode == self.wrap_mode) { + return result; + } + } + } + } + + // No-wrap path avoids allocations by using marker-based line widths. + if (width == 0 or self.wrap_mode == .none) { + const line_count = self.text_buffer.getLineCount(); + var max_width: u32 = 0; + var row: u32 = 0; + while (row < line_count) : (row += 1) { + max_width = @max(max_width, iter_mod.lineWidthAt(&self.text_buffer.rope, row)); + } + + const result = MeasureResult{ + .line_count = line_count, + .max_width = max_width, + }; + + self.cached_measure_width = width; + self.cached_measure_wrap_mode = self.wrap_mode; + self.cached_measure_result = result; + self.cached_measure_epoch = epoch; + self.cached_measure_buffer = self.text_buffer; + + return result; + } + + // Reuse arena capacity to avoid allocation overhead during streaming. + _ = self.measure_arena.reset(.retain_capacity); + const measure_allocator = self.measure_arena.allocator(); // Create temporary output structures var temp_virtual_lines = std.ArrayListUnmanaged(VirtualLine){}; @@ -719,7 +772,6 @@ pub const UnifiedTextBufferView = struct { }; // Use width for wrap calculation - // Special case: width=0 means get intrinsic width (no wrapping), so pass null const wrap_width_for_measure = if (self.wrap_mode != .none and width > 0) width else null; // Call generic calculation with temporary structures @@ -737,10 +789,18 @@ pub const UnifiedTextBufferView = struct { max_width = @max(max_width, w); } - return MeasureResult{ + const result = MeasureResult{ .line_count = @intCast(temp_virtual_lines.items.len), .max_width = max_width, }; + + self.cached_measure_width = width; + self.cached_measure_wrap_mode = self.wrap_mode; + self.cached_measure_result = result; + self.cached_measure_epoch = epoch; + self.cached_measure_buffer = self.text_buffer; + + return result; } /// Generic virtual line calculation that writes to provided output structures diff --git a/packages/core/src/zig/text-buffer.zig b/packages/core/src/zig/text-buffer.zig index 98b409dc4..7dd231a44 100644 --- a/packages/core/src/zig/text-buffer.zig +++ b/packages/core/src/zig/text-buffer.zig @@ -62,6 +62,10 @@ pub const UnifiedTextBuffer = struct { next_view_id: u32, free_view_ids: std.ArrayListUnmanaged(u32), + /// Monotonic counter that increments on every content change. Views use this + /// to detect stale caches even after clearViewDirty() runs. + content_epoch: u64, + // Per-line highlight cache (invalidated on edits) // Maps line_idx to highlights for that line line_highlights: std.ArrayListUnmanaged(std.ArrayListUnmanaged(Highlight)), @@ -118,6 +122,7 @@ pub const UnifiedTextBuffer = struct { .view_dirty_flags = view_dirty_flags, .next_view_id = 0, .free_view_ids = free_view_ids, + .content_epoch = 0, .line_highlights = .{}, .line_spans = .{}, .highlight_batch_depth = 0, @@ -198,7 +203,16 @@ pub const UnifiedTextBuffer = struct { } } + /// Returns the current content epoch. Use this to detect buffer changes + /// independent of the dirty flag (other code paths may clear dirty). + pub fn getContentEpoch(self: *const Self) u64 { + return self.content_epoch; + } + fn markAllViewsDirty(self: *Self) void { + // Increment epoch first so views see the new value when checking caches. + // Use wrapping add for safety, though u64 won't overflow in practice. + self.content_epoch +%= 1; for (self.view_dirty_flags.items) |*flag| { flag.* = true; } @@ -972,10 +986,15 @@ pub const UnifiedTextBuffer = struct { return self.tab_width; } - /// Set tab width (will be rounded up to nearest multiple of 2) + /// Set tab width, rounding up to nearest multiple of 2 (minimum 2). + /// Marks all views dirty if the width actually changes, since tab width + /// affects measured line widths and virtual line calculations. pub fn setTabWidth(self: *Self, width: u8) void { const clamped_width = @max(2, width); - self.tab_width = if (clamped_width % 2 == 0) clamped_width else clamped_width + 1; + const new_width = if (clamped_width % 2 == 0) clamped_width else clamped_width + 1; + if (self.tab_width == new_width) return; + self.tab_width = new_width; + self.markAllViewsDirty(); } /// Debug log the rope structure using rope.toText diff --git a/packages/solid/tests/__snapshots__/textarea.test.tsx.snap b/packages/solid/tests/__snapshots__/textarea.test.tsx.snap index faf26535d..d3f4b376b 100644 --- a/packages/solid/tests/__snapshots__/textarea.test.tsx.snap +++ b/packages/solid/tests/__snapshots__/textarea.test.tsx.snap @@ -519,5 +519,175 @@ start +" +`; + +exports[`Textarea Layout Tests Measure Cache Edge Cases should correctly measure text after content change 1`] = ` +"┌──────────────────────────────────────┐ +│Short text │ +└──────────────────────────────────────┘ + + + + + + + + + + + + +" +`; + +exports[`Textarea Layout Tests Measure Cache Edge Cases should correctly measure text after content change 2`] = ` +"┌──────────────────────────────────────┐ +│This is a much longer text that will │ +│definitely wrap to multiple lines │ +│when rendered │ +└──────────────────────────────────────┘ + + + + + + + + + + +" +`; + +exports[`Textarea Layout Tests Measure Cache Edge Cases should handle rapid content updates correctly 1`] = ` +"┌────────────────────────────┐ +│Update 4: some text here │ +└────────────────────────────┘ + + + + + + + +" +`; + +exports[`Textarea Layout Tests Measure Cache Edge Cases should handle width changes with cached measures 1`] = ` +"┌────────────────────────────┐ +│Content that will wrap │ +│differently at different │ +│widths │ +└────────────────────────────┘ + + + + + + + + + + +" +`; + +exports[`Textarea Layout Tests Measure Cache Edge Cases should handle width changes with cached measures 2`] = ` +"┌────────────────────────────────────────────────┐ +│Content that will wrap differently at different │ +│widths │ +└────────────────────────────────────────────────┘ + + + + + + + + + + + +" +`; + +exports[`Textarea Layout Tests Measure Cache Edge Cases should handle width changes with cached measures 3`] = ` +"┌──────────────────┐ +│Content that will │ +│wrap differently │ +│at different │ +│widths │ +└──────────────────┘ + + + + + + + + + +" +`; + +exports[`Textarea Layout Tests Measure Cache Edge Cases should handle empty to non-empty content transition 1`] = ` +"┌──────────────────────────────────────┐ +│ │ +└──────────────────────────────────────┘ + + + + + + + +" +`; + +exports[`Textarea Layout Tests Measure Cache Edge Cases should handle empty to non-empty content transition 2`] = ` +"┌──────────────────────────────────────┐ +│Now with content │ +└──────────────────────────────────────┘ + + + + + + + +" +`; + +exports[`Textarea Layout Tests Measure Cache Edge Cases should handle empty to non-empty content transition 3`] = ` +"┌──────────────────────────────────────┐ +│ │ +└──────────────────────────────────────┘ + + + + + + + +" +`; + +exports[`Textarea Layout Tests Measure Cache Edge Cases should correctly measure multiline content with unicode 1`] = ` +"┌────────────────────────────┐ +│Hello 世界 │ +│こんにちは │ +│🌟 Emoji 🚀 │ +└────────────────────────────┘ + + + + + + + + + + " `; diff --git a/packages/solid/tests/textarea.test.tsx b/packages/solid/tests/textarea.test.tsx index 05e82f3af..09140ba31 100644 --- a/packages/solid/tests/textarea.test.tsx +++ b/packages/solid/tests/textarea.test.tsx @@ -847,4 +847,138 @@ describe("Textarea Layout Tests", () => { expect(frame).toMatchSnapshot() }) }) + + describe("Measure Cache Edge Cases", () => { + it("should correctly measure text after content change", async () => { + const [value, setValue] = createSignal("Short text") + + testSetup = await testRender( + () => ( + + + {value()} + + + ), + { width: 50, height: 15 }, + ) + + await testSetup.renderOnce() + const initialFrame = testSetup.captureCharFrame() + + // Change to longer content that should cause more wrapping + setValue("This is a much longer text that will definitely wrap to multiple lines when rendered") + await testSetup.renderOnce() + const updatedFrame = testSetup.captureCharFrame() + + expect(initialFrame).toMatchSnapshot() + expect(updatedFrame).toMatchSnapshot() + expect(updatedFrame).not.toBe(initialFrame) + }) + + it("should handle rapid content updates correctly", async () => { + const [value, setValue] = createSignal("Initial") + + testSetup = await testRender( + () => ( + + + {value()} + + + ), + { width: 40, height: 10 }, + ) + + // Rapid updates to simulate typing + for (let i = 0; i < 5; i++) { + setValue(`Update ${i}: some text here`) + await testSetup.renderOnce() + } + + const finalFrame = testSetup.captureCharFrame() + expect(finalFrame).toMatchSnapshot() + }) + + it("should handle width changes with cached measures", async () => { + const [width, setWidth] = createSignal(30) + + testSetup = await testRender( + () => ( + + + Content that will wrap differently at different widths + + + ), + { width: 60, height: 15 }, + ) + + await testSetup.renderOnce() + const frame30 = testSetup.captureCharFrame() + + setWidth(50) + await testSetup.renderOnce() + const frame50 = testSetup.captureCharFrame() + + setWidth(20) + await testSetup.renderOnce() + const frame20 = testSetup.captureCharFrame() + + expect(frame30).toMatchSnapshot() + expect(frame50).toMatchSnapshot() + expect(frame20).toMatchSnapshot() + }) + + it("should handle empty to non-empty content transition", async () => { + const [value, setValue] = createSignal("") + + testSetup = await testRender( + () => ( + + + {value() || " "} + + + ), + { width: 50, height: 10 }, + ) + + await testSetup.renderOnce() + const emptyFrame = testSetup.captureCharFrame() + + setValue("Now with content") + await testSetup.renderOnce() + const contentFrame = testSetup.captureCharFrame() + + setValue("") + await testSetup.renderOnce() + const emptyAgainFrame = testSetup.captureCharFrame() + + expect(emptyFrame).toMatchSnapshot() + expect(contentFrame).toMatchSnapshot() + expect(emptyAgainFrame).toMatchSnapshot() + }) + + it("should correctly measure multiline content with unicode", async () => { + testSetup = await testRender( + () => ( + + + Hello 世界 +
+ こんにちは +
+ 🌟 Emoji 🚀 + + + ), + { width: 40, height: 15 }, + ) + + await testSetup.renderOnce() + const frame = testSetup.captureCharFrame() + expect(frame).toMatchSnapshot() + }) + }) })