Record paste and surface creation as append patches - #362
Conversation
merge now appends to the existing cloud in place and records only the previous point count, instead of cloning the whole cloud and keeping a snapshot. The result is delivered via a fresh PointCloud (newCloudView) because pcgol caches an unsafe float32 alias of Data keyed by its base pointer, which goes stale when the slice length changes in place. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## perf/patch-label #362 +/- ##
====================================================
+ Coverage 45.86% 46.49% +0.62%
====================================================
Files 9 9
Lines 1598 1624 +26
====================================================
+ Hits 733 755 +22
- Misses 813 815 +2
- Partials 52 54 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR changes how paste and surface creation (“merge”) operations are recorded for undo: instead of storing a full replacement snapshot of the previous cloud, it appends points in-place and records a minimal “append patch” containing only the prior point count and organized dimensions, with undo implemented as a truncation back to the prior count (restoring width/height as well). It also introduces a helper to return a fresh *pc.PointCloud view when the underlying Data slice length changes, to avoid stale cached unsafe views in pcgol.
Changes:
- Add
appendPatchto record pre-append(Points, Width, Height)and revert by truncatingData. - Add
newCloudViewto produce a fresh*pc.PointCloudwhenDatalength changes. - Update
editor.mergeto pushappendPatchand usenewCloudView; extend tests to cover append patch revert and encode/decode round-trip.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| patch.go | Adds newCloudView, introduces appendPatch, and extends patch decoding to support append patches. |
| patch_test.go | Adds unit test coverage for appendPatch revert and encode/decode round-trip. |
| editor.go | Updates merge to record append-only undo state and refresh the point cloud view after appending. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The decoded oldPoints could turn negative on an int width smaller than 64 bits and slip through the bounds checks into a negative slice bound. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rewrite the data bound in a division form matching the other guards and reject negative width/height, so the checks hold on any int width without relying on evaluation order. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adapt appendPatch to the encodeHead/payload interface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A corrupted replacePatch can restore a cloud whose decoded header yields a zero stride; the division in the bounds check then panicked instead of returning errBrokenPatch on the following undo. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Changes
Paste and surface creation (merge) now append to the cloud in place and record a patch holding only the previous point count, width and height. Undo just truncates back to the previous count.