From 139b15ee5342353e976044b0890193e43cbf4991 Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sun, 16 Aug 2026 16:37:42 +0900 Subject: [PATCH 01/13] Store undo history as serialized patches Replace the snapshot undo history (raw copies of the whole cloud plus a Go-side header list) with a patch-based history: each edit pushes a patch reverting it, serialized and stored uniformly on the JS heap. For now every edit type uses replacePatch, a whole-cloud snapshot, so behavior and memory characteristics are unchanged while the pipeline (push, serialized storage, undo by revert) is in place. Follow-ups replace the snapshot fallback with cheap per-operation patches and compress what remains. The non-js history stub becomes a real implementation (historyMem), making undo behavior testable with plain go test; randomized round-trip tests assert byte-exact restoration. Undo depth semantics of max_history are unchanged (N entries = N undos). Co-Authored-By: Claude Fable 5 --- command.go | 2 +- editor.go | 52 ++++++++++--- history_test.go | 124 +++++++++++++++++++++++++++++++ patch.go | 191 ++++++++++++++++++++++++++++++++++++++++++++++++ patch_test.go | 95 ++++++++++++++++++++++++ undo.go | 59 ++++++++++----- undo_js.go | 78 ++++++++++---------- 7 files changed, 531 insertions(+), 70 deletions(-) create mode 100644 history_test.go create mode 100644 patch.go create mode 100644 patch_test.go diff --git a/command.go b/command.go index a47b62a8..725ff6f3 100644 --- a/command.go +++ b/command.go @@ -631,8 +631,8 @@ func (c *commandContext) VoxelFilter(resolution float32) error { if selected { c.editor.passThrough(c.baseFilter(false)) - c.editor.pop() c.editor.merge(pcFiltered) + c.editor.squashLatest() } else { if err := c.editor.SetPointCloud(pcFiltered, cloudMain); err != nil { return err diff --git a/editor.go b/editor.go index be56099f..78bc24aa 100644 --- a/editor.go +++ b/editor.go @@ -36,14 +36,14 @@ func newEditor() *editor { type history interface { MaxHistory() int SetMaxHistory(m int) - push(pp *pc.PointCloud) *pc.PointCloud - pop() *pc.PointCloud - undo() (*pc.PointCloud, bool) + push(p patch) + squashLatest() + undo(pp *pc.PointCloud) (*pc.PointCloud, bool) clear() } func (e *editor) Undo() bool { - pp, ok := e.history.undo() + pp, ok := e.history.undo(e.pp) if ok { e.pp = pp } @@ -114,7 +114,13 @@ func (e *editor) SetPointCloud(pp *pc.PointCloud, id cloudID) error { } switch id { case cloudMain: - e.pp = e.push(pcNew) + if e.pp != nil { + e.push(&replacePatch{ + header: e.pp.PointCloudHeader.Clone(), + data: e.pp.Data, + }) + } + e.pp = pcNew case cloudSub: e.ppSub = pcNew it, err := pcNew.Vec3Iterator() @@ -161,7 +167,11 @@ func (e *editor) label(fn func(int, mat.Vec3) (uint32, bool)) error { itL.Incr() i++ } - e.pp = e.push(pcNew) + e.push(&replacePatch{ + header: e.pp.PointCloudHeader.Clone(), + data: e.pp.Data, + }) + e.pp = pcNew runtime.GC() return nil } @@ -171,7 +181,11 @@ func (e *editor) passThrough(fn func(int, mat.Vec3) bool) error { if err != nil { return err } - e.pp = e.push(pp) + e.push(&replacePatch{ + header: e.pp.PointCloudHeader.Clone(), + data: e.pp.Data, + }) + e.pp = pp runtime.GC() return nil } @@ -181,7 +195,11 @@ func (e *editor) passThroughByMask(sel []uint32, mask, val uint32) error { if err != nil { return err } - e.pp = e.push(pp) + e.push(&replacePatch{ + header: e.pp.PointCloudHeader.Clone(), + data: e.pp.Data, + }) + e.pp = pp runtime.GC() return nil } @@ -214,7 +232,11 @@ func (e *editor) relabelPointsInLabelRange(minLabel, maxLabel, newLabel uint32) lt.SetUint32(newLabel) } - e.pp = e.push(pcNew) + e.push(&replacePatch{ + header: e.pp.PointCloudHeader.Clone(), + data: e.pp.Data, + }) + e.pp = pcNew runtime.GC() return nil } @@ -255,7 +277,11 @@ func (e *editor) unlabelPoints(labelsToKeep []uint32) error { lt.SetUint32(0) } - e.pp = e.push(pcNew) + e.push(&replacePatch{ + header: e.pp.PointCloudHeader.Clone(), + data: e.pp.Data, + }) + e.pp = pcNew runtime.GC() return nil } @@ -356,6 +382,10 @@ func (e *editor) merge(pp *pc.PointCloud) { pcNew.Width = pcNew.Points pcNew.Height = 1 - e.pp = e.push(pcNew) + e.push(&replacePatch{ + header: e.pp.PointCloudHeader.Clone(), + data: e.pp.Data, + }) + e.pp = pcNew runtime.GC() } diff --git a/history_test.go b/history_test.go new file mode 100644 index 00000000..8a4bae7a --- /dev/null +++ b/history_test.go @@ -0,0 +1,124 @@ +package main + +import ( + "math/rand" + "reflect" + "testing" + + "github.com/seqsense/pcgol/mat" + "github.com/seqsense/pcgol/pc" +) + +func snapshotCloud(e *editor) *pc.PointCloud { + return cloneCloud(e.pp) +} + +func applyRandomEdit(t *testing.T, e *editor, rnd *rand.Rand) { + t.Helper() + switch rnd.Intn(5) { + case 0: // label by position + if err := e.label(func(i int, _ mat.Vec3) (uint32, bool) { + return uint32(rnd.Intn(4)), rnd.Intn(2) == 0 + }); err != nil { + t.Fatal(err) + } + case 1: // relabel range + if err := e.relabelPointsInLabelRange(0, uint32(rnd.Intn(3)), uint32(rnd.Intn(4))); err != nil { + t.Fatal(err) + } + case 2: // delete random points + if err := e.passThrough(func(i int, _ mat.Vec3) bool { + return rnd.Intn(4) != 0 + }); err != nil { + t.Fatal(err) + } + case 3: // paste + n := 1 + rnd.Intn(20) + e.merge(makeTestCloud(t, n, n, 1)) + case 4: // whole-cloud replacement + n := 50 + rnd.Intn(100) + if err := e.SetPointCloud(makeTestCloud(t, n, n, 1), cloudMain); err != nil { + t.Fatal(err) + } + } +} + +func TestEditorUndoRoundTrip(t *testing.T) { + for trial := int64(0); trial < 10; trial++ { + rnd := rand.New(rand.NewSource(trial)) + + e := newEditor() + e.SetMaxHistory(100) + if err := e.SetPointCloud(makeTestCloud(t, 200, 20, 10), cloudMain); err != nil { + t.Fatal(err) + } + + const nOps = 8 + snapshots := []*pc.PointCloud{snapshotCloud(e)} + for k := 0; k < nOps; k++ { + applyRandomEdit(t, e, rnd) + snapshots = append(snapshots, snapshotCloud(e)) + } + + for k := nOps; k > 0; k-- { + assertCloudEqual(t, snapshots[k], e.pp) + if !e.Undo() { + t.Fatalf("trial %d: undo %d failed", trial, nOps-k) + } + } + assertCloudEqual(t, snapshots[0], e.pp) + if !reflect.DeepEqual(snapshots[0].PointCloudHeader, e.pp.PointCloudHeader) { + t.Fatalf("trial %d: header mismatch after undoing all edits", trial) + } + + if e.Undo() { + t.Fatalf("trial %d: undo over the initial state must fail", trial) + } + } +} + +func TestHistoryMaxDepth(t *testing.T) { + rnd := rand.New(rand.NewSource(1)) + + e := newEditor() // maxHistoryDefault = 4 + if err := e.SetPointCloud(makeTestCloud(t, 100, 10, 10), cloudMain); err != nil { + t.Fatal(err) + } + + for k := 0; k < 6; k++ { + applyRandomEdit(t, e, rnd) + } + for k := 0; k < maxHistoryDefault; k++ { + if !e.Undo() { + t.Fatalf("undo %d must succeed", k) + } + } + if e.Undo() { + t.Fatal("undo deeper than max_history must fail") + } + + e.SetMaxHistory(0) + applyRandomEdit(t, e, rnd) + if e.Undo() { + t.Fatal("undo with max_history=0 must fail") + } +} + +func TestHistorySquashLatest(t *testing.T) { + e := newEditor() + if err := e.SetPointCloud(makeTestCloud(t, 100, 10, 10), cloudMain); err != nil { + t.Fatal(err) + } + orig := snapshotCloud(e) + + if err := e.passThrough(func(i int, _ mat.Vec3) bool { return i%2 == 0 }); err != nil { + t.Fatal(err) + } + e.merge(makeTestCloud(t, 10, 10, 1)) + e.squashLatest() + + if !e.Undo() { + t.Fatal("undo failed") + } + assertCloudEqual(t, orig, e.pp) +} diff --git a/patch.go b/patch.go new file mode 100644 index 00000000..1a6e41fd --- /dev/null +++ b/patch.go @@ -0,0 +1,191 @@ +package main + +import ( + "bytes" + "encoding/binary" + "errors" + "math" + + "github.com/seqsense/pcgol/pc" +) + +type patch interface { + // pp may be mutated; use the returned cloud + revert(pp *pc.PointCloud) (*pc.PointCloud, error) + encode(buf *bytes.Buffer) +} + +const ( + patchTypeLabel = iota + 1 + patchTypeDelete + patchTypeAppend + patchTypeReplace +) + +var ( + errBrokenPatch = errors.New("broken patch data") + errUnknownPatchType = errors.New("unknown patch type") +) + +type replacePatch struct { + header pc.PointCloudHeader + data []byte +} + +func (p *replacePatch) revert(_ *pc.PointCloud) (*pc.PointCloud, error) { + return &pc.PointCloud{ + PointCloudHeader: p.header, + Points: p.header.Width * p.header.Height, + Data: p.data, + }, nil +} + +func (p *replacePatch) encode(buf *bytes.Buffer) { + buf.WriteByte(patchTypeReplace) + writeUint32(buf, math.Float32bits(p.header.Version)) + writeUint32(buf, uint32(len(p.header.Fields))) + for i := range p.header.Fields { + writeString(buf, p.header.Fields[i]) + writeUint32(buf, uint32(p.header.Size[i])) + writeString(buf, p.header.Type[i]) + writeUint32(buf, uint32(p.header.Count[i])) + } + writeUint32(buf, uint32(p.header.Width)) + writeUint32(buf, uint32(p.header.Height)) + writeUint32(buf, uint32(len(p.header.Viewpoint))) + for _, v := range p.header.Viewpoint { + writeUint32(buf, math.Float32bits(v)) + } + writeUint32(buf, uint32(len(p.data))) + buf.Write(p.data) +} + +func encodePatches(buf *bytes.Buffer, ps []patch) { + for _, p := range ps { + p.encode(buf) + } +} + +// Decoded patches may reference b; do not reuse it afterwards +func decodePatches(b []byte) ([]patch, error) { + var ps []patch + for len(b) > 0 { + p, rest, err := decodePatch(b) + if err != nil { + return nil, err + } + ps = append(ps, p) + b = rest + } + return ps, nil +} + +func decodePatch(b []byte) (patch, []byte, error) { + if len(b) < 1 { + return nil, nil, errBrokenPatch + } + typ := b[0] + r := reader{b: b[1:]} + switch typ { + case patchTypeReplace: + p := &replacePatch{} + p.header.Version = math.Float32frombits(r.uint32()) + nFields := int(r.uint32()) + if r.err != nil || nFields < 0 || nFields > len(r.b) { + return nil, nil, errBrokenPatch + } + p.header.Fields = make([]string, nFields) + p.header.Size = make([]int, nFields) + p.header.Type = make([]string, nFields) + p.header.Count = make([]int, nFields) + for i := 0; i < nFields; i++ { + p.header.Fields[i] = r.string() + p.header.Size[i] = int(r.uint32()) + p.header.Type[i] = r.string() + p.header.Count[i] = int(r.uint32()) + } + p.header.Width = int(r.uint32()) + p.header.Height = int(r.uint32()) + nvp := int(r.uint32()) + if r.err != nil || nvp < 0 || nvp*4 > len(r.b) { + return nil, nil, errBrokenPatch + } + p.header.Viewpoint = make([]float32, nvp) + for i := range p.header.Viewpoint { + p.header.Viewpoint[i] = math.Float32frombits(r.uint32()) + } + p.data = r.bytes(int(r.uint32())) + if r.err != nil { + return nil, nil, r.err + } + return p, r.b, nil + } + return nil, nil, errUnknownPatchType +} + +func revertChunks(pp *pc.PointCloud, chunks [][]byte) (*pc.PointCloud, error) { + for i := len(chunks) - 1; i >= 0; i-- { + ps, err := decodePatches(chunks[i]) + if err != nil { + return nil, err + } + for j := len(ps) - 1; j >= 0; j-- { + if pp, err = ps[j].revert(pp); err != nil { + return nil, err + } + } + } + return pp, nil +} + +func packPatch(p patch) []byte { + var buf bytes.Buffer + p.encode(&buf) + return buf.Bytes() +} + +func writeUint32(buf *bytes.Buffer, v uint32) { + var b [4]byte + binary.LittleEndian.PutUint32(b[:], v) + buf.Write(b[:]) +} + +func writeString(buf *bytes.Buffer, s string) { + writeUint32(buf, uint32(len(s))) + buf.WriteString(s) +} + +type reader struct { + b []byte + err error +} + +func (r *reader) uint32() uint32 { + if r.err != nil { + return 0 + } + if len(r.b) < 4 { + r.err = errBrokenPatch + return 0 + } + v := binary.LittleEndian.Uint32(r.b) + r.b = r.b[4:] + return v +} + +func (r *reader) bytes(n int) []byte { + if r.err != nil { + return nil + } + if n < 0 || len(r.b) < n { + r.err = errBrokenPatch + return nil + } + b := r.b[:n] + r.b = r.b[n:] + return b +} + +func (r *reader) string() string { + return string(r.bytes(int(r.uint32()))) +} diff --git a/patch_test.go b/patch_test.go new file mode 100644 index 00000000..1d95637e --- /dev/null +++ b/patch_test.go @@ -0,0 +1,95 @@ +package main + +import ( + "bytes" + "math/rand" + "reflect" + "testing" + + "github.com/seqsense/pcgol/pc" +) + +func makeTestCloud(t *testing.T, n, width, height int) *pc.PointCloud { + t.Helper() + pp := &pc.PointCloud{ + PointCloudHeader: pc.PointCloudHeader{ + Version: 0.7, + Fields: []string{"x", "y", "z", "label"}, + Size: []int{4, 4, 4, 4}, + Type: []string{"F", "F", "F", "U"}, + Count: []int{1, 1, 1, 1}, + Width: width, + Height: height, + }, + Points: n, + } + pp.Data = make([]byte, n*pp.Stride()) + rnd := rand.New(rand.NewSource(int64(n))) + rnd.Read(pp.Data) + return pp +} + +func cloneCloud(pp *pc.PointCloud) *pc.PointCloud { + out := &pc.PointCloud{ + PointCloudHeader: pp.PointCloudHeader.Clone(), + Points: pp.Points, + Data: append([]byte{}, pp.Data...), + } + return out +} + +func assertCloudEqual(t *testing.T, expected, got *pc.PointCloud) { + t.Helper() + if expected.Points != got.Points { + t.Fatalf("Points: expected %d, got %d", expected.Points, got.Points) + } + if expected.Width != got.Width || expected.Height != got.Height { + t.Fatalf("Size: expected %dx%d, got %dx%d", + expected.Width, expected.Height, got.Width, got.Height) + } + if !bytes.Equal(expected.Data, got.Data) { + t.Fatal("Data mismatch after revert") + } +} + +func TestReplacePatchRevert(t *testing.T) { + orig := makeTestCloud(t, 100, 10, 10) + orig.Viewpoint = []float32{0, 0, 0, 1, 0, 0, 0} + pp := makeTestCloud(t, 5, 5, 1) + + p := &replacePatch{ + header: orig.PointCloudHeader.Clone(), + data: append([]byte{}, orig.Data...), + } + out, err := p.revert(pp) + if err != nil { + t.Fatal(err) + } + assertCloudEqual(t, orig, out) + if !reflect.DeepEqual(orig.PointCloudHeader, out.PointCloudHeader) { + t.Fatalf("Header: expected %+v, got %+v", orig.PointCloudHeader, out.PointCloudHeader) + } +} + +func TestPatchEncodeDecodeRoundTrip(t *testing.T) { + orig := makeTestCloud(t, 100, 10, 10) + orig.Viewpoint = []float32{1, 2, 3, 1, 0, 0, 0} + patches := []patch{ + &replacePatch{header: orig.PointCloudHeader.Clone(), data: orig.Data}, + } + + var buf bytes.Buffer + encodePatches(&buf, patches) + decoded, err := decodePatches(buf.Bytes()) + if err != nil { + t.Fatal(err) + } + if len(decoded) != len(patches) { + t.Fatalf("Expected %d patches, got %d", len(patches), len(decoded)) + } + for i := range patches { + if !reflect.DeepEqual(patches[i], decoded[i]) { + t.Errorf("Patch %d: expected %+v, got %+v", i, patches[i], decoded[i]) + } + } +} diff --git a/undo.go b/undo.go index 4db1bc44..6f72f721 100644 --- a/undo.go +++ b/undo.go @@ -1,3 +1,4 @@ +//go:build !js // +build !js package main @@ -6,35 +7,59 @@ import ( "github.com/seqsense/pcgol/pc" ) -// historyDummy is a dummy history implementation for testing. -type historyDummy struct { - latest *pc.PointCloud +type historyMem struct { + // entries[i] is a list of packed patch chunks forming one undo step + entries [][][]byte + maxHistory int } -func newHistory(_ int) history { - return &historyDummy{} +func newHistory(n int) history { + return &historyMem{maxHistory: n} } -func (historyDummy) MaxHistory() int { - return 0 +func (h *historyMem) MaxHistory() int { + return h.maxHistory } -func (historyDummy) SetMaxHistory(_ int) { +func (h *historyMem) SetMaxHistory(m int) { + if m < 0 { + m = 0 + } + h.maxHistory = m } -func (h *historyDummy) push(pp *pc.PointCloud) *pc.PointCloud { - h.latest = pp - return pp +func (h *historyMem) push(p patch) { + h.entries = append(h.entries, [][]byte{packPatch(p)}) + for len(h.entries) > h.maxHistory { + h.entries[0] = nil + h.entries = h.entries[1:] + } } -func (h *historyDummy) pop() *pc.PointCloud { - return h.latest +func (h *historyMem) squashLatest() { + if n := len(h.entries); n >= 2 { + h.entries[n-2] = append(h.entries[n-2], h.entries[n-1]...) + h.entries[n-1] = nil + h.entries = h.entries[:n-1] + } } -func (historyDummy) undo() (*pc.PointCloud, bool) { - return nil, false +func (h *historyMem) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { + n := len(h.entries) + if n == 0 { + return nil, false + } + entry := h.entries[n-1] + h.entries[n-1] = nil + h.entries = h.entries[:n-1] + + out, err := revertChunks(pp, entry) + if err != nil { + return nil, false + } + return out, true } -func (h *historyDummy) clear() { - h.latest = nil +func (h *historyMem) clear() { + h.entries = nil } diff --git a/undo_js.go b/undo_js.go index 482ea3c8..314e977d 100644 --- a/undo_js.go +++ b/undo_js.go @@ -6,10 +6,12 @@ import ( "github.com/seqsense/pcgol/pc" ) +// historyJS stores entries as JS Uint8Arrays to keep them out of the WASM +// linear memory, which never shrinks. type historyJS struct { - history []js.Value - historyHeader []pc.PointCloudHeader - maxHistory int + // entries[i] is a list of packed patch chunks forming one undo step + entries [][]js.Value + maxHistory int } func newHistory(n int) history { @@ -27,53 +29,47 @@ func (h *historyJS) SetMaxHistory(m int) { h.maxHistory = m } -func (h *historyJS) push(pp *pc.PointCloud) *pc.PointCloud { - header := pp.PointCloudHeader.Clone() - dataJS := js.Global().Get("Uint8Array").New(len(pp.Data)) - js.CopyBytesToJS(dataJS, pp.Data) - h.history = append(h.history, dataJS) - h.historyHeader = append(h.historyHeader, header) - if len(h.history) > h.MaxHistory()+1 { - h.history[0] = js.Null() - h.history = h.history[1:] - h.historyHeader = h.historyHeader[1:] +func (h *historyJS) push(p patch) { + packed := packPatch(p) + chunk := js.Global().Get("Uint8Array").New(len(packed)) + js.CopyBytesToJS(chunk, packed) + h.entries = append(h.entries, []js.Value{chunk}) + for len(h.entries) > h.maxHistory { + h.entries[0] = nil + h.entries = h.entries[1:] } - return pp } -func (h *historyJS) pop() *pc.PointCloud { - n := len(h.history) - back := h.history[n-1] - backHeader := h.historyHeader[n-1] - h.history[n-1] = js.Null() - h.history = h.history[:n-1] - h.historyHeader = h.historyHeader[:n-1] - - return h.reconstructPointCloud(backHeader, back) +func (h *historyJS) squashLatest() { + if n := len(h.entries); n >= 2 { + h.entries[n-2] = append(h.entries[n-2], h.entries[n-1]...) + h.entries[n-1] = nil + h.entries = h.entries[:n-1] + } } -func (h *historyJS) undo() (*pc.PointCloud, bool) { - if n := len(h.history); n > 1 { - h.history[n-1] = js.Null() - h.history = h.history[:n-1] - h.historyHeader = h.historyHeader[:n-1] - - return h.reconstructPointCloud(h.historyHeader[n-2], h.history[n-2]), true +func (h *historyJS) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { + n := len(h.entries) + if n == 0 { + return nil, false } - return nil, false -} + entry := h.entries[n-1] + h.entries[n-1] = nil + h.entries = h.entries[:n-1] -func (h *historyJS) reconstructPointCloud(header pc.PointCloudHeader, dataJS js.Value) *pc.PointCloud { - pp := &pc.PointCloud{ - PointCloudHeader: header, - Points: header.Width * header.Height, - Data: make([]byte, dataJS.Get("byteLength").Int()), + chunks := make([][]byte, len(entry)) + for i, c := range entry { + b := make([]byte, c.Get("byteLength").Int()) + js.CopyBytesToGo(b, c) + chunks[i] = b + } + out, err := revertChunks(pp, chunks) + if err != nil { + return nil, false } - js.CopyBytesToGo(pp.Data, dataJS) - return pp + return out, true } func (h *historyJS) clear() { - h.history = nil - h.historyHeader = nil + h.entries = nil } From bf9227e167324ffe8c2838687846b9d3084d354f Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sat, 22 Aug 2026 21:27:41 +0900 Subject: [PATCH 02/13] Pop the undo entry only after a successful revert A failed revert used to discard the entry; a later undo would then apply an older patch to a state it was not recorded against. Keep the history intact and block undo at the broken entry instead. Co-Authored-By: Claude Fable 5 --- history_test.go | 13 +++++++++++++ undo.go | 8 +++----- undo_js.go | 5 ++--- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/history_test.go b/history_test.go index 8a4bae7a..74e9b12c 100644 --- a/history_test.go +++ b/history_test.go @@ -122,3 +122,16 @@ func TestHistorySquashLatest(t *testing.T) { } assertCloudEqual(t, orig, e.pp) } + +func TestHistoryUndoKeepsEntryOnError(t *testing.T) { + h := &historyMem{ + maxHistory: 4, + entries: [][][]byte{{{0xFF}}}, // broken patch data + } + if _, ok := h.undo(nil); ok { + t.Fatal("undo of a broken entry must fail") + } + if len(h.entries) != 1 { + t.Fatal("a broken entry must not be dropped; a later undo would apply an older patch to a mismatched state") + } +} diff --git a/undo.go b/undo.go index 6f72f721..8b931ef8 100644 --- a/undo.go +++ b/undo.go @@ -49,14 +49,12 @@ func (h *historyMem) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { if n == 0 { return nil, false } - entry := h.entries[n-1] - h.entries[n-1] = nil - h.entries = h.entries[:n-1] - - out, err := revertChunks(pp, entry) + out, err := revertChunks(pp, h.entries[n-1]) if err != nil { return nil, false } + h.entries[n-1] = nil + h.entries = h.entries[:n-1] return out, true } diff --git a/undo_js.go b/undo_js.go index 314e977d..fc9fb4d8 100644 --- a/undo_js.go +++ b/undo_js.go @@ -54,9 +54,6 @@ func (h *historyJS) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { return nil, false } entry := h.entries[n-1] - h.entries[n-1] = nil - h.entries = h.entries[:n-1] - chunks := make([][]byte, len(entry)) for i, c := range entry { b := make([]byte, c.Get("byteLength").Int()) @@ -67,6 +64,8 @@ func (h *historyJS) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { if err != nil { return nil, false } + h.entries[n-1] = nil + h.entries = h.entries[:n-1] return out, true } From 42832bc13e95c69f7a5e2dd1e8cda8f8ece71704 Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sat, 22 Aug 2026 21:28:05 +0900 Subject: [PATCH 03/13] Copy packed patches into exact-sized slices buf.Bytes() retains the grown capacity of the buffer, which can be nearly twice the content size and is held long-term by historyMem. Co-Authored-By: Claude Fable 5 --- patch.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/patch.go b/patch.go index 1a6e41fd..0b64f23b 100644 --- a/patch.go +++ b/patch.go @@ -141,7 +141,9 @@ func revertChunks(pp *pc.PointCloud, chunks [][]byte) (*pc.PointCloud, error) { func packPatch(p patch) []byte { var buf bytes.Buffer p.encode(&buf) - return buf.Bytes() + packed := make([]byte, buf.Len()) + copy(packed, buf.Bytes()) + return packed } func writeUint32(buf *bytes.Buffer, v uint32) { From 87504524dd0f6d31711d2e77ceb386a94d8a0fe7 Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sat, 22 Aug 2026 21:39:35 +0900 Subject: [PATCH 04/13] Move the historyMem test into a non-js test file historyMem exists only in the non-js build; go vet for GOOS=js compiles test files too and failed on the reference. Co-Authored-By: Claude Fable 5 --- history_test.go | 13 ------------- undo_test.go | 21 +++++++++++++++++++++ 2 files changed, 21 insertions(+), 13 deletions(-) create mode 100644 undo_test.go diff --git a/history_test.go b/history_test.go index 74e9b12c..8a4bae7a 100644 --- a/history_test.go +++ b/history_test.go @@ -122,16 +122,3 @@ func TestHistorySquashLatest(t *testing.T) { } assertCloudEqual(t, orig, e.pp) } - -func TestHistoryUndoKeepsEntryOnError(t *testing.T) { - h := &historyMem{ - maxHistory: 4, - entries: [][][]byte{{{0xFF}}}, // broken patch data - } - if _, ok := h.undo(nil); ok { - t.Fatal("undo of a broken entry must fail") - } - if len(h.entries) != 1 { - t.Fatal("a broken entry must not be dropped; a later undo would apply an older patch to a mismatched state") - } -} diff --git a/undo_test.go b/undo_test.go new file mode 100644 index 00000000..e9a05f36 --- /dev/null +++ b/undo_test.go @@ -0,0 +1,21 @@ +//go:build !js +// +build !js + +package main + +import ( + "testing" +) + +func TestHistoryUndoKeepsEntryOnError(t *testing.T) { + h := &historyMem{ + maxHistory: 4, + entries: [][][]byte{{{0xFF}}}, // broken patch data + } + if _, ok := h.undo(nil); ok { + t.Fatal("undo of a broken entry must fail") + } + if len(h.entries) != 1 { + t.Fatal("a broken entry must not be dropped; a later undo would apply an older patch to a mismatched state") + } +} From 7c8418965114ee7a107e10660bd9d0d8d6200fdf Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sat, 22 Aug 2026 21:52:19 +0900 Subject: [PATCH 05/13] Tighten decode bounds for field and viewpoint counts Bound nFields by the minimal encoded field size so corrupted counts fail before allocating, and rewrite the viewpoint bound in the same multiplication-free form as the other guards. Co-Authored-By: Claude Fable 5 --- patch.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/patch.go b/patch.go index 0b64f23b..7226457c 100644 --- a/patch.go +++ b/patch.go @@ -91,7 +91,8 @@ func decodePatch(b []byte) (patch, []byte, error) { p := &replacePatch{} p.header.Version = math.Float32frombits(r.uint32()) nFields := int(r.uint32()) - if r.err != nil || nFields < 0 || nFields > len(r.b) { + // A field encodes to at least 16 bytes + if r.err != nil || nFields < 0 || nFields > len(r.b)/16 { return nil, nil, errBrokenPatch } p.header.Fields = make([]string, nFields) @@ -107,7 +108,7 @@ func decodePatch(b []byte) (patch, []byte, error) { p.header.Width = int(r.uint32()) p.header.Height = int(r.uint32()) nvp := int(r.uint32()) - if r.err != nil || nvp < 0 || nvp*4 > len(r.b) { + if r.err != nil || nvp < 0 || nvp > len(r.b)/4 { return nil, nil, errBrokenPatch } p.header.Viewpoint = make([]float32, nvp) From 04661d160a4e5e83dc6550330eebfca60ed6ec48 Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sat, 22 Aug 2026 21:53:36 +0900 Subject: [PATCH 06/13] Assemble history chunks on the JS heap without a WASM-side copy Pushing a replacePatch serialized the whole cloud into a Go buffer before copying it to the JS heap, transiently holding extra full-size copies in the WASM linear memory, which never shrinks. Split the patch wire form into a head and a raw payload (encodeHead/payload) and copy both straight into one Uint8Array, restoring the memory behavior of the previous direct-copy implementation for snapshots. Co-Authored-By: Claude Fable 5 --- patch.go | 20 ++++++++++++++------ undo_js.go | 10 +++++++--- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/patch.go b/patch.go index 7226457c..72895de1 100644 --- a/patch.go +++ b/patch.go @@ -12,7 +12,9 @@ import ( type patch interface { // pp may be mutated; use the returned cloud revert(pp *pc.PointCloud) (*pc.PointCloud, error) - encode(buf *bytes.Buffer) + // The wire form is the head followed by the raw payload + encodeHead(buf *bytes.Buffer) + payload() []byte } const ( @@ -40,7 +42,7 @@ func (p *replacePatch) revert(_ *pc.PointCloud) (*pc.PointCloud, error) { }, nil } -func (p *replacePatch) encode(buf *bytes.Buffer) { +func (p *replacePatch) encodeHead(buf *bytes.Buffer) { buf.WriteByte(patchTypeReplace) writeUint32(buf, math.Float32bits(p.header.Version)) writeUint32(buf, uint32(len(p.header.Fields))) @@ -57,12 +59,16 @@ func (p *replacePatch) encode(buf *bytes.Buffer) { writeUint32(buf, math.Float32bits(v)) } writeUint32(buf, uint32(len(p.data))) - buf.Write(p.data) +} + +func (p *replacePatch) payload() []byte { + return p.data } func encodePatches(buf *bytes.Buffer, ps []patch) { for _, p := range ps { - p.encode(buf) + p.encodeHead(buf) + buf.Write(p.payload()) } } @@ -141,9 +147,11 @@ func revertChunks(pp *pc.PointCloud, chunks [][]byte) (*pc.PointCloud, error) { func packPatch(p patch) []byte { var buf bytes.Buffer - p.encode(&buf) - packed := make([]byte, buf.Len()) + p.encodeHead(&buf) + data := p.payload() + packed := make([]byte, buf.Len()+len(data)) copy(packed, buf.Bytes()) + copy(packed[buf.Len():], data) return packed } diff --git a/undo_js.go b/undo_js.go index fc9fb4d8..fae3da9f 100644 --- a/undo_js.go +++ b/undo_js.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "syscall/js" "github.com/seqsense/pcgol/pc" @@ -30,9 +31,12 @@ func (h *historyJS) SetMaxHistory(m int) { } func (h *historyJS) push(p patch) { - packed := packPatch(p) - chunk := js.Global().Get("Uint8Array").New(len(packed)) - js.CopyBytesToJS(chunk, packed) + var head bytes.Buffer + p.encodeHead(&head) + data := p.payload() + chunk := js.Global().Get("Uint8Array").New(head.Len() + len(data)) + js.CopyBytesToJS(chunk, head.Bytes()) + js.CopyBytesToJS(chunk.Call("subarray", head.Len()), data) h.entries = append(h.entries, []js.Value{chunk}) for len(h.entries) > h.maxHistory { h.entries[0] = nil From cd21eec88105461d4adf8b232100b52bbd9bffa4 Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sun, 6 Sep 2026 14:59:37 +0900 Subject: [PATCH 07/13] Unify history implementations over a record storage abstraction Co-Authored-By: Claude Fable 5 --- editor.go | 11 +------ history.go | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++ patch.go | 25 ---------------- undo.go | 64 ++++++++++------------------------------ undo_js.go | 82 +++++++++++++--------------------------------------- undo_test.go | 14 ++++----- 6 files changed, 124 insertions(+), 154 deletions(-) create mode 100644 history.go diff --git a/editor.go b/editor.go index 78bc24aa..390c9dac 100644 --- a/editor.go +++ b/editor.go @@ -12,7 +12,7 @@ const ( ) type editor struct { - history + *history pp *pc.PointCloud ppSub *pc.PointCloud ppSubRect rect @@ -33,15 +33,6 @@ func newEditor() *editor { } } -type history interface { - MaxHistory() int - SetMaxHistory(m int) - push(p patch) - squashLatest() - undo(pp *pc.PointCloud) (*pc.PointCloud, bool) - clear() -} - func (e *editor) Undo() bool { pp, ok := e.history.undo(e.pp) if ok { diff --git a/history.go b/history.go new file mode 100644 index 00000000..3f3546fc --- /dev/null +++ b/history.go @@ -0,0 +1,82 @@ +package main + +import ( + "bytes" + + "github.com/seqsense/pcgol/pc" +) + +type recordStore interface { + store(parts ...[]byte) record +} + +type record interface { + load() []byte +} + +type undoStep []record + +type history struct { + store recordStore + steps []undoStep + maxHistory int +} + +func (h *history) MaxHistory() int { + return h.maxHistory +} + +func (h *history) SetMaxHistory(m int) { + if m < 0 { + m = 0 + } + h.maxHistory = m +} + +func (h *history) push(p patch) { + var head bytes.Buffer + p.encodeHead(&head) + h.steps = append(h.steps, undoStep{h.store.store(head.Bytes(), p.payload())}) + for len(h.steps) > h.maxHistory { + h.steps[0] = nil + h.steps = h.steps[1:] + } +} + +func (h *history) squashLatest() { + if n := len(h.steps); n >= 2 { + h.steps[n-2] = append(h.steps[n-2], h.steps[n-1]...) + h.steps[n-1] = nil + h.steps = h.steps[:n-1] + } +} + +func (h *history) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { + n := len(h.steps) + if n == 0 { + return nil, false + } + step := h.steps[n-1] + records := make([][]byte, len(step)) + for i, r := range step { + records[i] = r.load() + } + for i := len(records) - 1; i >= 0; i-- { + ps, err := decodePatches(records[i]) + if err != nil { + return nil, false + } + for j := len(ps) - 1; j >= 0; j-- { + if pp, err = ps[j].revert(pp); err != nil { + return nil, false + } + } + } + h.steps[n-1] = nil + h.steps = h.steps[:n-1] + return pp, true +} + +func (h *history) clear() { + h.steps = nil +} diff --git a/patch.go b/patch.go index 72895de1..7988cfd7 100644 --- a/patch.go +++ b/patch.go @@ -130,31 +130,6 @@ func decodePatch(b []byte) (patch, []byte, error) { return nil, nil, errUnknownPatchType } -func revertChunks(pp *pc.PointCloud, chunks [][]byte) (*pc.PointCloud, error) { - for i := len(chunks) - 1; i >= 0; i-- { - ps, err := decodePatches(chunks[i]) - if err != nil { - return nil, err - } - for j := len(ps) - 1; j >= 0; j-- { - if pp, err = ps[j].revert(pp); err != nil { - return nil, err - } - } - } - return pp, nil -} - -func packPatch(p patch) []byte { - var buf bytes.Buffer - p.encodeHead(&buf) - data := p.payload() - packed := make([]byte, buf.Len()+len(data)) - copy(packed, buf.Bytes()) - copy(packed[buf.Len():], data) - return packed -} - func writeUint32(buf *bytes.Buffer, v uint32) { var b [4]byte binary.LittleEndian.PutUint32(b[:], v) diff --git a/undo.go b/undo.go index 8b931ef8..92a2d357 100644 --- a/undo.go +++ b/undo.go @@ -3,61 +3,27 @@ package main -import ( - "github.com/seqsense/pcgol/pc" -) - -type historyMem struct { - // entries[i] is a list of packed patch chunks forming one undo step - entries [][][]byte - maxHistory int -} - -func newHistory(n int) history { - return &historyMem{maxHistory: n} +func newHistory(n int) *history { + return &history{store: memStore{}, maxHistory: n} } -func (h *historyMem) MaxHistory() int { - return h.maxHistory -} - -func (h *historyMem) SetMaxHistory(m int) { - if m < 0 { - m = 0 - } - h.maxHistory = m -} +// memStore keeps records on the Go heap. +type memStore struct{} -func (h *historyMem) push(p patch) { - h.entries = append(h.entries, [][]byte{packPatch(p)}) - for len(h.entries) > h.maxHistory { - h.entries[0] = nil - h.entries = h.entries[1:] +func (memStore) store(parts ...[]byte) record { + var total int + for _, d := range parts { + total += len(d) } -} - -func (h *historyMem) squashLatest() { - if n := len(h.entries); n >= 2 { - h.entries[n-2] = append(h.entries[n-2], h.entries[n-1]...) - h.entries[n-1] = nil - h.entries = h.entries[:n-1] + b := make([]byte, 0, total) + for _, d := range parts { + b = append(b, d...) } + return memRecord(b) } -func (h *historyMem) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { - n := len(h.entries) - if n == 0 { - return nil, false - } - out, err := revertChunks(pp, h.entries[n-1]) - if err != nil { - return nil, false - } - h.entries[n-1] = nil - h.entries = h.entries[:n-1] - return out, true -} +type memRecord []byte -func (h *historyMem) clear() { - h.entries = nil +func (r memRecord) load() []byte { + return r } diff --git a/undo_js.go b/undo_js.go index fae3da9f..621296e0 100644 --- a/undo_js.go +++ b/undo_js.go @@ -1,78 +1,36 @@ package main import ( - "bytes" "syscall/js" - - "github.com/seqsense/pcgol/pc" ) -// historyJS stores entries as JS Uint8Arrays to keep them out of the WASM -// linear memory, which never shrinks. -type historyJS struct { - // entries[i] is a list of packed patch chunks forming one undo step - entries [][]js.Value - maxHistory int -} - -func newHistory(n int) history { - return &historyJS{maxHistory: n} +func newHistory(n int) *history { + return &history{store: jsStore{}, maxHistory: n} } -func (h *historyJS) MaxHistory() int { - return h.maxHistory -} - -func (h *historyJS) SetMaxHistory(m int) { - if m < 0 { - m = 0 - } - h.maxHistory = m -} +// jsStore keeps records on the JS heap to avoid growing the WASM linear memory. +type jsStore struct{} -func (h *historyJS) push(p patch) { - var head bytes.Buffer - p.encodeHead(&head) - data := p.payload() - chunk := js.Global().Get("Uint8Array").New(head.Len() + len(data)) - js.CopyBytesToJS(chunk, head.Bytes()) - js.CopyBytesToJS(chunk.Call("subarray", head.Len()), data) - h.entries = append(h.entries, []js.Value{chunk}) - for len(h.entries) > h.maxHistory { - h.entries[0] = nil - h.entries = h.entries[1:] +func (jsStore) store(parts ...[]byte) record { + var total int + for _, d := range parts { + total += len(d) } -} - -func (h *historyJS) squashLatest() { - if n := len(h.entries); n >= 2 { - h.entries[n-2] = append(h.entries[n-2], h.entries[n-1]...) - h.entries[n-1] = nil - h.entries = h.entries[:n-1] + v := js.Global().Get("Uint8Array").New(total) + var off int + for _, d := range parts { + js.CopyBytesToJS(v.Call("subarray", off), d) + off += len(d) } + return jsRecord{v} } -func (h *historyJS) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { - n := len(h.entries) - if n == 0 { - return nil, false - } - entry := h.entries[n-1] - chunks := make([][]byte, len(entry)) - for i, c := range entry { - b := make([]byte, c.Get("byteLength").Int()) - js.CopyBytesToGo(b, c) - chunks[i] = b - } - out, err := revertChunks(pp, chunks) - if err != nil { - return nil, false - } - h.entries[n-1] = nil - h.entries = h.entries[:n-1] - return out, true +type jsRecord struct { + v js.Value } -func (h *historyJS) clear() { - h.entries = nil +func (r jsRecord) load() []byte { + b := make([]byte, r.v.Get("byteLength").Int()) + js.CopyBytesToGo(b, r.v) + return b } diff --git a/undo_test.go b/undo_test.go index e9a05f36..607821cc 100644 --- a/undo_test.go +++ b/undo_test.go @@ -7,15 +7,13 @@ import ( "testing" ) -func TestHistoryUndoKeepsEntryOnError(t *testing.T) { - h := &historyMem{ - maxHistory: 4, - entries: [][][]byte{{{0xFF}}}, // broken patch data - } +func TestHistoryUndoKeepsStepOnError(t *testing.T) { + h := newHistory(4) + h.steps = []undoStep{{memRecord{0xFF}}} // broken patch data if _, ok := h.undo(nil); ok { - t.Fatal("undo of a broken entry must fail") + t.Fatal("undo of a broken step must fail") } - if len(h.entries) != 1 { - t.Fatal("a broken entry must not be dropped; a later undo would apply an older patch to a mismatched state") + if len(h.steps) != 1 { + t.Fatal("a broken step must not be dropped; a later undo would apply an older patch to a mismatched state") } } From 5871493d3f4894ce08bc4ff8716fd28af304af7e Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sun, 6 Sep 2026 14:59:53 +0900 Subject: [PATCH 08/13] Load and apply history records one by one on undo Co-Authored-By: Claude Fable 5 --- history.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/history.go b/history.go index 3f3546fc..547fe4f5 100644 --- a/history.go +++ b/history.go @@ -57,12 +57,8 @@ func (h *history) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { return nil, false } step := h.steps[n-1] - records := make([][]byte, len(step)) - for i, r := range step { - records[i] = r.load() - } - for i := len(records) - 1; i >= 0; i-- { - ps, err := decodePatches(records[i]) + for i := len(step) - 1; i >= 0; i-- { + ps, err := decodePatches(step[i].load()) if err != nil { return nil, false } From a13934f32bcc4af12117697d91167a3a1be9177a Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sun, 6 Sep 2026 16:26:42 +0900 Subject: [PATCH 09/13] Propagate errors from history.push and editor.merge Co-Authored-By: Claude Fable 5 --- command.go | 12 +++++++++--- editor.go | 45 ++++++++++++++++++++++++++++++--------------- history.go | 3 ++- history_test.go | 8 ++++++-- 4 files changed, 47 insertions(+), 21 deletions(-) diff --git a/command.go b/command.go index 725ff6f3..fc485734 100644 --- a/command.go +++ b/command.go @@ -583,7 +583,9 @@ func (c *commandContext) AddSurface(resolution float32) bool { it.Incr() } } - c.editor.merge(pcNew) + if err := c.editor.merge(pcNew); err != nil { + return false + } c.setPointCloudUpdated() return true } @@ -631,7 +633,9 @@ func (c *commandContext) VoxelFilter(resolution float32) error { if selected { c.editor.passThrough(c.baseFilter(false)) - c.editor.merge(pcFiltered) + if err := c.editor.merge(pcFiltered); err != nil { + return err + } c.editor.squashLatest() } else { if err := c.editor.SetPointCloud(pcFiltered, cloudMain); err != nil { @@ -731,7 +735,9 @@ func (c *commandContext) FinalizeCurrentMode() error { for ; it.IsValid(); it.Incr() { it.SetVec3(trans.Transform(it.Vec3())) } - c.editor.merge(c.editor.ppSub) + if err := c.editor.merge(c.editor.ppSub); err != nil { + return err + } c.setPointCloudUpdated() c.UnsetCursors() } diff --git a/editor.go b/editor.go index 390c9dac..5e6ba6c4 100644 --- a/editor.go +++ b/editor.go @@ -106,10 +106,12 @@ func (e *editor) SetPointCloud(pp *pc.PointCloud, id cloudID) error { switch id { case cloudMain: if e.pp != nil { - e.push(&replacePatch{ + if err := e.push(&replacePatch{ header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, - }) + }); err != nil { + return err + } } e.pp = pcNew case cloudSub: @@ -158,10 +160,12 @@ func (e *editor) label(fn func(int, mat.Vec3) (uint32, bool)) error { itL.Incr() i++ } - e.push(&replacePatch{ + if err := e.push(&replacePatch{ header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, - }) + }); err != nil { + return err + } e.pp = pcNew runtime.GC() return nil @@ -172,10 +176,12 @@ func (e *editor) passThrough(fn func(int, mat.Vec3) bool) error { if err != nil { return err } - e.push(&replacePatch{ + if err := e.push(&replacePatch{ header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, - }) + }); err != nil { + return err + } e.pp = pp runtime.GC() return nil @@ -186,10 +192,12 @@ func (e *editor) passThroughByMask(sel []uint32, mask, val uint32) error { if err != nil { return err } - e.push(&replacePatch{ + if err := e.push(&replacePatch{ header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, - }) + }); err != nil { + return err + } e.pp = pp runtime.GC() return nil @@ -223,10 +231,12 @@ func (e *editor) relabelPointsInLabelRange(minLabel, maxLabel, newLabel uint32) lt.SetUint32(newLabel) } - e.push(&replacePatch{ + if err := e.push(&replacePatch{ header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, - }) + }); err != nil { + return err + } e.pp = pcNew runtime.GC() return nil @@ -268,10 +278,12 @@ func (e *editor) unlabelPoints(labelsToKeep []uint32) error { lt.SetUint32(0) } - e.push(&replacePatch{ + if err := e.push(&replacePatch{ header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, - }) + }); err != nil { + return err + } e.pp = pcNew runtime.GC() return nil @@ -364,7 +376,7 @@ func passThroughImpl(pp *pc.PointCloud, core func(_, _ *pc.PointCloud) int) (*pc return pcNew, nil } -func (e *editor) merge(pp *pc.PointCloud) { +func (e *editor) merge(pp *pc.PointCloud) error { pcNew := &pc.PointCloud{ PointCloudHeader: e.pp.PointCloudHeader.Clone(), Points: e.pp.Points + pp.Points, @@ -373,10 +385,13 @@ func (e *editor) merge(pp *pc.PointCloud) { pcNew.Width = pcNew.Points pcNew.Height = 1 - e.push(&replacePatch{ + if err := e.push(&replacePatch{ header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, - }) + }); err != nil { + return err + } e.pp = pcNew runtime.GC() + return nil } diff --git a/history.go b/history.go index 547fe4f5..0d4040ae 100644 --- a/history.go +++ b/history.go @@ -33,7 +33,7 @@ func (h *history) SetMaxHistory(m int) { h.maxHistory = m } -func (h *history) push(p patch) { +func (h *history) push(p patch) error { var head bytes.Buffer p.encodeHead(&head) h.steps = append(h.steps, undoStep{h.store.store(head.Bytes(), p.payload())}) @@ -41,6 +41,7 @@ func (h *history) push(p patch) { h.steps[0] = nil h.steps = h.steps[1:] } + return nil } func (h *history) squashLatest() { diff --git a/history_test.go b/history_test.go index 8a4bae7a..fe3f4dac 100644 --- a/history_test.go +++ b/history_test.go @@ -34,7 +34,9 @@ func applyRandomEdit(t *testing.T, e *editor, rnd *rand.Rand) { } case 3: // paste n := 1 + rnd.Intn(20) - e.merge(makeTestCloud(t, n, n, 1)) + if err := e.merge(makeTestCloud(t, n, n, 1)); err != nil { + t.Fatal(err) + } case 4: // whole-cloud replacement n := 50 + rnd.Intn(100) if err := e.SetPointCloud(makeTestCloud(t, n, n, 1), cloudMain); err != nil { @@ -114,7 +116,9 @@ func TestHistorySquashLatest(t *testing.T) { if err := e.passThrough(func(i int, _ mat.Vec3) bool { return i%2 == 0 }); err != nil { t.Fatal(err) } - e.merge(makeTestCloud(t, 10, 10, 1)) + if err := e.merge(makeTestCloud(t, 10, 10, 1)); err != nil { + t.Fatal(err) + } e.squashLatest() if !e.Undo() { From 1951bb77ba583250a800723f0f85eddd1dd24cba Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sun, 6 Sep 2026 16:26:54 +0900 Subject: [PATCH 10/13] Replace the custom patch codec with encoding/gob Co-Authored-By: Claude Fable 5 --- editor.go | 14 ++--- history.go | 12 ++-- patch.go | 170 ++++++++------------------------------------------ patch_test.go | 38 +++++------ 4 files changed, 57 insertions(+), 177 deletions(-) diff --git a/editor.go b/editor.go index 5e6ba6c4..1623827c 100644 --- a/editor.go +++ b/editor.go @@ -107,7 +107,7 @@ func (e *editor) SetPointCloud(pp *pc.PointCloud, id cloudID) error { case cloudMain: if e.pp != nil { if err := e.push(&replacePatch{ - header: e.pp.PointCloudHeader.Clone(), + Header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, }); err != nil { return err @@ -161,7 +161,7 @@ func (e *editor) label(fn func(int, mat.Vec3) (uint32, bool)) error { i++ } if err := e.push(&replacePatch{ - header: e.pp.PointCloudHeader.Clone(), + Header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, }); err != nil { return err @@ -177,7 +177,7 @@ func (e *editor) passThrough(fn func(int, mat.Vec3) bool) error { return err } if err := e.push(&replacePatch{ - header: e.pp.PointCloudHeader.Clone(), + Header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, }); err != nil { return err @@ -193,7 +193,7 @@ func (e *editor) passThroughByMask(sel []uint32, mask, val uint32) error { return err } if err := e.push(&replacePatch{ - header: e.pp.PointCloudHeader.Clone(), + Header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, }); err != nil { return err @@ -232,7 +232,7 @@ func (e *editor) relabelPointsInLabelRange(minLabel, maxLabel, newLabel uint32) } if err := e.push(&replacePatch{ - header: e.pp.PointCloudHeader.Clone(), + Header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, }); err != nil { return err @@ -279,7 +279,7 @@ func (e *editor) unlabelPoints(labelsToKeep []uint32) error { } if err := e.push(&replacePatch{ - header: e.pp.PointCloudHeader.Clone(), + Header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, }); err != nil { return err @@ -386,7 +386,7 @@ func (e *editor) merge(pp *pc.PointCloud) error { pcNew.Height = 1 if err := e.push(&replacePatch{ - header: e.pp.PointCloudHeader.Clone(), + Header: e.pp.PointCloudHeader.Clone(), data: e.pp.Data, }); err != nil { return err diff --git a/history.go b/history.go index 0d4040ae..3bc2f5fe 100644 --- a/history.go +++ b/history.go @@ -35,7 +35,9 @@ func (h *history) SetMaxHistory(m int) { func (h *history) push(p patch) error { var head bytes.Buffer - p.encodeHead(&head) + if err := encodePatch(&head, p); err != nil { + return err + } h.steps = append(h.steps, undoStep{h.store.store(head.Bytes(), p.payload())}) for len(h.steps) > h.maxHistory { h.steps[0] = nil @@ -59,14 +61,12 @@ func (h *history) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { } step := h.steps[n-1] for i := len(step) - 1; i >= 0; i-- { - ps, err := decodePatches(step[i].load()) + p, err := decodePatch(step[i].load()) if err != nil { return nil, false } - for j := len(ps) - 1; j >= 0; j-- { - if pp, err = ps[j].revert(pp); err != nil { - return nil, false - } + if pp, err = p.revert(pp); err != nil { + return nil, false } } h.steps[n-1] = nil diff --git a/patch.go b/patch.go index 7988cfd7..6940e384 100644 --- a/patch.go +++ b/patch.go @@ -2,176 +2,60 @@ package main import ( "bytes" - "encoding/binary" - "errors" - "math" + "encoding/gob" + "io" "github.com/seqsense/pcgol/pc" ) +// patch is a reverse edit restoring a point cloud to the state before an edit. type patch interface { - // pp may be mutated; use the returned cloud + // pp may be mutated; use the returned cloud. revert(pp *pc.PointCloud) (*pc.PointCloud, error) - // The wire form is the head followed by the raw payload - encodeHead(buf *bytes.Buffer) + // payload returns the bulk data kept out of the gob encoding to avoid copying it. payload() []byte + setPayload(data []byte) } -const ( - patchTypeLabel = iota + 1 - patchTypeDelete - patchTypeAppend - patchTypeReplace -) - -var ( - errBrokenPatch = errors.New("broken patch data") - errUnknownPatchType = errors.New("unknown patch type") -) +func init() { + gob.Register(&replacePatch{}) +} type replacePatch struct { - header pc.PointCloudHeader + Header pc.PointCloudHeader data []byte } func (p *replacePatch) revert(_ *pc.PointCloud) (*pc.PointCloud, error) { return &pc.PointCloud{ - PointCloudHeader: p.header, - Points: p.header.Width * p.header.Height, + PointCloudHeader: p.Header, + Points: p.Header.Width * p.Header.Height, Data: p.data, }, nil } -func (p *replacePatch) encodeHead(buf *bytes.Buffer) { - buf.WriteByte(patchTypeReplace) - writeUint32(buf, math.Float32bits(p.header.Version)) - writeUint32(buf, uint32(len(p.header.Fields))) - for i := range p.header.Fields { - writeString(buf, p.header.Fields[i]) - writeUint32(buf, uint32(p.header.Size[i])) - writeString(buf, p.header.Type[i]) - writeUint32(buf, uint32(p.header.Count[i])) - } - writeUint32(buf, uint32(p.header.Width)) - writeUint32(buf, uint32(p.header.Height)) - writeUint32(buf, uint32(len(p.header.Viewpoint))) - for _, v := range p.header.Viewpoint { - writeUint32(buf, math.Float32bits(v)) - } - writeUint32(buf, uint32(len(p.data))) -} - func (p *replacePatch) payload() []byte { return p.data } -func encodePatches(buf *bytes.Buffer, ps []patch) { - for _, p := range ps { - p.encodeHead(buf) - buf.Write(p.payload()) - } +func (p *replacePatch) setPayload(data []byte) { + p.data = data } -// Decoded patches may reference b; do not reuse it afterwards -func decodePatches(b []byte) ([]patch, error) { - var ps []patch - for len(b) > 0 { - p, rest, err := decodePatch(b) - if err != nil { - return nil, err - } - ps = append(ps, p) - b = rest - } - return ps, nil +// encodePatch writes the gob encoding of p to w, excluding the payload. +// A complete record is this encoding followed by the raw payload. +func encodePatch(w io.Writer, p patch) error { + return gob.NewEncoder(w).Encode(&p) } -func decodePatch(b []byte) (patch, []byte, error) { - if len(b) < 1 { - return nil, nil, errBrokenPatch - } - typ := b[0] - r := reader{b: b[1:]} - switch typ { - case patchTypeReplace: - p := &replacePatch{} - p.header.Version = math.Float32frombits(r.uint32()) - nFields := int(r.uint32()) - // A field encodes to at least 16 bytes - if r.err != nil || nFields < 0 || nFields > len(r.b)/16 { - return nil, nil, errBrokenPatch - } - p.header.Fields = make([]string, nFields) - p.header.Size = make([]int, nFields) - p.header.Type = make([]string, nFields) - p.header.Count = make([]int, nFields) - for i := 0; i < nFields; i++ { - p.header.Fields[i] = r.string() - p.header.Size[i] = int(r.uint32()) - p.header.Type[i] = r.string() - p.header.Count[i] = int(r.uint32()) - } - p.header.Width = int(r.uint32()) - p.header.Height = int(r.uint32()) - nvp := int(r.uint32()) - if r.err != nil || nvp < 0 || nvp > len(r.b)/4 { - return nil, nil, errBrokenPatch - } - p.header.Viewpoint = make([]float32, nvp) - for i := range p.header.Viewpoint { - p.header.Viewpoint[i] = math.Float32frombits(r.uint32()) - } - p.data = r.bytes(int(r.uint32())) - if r.err != nil { - return nil, nil, r.err - } - return p, r.b, nil +// The returned patch references b; do not reuse b afterwards. +func decodePatch(b []byte) (patch, error) { + r := bytes.NewReader(b) + var p patch + if err := gob.NewDecoder(r).Decode(&p); err != nil { + return nil, err } - return nil, nil, errUnknownPatchType -} - -func writeUint32(buf *bytes.Buffer, v uint32) { - var b [4]byte - binary.LittleEndian.PutUint32(b[:], v) - buf.Write(b[:]) -} - -func writeString(buf *bytes.Buffer, s string) { - writeUint32(buf, uint32(len(s))) - buf.WriteString(s) -} - -type reader struct { - b []byte - err error -} - -func (r *reader) uint32() uint32 { - if r.err != nil { - return 0 - } - if len(r.b) < 4 { - r.err = errBrokenPatch - return 0 - } - v := binary.LittleEndian.Uint32(r.b) - r.b = r.b[4:] - return v -} - -func (r *reader) bytes(n int) []byte { - if r.err != nil { - return nil - } - if n < 0 || len(r.b) < n { - r.err = errBrokenPatch - return nil - } - b := r.b[:n] - r.b = r.b[n:] - return b -} - -func (r *reader) string() string { - return string(r.bytes(int(r.uint32()))) + // gob reads exactly one value from a ByteReader; the rest is the payload + p.setPayload(b[len(b)-r.Len():]) + return p, nil } diff --git a/patch_test.go b/patch_test.go index 1d95637e..91b5e335 100644 --- a/patch_test.go +++ b/patch_test.go @@ -13,13 +13,14 @@ func makeTestCloud(t *testing.T, n, width, height int) *pc.PointCloud { t.Helper() pp := &pc.PointCloud{ PointCloudHeader: pc.PointCloudHeader{ - Version: 0.7, - Fields: []string{"x", "y", "z", "label"}, - Size: []int{4, 4, 4, 4}, - Type: []string{"F", "F", "F", "U"}, - Count: []int{1, 1, 1, 1}, - Width: width, - Height: height, + Version: 0.7, + Fields: []string{"x", "y", "z", "label"}, + Size: []int{4, 4, 4, 4}, + Type: []string{"F", "F", "F", "U"}, + Count: []int{1, 1, 1, 1}, + Width: width, + Height: height, + Viewpoint: []float32{0, 0, 0, 1, 0, 0, 0}, }, Points: n, } @@ -54,11 +55,10 @@ func assertCloudEqual(t *testing.T, expected, got *pc.PointCloud) { func TestReplacePatchRevert(t *testing.T) { orig := makeTestCloud(t, 100, 10, 10) - orig.Viewpoint = []float32{0, 0, 0, 1, 0, 0, 0} pp := makeTestCloud(t, 5, 5, 1) p := &replacePatch{ - header: orig.PointCloudHeader.Clone(), + Header: orig.PointCloudHeader.Clone(), data: append([]byte{}, orig.Data...), } out, err := p.revert(pp) @@ -74,22 +74,18 @@ func TestReplacePatchRevert(t *testing.T) { func TestPatchEncodeDecodeRoundTrip(t *testing.T) { orig := makeTestCloud(t, 100, 10, 10) orig.Viewpoint = []float32{1, 2, 3, 1, 0, 0, 0} - patches := []patch{ - &replacePatch{header: orig.PointCloudHeader.Clone(), data: orig.Data}, - } + p := &replacePatch{Header: orig.PointCloudHeader.Clone(), data: orig.Data} var buf bytes.Buffer - encodePatches(&buf, patches) - decoded, err := decodePatches(buf.Bytes()) - if err != nil { + if err := encodePatch(&buf, p); err != nil { t.Fatal(err) } - if len(decoded) != len(patches) { - t.Fatalf("Expected %d patches, got %d", len(patches), len(decoded)) + buf.Write(p.payload()) + decoded, err := decodePatch(buf.Bytes()) + if err != nil { + t.Fatal(err) } - for i := range patches { - if !reflect.DeepEqual(patches[i], decoded[i]) { - t.Errorf("Patch %d: expected %+v, got %+v", i, patches[i], decoded[i]) - } + if !reflect.DeepEqual(patch(p), decoded) { + t.Errorf("Expected %+v, got %+v", p, decoded) } } From 969963783b153dae96afd531f917a045d80fe341 Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sun, 6 Sep 2026 16:49:17 +0900 Subject: [PATCH 11/13] Rename patch types to describe what they store Co-Authored-By: Claude Fable 5 --- editor.go | 35 ++++------------- history.go | 10 ++--- patch.go | 61 ------------------------------ record.go | 66 +++++++++++++++++++++++++++++++++ patch_test.go => record_test.go | 21 +++++------ undo_test.go | 4 +- 6 files changed, 89 insertions(+), 108 deletions(-) delete mode 100644 patch.go create mode 100644 record.go rename patch_test.go => record_test.go (79%) diff --git a/editor.go b/editor.go index 1623827c..32f54e4b 100644 --- a/editor.go +++ b/editor.go @@ -106,10 +106,7 @@ func (e *editor) SetPointCloud(pp *pc.PointCloud, id cloudID) error { switch id { case cloudMain: if e.pp != nil { - if err := e.push(&replacePatch{ - Header: e.pp.PointCloudHeader.Clone(), - data: e.pp.Data, - }); err != nil { + if err := e.push(newPreviousCloud(e.pp)); err != nil { return err } } @@ -160,10 +157,7 @@ func (e *editor) label(fn func(int, mat.Vec3) (uint32, bool)) error { itL.Incr() i++ } - if err := e.push(&replacePatch{ - Header: e.pp.PointCloudHeader.Clone(), - data: e.pp.Data, - }); err != nil { + if err := e.push(newPreviousCloud(e.pp)); err != nil { return err } e.pp = pcNew @@ -176,10 +170,7 @@ func (e *editor) passThrough(fn func(int, mat.Vec3) bool) error { if err != nil { return err } - if err := e.push(&replacePatch{ - Header: e.pp.PointCloudHeader.Clone(), - data: e.pp.Data, - }); err != nil { + if err := e.push(newPreviousCloud(e.pp)); err != nil { return err } e.pp = pp @@ -192,10 +183,7 @@ func (e *editor) passThroughByMask(sel []uint32, mask, val uint32) error { if err != nil { return err } - if err := e.push(&replacePatch{ - Header: e.pp.PointCloudHeader.Clone(), - data: e.pp.Data, - }); err != nil { + if err := e.push(newPreviousCloud(e.pp)); err != nil { return err } e.pp = pp @@ -231,10 +219,7 @@ func (e *editor) relabelPointsInLabelRange(minLabel, maxLabel, newLabel uint32) lt.SetUint32(newLabel) } - if err := e.push(&replacePatch{ - Header: e.pp.PointCloudHeader.Clone(), - data: e.pp.Data, - }); err != nil { + if err := e.push(newPreviousCloud(e.pp)); err != nil { return err } e.pp = pcNew @@ -278,10 +263,7 @@ func (e *editor) unlabelPoints(labelsToKeep []uint32) error { lt.SetUint32(0) } - if err := e.push(&replacePatch{ - Header: e.pp.PointCloudHeader.Clone(), - data: e.pp.Data, - }); err != nil { + if err := e.push(newPreviousCloud(e.pp)); err != nil { return err } e.pp = pcNew @@ -385,10 +367,7 @@ func (e *editor) merge(pp *pc.PointCloud) error { pcNew.Width = pcNew.Points pcNew.Height = 1 - if err := e.push(&replacePatch{ - Header: e.pp.PointCloudHeader.Clone(), - data: e.pp.Data, - }); err != nil { + if err := e.push(newPreviousCloud(e.pp)); err != nil { return err } e.pp = pcNew diff --git a/history.go b/history.go index 3bc2f5fe..a9cc3a77 100644 --- a/history.go +++ b/history.go @@ -33,12 +33,12 @@ func (h *history) SetMaxHistory(m int) { h.maxHistory = m } -func (h *history) push(p patch) error { +func (h *history) push(d undoData) error { var head bytes.Buffer - if err := encodePatch(&head, p); err != nil { + if err := encodeUndoData(&head, d); err != nil { return err } - h.steps = append(h.steps, undoStep{h.store.store(head.Bytes(), p.payload())}) + h.steps = append(h.steps, undoStep{h.store.store(head.Bytes(), d.payload())}) for len(h.steps) > h.maxHistory { h.steps[0] = nil h.steps = h.steps[1:] @@ -61,11 +61,11 @@ func (h *history) undo(pp *pc.PointCloud) (*pc.PointCloud, bool) { } step := h.steps[n-1] for i := len(step) - 1; i >= 0; i-- { - p, err := decodePatch(step[i].load()) + d, err := decodeRecord(step[i].load()) if err != nil { return nil, false } - if pp, err = p.revert(pp); err != nil { + if pp, err = d.restore(pp); err != nil { return nil, false } } diff --git a/patch.go b/patch.go deleted file mode 100644 index 6940e384..00000000 --- a/patch.go +++ /dev/null @@ -1,61 +0,0 @@ -package main - -import ( - "bytes" - "encoding/gob" - "io" - - "github.com/seqsense/pcgol/pc" -) - -// patch is a reverse edit restoring a point cloud to the state before an edit. -type patch interface { - // pp may be mutated; use the returned cloud. - revert(pp *pc.PointCloud) (*pc.PointCloud, error) - // payload returns the bulk data kept out of the gob encoding to avoid copying it. - payload() []byte - setPayload(data []byte) -} - -func init() { - gob.Register(&replacePatch{}) -} - -type replacePatch struct { - Header pc.PointCloudHeader - data []byte -} - -func (p *replacePatch) revert(_ *pc.PointCloud) (*pc.PointCloud, error) { - return &pc.PointCloud{ - PointCloudHeader: p.Header, - Points: p.Header.Width * p.Header.Height, - Data: p.data, - }, nil -} - -func (p *replacePatch) payload() []byte { - return p.data -} - -func (p *replacePatch) setPayload(data []byte) { - p.data = data -} - -// encodePatch writes the gob encoding of p to w, excluding the payload. -// A complete record is this encoding followed by the raw payload. -func encodePatch(w io.Writer, p patch) error { - return gob.NewEncoder(w).Encode(&p) -} - -// The returned patch references b; do not reuse b afterwards. -func decodePatch(b []byte) (patch, error) { - r := bytes.NewReader(b) - var p patch - if err := gob.NewDecoder(r).Decode(&p); err != nil { - return nil, err - } - // gob reads exactly one value from a ByteReader; the rest is the payload - p.setPayload(b[len(b)-r.Len():]) - return p, nil -} diff --git a/record.go b/record.go new file mode 100644 index 00000000..ecc6b548 --- /dev/null +++ b/record.go @@ -0,0 +1,66 @@ +package main + +import ( + "bytes" + "encoding/gob" + "io" + + "github.com/seqsense/pcgol/pc" +) + +type undoData interface { + // pp may be mutated; use the returned cloud. + restore(pp *pc.PointCloud) (*pc.PointCloud, error) + // payload returns the bulk data kept out of the gob encoding to avoid copying it. + payload() []byte + setPayload(data []byte) +} + +func init() { + gob.Register(&previousCloud{}) +} + +type previousCloud struct { + Header pc.PointCloudHeader + data []byte +} + +func newPreviousCloud(pp *pc.PointCloud) *previousCloud { + return &previousCloud{ + Header: pp.PointCloudHeader.Clone(), + data: pp.Data, + } +} + +func (p *previousCloud) restore(_ *pc.PointCloud) (*pc.PointCloud, error) { + return &pc.PointCloud{ + PointCloudHeader: p.Header, + Points: p.Header.Width * p.Header.Height, + Data: p.data, + }, nil +} + +func (p *previousCloud) payload() []byte { + return p.data +} + +func (p *previousCloud) setPayload(data []byte) { + p.data = data +} + +// A record is this encoding followed by the raw payload. +func encodeUndoData(w io.Writer, d undoData) error { + return gob.NewEncoder(w).Encode(&d) +} + +// The returned undoData references b; do not reuse b afterwards. +func decodeRecord(b []byte) (undoData, error) { + r := bytes.NewReader(b) + var d undoData + if err := gob.NewDecoder(r).Decode(&d); err != nil { + return nil, err + } + // gob reads exactly one value from a ByteReader; the rest is the payload + d.setPayload(b[len(b)-r.Len():]) + return d, nil +} diff --git a/patch_test.go b/record_test.go similarity index 79% rename from patch_test.go rename to record_test.go index 91b5e335..2e143159 100644 --- a/patch_test.go +++ b/record_test.go @@ -49,19 +49,16 @@ func assertCloudEqual(t *testing.T, expected, got *pc.PointCloud) { expected.Width, expected.Height, got.Width, got.Height) } if !bytes.Equal(expected.Data, got.Data) { - t.Fatal("Data mismatch after revert") + t.Fatal("Data mismatch after restore") } } -func TestReplacePatchRevert(t *testing.T) { +func TestPreviousCloudRestore(t *testing.T) { orig := makeTestCloud(t, 100, 10, 10) pp := makeTestCloud(t, 5, 5, 1) - p := &replacePatch{ - Header: orig.PointCloudHeader.Clone(), - data: append([]byte{}, orig.Data...), - } - out, err := p.revert(pp) + p := newPreviousCloud(orig) + out, err := p.restore(pp) if err != nil { t.Fatal(err) } @@ -71,21 +68,21 @@ func TestReplacePatchRevert(t *testing.T) { } } -func TestPatchEncodeDecodeRoundTrip(t *testing.T) { +func TestRecordEncodeDecodeRoundTrip(t *testing.T) { orig := makeTestCloud(t, 100, 10, 10) orig.Viewpoint = []float32{1, 2, 3, 1, 0, 0, 0} - p := &replacePatch{Header: orig.PointCloudHeader.Clone(), data: orig.Data} + p := newPreviousCloud(orig) var buf bytes.Buffer - if err := encodePatch(&buf, p); err != nil { + if err := encodeUndoData(&buf, p); err != nil { t.Fatal(err) } buf.Write(p.payload()) - decoded, err := decodePatch(buf.Bytes()) + decoded, err := decodeRecord(buf.Bytes()) if err != nil { t.Fatal(err) } - if !reflect.DeepEqual(patch(p), decoded) { + if !reflect.DeepEqual(undoData(p), decoded) { t.Errorf("Expected %+v, got %+v", p, decoded) } } diff --git a/undo_test.go b/undo_test.go index 607821cc..830de184 100644 --- a/undo_test.go +++ b/undo_test.go @@ -9,11 +9,11 @@ import ( func TestHistoryUndoKeepsStepOnError(t *testing.T) { h := newHistory(4) - h.steps = []undoStep{{memRecord{0xFF}}} // broken patch data + h.steps = []undoStep{{memRecord{0xFF}}} // broken record data if _, ok := h.undo(nil); ok { t.Fatal("undo of a broken step must fail") } if len(h.steps) != 1 { - t.Fatal("a broken step must not be dropped; a later undo would apply an older patch to a mismatched state") + t.Fatal("a broken step must not be dropped; a later undo would apply an older record to a mismatched state") } } From 4b5479a6740589ce7260537eb87d88e3636273f8 Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sun, 6 Sep 2026 17:27:29 +0900 Subject: [PATCH 12/13] Define undo behavior with explicit small-example tests Co-Authored-By: Claude Fable 5 --- history_test.go | 283 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 235 insertions(+), 48 deletions(-) diff --git a/history_test.go b/history_test.go index fe3f4dac..1bd15568 100644 --- a/history_test.go +++ b/history_test.go @@ -9,6 +9,241 @@ import ( "github.com/seqsense/pcgol/pc" ) +func makeCloud(t *testing.T, points []mat.Vec3, labels []uint32) *pc.PointCloud { + t.Helper() + pp := &pc.PointCloud{ + PointCloudHeader: pc.PointCloudHeader{ + Version: 0.7, + Fields: []string{"x", "y", "z", "label"}, + Size: []int{4, 4, 4, 4}, + Type: []string{"F", "F", "F", "U"}, + Count: []int{1, 1, 1, 1}, + Width: len(points), + Height: 1, + Viewpoint: []float32{0, 0, 0, 1, 0, 0, 0}, + }, + Points: len(points), + } + pp.Data = make([]byte, len(points)*pp.Stride()) + it, err := pp.Vec3Iterator() + if err != nil { + t.Fatal(err) + } + lt, err := pp.Uint32Iterator("label") + if err != nil { + t.Fatal(err) + } + for i := range points { + it.SetVec3(points[i]) + lt.SetUint32(labels[i]) + it.Incr() + lt.Incr() + } + return pp +} + +func assertCloud(t *testing.T, pp *pc.PointCloud, points []mat.Vec3, labels []uint32) { + t.Helper() + if pp.Points != len(points) { + t.Fatalf("Expected %d points, got %d", len(points), pp.Points) + } + it, err := pp.Vec3Iterator() + if err != nil { + t.Fatal(err) + } + lt, err := pp.Uint32Iterator("label") + if err != nil { + t.Fatal(err) + } + for i := range points { + if !points[i].Equal(it.Vec3()) { + t.Fatalf("Point %d: expected %v, got %v", i, points[i], it.Vec3()) + } + if labels[i] != lt.Uint32() { + t.Fatalf("Label %d: expected %d, got %d", i, labels[i], lt.Uint32()) + } + it.Incr() + lt.Incr() + } +} + +func TestEditorUndo(t *testing.T) { + pts := []mat.Vec3{{1, 0, 0}, {2, 0, 0}, {3, 0, 0}} + + newTestEditor := func(t *testing.T, labels []uint32) *editor { + t.Helper() + e := newEditor() + if err := e.SetPointCloud(makeCloud(t, pts, labels), cloudMain); err != nil { + t.Fatal(err) + } + return e + } + undo := func(t *testing.T, e *editor) { + t.Helper() + if !e.Undo() { + t.Fatal("undo failed") + } + } + + t.Run("Label", func(t *testing.T) { + e := newTestEditor(t, []uint32{0, 0, 0}) + if err := e.label(func(i int, _ mat.Vec3) (uint32, bool) { + return 5, i != 1 + }); err != nil { + t.Fatal(err) + } + assertCloud(t, e.pp, pts, []uint32{5, 0, 5}) + undo(t, e) + assertCloud(t, e.pp, pts, []uint32{0, 0, 0}) + }) + t.Run("Delete", func(t *testing.T) { + e := newTestEditor(t, []uint32{1, 2, 3}) + if err := e.passThrough(func(i int, _ mat.Vec3) bool { + return i != 1 + }); err != nil { + t.Fatal(err) + } + assertCloud(t, e.pp, []mat.Vec3{pts[0], pts[2]}, []uint32{1, 3}) + undo(t, e) + assertCloud(t, e.pp, pts, []uint32{1, 2, 3}) + }) + t.Run("DeleteByMask", func(t *testing.T) { + e := newTestEditor(t, []uint32{1, 2, 3}) + sel := []uint32{0, selectBitmaskSegmentSelected, 0} + if err := e.passThroughByMask(sel, selectBitmaskSegmentSelected, 0); err != nil { + t.Fatal(err) + } + assertCloud(t, e.pp, []mat.Vec3{pts[0], pts[2]}, []uint32{1, 3}) + undo(t, e) + assertCloud(t, e.pp, pts, []uint32{1, 2, 3}) + }) + t.Run("Merge", func(t *testing.T) { + e := newTestEditor(t, []uint32{1, 2, 3}) + if err := e.merge(makeCloud(t, []mat.Vec3{{4, 0, 0}}, []uint32{7})); err != nil { + t.Fatal(err) + } + assertCloud(t, e.pp, append(pts, mat.Vec3{4, 0, 0}), []uint32{1, 2, 3, 7}) + undo(t, e) + assertCloud(t, e.pp, pts, []uint32{1, 2, 3}) + }) + t.Run("Relabel", func(t *testing.T) { + e := newTestEditor(t, []uint32{1, 2, 3}) + if err := e.relabelPointsInLabelRange(1, 2, 9); err != nil { + t.Fatal(err) + } + assertCloud(t, e.pp, pts, []uint32{9, 9, 3}) + undo(t, e) + assertCloud(t, e.pp, pts, []uint32{1, 2, 3}) + }) + t.Run("Unlabel", func(t *testing.T) { + e := newTestEditor(t, []uint32{1, 2, 3}) + if err := e.unlabelPoints([]uint32{2}); err != nil { + t.Fatal(err) + } + assertCloud(t, e.pp, pts, []uint32{0, 2, 0}) + undo(t, e) + assertCloud(t, e.pp, pts, []uint32{1, 2, 3}) + }) + t.Run("Replace", func(t *testing.T) { + e := newTestEditor(t, []uint32{1, 2, 3}) + pts2 := []mat.Vec3{{9, 9, 9}} + if err := e.SetPointCloud(makeCloud(t, pts2, []uint32{4}), cloudMain); err != nil { + t.Fatal(err) + } + assertCloud(t, e.pp, pts2, []uint32{4}) + undo(t, e) + assertCloud(t, e.pp, pts, []uint32{1, 2, 3}) + }) + t.Run("SequentialEdits", func(t *testing.T) { + e := newTestEditor(t, []uint32{0, 0, 0}) + if err := e.relabelPointsInLabelRange(0, 0, 1); err != nil { + t.Fatal(err) + } + if err := e.passThrough(func(i int, _ mat.Vec3) bool { + return i == 0 + }); err != nil { + t.Fatal(err) + } + assertCloud(t, e.pp, pts[:1], []uint32{1}) + undo(t, e) + assertCloud(t, e.pp, pts, []uint32{1, 1, 1}) + undo(t, e) + assertCloud(t, e.pp, pts, []uint32{0, 0, 0}) + }) + t.Run("NothingToUndo", func(t *testing.T) { + e := newTestEditor(t, []uint32{0, 0, 0}) + if e.Undo() { + t.Fatal("undo without edits must fail") + } + }) +} + +func TestHistoryMaxDepth(t *testing.T) { + pts := []mat.Vec3{{1, 0, 0}, {2, 0, 0}} + labelAll := func(t *testing.T, e *editor, l uint32) { + t.Helper() + if err := e.label(func(int, mat.Vec3) (uint32, bool) { + return l, true + }); err != nil { + t.Fatal(err) + } + } + + e := newEditor() + e.SetMaxHistory(2) + if err := e.SetPointCloud(makeCloud(t, pts, []uint32{0, 0}), cloudMain); err != nil { + t.Fatal(err) + } + labelAll(t, e, 1) + labelAll(t, e, 2) + labelAll(t, e, 3) + + if !e.Undo() { + t.Fatal("first undo must succeed") + } + assertCloud(t, e.pp, pts, []uint32{2, 2}) + if !e.Undo() { + t.Fatal("second undo must succeed") + } + assertCloud(t, e.pp, pts, []uint32{1, 1}) + if e.Undo() { + t.Fatal("undo deeper than max_history must fail") + } + assertCloud(t, e.pp, pts, []uint32{1, 1}) + + e.SetMaxHistory(0) + labelAll(t, e, 9) + if e.Undo() { + t.Fatal("undo with max_history=0 must fail") + } +} + +func TestHistorySquashLatest(t *testing.T) { + pts := []mat.Vec3{{1, 0, 0}, {2, 0, 0}} + + e := newEditor() + if err := e.SetPointCloud(makeCloud(t, pts, []uint32{0, 0}), cloudMain); err != nil { + t.Fatal(err) + } + if err := e.relabelPointsInLabelRange(0, 0, 1); err != nil { + t.Fatal(err) + } + if err := e.merge(makeCloud(t, []mat.Vec3{{3, 0, 0}}, []uint32{2})); err != nil { + t.Fatal(err) + } + e.squashLatest() + + if !e.Undo() { + t.Fatal("undo failed") + } + assertCloud(t, e.pp, pts, []uint32{0, 0}) + if e.Undo() { + t.Fatal("squashed edits must be undone as a single step") + } +} + +// The tests below fuzz the same behavior with randomized edit sequences. + func snapshotCloud(e *editor) *pc.PointCloud { return cloneCloud(e.pp) } @@ -78,51 +313,3 @@ func TestEditorUndoRoundTrip(t *testing.T) { } } } - -func TestHistoryMaxDepth(t *testing.T) { - rnd := rand.New(rand.NewSource(1)) - - e := newEditor() // maxHistoryDefault = 4 - if err := e.SetPointCloud(makeTestCloud(t, 100, 10, 10), cloudMain); err != nil { - t.Fatal(err) - } - - for k := 0; k < 6; k++ { - applyRandomEdit(t, e, rnd) - } - for k := 0; k < maxHistoryDefault; k++ { - if !e.Undo() { - t.Fatalf("undo %d must succeed", k) - } - } - if e.Undo() { - t.Fatal("undo deeper than max_history must fail") - } - - e.SetMaxHistory(0) - applyRandomEdit(t, e, rnd) - if e.Undo() { - t.Fatal("undo with max_history=0 must fail") - } -} - -func TestHistorySquashLatest(t *testing.T) { - e := newEditor() - if err := e.SetPointCloud(makeTestCloud(t, 100, 10, 10), cloudMain); err != nil { - t.Fatal(err) - } - orig := snapshotCloud(e) - - if err := e.passThrough(func(i int, _ mat.Vec3) bool { return i%2 == 0 }); err != nil { - t.Fatal(err) - } - if err := e.merge(makeTestCloud(t, 10, 10, 1)); err != nil { - t.Fatal(err) - } - e.squashLatest() - - if !e.Undo() { - t.Fatal("undo failed") - } - assertCloudEqual(t, orig, e.pp) -} From 786a3b357134c39f40a3c793ba44a6b9c6a7bf9d Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Mon, 7 Sep 2026 23:33:37 +0900 Subject: [PATCH 13/13] Name undoData implementations after the interface Co-Authored-By: Claude Fable 5 --- editor.go | 14 +++++++------- record.go | 14 +++++++------- record_test.go | 6 +++--- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/editor.go b/editor.go index 32f54e4b..590c9a00 100644 --- a/editor.go +++ b/editor.go @@ -106,7 +106,7 @@ func (e *editor) SetPointCloud(pp *pc.PointCloud, id cloudID) error { switch id { case cloudMain: if e.pp != nil { - if err := e.push(newPreviousCloud(e.pp)); err != nil { + if err := e.push(newUndoDataEntireCloud(e.pp)); err != nil { return err } } @@ -157,7 +157,7 @@ func (e *editor) label(fn func(int, mat.Vec3) (uint32, bool)) error { itL.Incr() i++ } - if err := e.push(newPreviousCloud(e.pp)); err != nil { + if err := e.push(newUndoDataEntireCloud(e.pp)); err != nil { return err } e.pp = pcNew @@ -170,7 +170,7 @@ func (e *editor) passThrough(fn func(int, mat.Vec3) bool) error { if err != nil { return err } - if err := e.push(newPreviousCloud(e.pp)); err != nil { + if err := e.push(newUndoDataEntireCloud(e.pp)); err != nil { return err } e.pp = pp @@ -183,7 +183,7 @@ func (e *editor) passThroughByMask(sel []uint32, mask, val uint32) error { if err != nil { return err } - if err := e.push(newPreviousCloud(e.pp)); err != nil { + if err := e.push(newUndoDataEntireCloud(e.pp)); err != nil { return err } e.pp = pp @@ -219,7 +219,7 @@ func (e *editor) relabelPointsInLabelRange(minLabel, maxLabel, newLabel uint32) lt.SetUint32(newLabel) } - if err := e.push(newPreviousCloud(e.pp)); err != nil { + if err := e.push(newUndoDataEntireCloud(e.pp)); err != nil { return err } e.pp = pcNew @@ -263,7 +263,7 @@ func (e *editor) unlabelPoints(labelsToKeep []uint32) error { lt.SetUint32(0) } - if err := e.push(newPreviousCloud(e.pp)); err != nil { + if err := e.push(newUndoDataEntireCloud(e.pp)); err != nil { return err } e.pp = pcNew @@ -367,7 +367,7 @@ func (e *editor) merge(pp *pc.PointCloud) error { pcNew.Width = pcNew.Points pcNew.Height = 1 - if err := e.push(newPreviousCloud(e.pp)); err != nil { + if err := e.push(newUndoDataEntireCloud(e.pp)); err != nil { return err } e.pp = pcNew diff --git a/record.go b/record.go index ecc6b548..a5303a9f 100644 --- a/record.go +++ b/record.go @@ -17,22 +17,22 @@ type undoData interface { } func init() { - gob.Register(&previousCloud{}) + gob.Register(&undoDataEntireCloud{}) } -type previousCloud struct { +type undoDataEntireCloud struct { Header pc.PointCloudHeader data []byte } -func newPreviousCloud(pp *pc.PointCloud) *previousCloud { - return &previousCloud{ +func newUndoDataEntireCloud(pp *pc.PointCloud) *undoDataEntireCloud { + return &undoDataEntireCloud{ Header: pp.PointCloudHeader.Clone(), data: pp.Data, } } -func (p *previousCloud) restore(_ *pc.PointCloud) (*pc.PointCloud, error) { +func (p *undoDataEntireCloud) restore(_ *pc.PointCloud) (*pc.PointCloud, error) { return &pc.PointCloud{ PointCloudHeader: p.Header, Points: p.Header.Width * p.Header.Height, @@ -40,11 +40,11 @@ func (p *previousCloud) restore(_ *pc.PointCloud) (*pc.PointCloud, error) { }, nil } -func (p *previousCloud) payload() []byte { +func (p *undoDataEntireCloud) payload() []byte { return p.data } -func (p *previousCloud) setPayload(data []byte) { +func (p *undoDataEntireCloud) setPayload(data []byte) { p.data = data } diff --git a/record_test.go b/record_test.go index 2e143159..656bd685 100644 --- a/record_test.go +++ b/record_test.go @@ -53,11 +53,11 @@ func assertCloudEqual(t *testing.T, expected, got *pc.PointCloud) { } } -func TestPreviousCloudRestore(t *testing.T) { +func TestUndoDataEntireCloudRestore(t *testing.T) { orig := makeTestCloud(t, 100, 10, 10) pp := makeTestCloud(t, 5, 5, 1) - p := newPreviousCloud(orig) + p := newUndoDataEntireCloud(orig) out, err := p.restore(pp) if err != nil { t.Fatal(err) @@ -71,7 +71,7 @@ func TestPreviousCloudRestore(t *testing.T) { func TestRecordEncodeDecodeRoundTrip(t *testing.T) { orig := makeTestCloud(t, 100, 10, 10) orig.Viewpoint = []float32{1, 2, 3, 1, 0, 0, 0} - p := newPreviousCloud(orig) + p := newUndoDataEntireCloud(orig) var buf bytes.Buffer if err := encodeUndoData(&buf, p); err != nil {