Introduce undo/redo support for arbitrary operations in `Buffer` by as-cii · Pull Request #11 · zed-industries/zed

as-cii · 2021-04-09T09:43:12Z

This is still not wired up and there is no undo/redo stack yet, but I feel confident about the implementation (randomized tests are passing) and given that the diff is non-trivial, I figured I'd open a pull request for you guys to take a 👀 and validate the approach. A few remarks:

The main idea is that we have an UndoMap struct that stores which undo operations have been done on which edits; then, based on the parity of the undo count we determine whether an edit is visible (see UndoMap::is_visible). Storing all the operations (as opposed to just the max undo count) allows for querying visibility in the past by supplying a version vector (see UndoMap::was_visible).
Fragment was also augmented to include the max undo IDs observed for that fragment (Fragment::max_undos) such that we could include the effect of the undo operation in FragmentSummary::max_version, which is used by methods like Buffer::changes_since(version) to navigate in parts of the tree that aren't covered by the supplied version.
We use the richer FragmentSummary::max_version information also during Buffer::splice_fragments to compute the version_in_range that is reported to remote peers when generating the edit operation. In fact, this is required so that when a remote peer applies an edit operation, it can deterministically compute whether a fragment was visible or not when the splice operation was generated, and delete local fragments accordingly.
Finally, applying an undo operation works by including the new undo count in UndoMap and - using the visibility query methods described above - finding all the fragments touched by the edit we are undoing/redoing and recomputing their visibility.

/cc: @nathansobo @maxbrunsfeld

...as opposed to the whole string.

We will report them in public higher-level methods like `Buffer::undo` and `Buffer::redo`.

maxbrunsfeld

⚡ Awesome job on this.

* Fix clippy * Remove increment request sequence (wrong while merging) * Send correct request to stop thread based on capabilities

…fault, cross-layer accumulation docs - Fix #11: commit_message_model doc said 'Default: true' but field is Option<LanguageModelSelection> - Fix #12: agent-settings.md said single_file_review default is false, but it's true - Fix #13: Add cross-layer accumulation docs to always_allow, always_confirm, always_deny

Replace bail! with graceful return Ok(()) when edit_predictions isn't an object. This allows subsequent migrations (m_2026_02_03, m_2026_02_04) to still run when edit_predictions has an unexpected type.

#3: Ensure worktree creation status cannot get stuck in Creating. Clone the WeakEntity handle before passing it to setup_new_workspace so failures can call set_worktree_creation_error to transition the status to Error. #8: Store the visual-test global observer subscription instead of detaching it, and explicitly drop it during teardown. #9: Delete the preserved temp directory after visual-test cleanup so repeated runs don't accumulate filesystem artifacts. #10: Add run_git_command helper that checks exit status and includes stderr/stdout in the error message for actionable diagnostics. #11: Change two open_new().detach() calls to detach_and_log_err(cx) so workspace creation failures are logged instead of silently swallowed.

- Fix #3: Add catch-all error handling around setup_new_workspace so worktree creation status cannot get stuck in Creating - Fix #8: Store visual-test workspace observer subscription instead of detaching it; explicitly drop during teardown - Fix #9: Delete preserved temp directory after visual test completion - Fix #10: Add run_git_command helper that checks exit status and includes stderr/stdout context in failure messages - Fix #11: Change two open_new().detach() calls to detach_and_log_err - Revert unrelated thread_history.rs changes (delete event refactoring, format_relative_time extraction, v_flex layout, iteration guard) - Restore assistant namespace conditional and CopyCode test assertions in agent_ui.rs

When atlas tiles are rapidly allocated and freed (e.g. watching a shared screen in Collab), a texture can become unreferenced and be removed while GPU uploads for it are still pending. On the next frame, `flush_uploads` indexes into the now-empty texture slot and panics: ``` thread 'main' panicked at crates/gpui_wgpu/src/wgpu_atlas.rs:231:40: texture must exist... #11 core::option::expect_failed #12 gpui_wgpu::wgpu_atlas::WgpuAtlas::before_frame #13 gpui_wgpu::wgpu_renderer::WgpuRenderer::draw ``` Drain pending uploads for a texture when it becomes unreferenced in `remove`, and skip uploads for missing textures in `flush_uploads` as a safety net.

When atlas tiles are rapidly allocated and freed (e.g. watching a shared screen in Collab), a texture can become unreferenced and be removed while GPU uploads for it are still pending. On the next frame, `flush_uploads` indexes into the now-empty texture slot and panics: ``` thread 'main' panicked at crates/gpui_wgpu/src/wgpu_atlas.rs:231:40: texture must exist... #11 core::option::expect_failed #12 gpui_wgpu::wgpu_atlas::WgpuAtlas::before_frame #13 gpui_wgpu::wgpu_renderer::WgpuRenderer::draw ``` This change drains pending uploads for a texture when it becomes unreferenced in `remove`, and skips uploads for missing textures in `flush_uploads` as a safety net. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed occasional crashes when viewing a screen share

