From eff3c82d935968cc35012f64dabbb6da7fa52084 Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sun, 16 Aug 2026 15:17:10 +0900 Subject: [PATCH 1/4] Record paste and surface creation as O(1) append patches 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 --- editor.go | 19 +++++++------------ patch.go | 42 ++++++++++++++++++++++++++++++++++++++++++ patch_test.go | 19 +++++++++++++++++++ 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/editor.go b/editor.go index bda8555..ea53b0d 100644 --- a/editor.go +++ b/editor.go @@ -326,18 +326,13 @@ func passThroughImpl(pp *pc.PointCloud, core func(_, _ *pc.PointCloud) int) (*pc } func (e *editor) merge(pp *pc.PointCloud) { - pcNew := &pc.PointCloud{ - PointCloudHeader: e.pp.PointCloudHeader.Clone(), - Points: e.pp.Points + pp.Points, - Data: append(e.pp.Data[:e.pp.Stride()*e.pp.Points], pp.Data...), - } - pcNew.Width = pcNew.Points - pcNew.Height = 1 - - e.push(&replacePatch{ - header: e.pp.PointCloudHeader.Clone(), - data: e.pp.Data, + e.push(&appendPatch{ + oldPoints: e.pp.Points, + oldWidth: e.pp.Width, + oldHeight: e.pp.Height, }) - e.pp = pcNew + n := e.pp.Points + pp.Points + data := append(e.pp.Data[:e.pp.Stride()*e.pp.Points], pp.Data...) + e.pp = newCloudView(e.pp, n, n, 1, data) runtime.GC() } diff --git a/patch.go b/patch.go index 1345151..c0bc258 100644 --- a/patch.go +++ b/patch.go @@ -28,6 +28,19 @@ var ( errNoLabelField = errors.New("point cloud has no label field") ) +// pcgol caches an unsafe float32 alias of Data keyed only by its base pointer, +// so a change of the Data length must be delivered in a fresh PointCloud. +func newCloudView(pp *pc.PointCloud, points, width, height int, data []byte) *pc.PointCloud { + out := &pc.PointCloud{ + PointCloudHeader: pp.PointCloudHeader.Clone(), + Points: points, + Data: data, + } + out.Width = width + out.Height = height + return out +} + func fieldByteOffset(h *pc.PointCloudHeader, name string) (int, bool) { offset := 0 for i, fn := range h.Fields { @@ -67,6 +80,25 @@ func (p *labelPatch) encode(buf *bytes.Buffer) { writeUint32s(buf, p.oldLabels) } +type appendPatch struct { + oldPoints, oldWidth, oldHeight int +} + +func (p *appendPatch) revert(pp *pc.PointCloud) (*pc.PointCloud, error) { + stride := pp.Stride() + if p.oldPoints > pp.Points || p.oldPoints*stride > len(pp.Data) { + return nil, errBrokenPatch + } + return newCloudView(pp, p.oldPoints, p.oldWidth, p.oldHeight, pp.Data[:p.oldPoints*stride]), nil +} + +func (p *appendPatch) encode(buf *bytes.Buffer) { + buf.WriteByte(patchTypeAppend) + writeUint32(buf, uint32(p.oldPoints)) + writeUint32(buf, uint32(p.oldWidth)) + writeUint32(buf, uint32(p.oldHeight)) +} + type replacePatch struct { header pc.PointCloudHeader data []byte @@ -137,6 +169,16 @@ func decodePatch(b []byte) (patch, []byte, error) { return nil, nil, r.err } return p, r.b, nil + case patchTypeAppend: + p := &appendPatch{ + oldPoints: int(r.uint32()), + oldWidth: int(r.uint32()), + oldHeight: int(r.uint32()), + } + if r.err != nil { + return nil, nil, r.err + } + return p, r.b, nil case patchTypeReplace: p := &replacePatch{} p.header.Version = math.Float32frombits(r.uint32()) diff --git a/patch_test.go b/patch_test.go index 42abd88..3b96125 100644 --- a/patch_test.go +++ b/patch_test.go @@ -73,6 +73,24 @@ func TestLabelPatchRevert(t *testing.T) { assertCloudEqual(t, orig, out) } +func TestAppendPatchRevert(t *testing.T) { + orig := makeTestCloud(t, 100, 10, 10) + pp := cloneCloud(orig) + + p := &appendPatch{oldPoints: pp.Points, oldWidth: pp.Width, oldHeight: pp.Height} + added := makeTestCloud(t, 10, 10, 1) + pp.Data = append(pp.Data, added.Data...) + pp.Points += added.Points + pp.Width = pp.Points + pp.Height = 1 + + out, err := p.revert(pp) + if err != nil { + t.Fatal(err) + } + assertCloudEqual(t, orig, out) +} + func TestReplacePatchRevert(t *testing.T) { orig := makeTestCloud(t, 100, 10, 10) orig.Viewpoint = []float32{0, 0, 0, 1, 0, 0, 0} @@ -97,6 +115,7 @@ func TestPatchEncodeDecodeRoundTrip(t *testing.T) { orig.Viewpoint = []float32{1, 2, 3, 1, 0, 0, 0} patches := []patch{ &labelPatch{indices: []uint32{1, 2, 42}, oldLabels: []uint32{7, 8, 9}}, + &appendPatch{oldPoints: 90, oldWidth: 9, oldHeight: 10}, &replacePatch{header: orig.PointCloudHeader.Clone(), data: orig.Data}, } From 129f36b6199419a8ce774d5f5cd9bc7e472906bd Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sat, 22 Aug 2026 21:09:20 +0900 Subject: [PATCH 2/4] Guard appendPatch against negative point counts 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 --- patch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patch.go b/patch.go index c0bc258..900652a 100644 --- a/patch.go +++ b/patch.go @@ -86,7 +86,7 @@ type appendPatch struct { func (p *appendPatch) revert(pp *pc.PointCloud) (*pc.PointCloud, error) { stride := pp.Stride() - if p.oldPoints > pp.Points || p.oldPoints*stride > len(pp.Data) { + if p.oldPoints < 0 || p.oldPoints > pp.Points || p.oldPoints*stride > len(pp.Data) { return nil, errBrokenPatch } return newCloudView(pp, p.oldPoints, p.oldWidth, p.oldHeight, pp.Data[:p.oldPoints*stride]), nil From 8a995f948240df427a11ce5b0bcd1a547e4bc60c Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sat, 22 Aug 2026 21:37:19 +0900 Subject: [PATCH 3/4] Validate appendPatch fields without multiplication 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 --- patch.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/patch.go b/patch.go index 900652a..f3ed3d1 100644 --- a/patch.go +++ b/patch.go @@ -86,7 +86,8 @@ type appendPatch struct { func (p *appendPatch) revert(pp *pc.PointCloud) (*pc.PointCloud, error) { stride := pp.Stride() - if p.oldPoints < 0 || p.oldPoints > pp.Points || p.oldPoints*stride > len(pp.Data) { + if p.oldPoints < 0 || p.oldWidth < 0 || p.oldHeight < 0 || + p.oldPoints > pp.Points || p.oldPoints > len(pp.Data)/stride { return nil, errBrokenPatch } return newCloudView(pp, p.oldPoints, p.oldWidth, p.oldHeight, pp.Data[:p.oldPoints*stride]), nil From 44b4edf0f19245212756495daf81234d5fcbeaa4 Mon Sep 17 00:00:00 2001 From: nabeya11 Date: Sat, 22 Aug 2026 22:19:07 +0900 Subject: [PATCH 4/4] Reject a non-positive stride in appendPatch.revert 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 --- patch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patch.go b/patch.go index e5e74ca..17d88e6 100644 --- a/patch.go +++ b/patch.go @@ -95,7 +95,7 @@ type appendPatch struct { func (p *appendPatch) revert(pp *pc.PointCloud) (*pc.PointCloud, error) { stride := pp.Stride() - if p.oldPoints < 0 || p.oldWidth < 0 || p.oldHeight < 0 || + if stride <= 0 || p.oldPoints < 0 || p.oldWidth < 0 || p.oldHeight < 0 || p.oldPoints > pp.Points || p.oldPoints > len(pp.Data)/stride { return nil, errBrokenPatch }