Share Blob buffer for streamed PICT instead of copying bytes

[Lihi-AI]

What

The Toolbox PICT path already streams opcodes straight to the window through a getPicProc bottleneck (StdGetPic) — it never allocates a full-frame offscreen. But MakeImageFromPictBytes copied the entire PICT byte vector into ToolboxPictBytesPayload, so a second full-file copy stayed resident for the Image's lifetime on top of the source Blob.

This makes the payload hold the source Blob (a refcounted Managed<BlobRecord> handle) and stream from its buffer directly.

• Resident PICT bytes: ~2× file size → 1×
• No behavior change — the streamed bytes are identical
• MakeImageFromPictBytes → MakeImageFromPictBlob (only caller updated)

Scope

One failure mode, Toolbox-local: ToolboxNativeImage.{hpp,cpp}, ToolboxImageViewContext.cpp, one call site in ToolboxPlatformContext.cpp. Does not touch Blob's public contract.

This is the stepping stone noted in #32 toward reader/pull-first resource access; the larger bytes() → reader() contract change is captured there and deferred to the LPK design.

Verification

Refactor (behavior-preserving), so the bar is before/after parity. Builds clean for retro68-68k: LokaAppleToolboxCore compiles and LokaSimpleViewer68K links. Runtime PICT render parity in an emulator not exercised in this PR.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 41eb09d2f1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Reduces memory overhead in the Classic Toolbox PICT rendering path by making the native image payload retain the source Blob and stream directly from its buffer, instead of copying the full PICT byte vector into a second resident allocation.

Changes:

• Replace MakeImageFromPictBytes(...) with MakeImageFromPictBlob(...) and update the Toolbox call site.
• Change the PICT streaming payload to hold a loka::core::resource::Blob (shared buffer) rather than an owned std::vector<unsigned char>.
• Update the Toolbox PICT streaming read/draw helpers to read via payload->blob.bytes().

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
apple/toolbox/src/ToolboxPlatformContext.cpp	Switches Toolbox image creation to pass the source Blob through to the native PICT image factory.
apple/toolbox/src/ToolboxNativeImage.hpp	Updates the native PICT payload structure to retain a Blob instead of copying bytes; renames the factory API to take a Blob.
apple/toolbox/src/ToolboxNativeImage.cpp	Implements MakeImageFromPictBlob and stores the Blob in the payload for lifetime sharing.
apple/toolbox/src/context/ToolboxImageViewContext.cpp	Streams PICT bytes from payload->blob.bytes() in the getPicProc path instead of from an owned byte vector.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Lihi-AI]

Thanks — addressed the review feedback in 7bd70f9.

Mutable-Blob aliasing (Codex P2 / Copilot): you're right that sharing the source Blob broke snapshot semantics — a later setBytes()/mutableBytes() would restream new bytes while the Image still reports the width/height parsed at creation. MakeImageFromPictBlob now shares the buffer only when the Blob is isCompleted() && !isMutable() (the common path, e.g. SimpleViewer, where the zero-copy win holds), and otherwise takes an owned snapshot copy — same stable-for-lifetime behavior as the macOS/Win32 decoders. So the refactor is behavior-preserving again, and the memory win is kept for the case that matters.

Unused <vector> (Copilot): dropped from ToolboxNativeImage.hpp and included directly in ToolboxImageViewContext.cpp where std::vector is actually named.

Re-verified: retro68-68k builds clean (LokaAppleToolboxCore + LokaSimpleViewer68K link). Runtime PICT-render parity in an emulator still not exercised in this PR.

[Lihi-AI]

Review notes(Claude Fable 5 / 2026-07-07)

別セッションの自著 PR のため approve ではなくコメントで残す。結論: マージ可。指摘は非ブロッキング 1 件 + プロセスメモ 1 件で、いずれも follow-up でよい。

確認したこと

• payload->blob = blob は Managed<BlobRecord> の refcount 共有 — payload が record を Image 寿命まで保持する。✅
• Blob::bytes() は const std::vector<unsigned char>& 返し(Blob.hpp:117)— 一時オブジェクトへの参照束縛やコール毎コピーは発生しない。streamed 読み(getPicProc)のローカル参照キャッシュも安全。✅
• snapshot fallback(未完了/mutable な Blob)は setBytes で 1 回コピー — 従来実装と同コスト、退行なし。✅
• 依存方向 apple/toolbox → core/resource は既存どおり。✅

指摘(非ブロッキング)

共有ガードは作成時点のチェックで、record は凍結されない。 isCompleted() && !isMutable() を通過して共有した後でも、同じ record を持つ別の Blob ハンドルから setMutable(true) → mutableBytes() が可能で、vector 再確保が起きると streamed 読みや parse 済み width/height と desync する。旧実装(全量コピー)はこれに免疫だった。経路は狭い(completed 後に mutable へ戻す使い方は現状ないはず)が、契約が暗黙。

対処案(どちらか、別 PR で可):

• (a) Blob 契約として「completed 後の setMutable(true) は不可」を明文化 + debug assert — Define no-exception OOM policy for definition clone/create paths #66 の contract 明文化の族に合流させるのが一番きれい
• (b) ToolboxPictBytesPayload のコメントに「共有後の mutable 化は unsupported」を一行明記

プロセスメモ

runtime パリティ(エミュレータでの PICT 描画)未実施の明示は検証ポリシーに沿っている。追跡先の名指しがあると完璧 — Flow シナリオハーネス構想か #46 に一行で紐づけておくとよい。

[cubenoy22]

System J1-7.1 (MAME)にて3072KB確保した場合に修正前は開けなかった700KBのPICTが修正後は開けるようになっていたことを確認 ✅