…ashboard.

…-02-26 Upstream merge 2026 02 26

) When atlas tiles are rapidly allocated and freed (e.g. watching a shared screen in Collab), a texture can become unreferenced and be removed while GPU uploads for it are still pending. On the next frame, `flush_uploads` indexes into the now-empty texture slot and panics: ``` thread 'main' panicked at crates/gpui_wgpu/src/wgpu_atlas.rs:231:40: texture must exist... zed-industries#11 core::option::expect_failed zed-industries#12 gpui_wgpu::wgpu_atlas::WgpuAtlas::before_frame zed-industries#13 gpui_wgpu::wgpu_renderer::WgpuRenderer::draw ``` This change drains pending uploads for a texture when it becomes unreferenced in `remove`, and skips uploads for missing textures in `flush_uploads` as a safety net. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed occasional crashes when viewing a screen share

…off (zed-industries#11)

…tries#11) Two-phase: server first (new MCP tool solution_agent.send_message_blocks wrapping the existing store.send_message_blocks which already takes Vec<acp::ContentBlock>; allow-list entry; tests), then mobile (picker UI via PhotoPicker + OpenDocument; pure-fn encoder in :core mapping mime → Text/Image content block or EncodingFailure; queue store v2 for block lists; optimistic-bubble dedupe via local-send-time).

) When atlas tiles are rapidly allocated and freed (e.g. watching a shared screen in Collab), a texture can become unreferenced and be removed while GPU uploads for it are still pending. On the next frame, `flush_uploads` indexes into the now-empty texture slot and panics: ``` thread 'main' panicked at crates/gpui_wgpu/src/wgpu_atlas.rs:231:40: texture must exist... zed-industries#11 core::option::expect_failed zed-industries#12 gpui_wgpu::wgpu_atlas::WgpuAtlas::before_frame zed-industries#13 gpui_wgpu::wgpu_renderer::WgpuRenderer::draw ``` This change drains pending uploads for a texture when it becomes unreferenced in `remove`, and skips uploads for missing textures in `flush_uploads` as a safety net. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed occasional crashes when viewing a screen share

Introduce undo/redo support for arbitrary operations in `Buffer`

…tions Closes out the rest of BUG_REPORT.md's optimization notes: - Skip the BMP encode+decode round trip. process_page used to encode pdfium's bitmap to BMP just so gpui's asset-cache pipeline would decode it right back on first paint. It now builds gpui's RenderImage/Frame directly from pdfium's RGBA-normalized buffer (one R/B swap to match gpui's BGRA convention) and hands it to img() via ImageSource::Render, skipping the decode step entirely. PageData.image is now Arc<RenderImage> instead of Arc<Image>. - Cap render target width (Medium zed-industries#11). process_page now clamps its requested width to MAX_RENDER_WIDTH (8192px) instead of asking pdfium for an arbitrarily large bitmap at high zoom - real tiling is a bigger change and still open. - Prioritize visible pages over prefetch. Pdf::request_page now takes a gpui::Priority; pdf_viewer.rs's prepaint spawns visible-range pages at Priority::High and the PRERENDER_PAGES window (plus search's adjacent-match prefetch) at Priority::Low, using BackgroundExecutor::spawn_with_priority. This is gpui's own weighted-probability scheduler rather than a hand-rolled queue - it still funnels everything through pdfium's single mutex, but a fast scroll's newly-visible pages no longer wait behind stale prefetch work on equal footing. Added test coverage for all three, plus the render/cache wiring in Pdf::request_page itself (previously untested): - pdf_renderer::tests::rendered_page_uses_gpuis_native_bgra_byte_order (against a new solid_red.pdf fixture) caught the channel swap being backwards on the first attempt at the BMP-round-trip removal. - pdf_renderer::tests::render_target_width_is_capped_at_extreme_zoom - pdf::tests::request_page_renders_once_then_short_circuits_from_cache, a #[gpui::test] against a real TestAppContext (needs allow_parking() since it does real PDFium FFI work on a background task). Required adding gpui/test-support as a dev-dependency, which - like db/test-support before it - was simply missing. BUG_REPORT.md and TEST_PLAN.md updated to reflect what's fixed/tested now. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…trip Closes out the last open optimization item (Medium zed-industries#11 / BUG_REPORT.md's "real tiling" note), scoped to vertical-strip rendering rather than a full 2D tile grid (Chromium's approach, but a much larger change for a case this crate rarely hits - most PDFs never come close to MAX_RENDER_WIDTH/HEIGHT). - process_page now computes height independently of the existing width cap: if a page's full height at the requested scale would exceed MAX_RENDER_HEIGHT (8192px - a real scenario, since the PDF spec allows pages up to 14,400pt tall), it renders a single MAX_RENDER_HEIGHT-tall vertical strip around a new visible_center_y parameter instead of downscaling the whole page. Uses pdfium's set_origin + render_into_bitmap_with_config with a bitmap smaller than the page's full scaled size - pdfium-render's own documented pattern for exactly this. - PageData gained page_offset_pt (where the strip starts), image_scale (the scale actually used to rasterize it, which can differ from the nominal `scale` field once width-capped), and covers_page_y() so callers can tell whether a cached render still covers a point they care about. - Pdf::request_page's cache-hit fast path now also checks covers_page_y, so scrolling within one oversized page re-renders the strip that's actually needed instead of reusing a stale one at the same nominal scale. - pdf_viewer.rs's prepaint computes visible_center_y per page from the actual viewport (clamped into each page's own bounds, so prerender pages get a sensible value too) and positions a rendered strip within its page box by page_offset_pt/covered_height_pt instead of stretching it - for an ordinary page these just work out to (0, full height), matching the previous size_full() behavior exactly. Search's adjacent-match prefetch uses the match's own position instead of defaulting to the page top. Added test_fixtures/tall_page.pdf (200x14,000pt, past MAX_RENDER_HEIGHT at 1x scale) and four new tests covering the strip math directly (capped height, offset following visible_center_y, covers_page_y correctness, different regions rendering different pixels) and Pdf::request_page's re-render-on-scroll behavior through a real TestAppContext. BUG_REPORT.md and TEST_PLAN.md updated; this closes the last item from the "finalize the optimizations" pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…tions Closes out the rest of BUG_REPORT.md's optimization notes: - Skip the BMP encode+decode round trip. process_page used to encode pdfium's bitmap to BMP just so gpui's asset-cache pipeline would decode it right back on first paint. It now builds gpui's RenderImage/Frame directly from pdfium's RGBA-normalized buffer (one R/B swap to match gpui's BGRA convention) and hands it to img() via ImageSource::Render, skipping the decode step entirely. PageData.image is now Arc<RenderImage> instead of Arc<Image>. - Cap render target width (Medium zed-industries#11). process_page now clamps its requested width to MAX_RENDER_WIDTH (8192px) instead of asking pdfium for an arbitrarily large bitmap at high zoom - real tiling is a bigger change and still open. - Prioritize visible pages over prefetch. Pdf::request_page now takes a gpui::Priority; pdf_viewer.rs's prepaint spawns visible-range pages at Priority::High and the PRERENDER_PAGES window (plus search's adjacent-match prefetch) at Priority::Low, using BackgroundExecutor::spawn_with_priority. This is gpui's own weighted-probability scheduler rather than a hand-rolled queue - it still funnels everything through pdfium's single mutex, but a fast scroll's newly-visible pages no longer wait behind stale prefetch work on equal footing. Added test coverage for all three, plus the render/cache wiring in Pdf::request_page itself (previously untested): - pdf_renderer::tests::rendered_page_uses_gpuis_native_bgra_byte_order (against a new solid_red.pdf fixture) caught the channel swap being backwards on the first attempt at the BMP-round-trip removal. - pdf_renderer::tests::render_target_width_is_capped_at_extreme_zoom - pdf::tests::request_page_renders_once_then_short_circuits_from_cache, a #[gpui::test] against a real TestAppContext (needs allow_parking() since it does real PDFium FFI work on a background task). Required adding gpui/test-support as a dev-dependency, which - like db/test-support before it - was simply missing. BUG_REPORT.md and TEST_PLAN.md updated to reflect what's fixed/tested now.

…trip Closes out the last open optimization item (Medium zed-industries#11 / BUG_REPORT.md's "real tiling" note), scoped to vertical-strip rendering rather than a full 2D tile grid (Chromium's approach, but a much larger change for a case this crate rarely hits - most PDFs never come close to MAX_RENDER_WIDTH/HEIGHT). - process_page now computes height independently of the existing width cap: if a page's full height at the requested scale would exceed MAX_RENDER_HEIGHT (8192px - a real scenario, since the PDF spec allows pages up to 14,400pt tall), it renders a single MAX_RENDER_HEIGHT-tall vertical strip around a new visible_center_y parameter instead of downscaling the whole page. Uses pdfium's set_origin + render_into_bitmap_with_config with a bitmap smaller than the page's full scaled size - pdfium-render's own documented pattern for exactly this. - PageData gained page_offset_pt (where the strip starts), image_scale (the scale actually used to rasterize it, which can differ from the nominal `scale` field once width-capped), and covers_page_y() so callers can tell whether a cached render still covers a point they care about. - Pdf::request_page's cache-hit fast path now also checks covers_page_y, so scrolling within one oversized page re-renders the strip that's actually needed instead of reusing a stale one at the same nominal scale. - pdf_viewer.rs's prepaint computes visible_center_y per page from the actual viewport (clamped into each page's own bounds, so prerender pages get a sensible value too) and positions a rendered strip within its page box by page_offset_pt/covered_height_pt instead of stretching it - for an ordinary page these just work out to (0, full height), matching the previous size_full() behavior exactly. Search's adjacent-match prefetch uses the match's own position instead of defaulting to the page top. Added test_fixtures/tall_page.pdf (200x14,000pt, past MAX_RENDER_HEIGHT at 1x scale) and four new tests covering the strip math directly (capped height, offset following visible_center_y, covers_page_y correctness, different regions rendering different pixels) and Pdf::request_page's re-render-on-scroll behavior through a real TestAppContext. BUG_REPORT.md and TEST_PLAN.md updated; this closes the last item from the "finalize the optimizations" pass.

tools/ggo-ide deleted in ggo 281fd557; MIGRATION records that plus a closing section (four real losses, three reported-lost-but-not, the deferred set, single-cover gaps, out-of-scope debts). One row moved the wrong way: inspector ✓ -> partial (Asset(_) is free text). Counts 55/36/2/11. Three stale citations fixed. Ten-crate sweep 703; merge drill zed-industries#11.

as-cii added 8 commits April 8, 2021 15:05

Introduce undo/redo support for arbitrary operations in Buffer

44ab38a

Fix Debug impl for Text to only display the slice a Text refers to

5e0d5b7

...as opposed to the whole string.

WIP

c20935d

Compute minimal version_in_range on edit and account for undo

7b6fbe1

Store only max undo by replica instead of individual undos in Fragment

2e7ca8e

Exercise arbitrary undos/redos in single-peer buffer randomized test

4d9ff11

Move UndoMap down

bf3a5d0

Don't report edits in Buffer::undo_or_redo

cba8730

We will report them in public higher-level methods like `Buffer::undo` and `Buffer::redo`.

as-cii force-pushed the undo branch from e95741e to cba8730 Compare April 9, 2021 12:15

maxbrunsfeld approved these changes Apr 9, 2021

View reviewed changes

as-cii merged commit 07b8a10 into master Apr 9, 2021

as-cii deleted the undo branch April 9, 2021 16:50

as-cii mentioned this pull request Apr 12, 2021

Undo/redo support #13

Merged

1 task

qarmin mentioned this pull request Jul 25, 2024

AddressSanitizer stack-use-after-scope crash when running app #15165

Closed

1 task

Anthony-Eid pushed a commit to Anthony-Eid/zed that referenced this pull request Aug 7, 2024

Fix use correct request to stop thread (zed-industries#11)

aa257ec

* Fix clippy * Remove increment request sequence (wrong while merging) * Send correct request to stop thread based on capabilities

CornuAmmonis mentioned this pull request Dec 28, 2024

[Linux KDE] Zed crashes when resizing #19254

Closed

1 task

mjbommar mentioned this pull request Aug 12, 2025

Hang after drawing first frame with NVIDIA 580 drivers before 580.82.07 on Linux Wayland #35948

Closed

StephenSmithwick mentioned this pull request Dec 19, 2025

MacOS Zed Hangs when loading zed: extensions #44013

Closed

chan-yuu pushed a commit to chan-yuu/zed that referenced this pull request Apr 18, 2026

feat: Add Feature zed-industries#11 Integrate task display into the d…

88af3a1

…ashboard.

m-bers pushed a commit to m-bers/zed that referenced this pull request Apr 19, 2026

Merge pull request zed-industries#11 from helixml/upstream-merge-2026…

b118f85

…-02-26 Upstream merge 2026 02 26

Sipaha pushed a commit to Sipaha/spk-editor that referenced this pull request May 4, 2026

FORK.md: document Linux focus-stealing-prevention limitation for hand…

8970f53

…off (zed-industries#11)

KJdotIO mentioned this pull request Jul 26, 2026

GitLab MR link absent from git blame hover popover #58841

Open

jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026

Merge pull request zed-industries#11 from zed-industries/undo

36ea197

Introduce undo/redo support for arbitrary operations in `Buffer`

Navigation Menu

Uh oh!

Uh oh!

Introduce undo/redo support for arbitrary operations in `Buffer` - #11

Introduce undo/redo support for arbitrary operations in `Buffer`#11
as-cii merged 8 commits into
masterfrom
undo

as-cii commented Apr 9, 2021

Uh oh!

maxbrunsfeld left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

as-cii commented Apr 9, 2021

Uh oh!

maxbrunsfeld left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants