From d8725210e7571d97f7646e6ce02b87854bcf35b3 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 23 Jul 2026 18:32:57 -0400 Subject: [PATCH 1/6] =?UTF-8?q?feat(banks):=20pure=20bank=5Fbook=20registr?= =?UTF-8?q?y=20=E2=80=94=20pool=20+=20named=20banks,=20move/copy,=20JSON?= =?UTF-8?q?=20round-trip=20(B1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CLAUDE.md | 4 +- CMakeLists.txt | 19 +- src/bank_book.cpp | 671 +++++++++++++++++++++++++++++++++++++++ src/bank_book.h | 208 ++++++++++++ tests/test_bank_book.cpp | 373 ++++++++++++++++++++++ 5 files changed, 1273 insertions(+), 2 deletions(-) create mode 100644 src/bank_book.cpp create mode 100644 src/bank_book.h create mode 100644 tests/test_bank_book.cpp diff --git a/CLAUDE.md b/CLAUDE.md index f11d23e..d9da0bc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,7 +22,7 @@ Vendors two submodules (see `.gitmodules`): cmake --build build ctest --test-dir build -Seven targets: +Eight targets: | Target | Kind | Purpose | |---|---|---| @@ -32,6 +32,7 @@ Seven targets: | `view_mode_model_tests` | executable | Pure unit tests for `view_mode_model` — no REAPER, no DAW. | | `view_tree_tests` | executable | Pure unit tests for `view_tree` — no REAPER, no DAW. | | `mode_switch_tests` | executable | Pure unit tests for `mode_switch` — no REAPER, no DAW. | +| `bank_book_tests` | executable | Pure unit tests for `bank_book` — no REAPER, no DAW. | | `reaper_reasampler` | loadable module | The actual extension binary (`.dll` / `.dylib` / `.so`). | ### macOS / Linux: SWELL dialog resources @@ -54,6 +55,7 @@ There is no hot-reload. Copy the built binary into REAPER's `UserPlugins/` folde - `view_mode_model` — Design View mode system: mode registry, GUID-keyed membership, folder-tree-aware visibility derivation, snapshot-based park/restore planner, JSON round-trip. Mirror of `bank_model` for the Design View phase. - `view_tree` — pure `I_FOLDERDEPTH`→FolderTree helper for the Design View shell; no REAPER types at the boundary. - `mode_switch` — REAPER-free segment layout + hit-test math for the bank_panel's Design View mode switch; divides a header rectangle into N equal segments and hit-tests a point to a segment. Mirror of `bank_grid`. +- `bank_book` — multi-bank registry (Phase B): an ordered set of banks (pool seeded as bank-zero + named banks), each wrapping a `BankIndex`. Owns create/rename/reorder/delete of named banks, pool privileges (un-deletable/un-renamable/un-evacuable, never zero banks) enforced in-model, active-bank id, index-only move/copy of a sample between banks, JSON round-trip + legacy-`bank_index`→pool migration. Wraps `BankIndex` (bank_model untouched; no `bankId` on `Sample`). **REAPER-facing shells:** - `capture` — `ICaptureBackend` interface; `OfflineRenderBackend` (deterministic default) and `RealtimeRecordBackend`. Input: `CaptureRequest`. Output: finished file + populated `Sample` handed to `bank_model`. diff --git a/CMakeLists.txt b/CMakeLists.txt index 76b76d6..fb7a61d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -108,6 +108,18 @@ add_library(tail_control STATIC src/tail_control.cpp) target_include_directories(tail_control PUBLIC src) target_link_libraries(tail_control PUBLIC render_settings) +# --------------------------------------------------------------------------- +# 2g') Pure bank_book library — NO REAPER, NO SWELL. The multi-bank phase heart +# (Phase B1): an ordered registry of banks (pool seeded as bank-zero + named +# banks), each wrapping a BankIndex; create/rename/reorder/delete named banks, +# pool privileges enforced in-model, active-bank id, index-only move/copy of a +# sample between banks, JSON round-trip + legacy-bank_index→pool migration. +# Mirror of bank_model / view_mode_model; wraps BankIndex (bank_model untouched). +# --------------------------------------------------------------------------- +add_library(bank_book STATIC src/bank_book.cpp) +target_include_directories(bank_book PUBLIC src) +target_link_libraries(bank_book PUBLIC bank_model) + # --------------------------------------------------------------------------- # 2g) Pure realtime_record library — NO REAPER, NO SWELL. The M8 realtime-record # logic: capture scope + FX-tap point -> I_RECMODE / I_RECMODE_FLAGS values, @@ -168,6 +180,10 @@ add_executable(realtime_record_tests tests/test_realtime_record.cpp) target_link_libraries(realtime_record_tests PRIVATE realtime_record) add_test(NAME realtime_record_tests COMMAND realtime_record_tests) +add_executable(bank_book_tests tests/test_bank_book.cpp) +target_link_libraries(bank_book_tests PRIVATE bank_book) +add_test(NAME bank_book_tests COMMAND bank_book_tests) + # --------------------------------------------------------------------------- # 4) The REAPER extension — a loadable module (dlopen'd by REAPER, not linked). # --------------------------------------------------------------------------- @@ -198,8 +214,9 @@ add_library(reaper_reasampler MODULE src/view.cpp src/track_guid.cpp src/actions.cpp + src/bank_book.cpp ) -target_link_libraries(reaper_reasampler PRIVATE bank_model capture_paths peaks bank_grid mode_switch view_mode_model insert_plan render_settings tail_control realtime_record) +target_link_libraries(reaper_reasampler PRIVATE bank_model capture_paths peaks bank_grid mode_switch view_mode_model insert_plan render_settings tail_control realtime_record bank_book) target_include_directories(reaper_reasampler PRIVATE ${SDK_INC} ${WDL_INC}) set_target_properties(reaper_reasampler PROPERTIES PREFIX "" OUTPUT_NAME "reaper_reasampler") diff --git a/src/bank_book.cpp b/src/bank_book.cpp new file mode 100644 index 0000000..9d6df3c --- /dev/null +++ b/src/bank_book.cpp @@ -0,0 +1,671 @@ +#include "bank_book.h" + +#include +#include + +// bank_book implementation. +// +// JSON is hand-rolled and self-contained, matching the house style of bank_model +// and view_mode_model (brief: keep the pure core dependency-free — no third-party +// JSON lib). The book blob nests one bank object per bank, each carrying that +// bank's BankIndex serialized by bank_model's OWN writer (BankIndex::serialize), +// so per-bank sample serialization stays owned by bank_model and is not duplicated +// here. The book writer emits the bank envelope (id / displayName / ordinal) plus a +// raw "index" member whose value is the BankIndex blob verbatim; the parser splits +// the book envelope, then hands each nested index blob straight to +// BankIndex::deserialize. Ints use %d; strings are escaped by writeEscaped. + +namespace reasampler { + +// --------------------------------------------------------------------------- +// BankBook — construction + bank lookup +// --------------------------------------------------------------------------- + +BankBook::BankBook() { + Bank pool; + pool.id = kPoolBankId; + pool.displayName = kPoolBankName; + pool.ordinal = 0; + banks_.push_back(std::move(pool)); + activeBankId_ = kPoolBankId; +} + +Bank* BankBook::bank(const std::string& id) { + for (auto& b : banks_) + if (b.id == id) return &b; + return nullptr; +} + +const Bank* BankBook::bank(const std::string& id) const { + for (const auto& b : banks_) + if (b.id == id) return &b; + return nullptr; +} + +BankIndex* BankBook::index(const std::string& id) { + Bank* b = bank(id); + return b ? &b->index : nullptr; +} + +const BankIndex* BankBook::index(const std::string& id) const { + const Bank* b = bank(id); + return b ? &b->index : nullptr; +} + +Bank& BankBook::pool() { + // The pool is seeded on construction and is un-deletable, so it always exists. + return *bank(kPoolBankId); +} + +const Bank& BankBook::pool() const { + return *bank(kPoolBankId); +} + +// --------------------------------------------------------------------------- +// Ordinal normalization +// --------------------------------------------------------------------------- + +void BankBook::normalizeOrdinals() { + // Stable-sort by ordinal with the pool pinned first, then rewrite ordinals to a + // contiguous 0..N-1. Stability preserves the caller's relative order among banks + // that share (or, after a reorder shuffle, tie on) an ordinal. + std::stable_sort(banks_.begin(), banks_.end(), [](const Bank& a, const Bank& b) { + if (a.isPool() != b.isPool()) return a.isPool(); // pool always first + return a.ordinal < b.ordinal; + }); + for (std::size_t i = 0; i < banks_.size(); ++i) + banks_[i].ordinal = static_cast(i); +} + +// --------------------------------------------------------------------------- +// Bank lifecycle +// --------------------------------------------------------------------------- + +bool BankBook::createBank(const std::string& id, const std::string& displayName) { + if (id.empty()) return false; // ids key the registry + if (id == kPoolBankId) return false; // reserved pool id + if (bank(id) != nullptr) return false; // duplicate + + Bank b; + b.id = id; + b.displayName = displayName; + b.ordinal = static_cast(banks_.size()); // append; normalize compacts it + banks_.push_back(std::move(b)); + normalizeOrdinals(); + return true; +} + +bool BankBook::renameBank(const std::string& id, const std::string& displayName) { + if (id == kPoolBankId) return false; // pool is un-renamable + Bank* b = bank(id); + if (b == nullptr) return false; + b->displayName = displayName; + return true; +} + +bool BankBook::deleteBank(const std::string& id) { + if (id == kPoolBankId) return false; // pool is un-deletable + auto it = std::find_if(banks_.begin(), banks_.end(), + [&](const Bank& b) { return b.id == id; }); + if (it == banks_.end()) return false; + + banks_.erase(it); + // If the active bank was the one deleted, fall back to the pool (invariant: the + // active id always names a live bank). + if (activeBankId_ == id) activeBankId_ = kPoolBankId; + normalizeOrdinals(); + return true; +} + +bool BankBook::reorderBank(const std::string& id, int newOrdinal) { + if (id == kPoolBankId) return false; // pool is pinned at ordinal 0 + if (bank(id) == nullptr) return false; + + // Work on the named banks as an ordered list (banks_ is already ordinal-sorted + // with the pool first, so named banks are banks_[1..]). Pull the target out and + // re-insert it at the requested position, clamped into the named-bank range + // [1..N], then rewrite ordinals contiguously. This is O(N) and obviously correct. + std::vector named; + named.reserve(banks_.size()); + for (auto& b : banks_) + if (!b.isPool()) named.push_back(std::move(b)); + + auto it = std::find_if(named.begin(), named.end(), + [&](const Bank& b) { return b.id == id; }); + Bank moved = std::move(*it); + named.erase(it); + + // Named ordinals are 1..N; convert to a 0-based insertion index into `named`. + const int hi = static_cast(named.size()); // insert-at range is [0..size] + int insertAt = std::max(0, std::min(newOrdinal - 1, hi)); + named.insert(named.begin() + insertAt, std::move(moved)); + + // Rebuild banks_: pool first, then the reordered named banks. Assign ordinals + // directly by position here — NOT via normalizeOrdinals(), whose stable_sort keys + // on the (now stale) ordinals and would undo the reinsertion order. + std::vector rebuilt; + rebuilt.reserve(named.size() + 1); + rebuilt.push_back(std::move(pool())); + for (auto& b : named) rebuilt.push_back(std::move(b)); + banks_ = std::move(rebuilt); + for (std::size_t i = 0; i < banks_.size(); ++i) + banks_[i].ordinal = static_cast(i); + return true; +} + +bool BankBook::evacuate(const std::string& id) { + if (id == kPoolBankId) return false; // pool is un-evacuable (it is the target) + Bank* src = bank(id); + if (src == nullptr) return false; + + // Move every member into the pool, index-only, observing destination collapse. + // Snapshot the members first, then clear the source — BankIndex has no bulk move, + // and adding into the pool must not alias the vector we are draining. + BankIndex& poolIndex = pool().index; + const std::vector members = src->index.all(); // copy + for (const auto& s : members) + poolIndex.add(s); // Added or Collapsed; either way the pool now holds the hash + src->index = BankIndex{}; // leave the evacuated bank empty + return true; +} + +// --------------------------------------------------------------------------- +// Active bank +// --------------------------------------------------------------------------- + +bool BankBook::setActiveBank(const std::string& id) { + if (bank(id) == nullptr) return false; // unknown id never corrupts state + activeBankId_ = id; + return true; +} + +BankIndex& BankBook::activeIndex() { + // activeBankId_ always names a live bank; it falls back to the pool on delete. + return bank(activeBankId_)->index; +} + +const BankIndex& BankBook::activeIndex() const { + return bank(activeBankId_)->index; +} + +// --------------------------------------------------------------------------- +// Sample movement (index-only) +// --------------------------------------------------------------------------- + +namespace { + +// Adds `s` to `dest` and maps the BankIndex outcome onto the transfer outcome for +// the "gained a NEW entry" case (`gained`) vs the collapse case. Rejected outcomes +// (absolute path / empty id) cannot occur here: the sample already passed add() on +// the source side, so its path and id are already valid. +TransferResult applyDestAdd(BankIndex& dest, const Sample& s, TransferResult gained) { + return dest.add(s) == AddResult::Collapsed ? TransferResult::Collapsed : gained; +} + +} // namespace + +TransferResult BankBook::moveSample(const std::string& sampleId, + const std::string& fromBankId, + const std::string& toBankId) { + Bank* from = bank(fromBankId); + Bank* to = bank(toBankId); + if (from == nullptr || to == nullptr) return TransferResult::RejectedUnknownBank; + if (fromBankId == toBankId) return TransferResult::RejectedSameBank; + + const Sample* s = from->index.query(sampleId); + if (s == nullptr) return TransferResult::RejectedSampleAbsent; + + // Copy the sample out before removing it: query returns a pointer into the + // source vector that remove() invalidates. + const Sample moved = *s; + from->index.remove(sampleId); // source loses the entry unconditionally on a move + return applyDestAdd(to->index, moved, TransferResult::Moved); +} + +TransferResult BankBook::copySample(const std::string& sampleId, + const std::string& fromBankId, + const std::string& toBankId) { + Bank* from = bank(fromBankId); + Bank* to = bank(toBankId); + if (from == nullptr || to == nullptr) return TransferResult::RejectedUnknownBank; + if (fromBankId == toBankId) return TransferResult::RejectedSameBank; + + const Sample* s = from->index.query(sampleId); + if (s == nullptr) return TransferResult::RejectedSampleAbsent; + + const Sample copy = *s; // source entry is left intact + return applyDestAdd(to->index, copy, TransferResult::Copied); +} + +// =========================================================================== +// JSON — writer +// =========================================================================== + +namespace { + +void writeEscaped(std::string& out, const std::string& s) { + out += '"'; + for (char c : s) { + switch (c) { + case '"': out += "\\\""; break; + case '\\': out += "\\\\"; break; + case '\b': out += "\\b"; break; + case '\f': out += "\\f"; break; + case '\n': out += "\\n"; break; + case '\r': out += "\\r"; break; + case '\t': out += "\\t"; break; + default: + if (static_cast(c) < 0x20) { + char buf[8]; + std::snprintf(buf, sizeof(buf), "\\u%04x", static_cast(c)); + out += buf; + } else { + out += c; + } + } + } + out += '"'; +} + +std::string intToStr(int v) { + char buf[16]; + std::snprintf(buf, sizeof(buf), "%d", v); + return buf; +} + +class ObjWriter { +public: + explicit ObjWriter(std::string& out) : out_(out) { out_ += '{'; } + ~ObjWriter() { out_ += '}'; } + + void keyRaw(const char* key, const std::string& rawValue) { + sep(); + writeEscaped(out_, key); + out_ += ':'; + out_ += rawValue; + } + void keyStr(const char* key, const std::string& value) { + sep(); + writeEscaped(out_, key); + out_ += ':'; + writeEscaped(out_, value); + } + void keyBegin(const char* key) { + sep(); + writeEscaped(out_, key); + out_ += ':'; + } + +private: + void sep() { if (first_) first_ = false; else out_ += ','; } + std::string& out_; + bool first_ = true; +}; + +} // namespace + +std::string BankBook::serialize() const { + std::string out; + { + ObjWriter root(out); + root.keyRaw("version", intToStr(1)); + root.keyStr("activeBank", activeBankId_); + + // banks: array of { id, displayName, ordinal, index: }. + // The pool rides in as bank-zero, persisted identically to any named bank. + root.keyBegin("banks"); + out += '['; + for (std::size_t i = 0; i < banks_.size(); ++i) { + if (i) out += ','; + ObjWriter b(out); + b.keyStr("id", banks_[i].id); + b.keyStr("displayName", banks_[i].displayName); + b.keyRaw("ordinal", intToStr(banks_[i].ordinal)); + // The nested index is bank_model's own JSON, emitted verbatim so the + // per-sample shape stays owned by BankIndex::serialize (not duplicated). + b.keyRaw("index", banks_[i].index.serialize()); + } + out += ']'; + } // root closes here (see bank_model note on NRVO + deferred close) + return out; +} + +// =========================================================================== +// JSON — parser (recursive descent; std::nullopt on any malformed input, never UB) +// =========================================================================== + +namespace { + +class Parser { +public: + explicit Parser(const std::string& s) : s_(s) {} + + // Parses a book blob into a bank set + active id. On success fills the out-params + // and returns true. Distinguishes the legacy shape (a bare bank_index object: has + // "samples", no "banks") from the book shape (has "banks"): a legacy blob yields a + // single pool bank carrying the migrated index and an empty active id (⇒ pool). The + // member deserialize() adopts the result (ordinal normalize + active resolve). + bool parseBook(std::vector& banks, std::string& activeBank); + +private: + const std::string& s_; + std::size_t pos_ = 0; + + bool eof() const { return pos_ >= s_.size(); } + + void skipWs() { + while (!eof()) { + char c = s_[pos_]; + if (c == ' ' || c == '\t' || c == '\n' || c == '\r') ++pos_; + else break; + } + } + + bool consume(char c) { + skipWs(); + if (eof() || s_[pos_] != c) return false; + ++pos_; + return true; + } + + bool parseString(std::string& out); + bool parseInt(int& out); + bool parseKey(std::string& key); + bool skipValue(); + // Captures the raw source text of one JSON value (object / array / string / + // scalar) verbatim, so a nested BankIndex blob can be handed to its own parser. + bool captureValue(std::string& raw); + + bool parseBank(Bank& out); +}; + +bool Parser::parseString(std::string& out) { + skipWs(); + if (eof() || s_[pos_] != '"') return false; + ++pos_; + out.clear(); + while (!eof()) { + char c = s_[pos_++]; + if (c == '"') return true; + if (c == '\\') { + if (eof()) return false; + char e = s_[pos_++]; + switch (e) { + case '"': out += '"'; break; + case '\\': out += '\\'; break; + case '/': out += '/'; break; + case 'b': out += '\b'; break; + case 'f': out += '\f'; break; + case 'n': out += '\n'; break; + case 'r': out += '\r'; break; + case 't': out += '\t'; break; + case 'u': { + auto readHex4 = [&](unsigned int& cp) -> bool { + if (pos_ + 4 > s_.size()) return false; + cp = 0; + for (int i = 0; i < 4; ++i) { + char h = s_[pos_++]; + cp <<= 4; + if (h >= '0' && h <= '9') cp |= static_cast(h - '0'); + else if (h >= 'a' && h <= 'f') cp |= static_cast(h - 'a' + 10); + else if (h >= 'A' && h <= 'F') cp |= static_cast(h - 'A' + 10); + else return false; + } + return true; + }; + unsigned int hi = 0; + if (!readHex4(hi)) return false; + unsigned int codePoint = hi; + if (hi >= 0xD800 && hi <= 0xDBFF) { + if (pos_ + 6 > s_.size()) return false; + if (s_[pos_] != '\\' || s_[pos_ + 1] != 'u') return false; + pos_ += 2; + unsigned int lo = 0; + if (!readHex4(lo)) return false; + if (lo < 0xDC00 || lo > 0xDFFF) return false; + codePoint = 0x10000 + ((hi - 0xD800) << 10) + (lo - 0xDC00); + } else if (hi >= 0xDC00 && hi <= 0xDFFF) { + return false; // unpaired low surrogate + } + if (codePoint <= 0x7F) { + out += static_cast(codePoint); + } else if (codePoint <= 0x7FF) { + out += static_cast(0xC0 | (codePoint >> 6)); + out += static_cast(0x80 | (codePoint & 0x3F)); + } else if (codePoint <= 0xFFFF) { + out += static_cast(0xE0 | (codePoint >> 12)); + out += static_cast(0x80 | ((codePoint >> 6) & 0x3F)); + out += static_cast(0x80 | (codePoint & 0x3F)); + } else { + out += static_cast(0xF0 | (codePoint >> 18)); + out += static_cast(0x80 | ((codePoint >> 12) & 0x3F)); + out += static_cast(0x80 | ((codePoint >> 6) & 0x3F)); + out += static_cast(0x80 | (codePoint & 0x3F)); + } + break; + } + default: return false; + } + } else { + out += c; + } + } + return false; // unterminated +} + +bool Parser::parseInt(int& out) { + skipWs(); + std::size_t start = pos_; + if (!eof() && (s_[pos_] == '-' || s_[pos_] == '+')) ++pos_; + std::size_t digitsStart = pos_; + while (!eof() && s_[pos_] >= '0' && s_[pos_] <= '9') ++pos_; + if (pos_ == digitsStart) return false; // no digits + long v = 0; + try { + v = std::stol(s_.substr(start, pos_ - start)); + } catch (...) { + return false; // out of long range → malformed + } + if (v < INT_MIN || v > INT_MAX) return false; + out = static_cast(v); + return true; +} + +bool Parser::parseKey(std::string& key) { + if (!parseString(key)) return false; + if (!consume(':')) return false; + return true; +} + +bool Parser::skipValue() { + std::string raw; + return captureValue(raw); +} + +// Records the raw source span of one JSON value starting at the current position +// (after whitespace) so it can be re-parsed by a nested parser. Handles nested +// objects/arrays with string-aware brace matching (braces inside strings ignored). +bool Parser::captureValue(std::string& raw) { + skipWs(); + if (eof()) return false; + std::size_t start = pos_; + char c = s_[pos_]; + if (c == '"') { + std::string tmp; + if (!parseString(tmp)) return false; + raw.assign(s_, start, pos_ - start); + return true; + } + if (c == '{' || c == '[') { + char open = c, close = (c == '{') ? '}' : ']'; + ++pos_; + int depth = 1; + while (!eof() && depth > 0) { + char d = s_[pos_]; + if (d == '"') { + std::string tmp; + if (!parseString(tmp)) return false; // advances past the string + continue; + } + if (d == open) ++depth; + else if (d == close) --depth; + ++pos_; + } + if (depth != 0) return false; + raw.assign(s_, start, pos_ - start); + return true; + } + // bare scalar (number / true / false / null) + while (!eof()) { + char d = s_[pos_]; + if (d == ',' || d == '}' || d == ']' || d == ' ' || d == '\t' || + d == '\n' || d == '\r') + break; + ++pos_; + } + if (pos_ == start) return false; + raw.assign(s_, start, pos_ - start); + return true; +} + +bool Parser::parseBank(Bank& b) { + if (!consume('{')) return false; + skipWs(); + if (consume('}')) return false; // a bank object must at least carry an id + + bool haveId = false; + bool haveIndex = false; + do { + std::string key; + if (!parseKey(key)) return false; + + if (key == "id") { + if (!parseString(b.id)) return false; + haveId = true; + } else if (key == "displayName") { + if (!parseString(b.displayName)) return false; + } else if (key == "ordinal") { + if (!parseInt(b.ordinal)) return false; + } else if (key == "index") { + std::string raw; + if (!captureValue(raw)) return false; + auto idx = BankIndex::deserialize(raw); + if (!idx) return false; // a malformed nested index fails the whole parse + b.index = std::move(*idx); + haveIndex = true; + } else { + if (!skipValue()) return false; // forward-compat unknown keys + } + } while (consume(',')); + + if (!consume('}')) return false; + if (!haveId || b.id.empty()) return false; // id keys the registry + if (!haveIndex) return false; // every bank persists its index + return true; +} + +bool Parser::parseBook(std::vector& banks, std::string& activeBank) { + banks.clear(); + activeBank.clear(); + if (!consume('{')) return false; + skipWs(); + if (consume('}')) return false; // an empty object is neither shape → malformed + + // Decide the shape by which structural key we saw. A "banks" key ⇒ book shape; a + // "samples" key with no "banks" ⇒ legacy shape (promote into the pool). + std::vector parsedBanks; + bool sawBanks = false; + bool sawSamples = false; + + do { + std::string key; + if (!parseKey(key)) return false; + + if (key == "banks") { + sawBanks = true; + if (!consume('[')) return false; + skipWs(); + if (!consume(']')) { + do { + Bank b; + if (!parseBank(b)) return false; + parsedBanks.push_back(std::move(b)); + } while (consume(',')); + if (!consume(']')) return false; + } + } else if (key == "activeBank") { + if (!parseString(activeBank)) return false; + } else if (key == "samples") { + // Legacy marker. The legacy index is re-parsed from the whole input below + // (BankIndex::deserialize owns that shape); here we only skip the value to + // keep the scan well-formed and note that we saw it. + sawSamples = true; + if (!skipValue()) return false; + } else { + if (!skipValue()) return false; // version, or unknown + } + } while (consume(',')); + + if (!consume('}')) return false; + skipWs(); + if (!eof()) return false; // trailing garbage + + // --- Legacy migration: a bare bank_index (samples, no banks) → pool. --- + if (!sawBanks) { + if (!sawSamples) return false; // neither shape's marker → malformed + auto legacy = BankIndex::deserialize(s_); + if (!legacy) return false; + Bank pool; + pool.id = kPoolBankId; + pool.displayName = kPoolBankName; + pool.ordinal = 0; + pool.index = std::move(*legacy); + banks.push_back(std::move(pool)); // { pool } with zero named banks + activeBank.clear(); // ⇒ pool (default) after adoption + return true; + } + + // --- Book shape: the parsed banks ARE the book (pool folded in). --- + // The pool must be present as bank-zero (serialize always emits it). Reject a + // book blob that omits it rather than silently re-seeding — a book without its + // pool is malformed, not a legacy blob. + bool hasPool = std::any_of(parsedBanks.begin(), parsedBanks.end(), + [](const Bank& b) { return b.isPool(); }); + if (!hasPool) return false; + + // Reject duplicate bank ids (ids key the registry; a dup would corrupt lookup). + for (std::size_t i = 0; i < parsedBanks.size(); ++i) + for (std::size_t j = i + 1; j < parsedBanks.size(); ++j) + if (parsedBanks[i].id == parsedBanks[j].id) return false; + + // Force the pool's fixed display name — it is not user-mutable, so we do not + // trust a persisted override for it (keeps kPoolBankName authoritative). + for (auto& b : parsedBanks) + if (b.isPool()) b.displayName = kPoolBankName; + + banks = std::move(parsedBanks); + return true; +} + +} // namespace + +void BankBook::adoptBanks(std::vector&& banks, const std::string& activeBank) { + banks_ = std::move(banks); + normalizeOrdinals(); + // Resolve the active bank defensively: fall back to the pool if the persisted id + // names no bank, so a corrupt active id never leaves a dangling capture target. + activeBankId_ = (bank(activeBank) != nullptr) ? activeBank : std::string(kPoolBankId); +} + +std::optional BankBook::deserialize(const std::string& json) { + std::vector banks; + std::string activeBank; + Parser p(json); + if (!p.parseBook(banks, activeBank)) return std::nullopt; + + BankBook book; + book.adoptBanks(std::move(banks), activeBank); + return book; +} + +} // namespace reasampler diff --git a/src/bank_book.h b/src/bank_book.h new file mode 100644 index 0000000..e4f1ed7 --- /dev/null +++ b/src/bank_book.h @@ -0,0 +1,208 @@ +#pragma once +// bank_book — the pure core of the multi-bank phase (Phase B), deliberately free +// of any REAPER type so it compiles and unit-tests OUTSIDE the DAW. It is the +// third instance of the same "pure registry + JSON round-trip, unit-tested outside +// the DAW" pattern as bank_model and view_mode_model. +// +// PURE MODULE (CLAUDE.md §load-bearing split): NO REAPER types, NO SWELL, NO +// vendor/ includes. Standard library only. +// +// -- What it is -------------------------------------------------------------- +// +// An ordered registry of banks. Each bank = { stable id, display name, ordinal, +// BankIndex }. The book WRAPS N BankIndex instances — bank_model / BankIndex are +// UNTOUCHED (additive: no bankId on Sample). Movement of samples between banks is +// index-only (remove from source's BankIndex, add to destination's); files never +// relocate — banks are logical groupings over one shared file pool. +// +// -- The pool (privileged, not special-cased) -------------------------------- +// +// Structurally the pool is bank-zero — one Bank among many, seeded on construction +// with a fixed id (kPoolBankId) and fixed display name (kPoolBankName), ordinal 0. +// Semantically it is privileged, and the privileges are enforced HERE in the pure +// rules layer (CONTEXT.md §Multi-bank guardrail — not deferred to a shell): +// * always exists (seeded on construction; the book never reaches zero banks) +// * un-deletable (deleteBank rejects the pool) +// * un-renamable (renameBank rejects the pool) +// * un-evacuable (evacuate rejects the pool — the pool is evacuation's +// destination, not a source) +// +// -- Id minting is the CALLER'S job (design decision) ------------------------ +// +// createBank takes a caller-supplied stable id, mirroring bank_model's "id +// assigned by the caller" and view_mode_model's mode ids. The pure core has no +// REAPER genGuid / RNG and deliberately introduces none: a fake in-model id source +// would not be a real GUID anyway, and keeping ids caller-supplied lets the B2 +// shell mint a genuine REAPER GUID while the model stays pure and deterministically +// testable. The model still enforces the invariants: non-empty, unique, not the +// reserved pool id. + +#include +#include +#include + +#include "bank_model.h" + +namespace reasampler { + +// The pool's fixed identity. The id is reserved: createBank rejects it, and the +// pool is always bank-zero. The name is fixed: renameBank rejects the pool. +inline constexpr const char* kPoolBankId = "pool"; +inline constexpr const char* kPoolBankName = "Pool"; + +// One bank: a stable id, a display name, an ordinal (tab/display order), and its +// own BankIndex. The pool is the bank whose id == kPoolBankId. +struct Bank { + std::string id; // stable, persisted; the pool's is kPoolBankId + std::string displayName; // mutable for named banks; fixed "Pool" for the pool + int ordinal = 0; // display order; pool is 0, named banks 1..N + BankIndex index; // this bank's samples + + bool isPool() const { return id == kPoolBankId; } + + bool operator==(const Bank& o) const { + return id == o.id && displayName == o.displayName && + ordinal == o.ordinal && index == o.index; + } +}; + +// Outcome of a cross-bank sample move/copy. Mirrors AddResult's honesty: the op +// reports what happened rather than silently mutating on a bad request. +// - Moved / Copied: the sample was transferred to the destination as a new entry. +// - Collapsed: the destination already held the hash; it collapsed onto the +// existing entry (a no-op add on the destination side). For a +// MOVE the source entry is STILL removed; for a COPY the source +// entry is (as always) retained. +// - RejectedUnknownBank: a source or destination id named no bank. +// - RejectedSampleAbsent: the sample id was not in the source bank. +// - RejectedSameBank: source and destination were the same bank (no-op). +enum class TransferResult { + Moved, + Copied, + Collapsed, + RejectedUnknownBank, + RejectedSampleAbsent, + RejectedSameBank, +}; + +// An ordered registry of banks with the pool seeded as bank-zero, per-bank sample +// indices, an active-bank pointer, and lossless JSON round-trip. The heart of the +// multi-bank phase — mirror of bank_model / view_mode_model. +class BankBook { +public: + BankBook(); // seeds the pool (id kPoolBankId, name kPoolBankName, ordinal 0); + // active bank = pool; zero named banks. + + // -- Bank lifecycle ------------------------------------------------------ + + // Creates a named bank with the caller-supplied stable id and display name, + // assigning the next ordinal. Rejects (returns false, no mutation) an empty id, + // a duplicate id, or the reserved pool id. Display name is not required unique. + bool createBank(const std::string& id, const std::string& displayName); + + // Renames a named bank. Rejects (false, no mutation) an unknown id or the pool. + bool renameBank(const std::string& id, const std::string& displayName); + + // Deletes a NAMED bank, removing it (and its member index entries) from the + // registry. Files are a shell/prune concern and are NOT touched here. Rejects + // (false, no mutation) an unknown id or the pool. Remaining banks' ordinals are + // compacted so the pool stays 0 and named banks stay contiguous 1..N. If the + // deleted bank was active, the active bank falls back to the pool. + bool deleteBank(const std::string& id); + + // Reorders a NAMED bank to `newOrdinal` (clamped into the named-bank range), + // shifting the others to keep ordinals contiguous. The pool is pinned at 0 and + // cannot be reordered. Rejects (false, no mutation) an unknown id or the pool. + bool reorderBank(const std::string& id, int newOrdinal); + + // Moves EVERY member of a named bank into the pool (index-only, observing the + // same destination-collapse-by-hash as a move), leaving the bank empty. Rejects + // (false, no mutation) an unknown id or the pool (the pool is the destination, + // never a source). Returns true on success even if the bank was already empty. + bool evacuate(const std::string& id); + + // -- Active bank --------------------------------------------------------- + + // The active bank's id (the capture target). Defaults to the pool. + const std::string& activeBankId() const { return activeBankId_; } + + // Sets the active bank. Rejects (returns false, no change) an id that names no + // bank — an invalid set never corrupts state. + bool setActiveBank(const std::string& id); + + // The active bank's BankIndex — the index the capture layer adds to. Always + // valid (the active id always names a live bank; it falls back to the pool). + BankIndex& activeIndex(); + const BankIndex& activeIndex() const; + + // -- Sample movement (index-only; files never relocate) ------------------ + + // Moves a sample by id from `fromBankId` to `toBankId`: removes it from the + // source index and adds it to the destination (observing destination + // collapse-by-hash). See TransferResult for the full outcome set. + TransferResult moveSample(const std::string& sampleId, + const std::string& fromBankId, + const std::string& toBankId); + + // Copies a sample by id from `fromBankId` to `toBankId`: the source entry is + // retained, the destination gains it (observing destination collapse-by-hash). + // Same hash may then live in both banks — cross-bank dedup is NOT enforced. + TransferResult copySample(const std::string& sampleId, + const std::string& fromBankId, + const std::string& toBankId); + + // -- Query --------------------------------------------------------------- + + // The bank with `id`, or nullptr. Pointer invalidated by any mutating call. + Bank* bank(const std::string& id); + const Bank* bank(const std::string& id) const; + + // The bank's BankIndex by id, or nullptr. Convenience over bank()->index. + BankIndex* index(const std::string& id); + const BankIndex* index(const std::string& id) const; + + // The pool (always present). Never null. + Bank& pool(); + const Bank& pool() const; + + // All banks in ordinal order (pool first). The pool is always banks()[0]. + const std::vector& banks() const { return banks_; } + + std::size_t size() const { return banks_.size(); } // >= 1 (the pool) + + bool operator==(const BankBook& o) const { + return banks_ == o.banks_ && activeBankId_ == o.activeBankId_; + } + + // -- Persistence --------------------------------------------------------- + + // Serializes the whole book to a JSON string (lossless round-trip): the pool + // folded in as bank-zero + named banks + per-bank indices + ordinals + active + // id. deserialize(serialize(x)) == x. + std::string serialize() const; + + // Parses a book JSON produced by serialize(). std::nullopt on malformed input. + // + // LEGACY MIGRATION: a bare legacy bank_index JSON (the pre-multi-bank shape, an + // object with a "samples" array and no "banks" key) is promoted into the pool's + // index, yielding a book of { pool } with zero named banks — one-way, lossless. + // After migration the book blob is authoritative (the caller persists the book + // shape going forward; the legacy key is retired by the B2 shell). + static std::optional deserialize(const std::string& json); + +private: + std::vector banks_; // ordinal order; banks_[0] is always the pool + std::string activeBankId_; // always names a live bank; defaults to pool + + // Re-sorts banks_ by ordinal (pool pinned first) and rewrites ordinals to a + // contiguous 0..N-1 so the pool is 0 and named banks are 1..N. Called after any + // structural change (create / delete / reorder). + void normalizeOrdinals(); + + // Replaces the book's banks with a parsed set, normalizes ordinals, and resolves + // the active bank (falling back to the pool if the id names no bank). Used only + // by deserialize; kept private so the public surface stays create/rename/etc. + void adoptBanks(std::vector&& banks, const std::string& activeBank); +}; + +} // namespace reasampler diff --git a/tests/test_bank_book.cpp b/tests/test_bank_book.cpp new file mode 100644 index 0000000..d0e7f3e --- /dev/null +++ b/tests/test_bank_book.cpp @@ -0,0 +1,373 @@ +// Standalone tests for reasampler::bank_book — no REAPER, no test framework. +// The heart of the multi-bank phase (Phase B1); the third instance of the pure +// "registry + JSON round-trip, unit-tested outside the DAW" pattern. +// +// Covers (PLAN.md B1 test cases): pool privileges (delete/rename/evacuate rejected, +// never zero banks); create / rename / reorder named banks; move source-loses / +// dest-gains; copy source-retained / dest-gains; evacuate empties source into pool +// with dest collapse; cross-bank same-hash coexistence; destination collapse on +// move/copy into a bank already holding the hash; active-bank get/set (defaults to +// pool, set named, invalid id); JSON round-trip lossless (full book); legacy +// bank_index → pool migration. + +#include "../src/bank_book.h" + +#include +#include + +using namespace reasampler; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +// A minimal, valid sample. `seed` disambiguates id + hash; `hash` overrides the +// content hash so tests can force collapses. relativePath is always relative. +static Sample sampleWith(const std::string& seed, const std::string& hash) { + Sample s; + s.id = "id-" + seed; + s.displayName = "sample " + seed; + s.relativePath = "bank/" + seed + ".wav"; + s.sourceMode = SourceMode::MasterMix; + s.channelCount = 2; + s.sampleRate = 48000; + s.tier = Tier::Scratch; + s.contentHash = hash; + s.createdTimestamp = 1753080000LL; + return s; +} +static Sample sampleWith(const std::string& seed) { return sampleWith(seed, "hash-" + seed); } + +// --------------------------------------------------------------------------- + +static void testPoolSeededAndDefaults() { + BankBook book; + // Pool present as bank-zero with fixed id + name + ordinal 0. + CHECK(book.size() == 1); + CHECK(book.banks()[0].id == kPoolBankId); + CHECK(book.banks()[0].displayName == std::string(kPoolBankName)); + CHECK(book.banks()[0].ordinal == 0); + CHECK(book.pool().id == kPoolBankId); + // Active bank defaults to the pool and resolves the pool's index. + CHECK(book.activeBankId() == std::string(kPoolBankId)); + CHECK(&book.activeIndex() == &book.pool().index); +} + +static void testPoolPrivileges() { + BankBook book; + CHECK(book.createBank("drums", "Drums")); + + // Delete-pool rejected; rename-pool rejected; evacuate-pool rejected. + CHECK(!book.deleteBank(kPoolBankId)); + CHECK(!book.renameBank(kPoolBankId, "NotPool")); + CHECK(!book.evacuate(kPoolBankId)); + CHECK(book.pool().displayName == std::string(kPoolBankName)); // unchanged + + // Reserved pool id cannot be minted as a named bank. + CHECK(!book.createBank(kPoolBankId, "Imposter")); + + // Deleting the only named bank still leaves the pool — never zero banks. + CHECK(book.deleteBank("drums")); + CHECK(book.size() == 1); + CHECK(book.pool().id == kPoolBankId); +} + +static void testCreateRenameReorder() { + BankBook book; + CHECK(book.createBank("a", "Alpha")); + CHECK(book.createBank("b", "Beta")); + CHECK(book.createBank("c", "Gamma")); + CHECK(book.size() == 4); // pool + 3 + + // Duplicate id rejected; empty id rejected. + CHECK(!book.createBank("a", "dup")); + CHECK(!book.createBank("", "empty")); + + // Ordinals: pool 0, named 1..3 in creation order. + CHECK(book.bank("a")->ordinal == 1); + CHECK(book.bank("b")->ordinal == 2); + CHECK(book.bank("c")->ordinal == 3); + + // Rename a named bank; pool rename still rejected. + CHECK(book.renameBank("b", "Beta-renamed")); + CHECK(book.bank("b")->displayName == "Beta-renamed"); + CHECK(!book.renameBank("missing", "x")); + + // Reorder: move "c" to the front of the named region (ordinal 1). + CHECK(book.reorderBank("c", 1)); + CHECK(book.pool().ordinal == 0); + CHECK(book.bank("c")->ordinal == 1); + CHECK(book.bank("a")->ordinal == 2); + CHECK(book.bank("b")->ordinal == 3); + // banks() is ordinal order, pool first. + CHECK(book.banks()[0].id == kPoolBankId); + CHECK(book.banks()[1].id == "c"); + CHECK(book.banks()[2].id == "a"); + CHECK(book.banks()[3].id == "b"); + + // Reorder past the end clamps to the last named slot. + CHECK(book.reorderBank("c", 999)); + CHECK(book.banks()[3].id == "c"); + // Reorder the pool is rejected; unknown id rejected. + CHECK(!book.reorderBank(kPoolBankId, 2)); + CHECK(!book.reorderBank("missing", 1)); +} + +static void testMoveSourceLosesDestGains() { + BankBook book; + CHECK(book.createBank("drums", "Drums")); + CHECK(book.pool().index.add(sampleWith("kick")) == AddResult::Added); + + // Move kick pool -> drums: source loses it, destination gains it. + CHECK(book.moveSample("id-kick", kPoolBankId, "drums") == TransferResult::Moved); + CHECK(book.pool().index.query("id-kick") == nullptr); // source lost it + CHECK(book.bank("drums")->index.query("id-kick") != nullptr); // dest gained it + CHECK(book.pool().index.empty()); + CHECK(book.bank("drums")->index.size() == 1); + + // Rejections: unknown bank, absent sample, same bank. + CHECK(book.moveSample("id-kick", "drums", "nope") == TransferResult::RejectedUnknownBank); + CHECK(book.moveSample("missing", "drums", kPoolBankId) == TransferResult::RejectedSampleAbsent); + CHECK(book.moveSample("id-kick", "drums", "drums") == TransferResult::RejectedSameBank); + // After the rejected ops the sample is still only in drums (state uncorrupted). + CHECK(book.bank("drums")->index.query("id-kick") != nullptr); +} + +static void testCopySourceRetainedDestGains() { + BankBook book; + CHECK(book.createBank("drums", "Drums")); + CHECK(book.pool().index.add(sampleWith("snare")) == AddResult::Added); + + // Copy: source retained, destination gains it — same hash in both banks (no + // cross-bank dedup: that is the point of copy). + CHECK(book.copySample("id-snare", kPoolBankId, "drums") == TransferResult::Copied); + CHECK(book.pool().index.query("id-snare") != nullptr); // source retained + CHECK(book.bank("drums")->index.query("id-snare") != nullptr); // dest gained it + CHECK(book.pool().index.findByHash("hash-snare") != nullptr); + CHECK(book.bank("drums")->index.findByHash("hash-snare") != nullptr); +} + +static void testMoveDestCollapse() { + BankBook book; + CHECK(book.createBank("drums", "Drums")); + // Same hash already in the destination under a DIFFERENT id. + Sample inPool = sampleWith("kick-a", "shared-hash"); + Sample inDrums = sampleWith("kick-b", "shared-hash"); + CHECK(book.pool().index.add(inPool) == AddResult::Added); + CHECK(book.bank("drums")->index.add(inDrums) == AddResult::Added); + + // Move the pool entry into drums: destination collapses onto its existing entry, + // but the source STILL loses the entry (move semantics). + CHECK(book.moveSample("id-kick-a", kPoolBankId, "drums") == TransferResult::Collapsed); + CHECK(book.pool().index.query("id-kick-a") == nullptr); // source lost it + CHECK(book.bank("drums")->index.size() == 1); // no duplicate + CHECK(book.bank("drums")->index.query("id-kick-b") != nullptr);// original kept + CHECK(book.bank("drums")->index.query("id-kick-a") == nullptr);// collapsed away +} + +static void testCopyDestCollapse() { + BankBook book; + CHECK(book.createBank("drums", "Drums")); + Sample inPool = sampleWith("hat-a", "hat-hash"); + Sample inDrums = sampleWith("hat-b", "hat-hash"); + CHECK(book.pool().index.add(inPool) == AddResult::Added); + CHECK(book.bank("drums")->index.add(inDrums) == AddResult::Added); + + // Copy into a bank already holding the hash: collapse; source retained. + CHECK(book.copySample("id-hat-a", kPoolBankId, "drums") == TransferResult::Collapsed); + CHECK(book.pool().index.query("id-hat-a") != nullptr); // source retained + CHECK(book.bank("drums")->index.size() == 1); // collapsed, no dup + CHECK(book.bank("drums")->index.query("id-hat-b") != nullptr); +} + +static void testCrossBankSameHashCoexistence() { + BankBook book; + CHECK(book.createBank("drums", "Drums")); + CHECK(book.createBank("hits", "Hits")); + CHECK(book.pool().index.add(sampleWith("clap", "clap-hash")) == AddResult::Added); + + // Copy the same sample into two named banks — all three banks hold the hash. + CHECK(book.copySample("id-clap", kPoolBankId, "drums") == TransferResult::Copied); + CHECK(book.copySample("id-clap", kPoolBankId, "hits") == TransferResult::Copied); + CHECK(book.pool().index.findByHash("clap-hash") != nullptr); + CHECK(book.bank("drums")->index.findByHash("clap-hash") != nullptr); + CHECK(book.bank("hits")->index.findByHash("clap-hash") != nullptr); +} + +static void testEvacuate() { + BankBook book; + CHECK(book.createBank("drums", "Drums")); + CHECK(book.bank("drums")->index.add(sampleWith("k1")) == AddResult::Added); + CHECK(book.bank("drums")->index.add(sampleWith("k2")) == AddResult::Added); + // A hash that ALSO lives in the pool already, to exercise destination collapse + // during evacuate. + CHECK(book.pool().index.add(sampleWith("dup-pool", "dup-hash")) == AddResult::Added); + CHECK(book.bank("drums")->index.add(sampleWith("dup-drums", "dup-hash")) == AddResult::Added); + + CHECK(book.evacuate("drums")); + // Source emptied. + CHECK(book.bank("drums")->index.empty()); + // Pool gained the two unique members; the dup collapsed onto the pool's existing. + CHECK(book.pool().index.query("id-k1") != nullptr); + CHECK(book.pool().index.query("id-k2") != nullptr); + CHECK(book.pool().index.query("id-dup-pool") != nullptr); // original kept + CHECK(book.pool().index.query("id-dup-drums") == nullptr); // collapsed away + CHECK(book.pool().index.size() == 3); // k1, k2, dup-pool + + // Evacuate an empty bank is a valid no-op success; unknown id rejected. + CHECK(book.evacuate("drums")); + CHECK(!book.evacuate("missing")); +} + +static void testActiveBank() { + BankBook book; + CHECK(book.createBank("drums", "Drums")); + // Defaults to pool. + CHECK(book.activeBankId() == std::string(kPoolBankId)); + + // Set to a named bank; activeIndex resolves it. + CHECK(book.setActiveBank("drums")); + CHECK(book.activeBankId() == "drums"); + CHECK(&book.activeIndex() == &book.bank("drums")->index); + + // Invalid id: rejected, state unchanged. + CHECK(!book.setActiveBank("missing")); + CHECK(book.activeBankId() == "drums"); + + // Deleting the active bank falls back to the pool. + CHECK(book.deleteBank("drums")); + CHECK(book.activeBankId() == std::string(kPoolBankId)); + CHECK(&book.activeIndex() == &book.pool().index); +} + +static void testJsonRoundTripFullBook() { + BankBook book; + CHECK(book.createBank("drums", "Drums \"kit\"\n")); // exercises escaping + CHECK(book.createBank("hits", "One-Shots")); + CHECK(book.setActiveBank("hits")); + + // Populate per-bank indices with distinct + shared-hash samples. + CHECK(book.pool().index.add(sampleWith("p1")) == AddResult::Added); + CHECK(book.bank("drums")->index.add(sampleWith("d1")) == AddResult::Added); + CHECK(book.bank("drums")->index.add(sampleWith("d2")) == AddResult::Added); + CHECK(book.bank("hits")->index.add(sampleWith("h1")) == AddResult::Added); + + std::string json = book.serialize(); + auto back = BankBook::deserialize(json); + CHECK(back.has_value()); + CHECK(back && *back == book); + // String form is idempotent too. + if (back) CHECK(back->serialize() == json); + + // Spot-check the reconstructed structure. + if (back) { + CHECK(back->activeBankId() == "hits"); + CHECK(back->size() == 3); + CHECK(back->bank("drums") != nullptr); + CHECK(back->bank("drums")->displayName == "Drums \"kit\"\n"); + CHECK(back->bank("drums")->index.size() == 2); + CHECK(back->bank("hits")->index.query("id-h1") != nullptr); + CHECK(back->pool().displayName == std::string(kPoolBankName)); + // Ordinals survived: pool 0, then named contiguously. + CHECK(back->banks()[0].ordinal == 0); + CHECK(back->banks()[1].ordinal == 1); + CHECK(back->banks()[2].ordinal == 2); + } +} + +static void testJsonEmptyBookRoundTrip() { + BankBook book; // pool only, empty index, active = pool + std::string json = book.serialize(); + auto back = BankBook::deserialize(json); + CHECK(back.has_value()); + CHECK(back && *back == book); + CHECK(back && back->size() == 1); + CHECK(back && back->pool().index.empty()); +} + +static void testLegacyMigration() { + // A bare legacy bank_index JSON (BankIndex::serialize output — has "samples", no + // "banks") must promote into the pool: a book of { pool } with zero named banks. + BankIndex legacy; + CHECK(legacy.add(sampleWith("old1")) == AddResult::Added); + CHECK(legacy.add(sampleWith("old2")) == AddResult::Added); + std::string legacyJson = legacy.serialize(); + + auto back = BankBook::deserialize(legacyJson); + CHECK(back.has_value()); + if (back) { + CHECK(back->size() == 1); // pool only + CHECK(back->pool().id == kPoolBankId); + CHECK(back->pool().displayName == std::string(kPoolBankName)); + CHECK(back->activeBankId() == std::string(kPoolBankId)); + CHECK(back->pool().index.size() == 2); // samples migrated + CHECK(back->pool().index.query("id-old1") != nullptr); + CHECK(back->pool().index.query("id-old2") != nullptr); + // The migrated index equals the legacy index (lossless). + CHECK(back->pool().index == legacy); + } + + // An EMPTY legacy index ("{\"samples\":[]}" style via serialize) also migrates. + BankIndex emptyLegacy; + auto back2 = BankBook::deserialize(emptyLegacy.serialize()); + CHECK(back2.has_value()); + CHECK(back2 && back2->size() == 1 && back2->pool().index.empty()); +} + +static void testMalformedJson() { + const char* bad[] = { + "", + "{", + "not json", + "{}", // neither shape marker + "{\"banks\":[", // truncated array + "{\"banks\":[{\"id\":\"x\"}]}", // bank missing its index + "{\"banks\":[{\"index\":{\"samples\":[]}}]}", // bank missing its id + "{\"banks\":[{\"id\":\"drums\",\"index\":{\"samples\":[]}}]}", // no pool + "{\"activeBank\":\"pool\"}", // no banks + no samples marker + "{\"banks\":[]}trailing", // trailing garbage + }; + for (const char* j : bad) { + auto r = BankBook::deserialize(j); + CHECK(!r.has_value()); + } + + // Duplicate bank ids are malformed (ids key the registry). + const char* dup = + "{\"activeBank\":\"pool\",\"banks\":[" + "{\"id\":\"pool\",\"displayName\":\"Pool\",\"ordinal\":0,\"index\":{\"samples\":[]}}," + "{\"id\":\"x\",\"displayName\":\"X\",\"ordinal\":1,\"index\":{\"samples\":[]}}," + "{\"id\":\"x\",\"displayName\":\"X2\",\"ordinal\":2,\"index\":{\"samples\":[]}}]}"; + CHECK(!BankBook::deserialize(dup).has_value()); +} + +static void testActiveBankResolveAfterCorruptPersistedId() { + // A book blob whose activeBank names no bank resolves to the pool (defensive). + const char* json = + "{\"activeBank\":\"ghost\",\"banks\":[" + "{\"id\":\"pool\",\"displayName\":\"Pool\",\"ordinal\":0,\"index\":{\"samples\":[]}}]}"; + auto back = BankBook::deserialize(json); + CHECK(back.has_value()); + CHECK(back && back->activeBankId() == std::string(kPoolBankId)); +} + +int main() { + testPoolSeededAndDefaults(); + testPoolPrivileges(); + testCreateRenameReorder(); + testMoveSourceLosesDestGains(); + testCopySourceRetainedDestGains(); + testMoveDestCollapse(); + testCopyDestCollapse(); + testCrossBankSameHashCoexistence(); + testEvacuate(); + testActiveBank(); + testJsonRoundTripFullBook(); + testJsonEmptyBookRoundTrip(); + testLegacyMigration(); + testMalformedJson(); + testActiveBankResolveAfterCorruptPersistedId(); + + if (g_fail == 0) std::printf("All tests passed.\n"); + return g_fail ? 1 : 0; +} From 85993ebc3e8287042b9e62e16280964cbb87f386 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Thu, 23 Jul 2026 20:26:15 -0400 Subject: [PATCH 2/6] feat(persist): persist BankBook under `banks` key, retire legacy bank_index, route capture to active bank (B2) --- src/bank_book.cpp | 20 +++++++++ src/bank_book.h | 16 +++++++ src/main.cpp | 20 +++++---- src/persist.cpp | 92 ++++++++++++++++++++++++++-------------- src/persist.h | 57 ++++++++++++++++++------- tests/test_bank_book.cpp | 68 +++++++++++++++++++++++++++++ 6 files changed, 218 insertions(+), 55 deletions(-) diff --git a/src/bank_book.cpp b/src/bank_book.cpp index 9d6df3c..bec9a03 100644 --- a/src/bank_book.cpp +++ b/src/bank_book.cpp @@ -668,4 +668,24 @@ std::optional BankBook::deserialize(const std::string& json) { return book; } +BankBook BankBook::loadFromPersisted(const std::string& banksJson, + const std::string& legacyJson) { + // Precedence 1: the authoritative `banks` blob. A present-but-malformed blob is + // an error, not an absence — degrade to an empty book rather than falling through + // to a stale legacy key (which would resurrect superseded single-bank state). + if (!banksJson.empty()) { + auto book = deserialize(banksJson); + return book ? std::move(*book) : BankBook{}; + } + // Precedence 2: no `banks` yet, but a legacy `bank_index` — one-way pool migration + // (deserialize's parse-time legacy path promotes it into the pool). A malformed + // legacy blob likewise degrades to empty. + if (!legacyJson.empty()) { + auto book = deserialize(legacyJson); + return book ? std::move(*book) : BankBook{}; + } + // Precedence 3: a brand-new / never-captured project — a fresh empty book. + return BankBook{}; +} + } // namespace reasampler diff --git a/src/bank_book.h b/src/bank_book.h index e4f1ed7..7aeece9 100644 --- a/src/bank_book.h +++ b/src/bank_book.h @@ -190,6 +190,22 @@ public: // shape going forward; the legacy key is retired by the B2 shell). static std::optional deserialize(const std::string& json); + // Resolve a BankBook from the two persisted ext-state values a project may carry: + // the authoritative `banks` blob and the retired-but-possibly-present legacy + // `bank_index` blob. The persist shell (B2) hands both raw strings straight here so + // the load-source decision stays REAPER-free and unit-tested. Precedence: + // 1. non-empty `banksJson` present -> deserialize it (authoritative). If it is + // MALFORMED, do NOT silently fall back to the legacy blob — a corrupt `banks` + // blob is an error, not an absence; return an empty book so a stale legacy key + // can never resurrect a superseded single-bank state over a broken book. + // 2. else non-empty `legacyJson` -> deserialize it (one-way pool migration). + // 3. else (both absent/empty) -> a fresh empty book (pool only). + // Never returns nullopt: an unloadable input degrades to the empty book (matching + // the shell's existing "malformed -> ignore, start empty" behaviour), so the caller + // has one branchless install path. + static BankBook loadFromPersisted(const std::string& banksJson, + const std::string& legacyJson); + private: std::vector banks_; // ordinal order; banks_[0] is always the pool std::string activeBankId_; // always names a live bank; defaults to pool diff --git a/src/main.cpp b/src/main.cpp index 200a11a..176d7e0 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -111,9 +111,10 @@ static int g_cmdCancelRealtime = 0; // The persistence session (M4): owns the in-memory BankIndex and bridges it to // project ext state. A timer tick drives g_session.poll() to detect project -// load / Save-As; capture adds Samples to g_session.bank(); after a capture we -// serialize the bank back into the active project's ext state so it travels with -// the .rpp. Replaces the M3 session-only g_bank. +// load / Save-As; capture adds Samples to g_session.bank() — which (B2) resolves to +// the ACTIVE bank's index inside the session's BankBook; after a capture we serialize +// the book back into the active project's ext state (the `banks` key) so it travels +// with the .rpp. Replaces the M3 session-only g_bank. static reasampler::ReaSamplerSession g_session; // --- M8 in-flight realtime capture (async, timer-driven) -------------------- @@ -133,8 +134,9 @@ static reasampler::RealtimeCaptureHandle g_rtCapture; static ReaProject* g_rtCaptureProject = nullptr; // Commit a finished realtime capture (a Done tick/abort with an Ok result): add the -// Sample to the bank, persist + MarkProjectDirty, log. Shared by the tick-completion -// path and the abort paths. On a non-Ok result, logs the failure only. +// Sample to the ACTIVE bank (g_session.bank() resolves to book.activeIndex() — B2), +// persist + MarkProjectDirty, log. Shared by the tick-completion path and the abort +// paths. On a non-Ok result, logs the failure only. static void CommitRealtimeResult(const reasampler::CaptureResult& res) { if (res.status != reasampler::CaptureStatus::Ok) @@ -547,10 +549,12 @@ static void RunCapture(const reasampler::CaptureActionDef& def) return; } + // Add to the ACTIVE bank: g_session.bank() resolves to book.activeIndex() (B2). reasampler::AddResult added = g_session.bank().add(res.sample); - // Persist the updated bank into the active project's ext state so the capture - // survives Save / close+reopen (M4) and travels with the .rpp. saveToActiveProject - // also calls MarkProjectDirty. Non-destructive: writes only our own ext-state key. + // Persist the updated book into the active project's ext state (the `banks` key) + // so the capture survives Save / close+reopen (M4) and travels with the .rpp. + // saveToActiveProject also clears the retired legacy key and calls MarkProjectDirty. + // Non-destructive: writes only our own ext-state keys. g_session.saveToActiveProject(); std::string log = "ReaSampler: " + res.message + "\n"; diff --git a/src/persist.cpp b/src/persist.cpp index e4c316e..4bb88b8 100644 --- a/src/persist.cpp +++ b/src/persist.cpp @@ -5,11 +5,15 @@ // WITHOUT REAPERAPI_IMPLEMENT — main.cpp is the one TU that defines the API // pointers; here they are extern (CLAUDE.md §contract). // -// Storage: SetProjExtState / GetProjExtState, namespace "reasampler", key -// "bank_index". Ext state is stored INSIDE the .rpp, so the index travels with -// the project automatically (CONTEXT.md §Persistence & paths). The only thing -// that does NOT travel for free is the physical bank folder; on Save-As to a new -// directory we relocate it so the index's relative paths still resolve. +// Storage: SetProjExtState / GetProjExtState, namespace "reasampler". Phase B: the +// whole BankBook (pool as bank-zero + named banks) is written under key "banks" +// (authoritative); the legacy single-bank key "bank_index" is RETIRED — cleared on +// save (SetProjExtState with "" deletes it) and read only once, to migrate a pre- +// multi-bank project's index into the pool. Ext state is stored INSIDE the .rpp, so +// the banks travel with the project automatically (CONTEXT.md §Persistence & paths). +// The only thing that does NOT travel for free is the physical bank folder; on +// Save-As to a new directory we relocate it so the indices' relative paths still +// resolve. // // PROJECT-LOAD / SAVE-AS DETECTION (chosen mechanism): // Driven by REAPER's "timer" register (main.cpp). Each poll() reads the active @@ -174,12 +178,22 @@ void ReaSamplerSession::saveToActiveProject() { if (!proj) return; // no active project — nothing to persist if (rppPath.empty()) return; // unsaved project — no .rpp to store into - const std::string json = bank_.serialize(); + // Phase B: the whole book (pool as bank-zero + named banks) is authoritative and + // rides in the `banks` key. + const std::string banksJson = book_.serialize(); SetProjExtState(static_cast(proj), kProjExtNamespace, - kProjExtIndexKey, json.c_str()); + kProjExtBanksKey, banksJson.c_str()); - // Additive: the Design-View model rides alongside the bank in its own key. - // Independent write — does not disturb the bank_index above. + // Retire the legacy single-bank `bank_index` key: SetProjExtState with an empty + // value DELETES the key (SDK header ~6288: val NULL or "" deletes the data). This + // realizes retirement concretely — after any save, a formerly-legacy project + // carries `banks` and NO `bank_index`, and going forward the legacy key is never + // written. Cheap and idempotent when the key is already absent. + SetProjExtState(static_cast(proj), kProjExtNamespace, + kProjExtIndexKey, ""); + + // Additive: the Design-View model rides alongside the banks in its own key. + // Independent write — does not disturb the `banks` blob above. const std::string viewJson = view_.serialize(); SetProjExtState(static_cast(proj), kProjExtNamespace, kProjExtViewKey, viewJson.c_str()); @@ -226,32 +240,46 @@ void ReaSamplerSession::loadFromProject(void* proj, const std::string& projectDi view_ = loadViewModel(static_cast(proj)); if (!proj) { - bank_ = BankIndex{}; + book_ = BankBook{}; return; } - const std::string json = - getProjExtStateString(static_cast(proj), kProjExtNamespace, - kProjExtIndexKey); - if (json.empty()) { - // No stored index (new or never-captured project) — start empty. - bank_ = BankIndex{}; - return; - } - std::optional loaded = BankIndex::deserialize(json); - if (!loaded) { - ShowConsoleMsg("ReaSampler: stored bank index is malformed — ignoring.\n"); - bank_ = BankIndex{}; - return; - } - bank_ = std::move(*loaded); - // Project-relative resolution is a READ-time concern: the index stores only - // relative paths (invariant), and consumers (M5 panel, M6 insert) resolve - // each entry against the CURRENT project dir via resolveBankFile(projectDir, - // relativePath). We do NOT rewrite the stored paths to absolute here — that - // would break the relative-only invariant and the travel-with-.rpp property. - // projectDir is threaded through for those consumers; nothing to do at load - // time beyond replacing the in-memory bank. + // Read both possible sources: the authoritative `banks` blob and the retired-but- + // possibly-still-present legacy `bank_index`. The precedence + migration decision + // (`banks` wins; else the legacy index migrates into the pool; else an empty book) + // is pure logic; it is inlined here rather than via BankBook::loadFromPersisted only + // so a malformed `banks` blob can be warned on the console (single parse) — a corrupt + // blob must read as "ignored", not silent loss, mirroring the prior malformed-index + // warning. A malformed `banks` degrades to an empty book and does NOT fall back to + // the stale legacy key (which would resurrect superseded single-bank state). + const std::string banksJson = + getProjExtStateString(static_cast(proj), kProjExtNamespace, + kProjExtBanksKey); + if (!banksJson.empty()) { + std::optional loaded = BankBook::deserialize(banksJson); + if (!loaded) { + ShowConsoleMsg("ReaSampler: stored banks are malformed — ignoring.\n"); + book_ = BankBook{}; + } else { + book_ = std::move(*loaded); + } + } else { + // No `banks` yet — fall back to the legacy `bank_index`, migrated into the pool + // by BankBook's parse-time promotion. loadFromPersisted covers the legacy-or- + // empty tail; passing "" for banksJson takes exactly that branch. + const std::string legacyJson = + getProjExtStateString(static_cast(proj), kProjExtNamespace, + kProjExtIndexKey); + book_ = BankBook::loadFromPersisted(std::string{}, legacyJson); + } + + // Project-relative resolution is a READ-time concern: every BankIndex in the book + // stores only relative paths (invariant, enforced per-bank at add()), and consumers + // (M5 panel, M6 insert) resolve each entry against the CURRENT project dir via + // resolveBankFile(projectDir, relativePath). We do NOT rewrite stored paths to + // absolute here — that would break the relative-only invariant and travel-with-.rpp. + // projectDir is threaded through for those consumers; nothing to do at load time + // beyond replacing the in-memory book. (void)projectDir; } diff --git a/src/persist.h b/src/persist.h index f00b271..8db7d1a 100644 --- a/src/persist.h +++ b/src/persist.h @@ -19,6 +19,7 @@ #include +#include "bank_book.h" #include "bank_model.h" #include "view_mode_model.h" @@ -28,10 +29,21 @@ namespace reasampler { // shipped: changing it orphans every already-saved project's index. inline constexpr const char* kProjExtNamespace = "reasampler"; -// The ext-state key the index JSON is stored under (one key holds the whole -// serialized BankIndex). FOREVER-STABLE for the same reason. +// The RETIRED legacy ext-state key: pre-multi-bank projects stored the whole +// serialized BankIndex here (single bank). Phase B2 no longer WRITES it — on save +// the key is cleared (SetProjExtState with "" deletes it) and the book is written +// under kProjExtBanksKey instead. It is still READ once, on load of a legacy +// project, to migrate its single index into the pool (BankBook's parse-time +// promotion). FOREVER-STABLE as a read key for that migration path. inline constexpr const char* kProjExtIndexKey = "bank_index"; +// The multi-bank ext-state key (Phase B): one key holds the whole serialized +// BankBook — the pool folded in as bank-zero plus every named bank, each with its +// own BankIndex, ordinals, and the active-bank id. AUTHORITATIVE going forward; +// supersedes kProjExtIndexKey. FOREVER-STABLE once shipped: changing it orphans +// every already-saved project's banks. +inline constexpr const char* kProjExtBanksKey = "banks"; + // The ext-state key the Design-View ViewModeModel JSON is stored under (one key // holds the whole serialized model: modes + membership + show-both + snapshots + // active mode). Distinct from kProjExtIndexKey — one namespace, two keys. @@ -45,8 +57,9 @@ inline constexpr const char* kProjExtViewKey = "view_state"; // it strands the identity of every already-saved project. See persist.cpp. inline constexpr const char* kProjExtGuidKey = "project_guid"; -// Owns the session's BankIndex and drives persistence against the active REAPER -// project. One instance lives for the extension's lifetime (main.cpp). It tracks +// Owns the session's BankBook (Phase B: pool + named banks) and drives persistence +// against the active REAPER project. One instance lives for the extension's +// lifetime (main.cpp). It tracks // the project identity it last saw so the timer tick can detect a project load // (a different project became active) and a Save-As (SAME project, path changed): // @@ -62,16 +75,28 @@ inline constexpr const char* kProjExtGuidKey = "project_guid"; // W12 defect that stopped the bank reloading); the pointer catches forks (Save-As // copies our GUID onto a distinct object — the W10 defect that clobbered a bank). // -// The bank itself is exposed for the capture/action layer to mutate; persist +// The book itself is exposed for the capture/action layer to mutate; persist // only reads it on save and replaces it on load. class ReaSamplerSession { public: ReaSamplerSession() = default; - // The in-memory bank. The action/capture layer adds captures here; persist - // serializes it on save and replaces it on project load. - BankIndex& bank() { return bank_; } - const BankIndex& bank() const { return bank_; } + // The multi-bank book (Phase B): the pool + named banks, each wrapping a + // BankIndex, plus the active-bank id. The action layer (B3) creates / renames / + // reorders / deletes banks and moves samples here; the panel (B4) reads it; + // persist serializes it under the `banks` key on save and replaces it on load. + BankBook& book() { return book_; } + const BankBook& book() const { return book_; } + + // The capture add-target: the ACTIVE bank's BankIndex (defaults to the pool). + // The capture path adds a captured Sample through this seam, so a capture lands + // in whichever bank is active — the single behavioural change B2 wires in over + // M7/M8 (the capture backends are untouched; only the target index moved). The + // panel/insert readers that displayed the single index continue to read it here + // unchanged; today it resolves to the pool (default active), matching prior + // single-bank behaviour, until B3/B4 let the user switch the active bank. + BankIndex& bank() { return book_.activeIndex(); } + const BankIndex& bank() const { return book_.activeIndex(); } // The in-memory Design-View model. The view/action layer mutates it (tag, // toggle, snapshot); persist serializes it on save and replaces it on project @@ -80,8 +105,9 @@ public: ViewModeModel& view() { return view_; } const ViewModeModel& view() const { return view_; } - // Serialize the current bank to the active project's ext state (namespace - // "reasampler"). Non-destructive beyond writing our own ext-state key. Safe + // Serialize the current book (under the `banks` key) and view model to the active + // project's ext state (namespace "reasampler"), and clear the retired legacy + // `bank_index` key. Non-destructive beyond writing our own ext-state keys. Safe // to call when there is no active/saved project (it no-ops). void saveToActiveProject(); @@ -103,7 +129,7 @@ public: bool consumeLoadSignal(); private: - BankIndex bank_; + BankBook book_; // The Design-View model. Default-constructed = Arrange + Design seeded, active // = Arrange; loadFromProject leaves this default when a project has no stored @@ -124,9 +150,10 @@ private: bool primed_ = false; // false until the first poll() observes state bool loadPending_ = false; // raised by loadFromProject; drained by consumeLoadSignal - // Load the index from the given project's ext state and resolve bank paths - // against projectDir. Replaces the in-memory bank. projectDir empty -> clears - // the bank (unsaved project has no resolvable bank). + // Load the book from the given project's ext state (the `banks` key, else the + // legacy `bank_index` key migrated into the pool) and resolve bank paths against + // projectDir at read time. Replaces the in-memory book. projectDir empty -> the + // book is reset to empty (unsaved project has no resolvable banks). void loadFromProject(void* proj, const std::string& projectDir); }; diff --git a/tests/test_bank_book.cpp b/tests/test_bank_book.cpp index d0e7f3e..f254da5 100644 --- a/tests/test_bank_book.cpp +++ b/tests/test_bank_book.cpp @@ -341,6 +341,70 @@ static void testMalformedJson() { CHECK(!BankBook::deserialize(dup).has_value()); } +// --- B2: persist-load source-precedence decision (pure) -------------------- +// loadFromPersisted picks the load source the persist shell will feed it from the +// two ext-state values a project may carry: the authoritative `banks` blob and the +// retired legacy `bank_index` blob. Precedence: banks > legacy > empty; a malformed +// banks blob degrades to empty WITHOUT falling back to the stale legacy key. + +static void testLoadPrefersBanksBlob() { + // A full book + a stale legacy index both present: `banks` wins, legacy ignored. + BankBook src; + CHECK(src.createBank("drums", "Drums")); + CHECK(src.setActiveBank("drums")); + CHECK(src.bank("drums")->index.add(sampleWith("new1")) == AddResult::Added); + const std::string banksJson = src.serialize(); + + BankIndex stale; + CHECK(stale.add(sampleWith("stale-old")) == AddResult::Added); + const std::string legacyJson = stale.serialize(); + + BankBook loaded = BankBook::loadFromPersisted(banksJson, legacyJson); + // The book equals the source book — the legacy key had NO effect. + CHECK(loaded == src); + CHECK(loaded.activeBankId() == "drums"); + CHECK(loaded.bank("drums") != nullptr); + CHECK(loaded.bank("drums")->index.query("id-new1") != nullptr); + // The stale legacy sample must NOT have leaked into the pool. + CHECK(loaded.pool().index.query("id-stale-old") == nullptr); +} + +static void testLoadMigratesLegacyWhenNoBanks() { + // No `banks` key, a legacy `bank_index` present: migrate into the pool, zero named. + BankIndex legacy; + CHECK(legacy.add(sampleWith("l1")) == AddResult::Added); + CHECK(legacy.add(sampleWith("l2")) == AddResult::Added); + const std::string legacyJson = legacy.serialize(); + + BankBook loaded = BankBook::loadFromPersisted(std::string{}, legacyJson); + CHECK(loaded.size() == 1); // pool only + CHECK(loaded.pool().id == kPoolBankId); + CHECK(loaded.activeBankId() == std::string(kPoolBankId)); + CHECK(loaded.pool().index.size() == 2); + CHECK(loaded.pool().index == legacy); // lossless +} + +static void testLoadEmptyWhenNeither() { + // Both absent: a fresh empty book (pool only, empty index, active = pool). + BankBook loaded = BankBook::loadFromPersisted(std::string{}, std::string{}); + CHECK(loaded == BankBook{}); + CHECK(loaded.size() == 1); + CHECK(loaded.pool().index.empty()); + CHECK(loaded.activeBankId() == std::string(kPoolBankId)); +} + +static void testLoadMalformedBanksDegradesWithoutLegacyFallback() { + // A present-but-malformed `banks` blob must degrade to an empty book and must NOT + // resurrect the stale legacy key (that would revive superseded single-bank state). + BankIndex stale; + CHECK(stale.add(sampleWith("stale")) == AddResult::Added); + const std::string legacyJson = stale.serialize(); + + BankBook loaded = BankBook::loadFromPersisted("{\"banks\":[", legacyJson); + CHECK(loaded == BankBook{}); // empty, NOT the legacy + CHECK(loaded.pool().index.query("id-stale") == nullptr); // legacy did not leak +} + static void testActiveBankResolveAfterCorruptPersistedId() { // A book blob whose activeBank names no bank resolves to the pool (defensive). const char* json = @@ -366,6 +430,10 @@ int main() { testJsonEmptyBookRoundTrip(); testLegacyMigration(); testMalformedJson(); + testLoadPrefersBanksBlob(); + testLoadMigratesLegacyWhenNoBanks(); + testLoadEmptyWhenNeither(); + testLoadMalformedBanksDegradesWithoutLegacyFallback(); testActiveBankResolveAfterCorruptPersistedId(); if (g_fail == 0) std::printf("All tests passed.\n"); From cbfc13c4e0db103a3f5ea9a55138f2077f983872 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 24 Jul 2026 05:35:16 -0400 Subject: [PATCH 3/6] =?UTF-8?q?feat(banks):=20bindable=20multi-bank=20acti?= =?UTF-8?q?on=20family=20=E2=80=94=20create/rename/delete/evacuate/activat?= =?UTF-8?q?e/move/copy=20+=20full-height=20toggles=20(B3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/actions.cpp | 361 ++++++++++++++++++++++++++++++++++++++- src/actions.h | 27 +++ src/bank_book.cpp | 16 ++ src/bank_book.h | 13 ++ src/bank_panel.cpp | 31 ++++ src/bank_panel.h | 30 ++++ src/main.cpp | 10 ++ tests/test_bank_book.cpp | 59 +++++++ 8 files changed, 546 insertions(+), 1 deletion(-) diff --git a/src/actions.cpp b/src/actions.cpp index efb961f..2a36c0e 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -24,7 +24,9 @@ #include #include -#include "persist.h" // ReaSamplerSession (owns view() model) +#include "bank_book.h" // BankBook, nextBankId, TransferResult, kPoolBankId (B1) +#include "bank_panel.h" // selection seam + full-height toggles (B3/B4) +#include "persist.h" // ReaSamplerSession (owns book() + view() model) #include "track_guid.h" // shared MediaTrack* -> canonical GUID key #include "view.h" // applyMode (D2 shell) #include "view_mode_model.h" @@ -37,6 +39,10 @@ #define REAPERAPI_WANT_EnumProjects #define REAPERAPI_WANT_Main_SaveProject #define REAPERAPI_WANT_ShowConsoleMsg +#define REAPERAPI_WANT_GetUserInputs +#define REAPERAPI_WANT_ShowMessageBox +#define REAPERAPI_WANT_genGuid +#define REAPERAPI_WANT_guidToString #include "reaper_plugin_functions.h" namespace reasampler { @@ -271,4 +277,357 @@ void designViewUnregisterActions(reaper_plugin_info_t* rec) { g_session = nullptr; } +// =========================================================================== +// Multi-bank action family (Phase B3) +// =========================================================================== +// +// Each action drives the B1 model on g_session->book() and persists via +// g_session->saveToActiveProject() so the change travels with the .rpp — exactly as +// the capture path persists a new Sample (main.cpp RunCapture). The book's rules +// (pool privileges, collapse-by-hash, active-fallback-to-pool) all live in bank_book; +// these handlers only call the model and react to the boolean / TransferResult. +// +// REFERENCE-INVALIDATION GUARDRAIL (B2 review): book().activeIndex() / bank()->index +// return a reference INTO the book's internal vector, which a create/delete can +// reallocate. No handler here caches a BankIndex& (or a Bank*) across a structural +// mutation — each resolves ids to strings up front and re-resolves after any +// create/delete. Move/copy pass ids (not references) straight to moveSample/copySample. + +namespace { + +// FOREVER-STABLE multi-bank action-id strings. Same CEREBELLUM_REASAMPLER_ family +// prefix; each is minted into a persistent command id user keybindings key off — +// NEVER change these after ship. +constexpr const char* kIdBankCreate = "CEREBELLUM_REASAMPLER_BANK_CREATE"; +constexpr const char* kIdBankRename = "CEREBELLUM_REASAMPLER_BANK_RENAME"; +constexpr const char* kIdBankDelete = "CEREBELLUM_REASAMPLER_BANK_DELETE"; +constexpr const char* kIdBankEvacuate = "CEREBELLUM_REASAMPLER_BANK_EVACUATE"; +constexpr const char* kIdBankActivateNext = "CEREBELLUM_REASAMPLER_BANK_ACTIVATE_NEXT"; +constexpr const char* kIdBankActivatePool = "CEREBELLUM_REASAMPLER_BANK_ACTIVATE_POOL"; +constexpr const char* kIdBankMoveSel = "CEREBELLUM_REASAMPLER_BANK_MOVE_SELECTED"; +constexpr const char* kIdBankCopySel = "CEREBELLUM_REASAMPLER_BANK_COPY_SELECTED"; +constexpr const char* kIdBankPoolFull = "CEREBELLUM_REASAMPLER_BANK_POOL_FULLHEIGHT"; +constexpr const char* kIdBankBanksFull = "CEREBELLUM_REASAMPLER_BANK_BANKS_FULLHEIGHT"; + +int g_cmdBankCreate = 0; +int g_cmdBankRename = 0; +int g_cmdBankDelete = 0; +int g_cmdBankEvacuate = 0; +int g_cmdBankActivateNext = 0; +int g_cmdBankActivatePool = 0; +int g_cmdBankMoveSel = 0; +int g_cmdBankCopySel = 0; +int g_cmdBankPoolFull = 0; +int g_cmdBankBanksFull = 0; + +gaccel_register_t g_accelBankCreate{}; +gaccel_register_t g_accelBankRename{}; +gaccel_register_t g_accelBankDelete{}; +gaccel_register_t g_accelBankEvacuate{}; +gaccel_register_t g_accelBankActivateNext{}; +gaccel_register_t g_accelBankActivatePool{}; +gaccel_register_t g_accelBankMoveSel{}; +gaccel_register_t g_accelBankCopySel{}; +gaccel_register_t g_accelBankPoolFull{}; +gaccel_register_t g_accelBankBanksFull{}; + +// Persists the book after a bank mutation. Mirrors the capture path (main.cpp +// RunCapture): a bank change is held in-session and written to the active project's +// ext state so it travels with the .rpp. No Save-As prompt here — saveToActiveProject +// no-ops on an unsaved project (the change stays valid for the session and persists +// on the user's next save), matching how capture persists. +void persistBook() { g_session->saveToActiveProject(); } + +// Prompts the user for a single line of text via REAPER's stock input dialog. +// GetUserInputs(title, num_inputs=1, captions_csv, retvals_csv, sz) -> false on +// cancel (SDK ~3808). `initial` pre-fills the field. Returns false (leaving `out` +// untouched) on cancel or an empty entry. Self-contained bindable-action name entry; +// B4's panel affordances supersede this with in-panel editing. +bool promptText(const char* title, const char* caption, const std::string& initial, + std::string& out) { + std::vector buf(512, '\0'); + // Pre-fill: GetUserInputs seeds the field from the retvals buffer's initial value. + std::snprintf(buf.data(), buf.size(), "%s", initial.c_str()); + if (!GetUserInputs(title, 1, caption, buf.data(), static_cast(buf.size()))) + return false; // user cancelled + std::string s(buf.data()); + if (s.empty()) return false; // an empty name is not a valid bank name + out = std::move(s); + return true; +} + +// Mints a fresh, genuine REAPER GUID string as a stable bank id (the B2/model design: +// ids are caller-supplied and stable; the model stays pure and mints none). Distinct +// from a track GUID by origin only — both are canonical guidToString output. +std::string mintBankId() { + GUID g{}; + genGuid(&g); + char buf[64] = {0}; // guidToString needs >=64 chars (SDK contract) + guidToString(&g, buf); + return std::string(buf); +} + +// Resolves a user-typed bank reference (a display name) to a bank id, scanning the +// book's banks in ordinal order. Case-sensitive exact match on displayName; "Pool" +// resolves the pool. Returns "" when no bank carries that name. Kept in the action +// layer (not the model) — it is UI name-resolution, not a model rule. +std::string bankIdByDisplayName(const std::string& name) { + for (const Bank& b : g_session->book().banks()) + if (b.displayName == name) return b.id; + return {}; +} + +// -- Action bodies --------------------------------------------------------- + +// Create a named bank: prompt for a display name, mint a stable GUID id, create it in +// the model, persist. The new bank is NOT auto-activated (create and activate are +// distinct acts — mirrors capture/placement separation). A duplicate-name is allowed +// (display names are not unique in the model); the fresh GUID keeps the id unique. +void doBankCreate() { + std::string name; + if (!promptText("ReaSampler: create bank", "Bank name:", "", name)) return; + const std::string id = mintBankId(); + if (!g_session->book().createBank(id, name)) { + ShowConsoleMsg("ReaSampler: could not create bank (id collision — try again).\n"); + return; + } + persistBook(); + ShowConsoleMsg(("ReaSampler: created bank \"" + name + "\".\n").c_str()); +} + +// Rename a bank: prompt for which bank (by current display name) and the new name. +// The pool is un-renamable (the model rejects it). Two prompts keep the bindable form +// self-contained; B4's panel renames in place on a tab. +void doBankRename() { + std::string which; + if (!promptText("ReaSampler: rename bank", "Bank to rename (current name):", "", + which)) + return; + const std::string id = bankIdByDisplayName(which); + if (id.empty()) { + ShowConsoleMsg(("ReaSampler: no bank named \"" + which + "\".\n").c_str()); + return; + } + std::string newName; + if (!promptText("ReaSampler: rename bank", "New name:", which, newName)) return; + if (!g_session->book().renameBank(id, newName)) { + ShowConsoleMsg("ReaSampler: cannot rename that bank (the pool is un-renamable).\n"); + return; + } + persistBook(); + ShowConsoleMsg(("ReaSampler: renamed \"" + which + "\" -> \"" + newName + "\".\n") + .c_str()); +} + +// Delete a named bank. Bindable safe-form of the confirm-on-non-empty guardrail: +// prompt for the bank; if it holds members, a YESNO ShowMessageBox names evacuate as +// the alternative before dropping them (a plain delete orphans those members' files +// until prune — CONTEXT.md §delete). An empty bank deletes with no prompt. The richer +// panel confirm (naming evacuate inline, with a one-click evacuate) arrives in B4. +void doBankDelete() { + std::string which; + if (!promptText("ReaSampler: delete bank", "Bank to delete:", "", which)) return; + const std::string id = bankIdByDisplayName(which); + if (id.empty()) { + ShowConsoleMsg(("ReaSampler: no bank named \"" + which + "\".\n").c_str()); + return; + } + // Read member count BEFORE deleting (the Bank* is invalidated by deleteBank; we do + // not cache it — resolve size to an int up front). + const Bank* b = g_session->book().bank(id); + if (!b) return; // race-safe: id resolved above but re-check + const std::size_t members = b->index.size(); + if (members > 0) { + const std::string msg = + "\"" + which + "\" holds " + std::to_string(members) + + (members == 1 ? " sample" : " samples") + + ".\n\nDeleting drops them from every bank (their files are NOT deleted, " + "but no bank will reference them until prune).\n\nTo keep the samples, " + "cancel and Evacuate the bank to the pool first.\n\nDelete anyway?"; + const int r = ShowMessageBox(msg.c_str(), "ReaSampler: delete non-empty bank", 4); + if (r != 6) return; // 6 == YES; anything else cancels (SDK ~6544) + } + if (!g_session->book().deleteBank(id)) { + ShowConsoleMsg("ReaSampler: cannot delete that bank (the pool is un-deletable).\n"); + return; + } + persistBook(); + ShowConsoleMsg(("ReaSampler: deleted bank \"" + which + "\".\n").c_str()); +} + +// Evacuate a named bank: move every member back to the pool (index-only, collapse by +// hash), leaving the bank empty. The pool is un-evacuable (the model rejects it). The +// intended "keep the samples" companion to delete. +void doBankEvacuate() { + std::string which; + if (!promptText("ReaSampler: evacuate bank", "Bank to evacuate to the pool:", "", + which)) + return; + const std::string id = bankIdByDisplayName(which); + if (id.empty()) { + ShowConsoleMsg(("ReaSampler: no bank named \"" + which + "\".\n").c_str()); + return; + } + if (!g_session->book().evacuate(id)) { + ShowConsoleMsg("ReaSampler: cannot evacuate that bank (the pool is the " + "destination, not a source).\n"); + return; + } + persistBook(); + ShowConsoleMsg(("ReaSampler: evacuated \"" + which + "\" to the pool.\n").c_str()); +} + +// Cycle the active bank forward in ordinal order (pool -> named -> ... -> pool), +// via the pure nextBankId helper. Activating a bank changes the CAPTURE TARGET (the +// next capture lands in the newly-active bank — B2's book().activeIndex() seam) and +// never touches the timeline. Persist so the active id travels with the .rpp. +void doBankActivateNext() { + std::vector ids; + ids.reserve(g_session->book().size()); + for (const Bank& b : g_session->book().banks()) ids.push_back(b.id); + const std::string target = nextBankId(ids, g_session->book().activeBankId()); + if (target.empty()) return; // degenerate (no banks) — cannot happen (pool seeded) + if (!g_session->book().setActiveBank(target)) return; + persistBook(); + const Bank* b = g_session->book().bank(target); + ShowConsoleMsg(("ReaSampler: active bank -> \"" + + (b ? b->displayName : target) + "\".\n") + .c_str()); +} + +// Activate the pool directly (the common "back to the default target" jump). Bindable +// direct-by-id form; a general activate-bank-by-name/menu is a B4 affordance. +void doBankActivatePool() { + if (!g_session->book().setActiveBank(kPoolBankId)) return; + persistBook(); + ShowConsoleMsg("ReaSampler: active bank -> \"Pool\".\n"); +} + +// Move or copy the panel's selected samples from the ACTIVE bank into a named +// destination bank (prompted by display name). The panel grid shows the active bank, +// so its selection ids are members of the active bank — that is the source. Both are +// index-only (files never relocate); move removes the source entry, copy retains it; +// both observe destination collapse-by-hash (bank_book). B4's "move to bank" menu will +// drive moveSample/copySample directly with a menu-chosen destination — this bindable +// form is the same operation with a text-prompt destination. +void doBankTransferSelected(bool copy) { + const std::vector selected = bankPanelSelectedSampleIds(); + if (selected.empty()) { + ShowConsoleMsg("ReaSampler: nothing selected in the bank panel to " + "move/copy.\n"); + return; + } + const char* verb = copy ? "copy" : "move"; + const std::string title = std::string("ReaSampler: ") + verb + " selected samples"; + std::string destName; + if (!promptText(title.c_str(), "Destination bank:", "", destName)) return; + const std::string destId = bankIdByDisplayName(destName); + if (destId.empty()) { + ShowConsoleMsg(("ReaSampler: no bank named \"" + destName + "\".\n").c_str()); + return; + } + // Source = the active bank (what the panel grid shows). Pass ids by value — no + // BankIndex& is cached across the loop's mutations. + const std::string srcId = g_session->book().activeBankId(); + if (srcId == destId) { + ShowConsoleMsg("ReaSampler: source and destination are the same bank.\n"); + return; + } + + int ok = 0, collapsed = 0, absent = 0; + for (const std::string& sampleId : selected) { + const TransferResult r = + copy ? g_session->book().copySample(sampleId, srcId, destId) + : g_session->book().moveSample(sampleId, srcId, destId); + switch (r) { + case TransferResult::Moved: + case TransferResult::Copied: ++ok; break; + case TransferResult::Collapsed: ++collapsed; break; + case TransferResult::RejectedSampleAbsent: ++absent; break; + // Unknown-bank / same-bank are pre-checked above; treat defensively as no-ops. + case TransferResult::RejectedUnknownBank: + case TransferResult::RejectedSameBank: break; + } + } + persistBook(); + std::string log = std::string("ReaSampler: ") + verb + " -> \"" + destName + + "\": " + std::to_string(ok) + " " + verb + "d"; + if (collapsed) log += ", " + std::to_string(collapsed) + " collapsed on hash"; + if (absent) log += ", " + std::to_string(absent) + " no longer present"; + log += ".\n"; + ShowConsoleMsg(log.c_str()); +} + +} // namespace + +void bankRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session) { + g_session = session; // shared with the Design View family; same live session + + g_cmdBankCreate = registerAction(rec, kIdBankCreate, g_accelBankCreate, + "ReaSampler: create bank"); + g_cmdBankRename = registerAction(rec, kIdBankRename, g_accelBankRename, + "ReaSampler: rename bank"); + g_cmdBankDelete = registerAction(rec, kIdBankDelete, g_accelBankDelete, + "ReaSampler: delete bank"); + g_cmdBankEvacuate = registerAction(rec, kIdBankEvacuate, g_accelBankEvacuate, + "ReaSampler: evacuate bank to pool"); + g_cmdBankActivateNext = registerAction(rec, kIdBankActivateNext, g_accelBankActivateNext, + "ReaSampler: activate next bank (cycle)"); + g_cmdBankActivatePool = registerAction(rec, kIdBankActivatePool, g_accelBankActivatePool, + "ReaSampler: activate pool"); + g_cmdBankMoveSel = registerAction(rec, kIdBankMoveSel, g_accelBankMoveSel, + "ReaSampler: move selected samples to bank"); + g_cmdBankCopySel = registerAction(rec, kIdBankCopySel, g_accelBankCopySel, + "ReaSampler: copy selected samples to bank"); + g_cmdBankPoolFull = registerAction(rec, kIdBankPoolFull, g_accelBankPoolFull, + "ReaSampler: toggle pool full-height"); + g_cmdBankBanksFull = registerAction(rec, kIdBankBanksFull, g_accelBankBanksFull, + "ReaSampler: toggle banks full-height"); +} + +bool bankHandleCommand(int command) { + if (command == 0 || !g_session) return false; + + if (command == g_cmdBankCreate) { doBankCreate(); return true; } + if (command == g_cmdBankRename) { doBankRename(); return true; } + if (command == g_cmdBankDelete) { doBankDelete(); return true; } + if (command == g_cmdBankEvacuate) { doBankEvacuate(); return true; } + if (command == g_cmdBankActivateNext) { doBankActivateNext(); return true; } + if (command == g_cmdBankActivatePool) { doBankActivatePool(); return true; } + if (command == g_cmdBankMoveSel) { doBankTransferSelected(false); return true; } + if (command == g_cmdBankCopySel) { doBankTransferSelected(true); return true; } + if (command == g_cmdBankPoolFull) { bankPanelToggledPoolFullHeight(); return true; } + if (command == g_cmdBankBanksFull) { bankPanelToggledBanksFullHeight(); return true; } + + return false; // not ours — caller's hookcommand keeps looking +} + +void bankUnregisterActions(reaper_plugin_info_t* rec) { + // Mirror-unregister with '-'-prefixed strings, reverse of registration order. + rec->Register("-gaccel", (void*)&g_accelBankBanksFull); + rec->Register("-command_id", (void*)kIdBankBanksFull); + rec->Register("-gaccel", (void*)&g_accelBankPoolFull); + rec->Register("-command_id", (void*)kIdBankPoolFull); + rec->Register("-gaccel", (void*)&g_accelBankCopySel); + rec->Register("-command_id", (void*)kIdBankCopySel); + rec->Register("-gaccel", (void*)&g_accelBankMoveSel); + rec->Register("-command_id", (void*)kIdBankMoveSel); + rec->Register("-gaccel", (void*)&g_accelBankActivatePool); + rec->Register("-command_id", (void*)kIdBankActivatePool); + rec->Register("-gaccel", (void*)&g_accelBankActivateNext); + rec->Register("-command_id", (void*)kIdBankActivateNext); + rec->Register("-gaccel", (void*)&g_accelBankEvacuate); + rec->Register("-command_id", (void*)kIdBankEvacuate); + rec->Register("-gaccel", (void*)&g_accelBankDelete); + rec->Register("-command_id", (void*)kIdBankDelete); + rec->Register("-gaccel", (void*)&g_accelBankRename); + rec->Register("-command_id", (void*)kIdBankRename); + rec->Register("-gaccel", (void*)&g_accelBankCreate); + rec->Register("-command_id", (void*)kIdBankCreate); + + // g_session is shared with the Design View family; designViewUnregisterActions + // also nulls it. Nulling twice is harmless. Leave it to whichever runs last. + g_session = nullptr; +} + } // namespace reasampler diff --git a/src/actions.h b/src/actions.h index 4b8d382..44bf442 100644 --- a/src/actions.h +++ b/src/actions.h @@ -39,4 +39,31 @@ bool designViewHandleCommand(int command); // '-'-prefixed strings (per the contract's unload rule). Call once on rec==nullptr. void designViewUnregisterActions(reaper_plugin_info_t* rec); +// --- Multi-bank action family (Phase B3) ----------------------------------- +// The bindable action set that drives the multi-bank workflow: create / rename / +// delete / evacuate a bank, activate a bank (direct pool/design-free + cycle), move / +// copy the panel's selected samples into a bank, and the two vertical-split +// full-height toggles. Every mutating action drives the B1 model on +// g_session.book() and persists via g_session.saveToActiveProject() so the change +// travels with the .rpp; the toggles flip the B4-rendered layout bit on the panel. +// +// Same registration/routing/unload contract as the Design View family above and the +// same shared g_session. Kept a distinct trio (not folded into the Design View one) +// because the two families are orthogonal pillars — but they share the single +// hookcommand main.cpp owns; each family's Handle claims only its own ids. + +// Registers the multi-bank family against `rec`. `session` is the live session (must +// outlive registration). Call exactly once at load. Shares g_session with the Design +// View family — pass the SAME session pointer. +void bankRegisterActions(reaper_plugin_info_t* rec, ReaSamplerSession* session); + +// Services one fired command for the multi-bank family. True iff it was one of this +// family's ids (and handled); false otherwise so the caller's hookcommand keeps +// looking. Safe for any command. +bool bankHandleCommand(int command); + +// Mirror-unregisters the multi-bank family with '-'-prefixed strings. Call once on +// rec==nullptr (before g_session is torn down). +void bankUnregisterActions(reaper_plugin_info_t* rec); + } // namespace reasampler diff --git a/src/bank_book.cpp b/src/bank_book.cpp index bec9a03..4fc4901 100644 --- a/src/bank_book.cpp +++ b/src/bank_book.cpp @@ -668,6 +668,22 @@ std::optional BankBook::deserialize(const std::string& json) { return book; } +// --------------------------------------------------------------------------- +// Active-bank cycle ordering (pure, free function — mirror of nextModeId) +// --------------------------------------------------------------------------- + +std::string nextBankId(const std::vector& orderedBankIds, + const std::string& currentBankId) { + if (orderedBankIds.empty()) return {}; // nothing to cycle to + for (std::size_t i = 0; i < orderedBankIds.size(); ++i) { + if (orderedBankIds[i] == currentBankId) + return orderedBankIds[(i + 1) % orderedBankIds.size()]; // wrap past the last + } + // Active id not in the list (stale/unknown) — jump to the first id as a sane + // home rather than returning "" (matches nextModeId's fallback). + return orderedBankIds.front(); +} + BankBook BankBook::loadFromPersisted(const std::string& banksJson, const std::string& legacyJson) { // Precedence 1: the authoritative `banks` blob. A present-but-malformed blob is diff --git a/src/bank_book.h b/src/bank_book.h index 7aeece9..5b9b8e7 100644 --- a/src/bank_book.h +++ b/src/bank_book.h @@ -221,4 +221,17 @@ private: void adoptBanks(std::vector&& banks, const std::string& activeBank); }; +// The next bank id to activate when cycling the active bank forward, in ordinal +// order (the ids arrive pool-first, named 1..N, matching banks()). Wraps: the id +// after the last returns the first (pool → named → … → pool). This is the pure +// decision behind the "cycle active bank" action — the shell reads the book's +// ordered bank ids + current active id, asks for the next, and activates it. +// * empty list -> "" (nothing to cycle to) +// * single id (pool-only) -> that id (a one-bank book stays put) +// * currentBankId not present -> the first id (a sane home to jump to) +// Exposed as a free function (not a BankBook member) so it is unit-testable against +// a bare id vector without a full book. Mirror of view_mode_model's nextModeId. +std::string nextBankId(const std::vector& orderedBankIds, + const std::string& currentBankId); + } // namespace reasampler diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 1b3a9e0..965c909 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -194,6 +194,13 @@ struct PanelState { // persistence is a noted follow-on. Mutated ONLY by a click in the footer strip. TailSetting tail; + // --- Vertical-split full-height layout (Phase B3) ------------------------- + // Which region(s) the vertical split shows: both (Split, default), pool only, + // or named-banks only. B3 actions flip it (bankPanelToggled*FullHeight); B4's + // panel renders from it. In-memory only (a UI-layout preference, not project + // state — it must not travel with the .rpp); resets to Split on unload. + BankPanelFullHeight fullHeight = BankPanelFullHeight::Split; + // --- Audition preview (Wave B) -------------------------------------------- // // The stock preview register we hand to PlayPreview/StopPreview. Its cs/mutex @@ -999,6 +1006,30 @@ TailSetting bankPanelTailSetting() { return s; } +BankPanelFullHeight bankPanelFullHeight() { + // In-memory for the extension's lifetime (g_panel is static), like the tail + // setting: survives panel open/close and bank changes, resets to Split on unload. + return g_panel.fullHeight; +} + +// Shared toggle body: enter `target` from any other state, or fall back to Split when +// already at `target` (a second press restores the split). Requests a repaint via the +// same InvalidateRect the refresh path uses, so an open panel reflects the change; a +// closed panel (hwnd null) simply stores the bit for B4 to render when it opens. +static void setFullHeight(BankPanelFullHeight target) { + g_panel.fullHeight = + (g_panel.fullHeight == target) ? BankPanelFullHeight::Split : target; + if (g_panel.hwnd) InvalidateRect(g_panel.hwnd, nullptr, FALSE); +} + +void bankPanelToggledPoolFullHeight() { + setFullHeight(BankPanelFullHeight::PoolOnly); +} + +void bankPanelToggledBanksFullHeight() { + setFullHeight(BankPanelFullHeight::BanksOnly); +} + void bankPanelShutdown() { closePanel(); // stops audition + destroys the window deinitPreview(); // destroy the preview lock (after the last stop) diff --git a/src/bank_panel.h b/src/bank_panel.h index 6e38528..95405a8 100644 --- a/src/bank_panel.h +++ b/src/bank_panel.h @@ -61,6 +61,36 @@ void bankPanelRefresh(); // state only; the toggle is mutated by a click inside the panel, never here. TailSetting bankPanelTailSetting(); +// The vertical-split full-height layout state (Phase B). The bank window splits +// vertically — pool on top, named-banks region below — and two toggles collapse the +// split: pool full-height (hide the named-banks region) and banks full-height (hide +// the pool). The two are mutually exclusive with the default (both regions shown), +// so one enum captures the whole state. +// +// This bit is B3-owned (the actions flip it); B4's panel RENDERS from it. It lives +// here beside the tail setting — the other session-level view-layout bit the panel +// reads — NOT in the persisted ReaSamplerSession: it is a UI-layout preference, not +// project state, so it must not travel with the .rpp. In-memory for the extension's +// lifetime; resets to Split on unload. +enum class BankPanelFullHeight { + Split, // default: pool region on top, named-banks region below + PoolOnly, // pool full-height — named-banks region hidden + BanksOnly, // banks full-height — pool region hidden +}; + +// The current full-height layout state (default Split). READ by B4's panel to decide +// which region(s) to draw. Safe before the panel has ever opened. +BankPanelFullHeight bankPanelFullHeight(); + +// Toggles pool full-height: Split <-> PoolOnly. From PoolOnly returns to Split; from +// either other state (Split or BanksOnly) enters PoolOnly. Bound to the "pool +// full-height" action. Requests a repaint so an open panel reflects the change. +void bankPanelToggledPoolFullHeight(); + +// Toggles banks full-height: Split <-> BanksOnly, symmetric to the pool toggle. +// Bound to the "banks full-height" action. Requests a repaint. +void bankPanelToggledBanksFullHeight(); + // Tears the panel down on extension unload: destroys the window and releases any // cached thumbnails / PCM handles. Mirror of bankPanelInit; safe if never opened. void bankPanelShutdown(); diff --git a/src/main.cpp b/src/main.cpp index 176d7e0..3e51bac 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -714,6 +714,8 @@ static bool OnHookCommand(int command, int /*flag*/) // Design View action family (D4). Claims only its own ids; returns false for the // rest so this hook keeps looking (per the contract). if (reasampler::designViewHandleCommand(command)) return true; + // Multi-bank action family (B3). Same contract: claims only its own ids. + if (reasampler::bankHandleCommand(command)) return true; return false; } @@ -761,6 +763,8 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT( // Tear down the Design View action family (D4) — mirror-unregisters each // gaccel + command_id with '-'-prefixed strings. After the hook is gone. reasampler::designViewUnregisterActions(g_rec); + // Tear down the multi-bank action family (B3) — same mirror-unregister. + reasampler::bankUnregisterActions(g_rec); g_rec->Register("-gaccel", (void*)&g_accelCancelRealtime); g_rec->Register("-command_id", (void*)(REASAMPLER_ACTION_PREFIX "CANCEL_REALTIME_CAPTURE")); @@ -910,6 +914,12 @@ extern "C" REAPER_PLUGIN_DLL_EXPORT int REAPER_PLUGIN_ENTRYPOINT( // the hook so every id is minted first. reasampler::designViewRegisterActions(rec, &g_session); + // Register the multi-bank action family (B3): create/rename/delete/evacuate bank, + // activate (cycle + pool), move/copy selected samples to a bank, and the two + // full-height layout toggles. Shares g_session with the Design View family; routed + // by the same hookcommand via bankHandleCommand. Registered before the hook. + reasampler::bankRegisterActions(rec, &g_session); + // One hookcommand routes every ReaSampler action (spike + toggle + Design View). // Registered once, after all command ids are minted. rec->Register("hookcommand", (void*)&OnHookCommand); diff --git a/tests/test_bank_book.cpp b/tests/test_bank_book.cpp index f254da5..24e3788 100644 --- a/tests/test_bank_book.cpp +++ b/tests/test_bank_book.cpp @@ -405,6 +405,60 @@ static void testLoadMalformedBanksDegradesWithoutLegacyFallback() { CHECK(loaded.pool().index.query("id-stale") == nullptr); // legacy did not leak } +// --- B3: active-bank cycle ordering (pure free function) ------------------- +// nextBankId(orderedIds, current) is the pure decision behind the "cycle active +// bank" action: given the book's ordered bank ids (pool-first) + the current active +// id, return the next id in ordinal order, wrapping pool -> named -> ... -> pool. + +static void testCycleOrderingWrapAround() { + // pool -> drums -> hits -> (wrap) pool. Exercises every step + the wrap. + const std::vector ids = {kPoolBankId, "drums", "hits"}; + CHECK(nextBankId(ids, kPoolBankId) == "drums"); + CHECK(nextBankId(ids, "drums") == "hits"); + CHECK(nextBankId(ids, "hits") == std::string(kPoolBankId)); // wrap past the last +} + +static void testCyclePoolOnlyStaysPool() { + // A pool-only book (no named banks) cycles to itself — the single id wraps to + // itself. The action becomes a no-op activation, which is correct. + const std::vector ids = {kPoolBankId}; + CHECK(nextBankId(ids, kPoolBankId) == std::string(kPoolBankId)); +} + +static void testCycleUnknownActiveResolvesToFirst() { + // A stale/unknown active id (e.g. the active bank was just deleted and the + // ordered list already dropped it) resolves to the first id — a sane home to jump + // to rather than "" — matching nextModeId's fallback. + const std::vector ids = {kPoolBankId, "drums"}; + CHECK(nextBankId(ids, "ghost") == std::string(kPoolBankId)); +} + +static void testCycleEmptyListYieldsEmpty() { + // Degenerate guard: an empty list has nothing to cycle to. (A real BankBook always + // seeds the pool, so this cannot arise from the book — but the pure helper must not + // index into an empty vector.) + const std::vector ids; + CHECK(nextBankId(ids, kPoolBankId).empty()); +} + +static void testCycleMatchesBookOrdinalOrder() { + // Integration-flavoured but still pure: drive the cycle off a real book's banks() + // order and confirm one full loop lands back on the pool, activating each bank in + // ordinal order. This is exactly what the action does (build ids from banks(), + // call nextBankId, setActiveBank). + BankBook book; + CHECK(book.createBank("a", "A")); + CHECK(book.createBank("b", "B")); // ordinals: pool 0, a 1, b 2 + + std::vector ids; + for (const Bank& bk : book.banks()) ids.push_back(bk.id); + + std::string cur = book.activeBankId(); // pool + cur = nextBankId(ids, cur); CHECK(cur == "a"); + cur = nextBankId(ids, cur); CHECK(cur == "b"); + cur = nextBankId(ids, cur); CHECK(cur == std::string(kPoolBankId)); // full loop +} + static void testActiveBankResolveAfterCorruptPersistedId() { // A book blob whose activeBank names no bank resolves to the pool (defensive). const char* json = @@ -435,6 +489,11 @@ int main() { testLoadEmptyWhenNeither(); testLoadMalformedBanksDegradesWithoutLegacyFallback(); testActiveBankResolveAfterCorruptPersistedId(); + testCycleOrderingWrapAround(); + testCyclePoolOnlyStaysPool(); + testCycleUnknownActiveResolvesToFirst(); + testCycleEmptyListYieldsEmpty(); + testCycleMatchesBookOrdinalOrder(); if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; From ccf65415c94db3b774b065e58e80e63388e9ce1a Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 25 Jul 2026 01:29:47 -0400 Subject: [PATCH 4/6] fix(banks): enforce unique bank display names in-model + B3 review minors --- CONTEXT.md | 12 +++++++-- docs/product/multi-bank.md | 13 ++++++--- src/actions.cpp | 54 +++++++++++++++++++++++++++++--------- src/bank_book.cpp | 46 +++++++++++++++++++++++++++++++- src/bank_book.h | 13 +++++++-- tests/test_bank_book.cpp | 53 +++++++++++++++++++++++++++++++++++++ 6 files changed, 169 insertions(+), 22 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 465d863..c97e120 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -547,7 +547,13 @@ arrange; the only change is *which* index the entry lands in. display name, ordinal, BankIndex }`. **`BankIndex` is untouched** — the multi-bank layer wraps it, it does not modify it (additive; no `bank-id` field on `Sample`). Bank id is the stable key (GUID-style, minted on bank create); display name and - ordinal are mutable (rename / reorder). The pool is the first, seeded, fixed-id + ordinal are mutable (rename / reorder). **Display names are unique**, enforced in the + pure model on create and rename: `createBank` / `renameBank` reject a name that + duplicates an existing bank's (renaming a bank to its own current name is a no-op + success). The comparison is **trimmed + case-insensitive (ASCII)**, so "Drums", + "drums", and " Drums " cannot coexist; the pool's reserved name "Pool" is protected + by the same check. Uniqueness makes by-name resolution in the action shell + unambiguous by construction. The pool is the first, seeded, fixed-id member. `bank_book` is the mirror of `bank_model` and `view_mode_model`: pure, no REAPER types, unit-tested outside the DAW, JSON round-trip. - **Active bank lives in the model, routes through the capture path.** `bank_book` @@ -637,7 +643,9 @@ Pure (no REAPER types, unit-tested — the mirror of `bank_model` / `view_mode_m - `bank_book` — ordered bank registry (`{ bank id, display name, ordinal, BankIndex }`); pool seeded with fixed id + name; create / rename / reorder / delete named banks (pool-privilege rules enforced here: reject delete/rename of - pool; delete drops member index entries); **evacuate** a bank (move every member to + pool; delete drops member index entries; **display names unique** — create/rename + reject a name that duplicates another bank's, trimmed + case-insensitive, "Pool" + protected); **evacuate** a bank (move every member to the pool, index-only, destination-collapse observed; pool cannot be evacuated); active-bank id (get/set, defaults to pool); **move** and **copy** a sample between banks (index-only, destination-collapse observed); query a bank's index; JSON diff --git a/docs/product/multi-bank.md b/docs/product/multi-bank.md index 9f7bc63..e8702da 100644 --- a/docs/product/multi-bank.md +++ b/docs/product/multi-bank.md @@ -90,9 +90,12 @@ reasons: So: `bank_book` is an ordered registry of `{ bank id, display name, ordinal, BankIndex }`, pool seeded as bank-zero. Bank id is the stable key (minted GUID-style -on create); name and ordinal are mutable. `BankIndex` is untouched. This is the -defer-the-feature, design-the-seam principle: the seam is a container above the -tested core, not a modification of it. +on create); name and ordinal are mutable. Display names are **unique** — two banks +cannot share a name (compared trimmed + case-insensitively, so "Drums" and "drums" +are the same name), enforced in the model on create and rename; the pool's "Pool" is +reserved by the same rule. `BankIndex` is untouched. This is the defer-the-feature, +design-the-seam principle: the seam is a container above the tested core, not a +modification of it. --- @@ -349,7 +352,9 @@ Mirrors the capture and Design View pillars exactly. - Ordered bank registry: `{ bank id, display name, ordinal, BankIndex }`; pool seeded with fixed id + fixed name. - Create / rename / reorder / delete named banks; pool-privilege rules enforced - here (reject delete-pool, reject rename-pool, never zero banks). + here (reject delete-pool, reject rename-pool, never zero banks). Display names are + unique — create/rename reject a name already used by another bank (trimmed + + case-insensitive; the pool's "Pool" is protected). - Active-bank id (get/set, defaults to pool); resolve the active bank's `BankIndex`. - Move / copy a sample between banks — index-only, destination collapse-by-hash observed, move removes the source entry. diff --git a/src/actions.cpp b/src/actions.cpp index 2a36c0e..24bf7ab 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -331,11 +331,14 @@ gaccel_register_t g_accelBankCopySel{}; gaccel_register_t g_accelBankPoolFull{}; gaccel_register_t g_accelBankBanksFull{}; -// Persists the book after a bank mutation. Mirrors the capture path (main.cpp -// RunCapture): a bank change is held in-session and written to the active project's -// ext state so it travels with the .rpp. No Save-As prompt here — saveToActiveProject -// no-ops on an unsaved project (the change stays valid for the session and persists -// on the user's next save), matching how capture persists. +// Persists the book after a bank mutation. Mirrors the CAPTURE path (main.cpp +// RunCapture), NOT the Design-View path: a bank change is held in-session and written +// to the active project's ext state so it travels with the .rpp. Deliberately no +// Save-As prompt — saveToActiveProject no-ops on an unsaved project (the change stays +// valid for the session and persists on the user's next save), exactly as capture +// persists. This is an intentional divergence from persistViewState (above), which +// DOES prompt Save-As on an unsaved project; do not "align" the two — a bank mutation +// follows capture's quiet-persist idiom, a Design-View mutation follows the prompt idiom. void persistBook() { g_session->saveToActiveProject(); } // Prompts the user for a single line of text via REAPER's stock input dialog. @@ -343,12 +346,21 @@ void persistBook() { g_session->saveToActiveProject(); } // cancel (SDK ~3808). `initial` pre-fills the field. Returns false (leaving `out` // untouched) on cancel or an empty entry. Self-contained bindable-action name entry; // B4's panel affordances supersede this with in-panel editing. +// +// COMMA GUARD: GetUserInputs splits the returned values on a separator that defaults +// to ',', so a bank name containing a comma would be truncated at the comma. We +// override the return separator to \x1f (ASCII unit separator, un-typeable in the +// dialog) via the documented `separator=X` extra caption field (SDK ~3806), so any +// printable name — commas included — round-trips whole. The captions_csv itself stays +// comma-joined: the single field caption, then the `separator=` directive as a +// trailing pseudo-caption (the directive redefines only the RETURN separator). bool promptText(const char* title, const char* caption, const std::string& initial, std::string& out) { std::vector buf(512, '\0'); // Pre-fill: GetUserInputs seeds the field from the retvals buffer's initial value. std::snprintf(buf.data(), buf.size(), "%s", initial.c_str()); - if (!GetUserInputs(title, 1, caption, buf.data(), static_cast(buf.size()))) + const std::string captions = std::string(caption) + ",separator=\x1f"; + if (!GetUserInputs(title, 1, captions.c_str(), buf.data(), static_cast(buf.size()))) return false; // user cancelled std::string s(buf.data()); if (s.empty()) return false; // an empty name is not a valid bank name @@ -368,9 +380,11 @@ std::string mintBankId() { } // Resolves a user-typed bank reference (a display name) to a bank id, scanning the -// book's banks in ordinal order. Case-sensitive exact match on displayName; "Pool" -// resolves the pool. Returns "" when no bank carries that name. Kept in the action -// layer (not the model) — it is UI name-resolution, not a model rule. +// book's banks in ordinal order. Exact match on displayName; "Pool" resolves the pool. +// Returns "" when no bank carries that name. Kept in the action layer (not the model) +// — it is UI name-resolution, not a model rule. First-match is unambiguous BY +// CONSTRUCTION: the model enforces unique display names (trimmed + case-insensitive), +// so at most one bank can carry a given name — no duplicate can shadow another here. std::string bankIdByDisplayName(const std::string& name) { for (const Bank& b : g_session->book().banks()) if (b.displayName == name) return b.id; @@ -381,14 +395,18 @@ std::string bankIdByDisplayName(const std::string& name) { // Create a named bank: prompt for a display name, mint a stable GUID id, create it in // the model, persist. The new bank is NOT auto-activated (create and activate are -// distinct acts — mirrors capture/placement separation). A duplicate-name is allowed -// (display names are not unique in the model); the fresh GUID keeps the id unique. +// distinct acts — mirrors capture/placement separation). The model rejects a display +// name that duplicates an existing bank's (trimmed + case-insensitive, incl. "Pool"); +// the create then fails and the user is told the name is taken. void doBankCreate() { std::string name; if (!promptText("ReaSampler: create bank", "Bank name:", "", name)) return; const std::string id = mintBankId(); if (!g_session->book().createBank(id, name)) { - ShowConsoleMsg("ReaSampler: could not create bank (id collision — try again).\n"); + ShowConsoleMsg( + ("ReaSampler: could not create bank \"" + name + + "\" (a bank with that name already exists).\n") + .c_str()); return; } persistBook(); @@ -411,7 +429,10 @@ void doBankRename() { std::string newName; if (!promptText("ReaSampler: rename bank", "New name:", which, newName)) return; if (!g_session->book().renameBank(id, newName)) { - ShowConsoleMsg("ReaSampler: cannot rename that bank (the pool is un-renamable).\n"); + // renameBank rejects the pool (un-renamable) or a name already used by another + // bank (unique display names, trimmed + case-insensitive). + ShowConsoleMsg("ReaSampler: cannot rename that bank (the pool is un-renamable, " + "or another bank already uses that name).\n"); return; } persistBook(); @@ -432,6 +453,13 @@ void doBankDelete() { ShowConsoleMsg(("ReaSampler: no bank named \"" + which + "\".\n").c_str()); return; } + // Pool early-out: the pool is un-deletable (the model rejects it). Catch it here, + // BEFORE the non-empty confirm, so typing "Pool" never shows a misleading + // "delete anyway?" prompt for an operation the model will refuse regardless. + if (id == kPoolBankId) { + ShowConsoleMsg("ReaSampler: the pool cannot be deleted.\n"); + return; + } // Read member count BEFORE deleting (the Bank* is invalidated by deleteBank; we do // not cache it — resolve size to an int up front). const Bank* b = g_session->book().bank(id); diff --git a/src/bank_book.cpp b/src/bank_book.cpp index 4fc4901..8f35d1d 100644 --- a/src/bank_book.cpp +++ b/src/bank_book.cpp @@ -77,6 +77,43 @@ void BankBook::normalizeOrdinals() { banks_[i].ordinal = static_cast(i); } +// --------------------------------------------------------------------------- +// Display-name uniqueness (trimmed + case-insensitive, ASCII) +// --------------------------------------------------------------------------- + +namespace { + +// Folds a display name to its uniqueness key: strip leading/trailing ASCII +// whitespace, lower-case ASCII letters. So "Drums", "drums", and " Drums " share one +// key and cannot coexist. ASCII-only by design — the pure core carries no locale +// facility and must not grow one; bank names are short user labels, not full Unicode +// case-folding candidates. +std::string nameKey(const std::string& s) { + std::size_t b = 0, e = s.size(); + auto isWs = [](char c) { return c == ' ' || c == '\t' || c == '\n' || c == '\r'; }; + while (b < e && isWs(s[b])) ++b; + while (e > b && isWs(s[e - 1])) --e; + std::string out; + out.reserve(e - b); + for (std::size_t i = b; i < e; ++i) { + char c = s[i]; + if (c >= 'A' && c <= 'Z') c = static_cast(c - 'A' + 'a'); + out += c; + } + return out; +} + +} // namespace + +// True if any bank OTHER than `exceptId` already carries `name`'s uniqueness key. The +// exception lets renameBank accept a bank keeping (or re-casing/-spacing) its own name. +bool BankBook::displayNameTaken(const std::string& name, const std::string& exceptId) const { + const std::string key = nameKey(name); + for (const auto& b : banks_) + if (b.id != exceptId && nameKey(b.displayName) == key) return true; + return false; +} + // --------------------------------------------------------------------------- // Bank lifecycle // --------------------------------------------------------------------------- @@ -84,7 +121,10 @@ void BankBook::normalizeOrdinals() { bool BankBook::createBank(const std::string& id, const std::string& displayName) { if (id.empty()) return false; // ids key the registry if (id == kPoolBankId) return false; // reserved pool id - if (bank(id) != nullptr) return false; // duplicate + if (bank(id) != nullptr) return false; // duplicate id + // Display names are unique (trimmed + case-insensitive); the pool's "Pool" is a + // reserved name and is caught here like any other collision. + if (displayNameTaken(displayName, /*exceptId=*/id)) return false; Bank b; b.id = id; @@ -99,6 +139,10 @@ bool BankBook::renameBank(const std::string& id, const std::string& displayName) if (id == kPoolBankId) return false; // pool is un-renamable Bank* b = bank(id); if (b == nullptr) return false; + // Reject a name already used by a DIFFERENT bank. Renaming a bank to its own + // current name (or a case/space variant of it) is a no-op success, not a + // rejection — exceptId=id excludes the bank itself from the collision scan. + if (displayNameTaken(displayName, /*exceptId=*/id)) return false; b->displayName = displayName; return true; } diff --git a/src/bank_book.h b/src/bank_book.h index 5b9b8e7..2a6084b 100644 --- a/src/bank_book.h +++ b/src/bank_book.h @@ -97,10 +97,14 @@ public: // Creates a named bank with the caller-supplied stable id and display name, // assigning the next ordinal. Rejects (returns false, no mutation) an empty id, - // a duplicate id, or the reserved pool id. Display name is not required unique. + // a duplicate id, the reserved pool id, or a display name that duplicates an + // existing bank's name (including the pool's "Pool"). Display-name uniqueness is + // trimmed + case-insensitive (ASCII): "Drums", "drums", and " Drums " collide. bool createBank(const std::string& id, const std::string& displayName); - // Renames a named bank. Rejects (false, no mutation) an unknown id or the pool. + // Renames a named bank. Rejects (false, no mutation) an unknown id, the pool, or a + // target name already used by a DIFFERENT bank (trimmed + case-insensitive, as + // createBank). Renaming a bank to its own current name is a no-op success. bool renameBank(const std::string& id, const std::string& displayName); // Deletes a NAMED bank, removing it (and its member index entries) from the @@ -210,6 +214,11 @@ private: std::vector banks_; // ordinal order; banks_[0] is always the pool std::string activeBankId_; // always names a live bank; defaults to pool + // True if a bank OTHER than `exceptId` already carries `name`'s uniqueness key + // (trimmed + case-insensitive, ASCII). Backs the create/rename uniqueness check; + // pass exceptId=id to let a bank keep (or re-case/-space) its own name. + bool displayNameTaken(const std::string& name, const std::string& exceptId) const; + // Re-sorts banks_ by ordinal (pool pinned first) and rewrites ordinals to a // contiguous 0..N-1 so the pool is 0 and named banks are 1..N. Called after any // structural change (create / delete / reorder). diff --git a/tests/test_bank_book.cpp b/tests/test_bank_book.cpp index 24e3788..5c96e22 100644 --- a/tests/test_bank_book.cpp +++ b/tests/test_bank_book.cpp @@ -92,6 +92,8 @@ static void testCreateRenameReorder() { CHECK(book.renameBank("b", "Beta-renamed")); CHECK(book.bank("b")->displayName == "Beta-renamed"); CHECK(!book.renameBank("missing", "x")); + // Renaming back to a non-colliding name keeps working. + CHECK(book.renameBank("b", "Beta")); // Reorder: move "c" to the front of the named region (ordinal 1). CHECK(book.reorderBank("c", 1)); @@ -113,6 +115,56 @@ static void testCreateRenameReorder() { CHECK(!book.reorderBank("missing", 1)); } +static void testDisplayNameUniqueness() { + BankBook book; + CHECK(book.createBank("a", "Drums")); + + // A unique name is accepted. + CHECK(book.createBank("b", "Bass")); + + // Exact duplicate rejected, no mutation (size unchanged, the collided id absent). + CHECK(!book.createBank("c", "Drums")); + CHECK(book.bank("c") == nullptr); + CHECK(book.size() == 3); // pool + a + b only + + // Trimmed + case-insensitive collisions: "drums", " Drums ", "DRUMS" all collide. + CHECK(!book.createBank("c", "drums")); + CHECK(!book.createBank("c", " Drums ")); + CHECK(!book.createBank("c", "DRUMS")); + CHECK(book.bank("c") == nullptr); + + // The pool's reserved name "Pool" (and its variants) cannot be taken by a new bank. + CHECK(!book.createBank("c", "Pool")); + CHECK(!book.createBank("c", " pool ")); + CHECK(book.bank("c") == nullptr); + + // -- renameBank uniqueness -------------------------------------------------- + // Rename to a name used by ANOTHER bank is rejected (no mutation). + CHECK(!book.renameBank("b", "Drums")); + CHECK(book.bank("b")->displayName == "Bass"); // unchanged + CHECK(!book.renameBank("b", "drums")); // case-insensitive collision too + CHECK(!book.renameBank("b", " Drums ")); // trimmed collision too + + // Renaming a bank to its OWN current name is a no-op success (not a rejection). + CHECK(book.renameBank("a", "Drums")); + CHECK(book.bank("a")->displayName == "Drums"); + // Re-casing/-spacing its own name is likewise allowed (it collides only with self). + CHECK(book.renameBank("a", " drums ")); + CHECK(book.bank("a")->displayName == " drums "); + + // Renaming to the pool's reserved name is rejected (pool is the "other" bank here). + CHECK(!book.renameBank("b", "Pool")); + CHECK(book.bank("b")->displayName == "Bass"); + + // A genuinely fresh unique name still renames fine. + CHECK(book.renameBank("b", "Low End")); + CHECK(book.bank("b")->displayName == "Low End"); + + // After the rejections, the previously-freed name is now reusable by a new bank. + CHECK(book.createBank("c", "Bass")); + CHECK(book.bank("c")->displayName == "Bass"); +} + static void testMoveSourceLosesDestGains() { BankBook book; CHECK(book.createBank("drums", "Drums")); @@ -473,6 +525,7 @@ int main() { testPoolSeededAndDefaults(); testPoolPrivileges(); testCreateRenameReorder(); + testDisplayNameUniqueness(); testMoveSourceLosesDestGains(); testCopySourceRetainedDestGains(); testMoveDestCollapse(); From a67a2f9479756eb890b49cbb721a4e96e8e2bcc8 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 25 Jul 2026 14:37:57 -0400 Subject: [PATCH 5/6] =?UTF-8?q?feat(bank=5Fpanel):=20B4=20vertical-split?= =?UTF-8?q?=20UI=20=E2=80=94=20LICE=20tab=20strip,=20id-keyed=20bank=20ops?= =?UTF-8?q?,=20move/copy=20drag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pool grid on top, LICE-drawn named-banks tab strip below with overflow-scroll (new pure tab_strip seam, unit-tested). Full-height toggles, unmistakable active-bank readout distinct from the shown tab, tab context menu (activate/rename/delete/evacuate/create) with rich confirm-on-non-empty-delete, and move/copy via menu + drag with drop-highlighting. Fold-in: deserialize auto-disambiguates duplicate folded bank names instead of rejecting the book. --- CMakeLists.txt | 18 +- src/actions.cpp | 17 +- src/bank_book.cpp | 43 ++ src/bank_panel.cpp | 1556 +++++++++++++++++++++++++++----------- src/bank_panel.h | 15 + src/insert.cpp | 8 +- src/tab_strip.cpp | 112 +++ src/tab_strip.h | 135 ++++ tests/test_bank_book.cpp | 120 +++ tests/test_tab_strip.cpp | 202 +++++ 10 files changed, 1769 insertions(+), 457 deletions(-) create mode 100644 src/tab_strip.cpp create mode 100644 src/tab_strip.h create mode 100644 tests/test_tab_strip.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index fb7a61d..662266a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -55,6 +55,17 @@ target_include_directories(bank_grid PUBLIC src) add_library(mode_switch STATIC src/mode_switch.cpp) target_include_directories(mode_switch PUBLIC src) +# --------------------------------------------------------------------------- +# 2c'') Pure tab_strip layout — NO REAPER, NO SWELL. The named-banks tab-strip +# geometry (B4): strip rect + N tabs at a fixed tab width + scroll offset -> +# per-tab rects (overflow-clipped), overflow chevron reservation + maxScroll, +# and point -> tab / chevron hit-test. Split out so the strip's layout + +# overflow/scroll math is unit-tested outside the DAW; the bank_panel region +# that draws it and routes clicks is DAW-verified. Mirror of mode_switch. +# --------------------------------------------------------------------------- +add_library(tab_strip STATIC src/tab_strip.cpp) +target_include_directories(tab_strip PUBLIC src) + # --------------------------------------------------------------------------- # 2d) Pure view_mode_model library — NO REAPER, NO SWELL. The Design View heart # (Phase D1): mode registry + GUID-keyed membership index + folder-tree-aware @@ -156,6 +167,10 @@ add_executable(mode_switch_tests tests/test_mode_switch.cpp) target_link_libraries(mode_switch_tests PRIVATE mode_switch) add_test(NAME mode_switch_tests COMMAND mode_switch_tests) +add_executable(tab_strip_tests tests/test_tab_strip.cpp) +target_link_libraries(tab_strip_tests PRIVATE tab_strip) +add_test(NAME tab_strip_tests COMMAND tab_strip_tests) + add_executable(view_mode_model_tests tests/test_view_mode_model.cpp) target_link_libraries(view_mode_model_tests PRIVATE view_mode_model) add_test(NAME view_mode_model_tests COMMAND view_mode_model_tests) @@ -206,6 +221,7 @@ add_library(reaper_reasampler MODULE src/persist.cpp src/bank_panel.cpp src/mode_switch.cpp + src/tab_strip.cpp src/insert.cpp src/insert_plan.cpp ${LICE_SRC} @@ -216,7 +232,7 @@ add_library(reaper_reasampler MODULE src/actions.cpp src/bank_book.cpp ) -target_link_libraries(reaper_reasampler PRIVATE bank_model capture_paths peaks bank_grid mode_switch view_mode_model insert_plan render_settings tail_control realtime_record bank_book) +target_link_libraries(reaper_reasampler PRIVATE bank_model capture_paths peaks bank_grid mode_switch tab_strip view_mode_model insert_plan render_settings tail_control realtime_record bank_book) target_include_directories(reaper_reasampler PRIVATE ${SDK_INC} ${WDL_INC}) set_target_properties(reaper_reasampler PROPERTIES PREFIX "" OUTPUT_NAME "reaper_reasampler") diff --git a/src/actions.cpp b/src/actions.cpp index 24bf7ab..dfcab73 100644 --- a/src/actions.cpp +++ b/src/actions.cpp @@ -531,12 +531,13 @@ void doBankActivatePool() { ShowConsoleMsg("ReaSampler: active bank -> \"Pool\".\n"); } -// Move or copy the panel's selected samples from the ACTIVE bank into a named -// destination bank (prompted by display name). The panel grid shows the active bank, -// so its selection ids are members of the active bank — that is the source. Both are +// Move or copy the panel's selected samples into a named destination bank (prompted +// by display name). The SOURCE is the bank the selection lives in — the focused +// region's displayed bank (bankPanelSelectedSourceBankId), which under B4's vertical +// split is NOT necessarily the active/capture-target bank (active ≠ shown). Both are // index-only (files never relocate); move removes the source entry, copy retains it; -// both observe destination collapse-by-hash (bank_book). B4's "move to bank" menu will -// drive moveSample/copySample directly with a menu-chosen destination — this bindable +// both observe destination collapse-by-hash (bank_book). B4's "move to bank" menu +// drives moveSample/copySample directly with a menu-chosen destination — this bindable // form is the same operation with a text-prompt destination. void doBankTransferSelected(bool copy) { const std::vector selected = bankPanelSelectedSampleIds(); @@ -554,9 +555,9 @@ void doBankTransferSelected(bool copy) { ShowConsoleMsg(("ReaSampler: no bank named \"" + destName + "\".\n").c_str()); return; } - // Source = the active bank (what the panel grid shows). Pass ids by value — no - // BankIndex& is cached across the loop's mutations. - const std::string srcId = g_session->book().activeBankId(); + // Source = the bank the selection lives in (the focused region's displayed bank). + // Pass ids by value — no BankIndex& is cached across the loop's mutations. + const std::string srcId = bankPanelSelectedSourceBankId(); if (srcId == destId) { ShowConsoleMsg("ReaSampler: source and destination are the same bank.\n"); return; diff --git a/src/bank_book.cpp b/src/bank_book.cpp index 8f35d1d..9daebd3 100644 --- a/src/bank_book.cpp +++ b/src/bank_book.cpp @@ -687,6 +687,49 @@ bool Parser::parseBook(std::vector& banks, std::string& activeBank) { for (auto& b : parsedBanks) if (b.isPool()) b.displayName = kPoolBankName; + // --- Coalesce duplicate folded display names (B4 re-review fold-in). -------- + // The in-model create/rename path enforces unique display names under nameKey, + // but a hand-edited .rpp blob can smuggle in two banks whose names fold to the + // same key ("Drums" and " drums "). Rejecting the whole book over one collision + // would degrade the user's entire library to empty, so instead we AUTO- + // DISAMBIGUATE the later duplicate deterministically: scan in parse order, and + // the first time a folded key repeats, suffix that bank's display name (" 2", + // " 3", …) until its folded key is unique among all names seen so far. The FIRST + // bank to carry a key keeps its name verbatim; only subsequent collisions are + // renamed. No bank or sample is lost, and ids are untouched. The pool is included + // in the seen-set (its "Pool" key is reserved) so a named bank folding to "pool" + // is disambiguated away from it, never the reverse. + { + std::vector seenKeys; + seenKeys.reserve(parsedBanks.size()); + for (auto& b : parsedBanks) { + if (b.isPool()) { // pool's name is fixed; reserve its key + seenKeys.push_back(nameKey(b.displayName)); + continue; + } + const auto taken = [&](const std::string& k) { + return std::find(seenKeys.begin(), seenKeys.end(), k) != seenKeys.end(); + }; + std::string key = nameKey(b.displayName); + if (taken(key)) { + // Suffix with an ascending integer until the folded key is free. Guard + // against a pathological blob whose base name already ends in a number + // by folding the candidate each attempt (nameKey normalizes it). + const std::string base = b.displayName; + for (int n = 2;; ++n) { + const std::string candidate = base + " " + std::to_string(n); + const std::string candKey = nameKey(candidate); + if (!taken(candKey)) { + b.displayName = candidate; + key = candKey; + break; + } + } + } + seenKeys.push_back(key); + } + } + banks = std::move(parsedBanks); return true; } diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 965c909..4b2aef6 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -1,4 +1,5 @@ -// bank_panel.cpp — REAPER-facing docked grid (M5, Wave A). See bank_panel.h. +// bank_panel.cpp — REAPER-facing docked grid (M5 Wave A/B + Phase B4). See +// bank_panel.h. // // Compiled into the reaper_reasampler MODULE. Includes reaper_plugin_functions.h // WITHOUT REAPERAPI_IMPLEMENT — main.cpp owns the API pointers; here they are @@ -8,47 +9,54 @@ // tested): // * a SWELL dialog (IDD_BANK_PANEL) docked via DockWindowAddEx / undocked via // DockWindowRemove; toggled open/closed. -// * WM_PAINT: draw the current bank as a grid of waveform thumbnails using LICE, -// or a centered empty-state string when the bank is empty. -// * per-sample PCM read via PCM_source (PCM_Source_CreateFromFile + -// PCM_source::GetSamples) fed to peaks::computeEnvelope at the cell width. -// * an in-memory thumbnail cache keyed by (sample id, draw width, bank -// generation) so paint does not recompute envelopes every frame. +// * WM_PAINT: a VERTICAL SPLIT (Phase B4) — the pool grid region on top, a +// LICE-drawn named-banks tab-page region below (one tab per named bank, an +// overflow/scroll strip), and two full-height toggles that collapse the split. +// Each region reuses the M5 grid render loop (waveform thumbnails / empty state). +// * per-sample PCM read via PCM_source fed to peaks::computeEnvelope at cell width. +// * an in-memory thumbnail cache keyed by (sample id, draw width, bank generation). +// * id-keyed bank management (create / rename / delete / evacuate / activate) and +// sample move/copy — driven from a tab context menu and a drag — against the B1 +// BankBook model on g_session.book(), persisted via g_session.saveToActiveProject(). // -// READ-ONLY (load-bearing principle): this panel never inserts into the arrange -// and never mutates the project or the bank. It only reads g_session.bank() and -// reads sample files off disk. +// READ-ONLY of the TIMELINE (load-bearing principle): this panel never inserts into +// the arrange. It DOES mutate the bank BOOK (create/rename/move/etc.) — that is the +// whole point of B4 — but only the index/model + ext-state, never the arrange, never +// a sample file on disk (bank ops are index-only; files stay put — CONTEXT.md §Multi-bank). // -// THUMBNAIL-CACHE DECISION (CONTEXT.md §Open questions "recompute vs store peak -// bins alongside the index"): for Wave A we RECOMPUTE into an in-memory cache and -// do NOT persist peak bins in the index. Rationale: the persisted index stays -// lean and format-stable; envelopes are cheap to recompute on demand and must be -// recomputed anyway whenever the panel width (bin count) changes, which a stored -// fixed-resolution bin set could not satisfy. Storing bins is a later optimization -// if profiling shows recompute cost matters (it is bounded: one read + one O(frames) -// pass per sample, only on cache miss). +// THE PURE SEAMS: grid tiling / hit-test / selection math live in bank_grid; the +// mode-switch geometry in mode_switch; the named-banks TAB-STRIP layout, overflow/ +// scroll, and hit-test in tab_strip. All three are unit-tested outside the DAW; only +// draw + input routing + the model calls live here. +// +// REFERENCE-INVALIDATION GUARDRAIL (CONTEXT.md §Multi-bank): a bank-structural +// mutation (create/delete/evacuate/activate/move) can reallocate the book's vector, +// so a BankIndex& / Bank* must NEVER be cached across one. Every handler below +// resolves fresh AFTER any mutation and passes bank IDS (not references) into the +// model ops. #include "bank_panel.h" #include +#include // std::abs (drag threshold) #include #include #include #include +#include "bank_book.h" #include "bank_grid.h" #include "bank_model.h" #include "capture_paths.h" #include "mode_switch.h" #include "peaks.h" #include "persist.h" +#include "tab_strip.h" #include "tail_control.h" // TailSetting, cycleTailMode, tailToggleLabel (pure) #include "view.h" // applyMode — the D2/D4 mode-activation entrypoint the switch fires // SWELL / LICE. On macOS/Linux SWELL is provided by the host (SWELL_PROVIDED_BY_APP); // on Windows we use native Win32 (windows.h first, then swell.h no-ops on _WIN32). -// LICE routes its GDI through whichever backend is active. wdltypes.h gives -// WDL_DLGRET (the platform dialog-proc return type). #ifdef _WIN32 #include #include // GET_X_LPARAM / GET_Y_LPARAM (SWELL supplies them on mac/linux) @@ -61,9 +69,6 @@ #include "resource.h" -// reaper_plugin.h defines preview_register_t (the stock preview struct) and the -// REAPER_PLUGIN_HINSTANCE / registration types. main.cpp includes it with -// REAPERAPI_IMPLEMENT; here we only need the type declarations. #include "reaper_plugin.h" #define REAPERAPI_MINIMAL @@ -74,15 +79,15 @@ #define REAPERAPI_WANT_GetMainHwnd #define REAPERAPI_WANT_PCM_Source_CreateFromFile #define REAPERAPI_WANT_PCM_Source_Destroy -// Stock preview API (verified against reaper_plugin.h / reaper_plugin_functions.h): -// PlayPreview/StopPreview drive a caller-owned preview_register_t. These are the -// STOCK symbols (not SWS-only) — see the audition section below. #define REAPERAPI_WANT_PlayPreview #define REAPERAPI_WANT_StopPreview +#define REAPERAPI_WANT_GetUserInputs +#define REAPERAPI_WANT_ShowMessageBox +#define REAPERAPI_WANT_genGuid +#define REAPERAPI_WANT_guidToString #include "reaper_plugin_functions.h" -// main.cpp owns the module instance handle (needed to load the dialog resource) -// and REAPER's dispatch struct (needed to register the keyboard accelerator hook). +// main.cpp owns the module instance handle and REAPER's dispatch struct. extern REAPER_PLUGIN_HINSTANCE g_hInst; extern reaper_plugin_info_t* g_rec; @@ -92,18 +97,10 @@ namespace { namespace fs = std::filesystem; -// --- Layout / palette constants (Wave A: fixed, no user config — YAGNI) ------- +// --- Layout / palette constants ---------------------------------------------- -// Cell size + spacing for the grid. Tuned for a legible thumbnail at a glance; -// revisit when audition/selection UI lands (Wave B) and cells gain chrome. const GridSpec kGrid{/*cellWidth=*/140, /*cellHeight=*/84, /*gap=*/10}; -// How many PCM frames to pull per sample for the thumbnail. The envelope is drawn -// at cell width (~140 bins), so a few thousand frames per bin is ample; capping -// the read keeps a long sample's thumbnail cheap without a streaming loop. A -// captured one-shot/loop is short; a full-mix bounce is downsampled visually -// anyway. If a sample is longer than this, the thumbnail shows its head — an -// acceptable Wave-A approximation, flagged for Wave B (whole-file overview). constexpr int kMaxThumbnailFrames = 1 << 20; // ~1M frames (~22s @ 48k) const LICE_pixel kColBackground = LICE_RGBA(28, 28, 30, 255); @@ -111,140 +108,179 @@ const LICE_pixel kColCellBg = LICE_RGBA(44, 44, 48, 255); const LICE_pixel kColCellBorder = LICE_RGBA(70, 70, 76, 255); const LICE_pixel kColWaveform = LICE_RGBA(120, 200, 160, 255); const LICE_pixel kColMidline = LICE_RGBA(60, 60, 66, 255); -const LICE_pixel kColText = LICE_RGBA(200, 200, 205, 255); -// Selection chrome (Wave B). Selected cells get a tinted fill + brighter border; -// the focused cell (audition/nav target) gets a distinct accent border so it is -// distinguishable within a multi-selection. -const LICE_pixel kColSelBg = LICE_RGBA(38, 66, 58, 255); // selected fill tint -const LICE_pixel kColSelBorder = LICE_RGBA(120, 200, 160, 255);// selected border -const LICE_pixel kColFocusBorder = LICE_RGBA(210, 230, 220, 255);// focused-cell border +const LICE_pixel kColSelBg = LICE_RGBA(38, 66, 58, 255); +const LICE_pixel kColSelBorder = LICE_RGBA(120, 200, 160, 255); +const LICE_pixel kColFocusBorder = LICE_RGBA(210, 230, 220, 255); // --- Mode-switch header (D5) -------------------------------------------------- -// A fixed-height segmented control at the top of the client area: one segment per -// registered Design-View mode, the active one lit. The grid is offset below it. -// Layout math (segment rects, hit-test) lives in the pure mode_switch module; only -// the draw + click routing is here. -constexpr int kHeaderHeight = 30; // px; fixed strip, grid starts below it +constexpr int kHeaderHeight = 30; -const LICE_pixel kColHeaderBg = LICE_RGBA(20, 20, 22, 255); // header strip fill -const LICE_pixel kColSegBg = LICE_RGBA(44, 44, 48, 255); // inactive segment -const LICE_pixel kColSegActiveBg = LICE_RGBA(58, 96, 84, 255); // active (lit) segment -const LICE_pixel kColSegBorder = LICE_RGBA(70, 70, 76, 255); // segment divider +const LICE_pixel kColHeaderBg = LICE_RGBA(20, 20, 22, 255); +const LICE_pixel kColSegBg = LICE_RGBA(44, 44, 48, 255); +const LICE_pixel kColSegActiveBg = LICE_RGBA(58, 96, 84, 255); +const LICE_pixel kColSegBorder = LICE_RGBA(70, 70, 76, 255); -// Segment label colors are COLORREFs (SetTextColor takes RGB, not LICE_pixel). -const COLORREF kRgbSegText = RGB(170, 170, 176); // inactive label -const COLORREF kRgbSegActiveText = RGB(220, 235, 228); // active label +const COLORREF kRgbSegText = RGB(170, 170, 176); +const COLORREF kRgbSegActiveText = RGB(220, 235, 228); // --- Tail-mode footer (T1 exposure) ------------------------------------------- -// A fixed-height strip at the BOTTOM of the client area holding the tail-mode -// toggle ("Tail: Off / Auto / Manual"). Clicking anywhere in it cycles the mode -// (None -> Auto -> Manual -> None). Display/settings only: it mutates the panel's -// in-memory tail setting the plain capture actions read — NEVER the project/bank/ -// arrange. The cycle/label logic is the pure tail_control module; only the draw + -// click routing is here. The grid viewport is shortened by this strip's height so -// cells never draw under it. -constexpr int kFooterHeight = 26; // px; fixed strip at the bottom +constexpr int kFooterHeight = 26; -const LICE_pixel kColFooterBg = LICE_RGBA(20, 20, 22, 255); // footer strip fill -const LICE_pixel kColFooterBorder = LICE_RGBA(70, 70, 76, 255); // top divider -const COLORREF kRgbFooterText = RGB(190, 205, 198); // toggle label +const LICE_pixel kColFooterBg = LICE_RGBA(20, 20, 22, 255); +const LICE_pixel kColFooterBorder = LICE_RGBA(70, 70, 76, 255); +const COLORREF kRgbFooterText = RGB(190, 205, 198); + +// --- Vertical split + region headers + tab strip (Phase B4) ------------------- +// +// The client area, top to bottom: mode-switch header (kHeaderHeight) | split body | +// tail footer (kFooterHeight). The split body holds the pool region (top) and the +// named-banks region (bottom). Each region opens with a REGION HEADER band: a title, +// the active-bank readout, and a full-height toggle button. The named-banks region's +// header ALSO hosts the LICE tab strip and a "+" create button. +constexpr int kRegionHeaderHeight = 24; // per-region title/toggle band +constexpr int kTabStripHeight = 26; // the named-banks tab strip band +constexpr int kSplitDividerHeight = 3; // the horizontal divider between regions +constexpr int kFullHtBtnWidth = 22; // the square full-height toggle button +constexpr int kCreateBtnWidth = 22; // the "+" create-bank button + +// Tab strip metrics (the pure tab_strip owns the math; these are its inputs). +const TabStripSpec kTabSpec{/*tabWidth=*/96, /*chevronWidth=*/20}; + +const LICE_pixel kColRegionHeaderBg = LICE_RGBA(24, 24, 26, 255); +const LICE_pixel kColRegionBorder = LICE_RGBA(70, 70, 76, 255); +const LICE_pixel kColDivider = LICE_RGBA(12, 12, 14, 255); +const LICE_pixel kColBtnBg = LICE_RGBA(48, 48, 52, 255); +const LICE_pixel kColBtnBorder = LICE_RGBA(90, 90, 96, 255); + +const LICE_pixel kColTabBg = LICE_RGBA(40, 40, 44, 255); +const LICE_pixel kColTabShownBg = LICE_RGBA(58, 58, 64, 255); // the shown tab (browsed) +const LICE_pixel kColTabActiveBg = LICE_RGBA(58, 96, 84, 255); // the ACTIVE bank (capture target) +const LICE_pixel kColTabBorder = LICE_RGBA(70, 70, 76, 255); +const LICE_pixel kColTabActiveBorder= LICE_RGBA(150, 230, 190, 255);// active-tab accent +const LICE_pixel kColChevronBg = LICE_RGBA(32, 32, 36, 255); +// Drop-target highlight during a drag (unmistakable accent over the destination). +const LICE_pixel kColDropTarget = LICE_RGBA(90, 150, 120, 255); + +const COLORREF kRgbRegionTitle = RGB(200, 205, 210); +const COLORREF kRgbActiveReadout = RGB(150, 230, 190); // "Active: …" accent +const COLORREF kRgbTabText = RGB(200, 200, 205); +const COLORREF kRgbTabActiveText = RGB(230, 245, 238); +const COLORREF kRgbBtnText = RGB(210, 215, 220); // --- Panel state -------------------------------------------------------------- -// A computed thumbnail: the per-channel envelope at a known width. Held in the -// cache so paint reuses it until the sample, width, or bank generation changes. struct CachedThumbnail { - Envelope envelope; // one ChannelEnvelope per channel, `width` bins each - int width = 0; // bins per channel this envelope was computed at + Envelope envelope; + int width = 0; }; +// Which of the two split regions currently owns the selection / receives keyboard +// input. The move/copy source is the focused region's displayed bank. +enum class Region { Pool, Banks }; + +// What a drag is dropping onto, resolved live under the pointer during a drag. +enum class DropKind { None, PoolRegion, Tab }; + struct PanelState { ReaSamplerSession* session = nullptr; - HWND hwnd = nullptr; // the docked dialog, null when closed + HWND hwnd = nullptr; bool open = false; - // Bank-change detection: a cheap fingerprint of the bank (count + ids + - // relative paths). When it changes we bump `generation`, which invalidates - // every cache entry (keyed by generation) and forces a repaint. Simpler than - // adding a mutation counter to BankIndex, and correct across same-count - // project-load swaps (the fingerprint includes ids/paths, not just size). std::string bankFingerprint; std::uint64_t generation = 0; - // Thumbnail cache: key string (bank_grid::thumbnailKeyString) -> envelope. - // Entries for stale generations are lazily overwritten on next miss; a bank - // change also clears it wholesale (see refreshFingerprint) to bound memory. std::unordered_map cache; - // --- Interaction (Wave B) ------------------------------------------------- - - // The current cell selection (indices into bank->all(), focus, anchor). Pure - // math lives in bank_grid; this holds the live state the pointer/keyboard - // mutate. A bank change (generation bump) resets it (indices could dangle). + // --- Selection (per focused region) --------------------------------------- + // One live selection, scoped to `focusedRegion`. Switching regions moves the + // selection with the focus (a click in the other region reseeds it there). Selection selection; + int selItemCount = 0; + Region focusedRegion = Region::Pool; - // The item count the selection was last validated against. On a bank change we - // clear the selection rather than risk indices pointing past the new count. - int selItemCount = 0; - - // --- Tail-mode toggle (T1 exposure) --------------------------------------- - // The current tail setting the plain capture actions read (bankPanelTailSetting). - // Default None (exact bounds). In-memory only — resets on panel teardown; project - // persistence is a noted follow-on. Mutated ONLY by a click in the footer strip. - TailSetting tail; - - // --- Vertical-split full-height layout (Phase B3) ------------------------- - // Which region(s) the vertical split shows: both (Split, default), pool only, - // or named-banks only. B3 actions flip it (bankPanelToggled*FullHeight); B4's - // panel renders from it. In-memory only (a UI-layout preference, not project - // state — it must not travel with the .rpp); resets to Split on unload. + // --- Vertical-split state ------------------------------------------------- BankPanelFullHeight fullHeight = BankPanelFullHeight::Split; - // --- Audition preview (Wave B) -------------------------------------------- - // - // The stock preview register we hand to PlayPreview/StopPreview. Its cs/mutex - // is initialized ONCE (initPreview) and destroyed ONCE (deinitPreview) across - // the panel's lifetime — NOT per playback — because REAPER's audio thread may - // touch the register's guarded fields. `previewSrc` is the PCM_source currently - // owned by `preview.src`; non-null exactly while auditioning. `previewActive` - // tracks whether PlayPreview succeeded and StopPreview is still owed. + // The named bank whose grid the banks region shows (the SHOWN tab) — DISTINCT + // from the active/capture-target bank (book().activeBankId()). Empty when there + // are no named banks. Reconciled each fingerprint pass so it always names a live + // named bank (or is empty). + std::string shownBankId; + + // Tab-strip horizontal scroll offset (px), clamped to the strip's max each frame. + int tabScroll = 0; + + // --- Drag (sample move between regions/onto a tab) ------------------------ + // A drag begins only after the pointer moves past a threshold from a press that + // landed on a SELECTED grid cell — this is how it is disambiguated from the M5 + // multi-select drag (which begins immediately on any grid press). See onLBtnDown/ + // onMouseMove. dragging is true once the threshold is crossed. + bool dragArmed = false; // pressed on a selected cell; watching for threshold + bool dragging = false; // threshold crossed; a move-drag is in progress + int dragStartX = 0, dragStartY = 0; + Region dragSourceRegion = Region::Pool; + std::string dragSourceBankId; // the bank the dragged samples come from + std::vector dragSampleIds;// snapshot of the selection at drag start + DropKind dropKind = DropKind::None; // live drop target under the pointer + std::string dropBankId; // destination bank id when dropKind==Tab + + // --- Tail-mode toggle ----------------------------------------------------- + TailSetting tail; + + // --- Audition preview ----------------------------------------------------- preview_register_t preview{}; PCM_source* previewSrc = nullptr; bool previewActive = false; - bool previewInited = false; // guards double init / deinit + bool previewInited = false; }; PanelState g_panel; -// Forward declarations for the interaction/audition helpers defined lower down but -// referenced by earlier sections (e.g. refreshFingerprint stops audition on a bank -// change). Definitions live in the "Audition preview" / "Selection + input" blocks. void stopAudition(); // --- Current-project directory (mirrors persist.cpp's derivation) ------------- -// -// The index stores relative paths; resolving a bank file needs the current .rpp -// directory. persist.cpp derives this the same way for load; the panel is its own -// shell so it reads it directly rather than threading state through the session. -// FOLLOW-UP: capture.cpp, persist.cpp, and now bank_panel.cpp each carry this -// two-line derivation — a shared REAPER helper ("current project dir") is a clean -// small refactor once a third consumer exists (now it does). Out of scope this wave. std::string currentProjectDir() { std::vector buf(4096, '\0'); EnumProjects(-1, buf.data(), static_cast(buf.size())); std::string rpp(buf.data()); - if (rpp.empty()) return {}; // unsaved project: no resolvable bank + if (rpp.empty()) return {}; return normalizeSlashes(fs::path(rpp).parent_path().string()); } -// --- Thumbnail computation ---------------------------------------------------- +// --- Book / bank accessors ---------------------------------------------------- + +BankBook* book() { return g_panel.session ? &g_panel.session->book() : nullptr; } + +// The BankIndex a region currently displays. Pool region -> the pool; banks region -> +// the shown tab's bank (or nullptr when no named banks / the id went stale). Resolved +// FRESH every call (never cached across a mutation). +const BankIndex* indexForRegion(Region r) { + BankBook* b = book(); + if (!b) return nullptr; + if (r == Region::Pool) return &b->pool().index; + if (g_panel.shownBankId.empty()) return nullptr; + return b->index(g_panel.shownBankId); +} + +// The bank id a region displays (pool id, or the shown tab's id; "" when none). +std::string bankIdForRegion(Region r) { + if (r == Region::Pool) return std::string(kPoolBankId); + return g_panel.shownBankId; +} + +// The named banks in ordinal order (pool excluded) — the tabs. Resolved fresh. +std::vector namedBanks() { + std::vector out; + BankBook* b = book(); + if (!b) return out; + for (const Bank& bk : b->banks()) + if (!bk.isPool()) out.push_back(&bk); + return out; +} + +// --- Thumbnail computation (unchanged from M5) -------------------------------- -// Reads up to kMaxThumbnailFrames of interleaved PCM from `absPath` and computes a -// per-channel min/max envelope at `width` bins. Returns an empty envelope on any -// failure (missing file, unreadable source, zero-length) — the caller draws an -// empty cell rather than propagating an error. READ-ONLY: opens the file through -// a PCM_source and destroys it; never touches the project. Envelope computeThumbnail(const std::string& absPath, int width) { if (width <= 0 || absPath.empty()) return {}; @@ -259,16 +295,12 @@ Envelope computeThumbnail(const std::string& absPath, int width) { return {}; } - // Frames to read: the whole sample, capped so a long bounce stays cheap. std::int64_t totalFrames = static_cast(lengthSec * srate); if (totalFrames <= 0) { PCM_Source_Destroy(src); return {}; } int frames = totalFrames > kMaxThumbnailFrames ? kMaxThumbnailFrames : static_cast(totalFrames); - // One GetSamples call filling a caller-allocated interleaved buffer. block.length - // is the requested frame count; samples_out reports what was actually rendered - // (may be short at end-of-file). We ask at the source's own rate so no resample. std::vector buf(static_cast(frames) * nch, 0.0); PCM_source_transfer_t block{}; block.time_s = 0.0; @@ -284,11 +316,6 @@ Envelope computeThumbnail(const std::string& absPath, int width) { const int got = block.samples_out; if (got <= 0) return {}; - // ReaSample is double in some builds, float in others; peaks consumes float - // (peaks::Sample is a float alias, its native buffer type). Convert at this - // boundary — use `float` explicitly, NOT `reasampler::Sample`, because that - // name also denotes bank_model's metadata struct in this same namespace when - // both headers are visible (they are here in the module). const std::size_t sampleCount = static_cast(got) * nch; std::vector pcm(sampleCount); for (std::size_t i = 0; i < sampleCount; ++i) @@ -299,9 +326,6 @@ Envelope computeThumbnail(const std::string& absPath, int width) { static_cast(width)); } -// Returns the cached envelope for `sample` at `width`, computing+inserting it on a -// miss. Keyed by (id, width, current generation) so a resize or bank change misses -// and recomputes. `projectDir` resolves the sample's relative path to disk. const Envelope& thumbnailFor(const Sample& sample, int width, const std::string& projectDir) { ThumbnailKey key{sample.id, width, g_panel.generation}; @@ -318,14 +342,8 @@ const Envelope& thumbnailFor(const Sample& sample, int width, return ins.first->second.envelope; } -// --- Drawing ------------------------------------------------------------------ +// --- Drawing: thumbnails (unchanged from M5) ---------------------------------- -// Draws one sample's envelope into `rect` of `bmp`: a cell background, border, a -// zero midline, and the min/max waveform. Multi-channel envelopes are stacked -// vertically (each channel gets an equal horizontal band) so a stereo sample shows -// both channels without folding (precision invariant: no stereo fold). -// `selected` tints the fill and brightens the border; `focused` overrides the -// border with the accent color so the caret cell reads within a multi-selection. void drawThumbnail(LICE_IBitmap* bmp, const CellRect& rect, const Envelope& env, bool selected, bool focused) { const LICE_pixel bg = selected ? kColSelBg : kColCellBg; @@ -334,15 +352,11 @@ void drawThumbnail(LICE_IBitmap* bmp, const CellRect& rect, const Envelope& env, LICE_FillRect(bmp, rect.x, rect.y, rect.width, rect.height, bg, 1.0f, 0); LICE_DrawRect(bmp, rect.x, rect.y, rect.width, rect.height, border, 1.0f, 0); - // The focused cell gets a second inset rectangle so it stays distinct even when - // its neighbors are also selected (double outline reads as "the active one"). if (focused) LICE_DrawRect(bmp, rect.x + 1, rect.y + 1, rect.width - 2, rect.height - 2, border, 1.0f, 0); if (env.empty()) { - // Unreadable / empty sample: cell drawn, no waveform. A single midline - // signals "cell present, no data" without an error dialog. const int midY = rect.y + rect.height / 2; LICE_Line(bmp, rect.x + 2, midY, rect.x + rect.width - 2, midY, kColMidline, 1.0f, 0, false); @@ -356,8 +370,6 @@ void drawThumbnail(LICE_IBitmap* bmp, const CellRect& rect, const Envelope& env, const ChannelEnvelope& bins = env[ch]; const int bandTop = rect.y + ch * bandH; const int midY = bandTop + bandH / 2; - // half-height in pixels a full-scale (|value|==1) sample reaches, minus a - // 2px inset so the waveform never touches the cell border. const double halfSpan = (bandH / 2) - 2; LICE_Line(bmp, rect.x + 2, midY, rect.x + rect.width - 2, midY, @@ -366,16 +378,11 @@ void drawThumbnail(LICE_IBitmap* bmp, const CellRect& rect, const Envelope& env, const int nbins = static_cast(bins.size()); if (nbins <= 0) continue; - // Map bin i -> a column x within the cell's inner width. The envelope was - // computed at `width` bins == the cell's drawable columns, so bin i maps - // to column i; guard anyway if they differ (e.g. cached at another width). - const int innerW = rect.width - 4; // 2px inset each side + const int innerW = rect.width - 4; for (int i = 0; i < nbins; ++i) { const int x = rect.x + 2 + (nbins > 1 ? (i * (innerW - 1)) / (nbins - 1) : 0); - // min<=max always (peaks invariant). Draw a vertical line from the - // min sample to the max sample, clamped to the band. - int yMax = midY - static_cast(bins[i].max * halfSpan); // max -> up - int yMin = midY - static_cast(bins[i].min * halfSpan); // min -> down + int yMax = midY - static_cast(bins[i].max * halfSpan); + int yMin = midY - static_cast(bins[i].min * halfSpan); if (yMax < bandTop) yMax = bandTop; if (yMin > bandTop + bandH - 1) yMin = bandTop + bandH - 1; LICE_Line(bmp, x, yMin, x, yMax, kColWaveform, 1.0f, 0, false); @@ -383,43 +390,32 @@ void drawThumbnail(LICE_IBitmap* bmp, const CellRect& rect, const Envelope& env, } } -// Draws the empty-state message centered in the client area. -void drawEmptyState(HWND hwnd, LICE_IBitmap* bmp, int w, int h) { - (void)hwnd; - LICE_Clear(bmp, kColBackground); +// Draws a centered single-line label into a rect (COLORREF text). +void drawCenteredText(LICE_IBitmap* bmp, const RECT& rc, const char* text, + COLORREF color, UINT fmt) { HDC dc = bmp->getDC(); if (!dc) return; - const char* msg = "No samples in this project's bank yet. Capture one to see it here."; - RECT rc{0, 0, w, h}; - SetTextColor(dc, RGB(200, 200, 205)); + RECT r = rc; + SetTextColor(dc, color); SetBkMode(dc, TRANSPARENT); - DrawText(dc, msg, -1, &rc, DT_CENTER | DT_VCENTER | DT_SINGLELINE | DT_WORDBREAK); + DrawText(dc, text, -1, &r, fmt | DT_VCENTER | DT_SINGLELINE | DT_END_ELLIPSIS); } -// The mode-switch header rect for a client of width `w`: the full-width strip of -// fixed height at the top. Pure geometry (mode_switch owns the segment division); -// this just sizes the band. Shared by paint and click routing so both agree. -HeaderRect panelHeader(int w) { - return HeaderRect{0, 0, w, kHeaderHeight}; -} +// --- Mode-switch header (D5, unchanged) --------------------------------------- + +HeaderRect panelHeader(int w) { return HeaderRect{0, 0, w, kHeaderHeight}; } -// The number of registered Design-View modes (segments to draw). 0 when no session. int modeCount() { if (!g_panel.session) return 0; return static_cast(g_panel.session->view().modes().size()); } -// Draws the segmented mode switch into the header strip of width `w`: one segment -// per registered mode (ordinal order), the active mode lit, each labeled with its -// display name. READ-ONLY: reads g_session->view() live; never mutates the model here -// (activation happens on click, in handleClick). void drawModeSwitch(LICE_IBitmap* bmp, int w) { if (!g_panel.session) return; const ViewModeModel& view = g_panel.session->view(); const std::vector& modes = view.modes().all(); const int n = static_cast(modes.size()); - // Strip background first (so an empty/absent switch still reads as a header band). LICE_FillRect(bmp, 0, 0, w, kHeaderHeight, kColHeaderBg, 1.0f, 0); if (n <= 0) return; @@ -440,78 +436,301 @@ void drawModeSwitch(LICE_IBitmap* bmp, int w) { LICE_DrawRect(bmp, s.x, s.y, s.width, s.height, kColSegBorder, 1.0f, 0); if (!dc) continue; - - // Display name centered in the segment. A single-line centered label; - // the segment is wide enough for the seed modes' short names. - const std::string& label = mode.displayName; RECT rc{s.x, s.y, s.x + s.width, s.y + s.height}; SetTextColor(dc, active ? kRgbSegActiveText : kRgbSegText); SetBkMode(dc, TRANSPARENT); - DrawText(dc, label.c_str(), -1, &rc, + DrawText(dc, mode.displayName.c_str(), -1, &rc, DT_CENTER | DT_VCENTER | DT_SINGLELINE | DT_END_ELLIPSIS); } } -// The tail-toggle footer rect for a client of width `w` and height `h`: the -// full-width strip of fixed height pinned to the BOTTOM. A RECT (not HeaderRect) -// since the whole strip is one hit target — a click anywhere in it cycles the mode. -// Shared by paint and click routing so both agree on the band. Degenerate (empty) -// when the client is too short to host it above the header. +// --- Tail-mode footer (T1, unchanged) ----------------------------------------- + RECT panelFooter(int w, int h) { RECT rc{}; rc.left = 0; rc.right = w; rc.top = h - kFooterHeight; rc.bottom = h; - // Clamp so the footer never rides up into (or above) the header band on a very - // short panel — it collapses to empty rather than overlapping the mode switch. - if (rc.top < kHeaderHeight) rc.top = rc.bottom; // empty: top == bottom + if (rc.top < kHeaderHeight) rc.top = rc.bottom; return rc; } -// Draws the tail-mode toggle into the footer strip: a filled band, a top divider, -// and the current mode's label ("Tail: Off / Auto / Manual") from the pure -// tail_control module. READ-ONLY: reads g_panel.tail; the click handler mutates it. void drawTailFooter(LICE_IBitmap* bmp, int w, int h) { const RECT f = panelFooter(w, h); - if (f.top >= f.bottom) return; // no room — skip (short panel) + if (f.top >= f.bottom) return; LICE_FillRect(bmp, f.left, f.top, w, kFooterHeight, kColFooterBg, 1.0f, 0); - // Top divider so the strip reads as distinct from the grid above it. LICE_Line(bmp, f.left, f.top, f.right, f.top, kColFooterBorder, 1.0f, 0, false); HDC dc = bmp->getDC(); if (!dc) return; const std::string label = tailToggleLabel(g_panel.tail); RECT rc = f; - rc.left += 8; // small left pad so the label is not flush against the edge + rc.left += 8; SetTextColor(dc, kRgbFooterText); SetBkMode(dc, TRANSPARENT); DrawText(dc, label.c_str(), -1, &rc, DT_LEFT | DT_VCENTER | DT_SINGLELINE | DT_END_ELLIPSIS); } -// The cell rects for the panel's CURRENT client width and bank size, translated -// DOWN by the header height so the grid sits below the mode switch. Both paint and -// mouse hit-testing call this so they share identical geometry (no drift between -// what is drawn and what a click resolves to). Returns empty when the window is -// gone or the bank is empty. -std::vector panelRects() { - if (!g_panel.hwnd) return {}; - const BankIndex* bank = g_panel.session ? &g_panel.session->bank() : nullptr; - if (!bank || bank->empty()) return {}; - RECT cr{}; - GetClientRect(g_panel.hwnd, &cr); - const int w = cr.right - cr.left; +// --- Split geometry ----------------------------------------------------------- +// +// Every rect below is derived from the client size + fullHeight state, and BOTH paint +// and hit-testing call these so they never drift. All are top-left origin. + +// The body band between the mode-switch header and the tail footer. +RECT splitBody(int w, int h) { + RECT rc{}; + rc.left = 0; + rc.right = w; + rc.top = kHeaderHeight; + const RECT footer = panelFooter(w, h); + rc.bottom = (footer.top < footer.bottom) ? footer.top : h; + if (rc.bottom < rc.top) rc.bottom = rc.top; + return rc; +} + +// True when both regions are shown (the split is live). Otherwise one region fills +// the body. +bool poolShown() { return g_panel.fullHeight != BankPanelFullHeight::BanksOnly; } +bool banksShown() { return g_panel.fullHeight != BankPanelFullHeight::PoolOnly; } + +// The pool region's rect (whole-region: header band + grid). Empty when hidden. +RECT poolRegionRect(int w, int h) { + const RECT body = splitBody(w, h); + if (!poolShown()) return RECT{0, 0, 0, 0}; + if (!banksShown()) return body; // pool full-height: the whole body + // Split: pool gets the top half (minus the divider). + RECT rc = body; + rc.bottom = body.top + (body.bottom - body.top - kSplitDividerHeight) / 2; + if (rc.bottom < rc.top) rc.bottom = rc.top; + return rc; +} + +// The named-banks region's rect (whole-region: header band + tab strip + grid). +RECT banksRegionRect(int w, int h) { + const RECT body = splitBody(w, h); + if (!banksShown()) return RECT{0, 0, 0, 0}; + if (!poolShown()) return body; // banks full-height: the whole body + RECT rc = body; + rc.top = body.top + (body.bottom - body.top - kSplitDividerHeight) / 2 + + kSplitDividerHeight; + if (rc.top > rc.bottom) rc.top = rc.bottom; + return rc; +} + +// A region's header band (the top kRegionHeaderHeight of the region). +RECT regionHeaderRect(const RECT& region) { + RECT rc = region; + rc.bottom = region.top + kRegionHeaderHeight; + if (rc.bottom > region.bottom) rc.bottom = region.bottom; + return rc; +} + +// The named-banks region's tab strip (below its header band). +TabStripRect banksTabStripRect(const RECT& region) { + const RECT hdr = regionHeaderRect(region); + TabStripRect s; + s.x = region.left; + s.y = hdr.bottom; + s.width = region.right - region.left; + s.height = kTabStripHeight; + if (s.y + s.height > region.bottom) s.height = region.bottom - s.y; + if (s.height < 0) s.height = 0; + return s; +} + +// A region's grid viewport (below the header band, and below the tab strip for the +// banks region). This is where cells tile. +RECT regionGridRect(const RECT& region, bool isBanks) { + RECT rc = region; + rc.top = region.top + kRegionHeaderHeight; + if (isBanks) rc.top += kTabStripHeight; + if (rc.top > rc.bottom) rc.top = rc.bottom; + return rc; +} + +// The full-height toggle button rect inside a region header (right-aligned). +RECT fullHtBtnRect(const RECT& region) { + const RECT hdr = regionHeaderRect(region); + RECT rc = hdr; + rc.right = hdr.right - 4; + rc.left = rc.right - kFullHtBtnWidth; + rc.top = hdr.top + 2; + rc.bottom = hdr.bottom - 2; + return rc; +} + +// The "+" create-bank button rect inside the named-banks region header (left of the +// full-height button). +RECT createBtnRect(const RECT& region) { + RECT ft = fullHtBtnRect(region); + RECT rc = ft; + rc.right = ft.left - 4; + rc.left = rc.right - kCreateBtnWidth; + return rc; +} + +// The cell rects for a region's grid, translated into the region's grid viewport. +// Both paint and hit-testing call this. Empty when the index is null/empty. +std::vector regionCellRects(const RECT& region, bool isBanks, + const BankIndex* index) { + if (!index || index->empty()) return {}; + const RECT grid = regionGridRect(region, isBanks); + const int w = grid.right - grid.left; if (w <= 0) return {}; std::vector rects = - computeCellRects(static_cast(bank->size()), w, kGrid); - for (CellRect& r : rects) r.y += kHeaderHeight; // offset below the header + computeCellRects(static_cast(index->size()), w, kGrid); + for (CellRect& r : rects) { r.x += grid.left; r.y += grid.top; } return rects; } -// The full paint: build/refresh the LICE backing bitmap at client size, draw the -// grid (or empty state), then blit to the window HDC. +// --- Drawing: a grid region --------------------------------------------------- + +// Draws one region's grid of thumbnails (or an empty-state line) clipped to its +// viewport. `selectionOwner` is true when this region holds the live selection, so +// its cells show selection/focus chrome; the other region draws plain. +void drawRegionGrid(LICE_IBitmap* bmp, const RECT& region, bool isBanks, + const BankIndex* index, const std::string& emptyMsg, + bool selectionOwner, const std::string& projectDir) { + const RECT grid = regionGridRect(region, isBanks); + if (grid.bottom <= grid.top) return; + + if (!index || index->empty()) { + drawCenteredText(bmp, grid, emptyMsg.c_str(), RGB(150, 150, 156), DT_CENTER); + return; + } + + const std::vector& samples = index->all(); + const std::vector rects = regionCellRects(region, isBanks, index); + const int binWidth = kGrid.cellWidth - 4; + for (std::size_t i = 0; i < rects.size(); ++i) { + const CellRect& rect = rects[i]; + if (rect.y >= grid.bottom) continue; // below the viewport: skip (no scroll) + const int idx = static_cast(i); + const bool selected = selectionOwner && g_panel.selection.contains(idx); + const bool focused = selectionOwner && g_panel.selection.focus == idx; + const Envelope& env = thumbnailFor(samples[i], binWidth, projectDir); + drawThumbnail(bmp, rect, env, selected, focused); + } +} + +// Draws a region header: title, the active-bank readout, and the full-height button. +void drawRegionHeader(LICE_IBitmap* bmp, const RECT& region, const char* title, + const std::string& activeName, bool poolBtnIsPool) { + const RECT hdr = regionHeaderRect(region); + LICE_FillRect(bmp, hdr.left, hdr.top, hdr.right - hdr.left, + hdr.bottom - hdr.top, kColRegionHeaderBg, 1.0f, 0); + LICE_Line(bmp, hdr.left, hdr.bottom - 1, hdr.right, hdr.bottom - 1, + kColRegionBorder, 1.0f, 0, false); + + // Title, left. + RECT titleRc = hdr; + titleRc.left += 8; + titleRc.right = titleRc.left + 120; + drawCenteredText(bmp, titleRc, title, kRgbRegionTitle, DT_LEFT); + + // Active-bank readout, centered — the UNMISTAKABLE indicator (settled B4 + // constraint). It names the active/capture-target bank in an accent color in + // BOTH region headers, so the active bank is legible even when it is not the + // shown tab and even when it is the pool (no tab exists for it). + const std::string readout = "Active: " + activeName; + RECT actRc = hdr; + actRc.left = titleRc.right + 6; + actRc.right = createBtnRect(region).left - 6; + if (actRc.right > actRc.left) + drawCenteredText(bmp, actRc, readout.c_str(), kRgbActiveReadout, DT_LEFT); + + // Full-height toggle button: an arrow glyph. In split it means "maximize this + // region"; when this region is already full it means "restore the split". + const RECT btn = fullHtBtnRect(region); + LICE_FillRect(bmp, btn.left, btn.top, btn.right - btn.left, + btn.bottom - btn.top, kColBtnBg, 1.0f, 0); + LICE_DrawRect(bmp, btn.left, btn.top, btn.right - btn.left, + btn.bottom - btn.top, kColBtnBorder, 1.0f, 0); + const bool thisFull = + poolBtnIsPool ? (g_panel.fullHeight == BankPanelFullHeight::PoolOnly) + : (g_panel.fullHeight == BankPanelFullHeight::BanksOnly); + drawCenteredText(bmp, btn, thisFull ? "\xE2\x87\x85" : "\xE2\x87\x83", // ⇅ / ⇃ + kRgbBtnText, DT_CENTER); +} + +// Draws the named-banks tab strip: one tab per named bank (ordinal order), the SHOWN +// tab highlighted, the ACTIVE bank's tab lit with the accent border, overflow +// chevrons when present, plus the "+" create button in the header. During a drag, +// the tab under the pointer gets the drop-target highlight. +void drawTabStrip(LICE_IBitmap* bmp, const RECT& region) { + const TabStripRect strip = banksTabStripRect(region); + if (strip.height <= 0) return; + LICE_FillRect(bmp, strip.x, strip.y, strip.width, strip.height, + kColRegionHeaderBg, 1.0f, 0); + + const std::vector tabs = namedBanks(); + const int n = static_cast(tabs.size()); + if (n == 0) { + RECT r{strip.x + 8, strip.y, strip.x + strip.width, strip.y + strip.height}; + drawCenteredText(bmp, r, "No named banks — click + to create one.", + RGB(140, 140, 146), DT_LEFT); + return; + } + + const TabStripLayout layout = + computeTabStripLayout(strip, n, kTabSpec, g_panel.tabScroll); + + // Chevrons (drawn first so tabs sit above their inner edges). + if (layout.overflow) { + LICE_FillRect(bmp, strip.x, strip.y, kTabSpec.chevronWidth, strip.height, + kColChevronBg, 1.0f, 0); + LICE_FillRect(bmp, strip.x + strip.width - kTabSpec.chevronWidth, strip.y, + kTabSpec.chevronWidth, strip.height, kColChevronBg, 1.0f, 0); + RECT lc{strip.x, strip.y, strip.x + kTabSpec.chevronWidth, + strip.y + strip.height}; + RECT rc{strip.x + strip.width - kTabSpec.chevronWidth, strip.y, + strip.x + strip.width, strip.y + strip.height}; + drawCenteredText(bmp, lc, "\xE2\x80\xB9", kRgbTabText, DT_CENTER); // ‹ + drawCenteredText(bmp, rc, "\xE2\x80\xBA", kRgbTabText, DT_CENTER); // › + } + + const std::string activeId = book() ? book()->activeBankId() : std::string(); + const std::vector rects = + computeTabRects(strip, n, kTabSpec, g_panel.tabScroll); + for (const TabRect& tr : rects) { + const Bank* bk = tabs[static_cast(tr.index)]; + const bool shown = bk->id == g_panel.shownBankId; + const bool active = bk->id == activeId; + const bool dropHere = g_panel.dragging && + g_panel.dropKind == DropKind::Tab && + g_panel.dropBankId == bk->id; + + LICE_pixel bg = shown ? kColTabShownBg : kColTabBg; + if (active) bg = kColTabActiveBg; + if (dropHere) bg = kColDropTarget; + LICE_FillRect(bmp, tr.x, tr.y, tr.width, tr.height, bg, 1.0f, 0); + // The active bank's tab gets a bright accent border (unmistakable), distinct + // from the shown tab's fill highlight — active ≠ shown, made visible. + LICE_DrawRect(bmp, tr.x, tr.y, tr.width, tr.height, + active ? kColTabActiveBorder : kColTabBorder, 1.0f, 0); + if (active) + LICE_DrawRect(bmp, tr.x + 1, tr.y + 1, tr.width - 2, tr.height - 2, + kColTabActiveBorder, 1.0f, 0); + + RECT lr{tr.x + 4, tr.y, tr.x + tr.width - 4, tr.y + tr.height}; + drawCenteredText(bmp, lr, bk->displayName.c_str(), + active ? kRgbTabActiveText : kRgbTabText, DT_CENTER); + } +} + +// The active bank's display name (for the readout). "Pool" when the pool is active. +std::string activeBankName() { + BankBook* b = book(); + if (!b) return std::string(kPoolBankName); + const Bank* bk = b->bank(b->activeBankId()); + return bk ? bk->displayName : std::string(kPoolBankName); +} + +// --- Full paint --------------------------------------------------------------- + void paintPanel(HWND hwnd, HDC hdc) { RECT cr{}; GetClientRect(hwnd, &cr); @@ -519,42 +738,55 @@ void paintPanel(HWND hwnd, HDC hdc) { const int h = cr.bottom - cr.top; if (w <= 0 || h <= 0) return; - // A per-paint sysbitmap. Cheap to construct; sized to the client. (Wave A - // keeps it local; if repaint cost ever matters, cache it across paints.) LICE_SysBitmap bmp(w, h); + LICE_Clear(&bmp, kColBackground); - const BankIndex* bank = g_panel.session ? &g_panel.session->bank() : nullptr; - if (!bank || bank->empty()) { - drawEmptyState(hwnd, &bmp, w, h); - } else { - LICE_Clear(&bmp, kColBackground); - const std::string projectDir = currentProjectDir(); - const std::vector& samples = bank->all(); - std::vector rects = - computeCellRects(static_cast(samples.size()), w, kGrid); - for (CellRect& r : rects) r.y += kHeaderHeight; // grid sits below the header - // Draw each cell's thumbnail. Inner drawable width == cell width - inset; - // compute the envelope at the cell's inner column count so bins map 1:1. - const int binWidth = kGrid.cellWidth - 4; - // Cells must not draw under the footer strip: the visible grid stops at the - // footer top (or the client bottom when the panel is too short for a footer). - const RECT footer = panelFooter(w, h); - const int gridBottom = footer.top < footer.bottom ? footer.top : h; - for (std::size_t i = 0; i < rects.size(); ++i) { - const CellRect& rect = rects[i]; - // Skip cells entirely below the visible grid area (Wave A has no scroll; - // this just avoids computing thumbnails that cannot be seen). - if (rect.y >= gridBottom) continue; - const int idx = static_cast(i); - const bool selected = g_panel.selection.contains(idx); - const bool focused = g_panel.selection.focus == idx; - const Envelope& env = thumbnailFor(samples[i], binWidth, projectDir); - drawThumbnail(&bmp, rect, env, selected, focused); + const std::string projectDir = currentProjectDir(); + const std::string activeName = activeBankName(); + + // Pool region (top). + if (poolShown()) { + const RECT region = poolRegionRect(w, h); + drawRegionHeader(&bmp, region, "Pool", activeName, /*poolBtnIsPool=*/true); + drawRegionGrid(&bmp, region, /*isBanks=*/false, indexForRegion(Region::Pool), + "No samples in the pool yet. Capture one to see it here.", + g_panel.focusedRegion == Region::Pool, projectDir); + // Drop-target highlight for the pool region during a drag. + if (g_panel.dragging && g_panel.dropKind == DropKind::PoolRegion) { + const RECT grid = regionGridRect(region, false); + LICE_DrawRect(&bmp, grid.left + 1, grid.top + 1, + grid.right - grid.left - 2, grid.bottom - grid.top - 2, + kColDropTarget, 1.0f, 0); } } - // The mode switch and tail footer draw LAST so their bands overlay the top/bottom - // of the grid / empty-state area regardless of which branch ran above. + // Split divider. + if (poolShown() && banksShown()) { + const RECT body = splitBody(w, h); + const int dy = body.top + (body.bottom - body.top - kSplitDividerHeight) / 2; + LICE_FillRect(&bmp, 0, dy, w, kSplitDividerHeight, kColDivider, 1.0f, 0); + } + + // Named-banks region (bottom). + if (banksShown()) { + const RECT region = banksRegionRect(w, h); + drawRegionHeader(&bmp, region, "Banks", activeName, /*poolBtnIsPool=*/false); + // "+" create button (drawn as part of the banks header). + const RECT cbtn = createBtnRect(region); + LICE_FillRect(&bmp, cbtn.left, cbtn.top, cbtn.right - cbtn.left, + cbtn.bottom - cbtn.top, kColBtnBg, 1.0f, 0); + LICE_DrawRect(&bmp, cbtn.left, cbtn.top, cbtn.right - cbtn.left, + cbtn.bottom - cbtn.top, kColBtnBorder, 1.0f, 0); + drawCenteredText(&bmp, cbtn, "+", kRgbBtnText, DT_CENTER); + + drawTabStrip(&bmp, region); + drawRegionGrid(&bmp, region, /*isBanks=*/true, indexForRegion(Region::Banks), + g_panel.shownBankId.empty() + ? "Select or create a named bank." + : "This bank is empty. Move samples here from the pool.", + g_panel.focusedRegion == Region::Banks, projectDir); + } + drawModeSwitch(&bmp, w); drawTailFooter(&bmp, w, h); @@ -563,65 +795,64 @@ void paintPanel(HWND hwnd, HDC hdc) { // --- Bank-change detection ---------------------------------------------------- -// A cheap fingerprint of the bank: count + each sample's id and relative path. -// Ids are unique and stable; including relative paths catches an in-place file -// swap. Cheaper than hashing PCM, sufficient to know "the grid must redraw". -std::string bankFingerprint(const BankIndex& bank) { - std::string fp = std::to_string(bank.size()); - for (const Sample& s : bank.all()) { - fp += '\x1f'; - fp += s.id; - fp += '\x1f'; - fp += s.relativePath; +// A fingerprint of the WHOLE BOOK: for each bank, its id + display name + active flag +// + per-sample id/path. Catches every mutation the panel must redraw for: capture, +// project load, and B4's own create/rename/delete/move/activate. +std::string bookFingerprint() { + BankBook* b = book(); + if (!b) return {}; + std::string fp = std::to_string(b->size()); + fp += '\x1e'; fp += b->activeBankId(); + for (const Bank& bk : b->banks()) { + fp += '\x1d'; + fp += bk.id; + fp += '\x1c'; + fp += bk.displayName; + for (const Sample& s : bk.index.all()) { + fp += '\x1f'; + fp += s.id; + fp += '\x1f'; + fp += s.relativePath; + } } return fp; } -// Recomputes the fingerprint; on change, bumps the generation and clears the -// cache (bounding memory and invalidating every stale-generation entry). Returns -// true if the bank changed since last check. +// Reconciles shownBankId against the live named banks: keep it if it still names a +// named bank; otherwise fall to the first named bank (or empty when none). Keeps the +// banks region always showing a valid tab. Never touches the ACTIVE bank. +void reconcileShownBank() { + BankBook* b = book(); + if (!b) { g_panel.shownBankId.clear(); return; } + if (!g_panel.shownBankId.empty()) { + const Bank* bk = b->bank(g_panel.shownBankId); + if (bk && !bk->isPool()) return; // still valid + } + const std::vector named = namedBanks(); + g_panel.shownBankId = named.empty() ? std::string() : named.front()->id; +} + bool refreshFingerprint() { - if (!g_panel.session) return false; - std::string fp = bankFingerprint(g_panel.session->bank()); + if (!book()) return false; + std::string fp = bookFingerprint(); if (fp == g_panel.bankFingerprint) return false; g_panel.bankFingerprint = std::move(fp); ++g_panel.generation; g_panel.cache.clear(); - // The selection indexes into the OLD bank order; a bank change (capture / - // project load) can invalidate those indices, so clear it and stop any - // audition of a sample that may no longer exist at the same index. + // The selection indexes into the OLD order; a change can invalidate those, so + // clear it and stop any audition. if (!g_panel.selection.empty() || g_panel.selection.focus >= 0) { g_panel.selection = Selection{}; stopAudition(); } - g_panel.selItemCount = static_cast(g_panel.session->bank().size()); + reconcileShownBank(); + const BankIndex* idx = indexForRegion(g_panel.focusedRegion); + g_panel.selItemCount = idx ? static_cast(idx->size()) : 0; return true; } -// --- Audition preview --------------------------------------------------------- -// -// READ-ONLY / NON-DESTRUCTIVE (load-bearing principle): audition is PREVIEW -// playback only. It NEVER inserts into the arrange, creates items/tracks, or -// mutates the project or bank. PlayPreview streams a caller-owned PCM_source -// through REAPER's preview bus and touches nothing in the project. -// -// FLAGGED RUNTIME ASSUMPTIONS (header does not specify these; verified only by -// signature/struct, not semantics — DAW-verify): -// 1. REAPER's audio thread reads the preview_register_t by POINTER while the -// preview is active (the struct's own comment mandates a cs/mutex we init), -// so the register must outlive playback — we hold it in g_panel (static), -// never on the stack. -// 2. StopPreview is assumed to detach the source from the audio thread BEFORE it -// returns, making it safe to PCM_Source_Destroy the source immediately after. -// This is the conventional contract (SWS' preview helpers rely on it) but is -// NOT documented in the header — flagged. If a rare race surfaced, the fix is -// a StartPreviewFade + deferred free; not done now (YAGNI, no evidence). -// 3. m_out_chan == 0 routes to the first hardware output pair (stereo). We do not -// set mono (&1024). volume 1.0, loop false, curpos 0. +// --- Audition preview (unchanged from M5) ------------------------------------- -// Initializes the preview register's cs/mutex ONCE for the panel's lifetime. The -// preview struct guards its fields with a platform lock the caller must set up -// (reaper_plugin.h). Idempotent. void initPreview() { if (g_panel.previewInited) return; #ifdef _WIN32 @@ -632,16 +863,11 @@ void initPreview() { g_panel.previewInited = true; } -// Stops any active preview and frees the owned PCM_source. Safe to call when -// nothing is playing (no-op). Every stop path funnels through here so the source -// is freed exactly once and never dangles. void stopAudition() { if (g_panel.previewActive) { StopPreview(&g_panel.preview); g_panel.previewActive = false; } - // Free the source AFTER StopPreview has detached it (assumption #2). Clear the - // register's src so a stale pointer can never be handed back to PlayPreview. if (g_panel.previewSrc) { PCM_Source_Destroy(g_panel.previewSrc); g_panel.previewSrc = nullptr; @@ -649,7 +875,6 @@ void stopAudition() { g_panel.preview.src = nullptr; } -// Destroys the preview register's cs/mutex on panel teardown, after stopAudition. void deinitPreview() { if (!g_panel.previewInited) return; #ifdef _WIN32 @@ -660,31 +885,24 @@ void deinitPreview() { g_panel.previewInited = false; } -// Auditions the sample at bank index `idx`: stops any prior preview, loads the -// sample's file as a PCM_source, and starts stock preview playback. Re-audition -// (calling with a new idx while one plays) stops the previous first. On any -// failure (bad index, unsaved project, unreadable file, PlayPreview refusal) it -// leaves nothing playing and no source leaked. +// Auditions sample `idx` of the FOCUSED region's displayed bank. void startAudition(int idx) { - // Always stop+free the previous first — re-audition semantics, and it clears - // previewSrc so the load below starts clean. stopAudition(); - const BankIndex* bank = g_panel.session ? &g_panel.session->bank() : nullptr; - if (!bank) return; - const std::vector& samples = bank->all(); + const BankIndex* index = indexForRegion(g_panel.focusedRegion); + if (!index) return; + const std::vector& samples = index->all(); if (idx < 0 || idx >= static_cast(samples.size())) return; const std::string projectDir = currentProjectDir(); const std::string abs = resolveBankFile(projectDir, samples[idx].relativePath); - if (abs.empty()) return; // unsaved project / unresolvable — nothing to play + if (abs.empty()) return; PCM_source* src = PCM_Source_CreateFromFile(abs.c_str()); - if (!src) return; // unreadable file — no preview, no leak + if (!src) return; - // Fill the register. cs/mutex already initialized (initPreview at panel open). g_panel.preview.src = src; - g_panel.preview.m_out_chan = 0; // first hardware output pair (assumption #3) + g_panel.preview.m_out_chan = 0; g_panel.preview.curpos = 0.0; g_panel.preview.loop = false; g_panel.preview.volume = 1.0; @@ -693,120 +911,477 @@ void startAudition(int idx) { g_panel.preview.preview_track = nullptr; if (PlayPreview(&g_panel.preview) != 0) { - g_panel.previewSrc = src; // we now own it until stopAudition frees it + g_panel.previewSrc = src; g_panel.previewActive = true; } else { - // PlayPreview refused — free the source we created rather than leak it. PCM_Source_Destroy(src); g_panel.preview.src = nullptr; } } -// --- Selection + input -------------------------------------------------------- +// --- Input helpers ------------------------------------------------------------ -// True while VK_CONTROL / VK_SHIFT is physically down. SWELL does NOT set MK_* bits -// in a mouse message's wParam (swell-types.h), so modifier state is read live via -// GetAsyncKeyState — the portable path (Win/mac/GDK all support these two VKs). bool ctrlDown() { return (GetAsyncKeyState(VK_CONTROL) & 0x8000) != 0; } bool shiftDown() { return (GetAsyncKeyState(VK_SHIFT) & 0x8000) != 0; } -// The current bank item count (0 when no session/bank). -int bankItemCount() { - const BankIndex* bank = g_panel.session ? &g_panel.session->bank() : nullptr; - return bank ? static_cast(bank->size()) : 0; -} - -// Requests a repaint of the whole client area (selection/focus chrome changed). void invalidatePanel() { if (g_panel.hwnd) InvalidateRect(g_panel.hwnd, nullptr, FALSE); } -// Handles a left-button click at client (x, y): hit-test to a cell, update the -// selection through the pure model with the live modifier state, repaint. A click -// on empty space (gap/margin/below grid) clears the selection AND stops audition -// (deselect stop path). READ-ONLY: never mutates the bank/project. -void handleClick(int x, int y) { - // Mode-switch header takes precedence: a click in the header band activates the - // clicked mode via the D2/D4 view shell (the same action the user can bind) and - // repaints. Load-bearing principle preserved — this fires a Design-View toggle, - // it never inserts into the arrange or mutates the bank. - if (g_panel.session) { - RECT cr{}; - GetClientRect(g_panel.hwnd, &cr); - const int w = cr.right - cr.left; - const int n = modeCount(); - const int seg = hitTestSegment(x, y, panelHeader(w), n); - if (seg >= 0) { - const std::vector& modes = g_panel.session->view().modes().all(); - if (seg < static_cast(modes.size())) { - applyMode(g_panel.session->view(), modes[static_cast(seg)].id, - nullptr); - invalidatePanel(); // active-segment highlight + parked-track redraw - } - return; // header click consumed; do NOT fall through to grid selection +// The item count of the focused region's bank (0 when none). +int focusedItemCount() { + const BankIndex* idx = indexForRegion(g_panel.focusedRegion); + return idx ? static_cast(idx->size()) : 0; +} + +// Which region (if any) contains client point (x, y); returns false via `out` set to +// Pool by default when the point is in neither region body. +bool regionAt(int x, int y, Region& out) { + RECT cr{}; + GetClientRect(g_panel.hwnd, &cr); + const int w = cr.right - cr.left, h = cr.bottom - cr.top; + if (poolShown()) { + const RECT r = poolRegionRect(w, h); + if (x >= r.left && x < r.right && y >= r.top && y < r.bottom) { + out = Region::Pool; return true; } } - - // Tail-mode footer: a click anywhere in the bottom strip cycles the tail mode - // (None -> Auto -> Manual -> None) and repaints. Settings-only — it mutates the - // panel's in-memory tail setting the capture actions read, and NOTHING in the - // project/bank/arrange. Checked before the grid so a footer click never selects. - { - RECT cr{}; - GetClientRect(g_panel.hwnd, &cr); - const int w = cr.right - cr.left; - const int h = cr.bottom - cr.top; - const RECT f = panelFooter(w, h); - if (f.top < f.bottom && x >= f.left && x < f.right && y >= f.top && y < f.bottom) { - g_panel.tail.mode = cycleTailMode(g_panel.tail.mode); - invalidatePanel(); - return; // footer click consumed; do NOT fall through to grid selection + if (banksShown()) { + const RECT r = banksRegionRect(w, h); + if (x >= r.left && x < r.right && y >= r.top && y < r.bottom) { + out = Region::Banks; return true; } } + return false; +} - const std::vector rects = panelRects(); - const int hit = hitTestCell(x, y, rects); - const int count = bankItemCount(); +// --- Bank management ops (id-keyed; drive the B1 model + persist) -------------- +// +// Each op mutates g_session.book() then persists via saveToActiveProject(). After a +// STRUCTURAL mutation (create/delete/evacuate) any Bank*/BankIndex& is invalid — we +// resolve fresh, pass ids, and let the next refreshFingerprint repaint. persistBook +// no-ops on an unsaved project (matches the capture/B3 quiet-persist idiom). - if (hit < 0) { - // Click on empty space clears the selection and stops any audition. - if (!g_panel.selection.empty() || g_panel.selection.focus >= 0) { - g_panel.selection = Selection{}; - stopAudition(); - invalidatePanel(); +void persistBook() { + if (g_panel.session) g_panel.session->saveToActiveProject(); +} + +// REAPER's stock single-line input (comma-safe via the \x1f return separator, as B3). +bool promptText(const char* title, const char* caption, const std::string& initial, + std::string& out) { + std::vector buf(512, '\0'); + std::snprintf(buf.data(), buf.size(), "%s", initial.c_str()); + const std::string captions = std::string(caption) + ",separator=\x1f"; + if (!GetUserInputs(title, 1, captions.c_str(), buf.data(), + static_cast(buf.size()))) + return false; + std::string s(buf.data()); + if (s.empty()) return false; + out = std::move(s); + return true; +} + +// Mints a genuine REAPER GUID string as a stable bank id (same as B3 mintBankId). +std::string mintBankId() { + GUID g{}; + genGuid(&g); + char buf[64] = {0}; + guidToString(&g, buf); + return std::string(buf); +} + +void doCreateBank() { + std::string name; + if (!promptText("ReaSampler: create bank", "Bank name:", "", name)) return; + const std::string id = mintBankId(); + if (!book()->createBank(id, name)) { + ShowMessageBox("A bank with that name already exists.", + "ReaSampler: create bank", 0); + return; + } + g_panel.shownBankId = id; // show the freshly-created bank + g_panel.focusedRegion = Region::Banks; + persistBook(); + invalidatePanel(); +} + +void doRenameBank(const std::string& bankId) { + const Bank* bk = book()->bank(bankId); + if (!bk || bk->isPool()) return; + const std::string current = bk->displayName; // copy before any mutation + std::string newName; + if (!promptText("ReaSampler: rename bank", "New name:", current, newName)) return; + if (!book()->renameBank(bankId, newName)) { + ShowMessageBox("Another bank already uses that name.", + "ReaSampler: rename bank", 0); + return; + } + persistBook(); + invalidatePanel(); +} + +// Delete with the RICHER confirm-on-non-empty affordance (B4): the confirm names the +// member count AND offers evacuate as the one-click alternative (Yes=delete anyway, +// No=evacuate-then-keep, Cancel=abort) — richer than B3's basic YESNO. +void doDeleteBank(const std::string& bankId) { + const Bank* bk = book()->bank(bankId); + if (!bk || bk->isPool()) return; + const std::size_t members = bk->index.size(); // read BEFORE any mutation + const std::string name = bk->displayName; + + if (members > 0) { + const std::string msg = + "\"" + name + "\" holds " + std::to_string(members) + + (members == 1 ? " sample" : " samples") + + ".\n\nYes — delete the bank AND drop its samples (files are kept on disk " + "but no bank references them until prune).\nNo — Evacuate them to the " + "pool first, then delete the empty bank (keeps the samples).\nCancel — " + "do nothing."; + // 3 == MB_YESNOCANCEL. 6=Yes, 7=No, 2=Cancel (SDK). + const int r = ShowMessageBox(msg.c_str(), + "ReaSampler: delete non-empty bank", 3); + if (r == 2) return; // Cancel + if (r == 7) { // No -> evacuate, then delete empty + if (!book()->evacuate(bankId)) return; + // book() may have reallocated; re-resolve nothing (we pass the id again). } + // r == 6 (Yes) falls through to a plain delete (drops members). + } + if (!book()->deleteBank(bankId)) return; + persistBook(); + // shownBankId is reconciled by the next fingerprint pass; nudge focus to pool if + // no named banks remain so the selection has a valid home. + invalidatePanel(); +} + +void doEvacuateBank(const std::string& bankId) { + const Bank* bk = book()->bank(bankId); + if (!bk || bk->isPool()) return; + if (!book()->evacuate(bankId)) return; + persistBook(); + invalidatePanel(); +} + +void doActivateBank(const std::string& bankId) { + if (!book()->setActiveBank(bankId)) return; // rejects an unknown id + persistBook(); + invalidatePanel(); +} + +// Move or copy `sampleIds` from `srcBankId` to `destBankId` (index-only). Both pass +// ids straight to the model op (no BankIndex& cached across the loop's mutations). +void transferSamples(const std::vector& sampleIds, + const std::string& srcBankId, const std::string& destBankId, + bool copy) { + if (sampleIds.empty() || srcBankId == destBankId) return; + if (!book()->bank(srcBankId) || !book()->bank(destBankId)) return; + for (const std::string& sid : sampleIds) { + if (copy) book()->copySample(sid, srcBankId, destBankId); + else book()->moveSample(sid, srcBankId, destBankId); + } + persistBook(); + // The selection indexed into the source; after a move those indices are stale, so + // clear it (the fingerprint pass will also clear, but do it now for immediacy). + g_panel.selection = Selection{}; + invalidatePanel(); +} + +// The selection's sample ids resolved against the FOCUSED region's bank (source of a +// move/copy). Returns ids in bank order; empty when nothing selected. +std::vector focusedSelectionIds() { + std::vector ids; + const BankIndex* idx = indexForRegion(g_panel.focusedRegion); + if (!idx) return ids; + const std::vector& samples = idx->all(); + const int count = static_cast(samples.size()); + for (int i : g_panel.selection.indices) + if (i >= 0 && i < count) ids.push_back(samples[static_cast(i)].id); + return ids; +} + +// --- Popup menus -------------------------------------------------------------- +// +// SWELL/Win32 both expose CreatePopupMenu / InsertMenu (SWELL aliases SWELL_InsertMenu +// -> InsertMenu) / TrackPopupMenu(TPM_RETURNCMD) / DestroyMenu. We build a menu of +// (label -> small int command), track it at screen coords, and switch on the return. +// Menu command ids are LOCAL to the popup (not REAPER action ids) — TPM_RETURNCMD +// hands the chosen id straight back, so no hookcommand routing is involved. + +// Appends a string item (id) to `menu` at its end. Portable over Win32/SWELL: both +// accept InsertMenu(menu, pos, MF_BYPOSITION|MF_STRING, id, text) with pos past the +// end appending. -1 as an unsigned position appends on Win32; SWELL clamps a large +// pos to the end. +void menuAppend(HMENU menu, unsigned int id, const char* text, bool grayed = false) { + UINT flags = MF_BYPOSITION | MF_STRING; + if (grayed) flags |= MF_GRAYED; + InsertMenu(menu, -1, flags, id, text); +} +void menuSeparator(HMENU menu) { + InsertMenu(menu, -1, MF_BYPOSITION | MF_SEPARATOR, 0, nullptr); +} + +// Menu command ids (local to a popup). +enum : unsigned int { + kMenuNone = 0, + kMenuActivate = 100, + kMenuRename, + kMenuDelete, + kMenuEvacuate, + kMenuCreate, + kMenuMoveBase = 1000, // move-to-bank: kMenuMoveBase + destination index + kMenuCopyBase = 2000, // copy-to-bank: kMenuCopyBase + destination index +}; + +// Shows the right-click context menu for a named-bank TAB: activate / rename / delete +// / evacuate that bank, plus a create entry. Drives the id-keyed ops. +void showTabMenu(int screenX, int screenY, const std::string& bankId) { + const Bank* bk = book()->bank(bankId); + if (!bk || bk->isPool()) return; + const bool isActive = book()->activeBankId() == bankId; + const bool nonEmpty = !bk->index.empty(); + + HMENU menu = CreatePopupMenu(); + menuAppend(menu, kMenuActivate, + isActive ? "Active (capture target)" : "Activate (make capture target)", + /*grayed=*/isActive); + menuSeparator(menu); + menuAppend(menu, kMenuRename, "Rename\xE2\x80\xA6"); + menuAppend(menu, kMenuEvacuate, "Evacuate to pool", /*grayed=*/!nonEmpty); + menuAppend(menu, kMenuDelete, "Delete\xE2\x80\xA6"); + menuSeparator(menu); + menuAppend(menu, kMenuCreate, "New bank\xE2\x80\xA6"); + + const int cmd = TrackPopupMenu(menu, TPM_RETURNCMD, screenX, screenY, 0, + g_panel.hwnd, nullptr); + DestroyMenu(menu); + + switch (cmd) { + case kMenuActivate: doActivateBank(bankId); break; + case kMenuRename: doRenameBank(bankId); break; + case kMenuEvacuate: doEvacuateBank(bankId); break; + case kMenuDelete: doDeleteBank(bankId); break; + case kMenuCreate: doCreateBank(); break; + default: break; + } +} + +// Shows the move/copy menu for the current selection (the SOURCE is the focused +// region's bank). Lists every OTHER bank (pool + named) as a move destination, then a +// copy submenu-free flat list (copy entries follow the move block). Move is the +// default (listed first); copy is the deliberate secondary act. +void showSelectionMenu(int screenX, int screenY) { + const std::vector sel = focusedSelectionIds(); + if (sel.empty()) return; + const std::string srcId = bankIdForRegion(g_panel.focusedRegion); + + // Destinations: pool + named banks, excluding the source. Ordinal order. + struct Dest { std::string id; std::string name; }; + std::vector dests; + if (srcId != std::string(kPoolBankId)) + dests.push_back({std::string(kPoolBankId), std::string(kPoolBankName)}); + for (const Bank* bk : namedBanks()) + if (bk->id != srcId) dests.push_back({bk->id, bk->displayName}); + + HMENU menu = CreatePopupMenu(); + if (dests.empty()) { + menuAppend(menu, kMenuNone, "No other bank to move to", /*grayed=*/true); + TrackPopupMenu(menu, TPM_RETURNCMD, screenX, screenY, 0, g_panel.hwnd, nullptr); + DestroyMenu(menu); return; } - g_panel.selection = - applyClick(g_panel.selection, hit, ctrlDown(), shiftDown(), count); + const std::string label = std::to_string(sel.size()) + + (sel.size() == 1 ? " sample" : " samples"); + menuAppend(menu, kMenuNone, ("Move " + label + " to:").c_str(), /*grayed=*/true); + for (std::size_t i = 0; i < dests.size(); ++i) + menuAppend(menu, kMenuMoveBase + static_cast(i), + (" " + dests[i].name).c_str()); + menuSeparator(menu); + menuAppend(menu, kMenuNone, ("Copy " + label + " to:").c_str(), /*grayed=*/true); + for (std::size_t i = 0; i < dests.size(); ++i) + menuAppend(menu, kMenuCopyBase + static_cast(i), + (" " + dests[i].name).c_str()); + + const int cmd = TrackPopupMenu(menu, TPM_RETURNCMD, screenX, screenY, 0, + g_panel.hwnd, nullptr); + DestroyMenu(menu); + if (cmd >= static_cast(kMenuMoveBase) && + cmd < static_cast(kMenuMoveBase + dests.size())) { + transferSamples(sel, srcId, dests[cmd - kMenuMoveBase].id, /*copy=*/false); + } else if (cmd >= static_cast(kMenuCopyBase) && + cmd < static_cast(kMenuCopyBase + dests.size())) { + transferSamples(sel, srcId, dests[cmd - kMenuCopyBase].id, /*copy=*/true); + } +} + +// --- Click routing ------------------------------------------------------------ + +// Handles a header/tab-strip/button click for the banks region. Returns true if the +// click was consumed (a region-chrome hit), false to fall through to grid selection. +bool handleBanksChromeClick(int x, int y, const RECT& region) { + // Full-height toggle button. + const RECT ftb = fullHtBtnRect(region); + if (x >= ftb.left && x < ftb.right && y >= ftb.top && y < ftb.bottom) { + bankPanelToggledBanksFullHeight(); + return true; + } + // "+" create button. + const RECT cb = createBtnRect(region); + if (x >= cb.left && x < cb.right && y >= cb.top && y < cb.bottom) { + doCreateBank(); + return true; + } + // Tab strip: chevrons scroll, a tab click SHOWS that bank (browse — NOT activate). + const TabStripRect strip = banksTabStripRect(region); + const std::vector tabs = namedBanks(); + const int n = static_cast(tabs.size()); + const TabHit hit = hitTestTabStrip(x, y, strip, n, kTabSpec, g_panel.tabScroll); + if (hit.kind == TabHitKind::ScrollLeft || hit.kind == TabHitKind::ScrollRight) { + const TabStripLayout layout = + computeTabStripLayout(strip, n, kTabSpec, g_panel.tabScroll); + const int step = kTabSpec.tabWidth; + const int desired = g_panel.tabScroll + + (hit.kind == TabHitKind::ScrollLeft ? -step : step); + g_panel.tabScroll = clampTabScroll(desired, layout); + invalidatePanel(); + return true; + } + if (hit.kind == TabHitKind::Tab) { + const Bank* bk = tabs[static_cast(hit.index)]; + if (bk->id != g_panel.shownBankId) { + g_panel.shownBankId = bk->id; // browse: show this bank's grid + g_panel.selection = Selection{}; // grid changed — reset selection + stopAudition(); + } + g_panel.focusedRegion = Region::Banks; + invalidatePanel(); + return true; + } + return false; +} + +// Handles the pool region's full-height toggle. Returns true if consumed. +bool handlePoolChromeClick(int x, int y, const RECT& region) { + const RECT ftb = fullHtBtnRect(region); + if (x >= ftb.left && x < ftb.right && y >= ftb.top && y < ftb.bottom) { + bankPanelToggledPoolFullHeight(); + return true; + } + return false; +} + +// Applies a left-click at (x, y): route to mode switch / footer / region chrome / +// grid selection, and arm a potential drag when the click lands on a selected cell. +void handleClick(int x, int y) { + RECT cr{}; + GetClientRect(g_panel.hwnd, &cr); + const int w = cr.right - cr.left, h = cr.bottom - cr.top; + + // Mode-switch header (D5) takes precedence. + if (g_panel.session) { + const int seg = hitTestSegment(x, y, panelHeader(w), modeCount()); + if (seg >= 0) { + const std::vector& modes = g_panel.session->view().modes().all(); + if (seg < static_cast(modes.size())) { + applyMode(g_panel.session->view(), + modes[static_cast(seg)].id, nullptr); + invalidatePanel(); + } + return; + } + } + + // Tail footer: a click anywhere cycles the tail mode. + const RECT f = panelFooter(w, h); + if (f.top < f.bottom && x >= f.left && x < f.right && y >= f.top && y < f.bottom) { + g_panel.tail.mode = cycleTailMode(g_panel.tail.mode); + invalidatePanel(); + return; + } + + // Region chrome (headers, tab strip, buttons). + if (poolShown()) { + const RECT pr = poolRegionRect(w, h); + if (y >= pr.top && y < regionGridRect(pr, false).top) { + if (handlePoolChromeClick(x, y, pr)) return; + } + } + if (banksShown()) { + const RECT br = banksRegionRect(w, h); + if (y >= br.top && y < regionGridRect(br, true).top) { + if (handleBanksChromeClick(x, y, br)) return; + } + } + + // Grid selection. Resolve which region's grid the point is in. + Region reg = Region::Pool; + if (!regionAt(x, y, reg)) return; + const bool isBanks = reg == Region::Banks; + const RECT region = isBanks ? banksRegionRect(w, h) : poolRegionRect(w, h); + const BankIndex* index = indexForRegion(reg); + const std::vector rects = regionCellRects(region, isBanks, index); + const int hit = hitTestCell(x, y, rects); + const int count = index ? static_cast(index->size()) : 0; + + // Switching focus region reseeds the selection there. + if (g_panel.focusedRegion != reg) { + g_panel.focusedRegion = reg; + g_panel.selection = Selection{}; + stopAudition(); + } + + if (hit < 0) { + if (!g_panel.selection.empty() || g_panel.selection.focus >= 0) { + g_panel.selection = Selection{}; + stopAudition(); + } + invalidatePanel(); + return; + } + + // If the pressed cell is already SELECTED and no modifier is held, arm a drag — + // the actual selection change is deferred to LBUTTONUP if no drag begins (so a + // plain click on a multi-selection can start a drag without collapsing it first). + // Otherwise apply the click immediately. This is the M5-multi-select-drag + // disambiguation: M5 has no cell drag; a drag here begins only from a selected + // cell past a movement threshold (see onMouseMove), so plain click/shift/ctrl + // multi-select is untouched. + const bool onSelected = g_panel.selection.contains(hit); + if (onSelected && !ctrlDown() && !shiftDown()) { + g_panel.dragArmed = true; + g_panel.dragStartX = x; + g_panel.dragStartY = y; + g_panel.dragSourceRegion = reg; + // Keep the current (multi-)selection as the drag payload candidate. + g_panel.selection.focus = hit; // move the caret to the pressed cell + invalidatePanel(); + return; + } + + g_panel.selection = applyClick(g_panel.selection, hit, ctrlDown(), shiftDown(), count); g_panel.selItemCount = count; invalidatePanel(); } -// The column count for the panel's CURRENT client width (nav needs the same wrap -// the layout uses). >= 1. -int columnsNow() { - if (!g_panel.hwnd) return 1; +// The column count for a region's current grid width (nav needs the layout's wrap). +int columnsForRegion(Region reg) { RECT cr{}; GetClientRect(g_panel.hwnd, &cr); - return columnsForWidth(cr.right - cr.left, kGrid); + const int w = cr.right - cr.left, h = cr.bottom - cr.top; + const RECT region = reg == Region::Banks ? banksRegionRect(w, h) + : poolRegionRect(w, h); + const RECT grid = regionGridRect(region, reg == Region::Banks); + return columnsForWidth(grid.right - grid.left, kGrid); } -// True iff `hwnd` is our panel window or a descendant of it (the accelerator hook -// only claims keys when focus is inside the panel). Walks the parent chain. bool isOurWindow(HWND hwnd) { for (HWND w = hwnd; w; w = GetParent(w)) if (w == g_panel.hwnd) return true; return false; } -// Handles a key-down (virtual key `vk`) while the panel is focused. Returns true if -// the key was consumed (arrow nav / Enter/Space audition / Esc stop), false to let -// REAPER handle it. Arrow keys mutate the selection through the pure nav model and -// repaint; Shift extends. READ-ONLY: never mutates the bank/project. bool handleKey(int vk) { - const int count = bankItemCount(); + const int count = focusedItemCount(); if (count <= 0) return false; switch (vk) { @@ -818,23 +1393,19 @@ bool handleKey(int vk) { : vk == VK_RIGHT ? NavKey::Right : vk == VK_UP ? NavKey::Up : NavKey::Down; - g_panel.selection = - navigate(g_panel.selection, nk, columnsNow(), count, shiftDown()); + g_panel.selection = navigate(g_panel.selection, nk, + columnsForRegion(g_panel.focusedRegion), + count, shiftDown()); g_panel.selItemCount = count; invalidatePanel(); return true; } case VK_RETURN: case VK_SPACE: - // Audition the focused cell. Enter/Space with no focus does nothing - // (nothing to play). Re-audition stops the previous inside startAudition. if (g_panel.selection.focus >= 0) startAudition(g_panel.selection.focus); return true; case VK_ESCAPE: - // Stop audition (does not clear the selection — Esc is "stop", not - // "deselect"). No-op when nothing is playing; still consume so REAPER - // does not treat Esc as a global stop while the panel is focused. stopAudition(); return true; default: @@ -842,35 +1413,149 @@ bool handleKey(int vk) { } } -// The keyboard accelerator hook (registered with "accelerator"). REAPER calls this -// for every keystroke; we claim arrow/Enter/Space/Esc ONLY when focus is inside the -// panel, eating them so REAPER does not steal arrows for the arrange. Returns 1 to -// eat, 0 to pass on (not our window / not our key). int translateAccel(MSG* msg, accelerator_register_t* /*ctx*/) { - if (!msg || msg->message != WM_KEYDOWN) return 0; // key-down only + if (!msg || msg->message != WM_KEYDOWN) return 0; if (!g_panel.open || !g_panel.hwnd) return 0; - if (!isOurWindow(GetFocus())) return 0; // focus not in the panel + if (!isOurWindow(GetFocus())) return 0; return handleKey(static_cast(msg->wParam)) ? 1 : 0; } accelerator_register_t g_accel{translateAccel, true, nullptr}; bool g_accelRegistered = false; -// Registers the keyboard hook once (on first panel open). isLocal must be true -// (reaper_plugin.h). Safe to call repeatedly. void registerAccel() { if (g_accelRegistered || !g_rec) return; g_rec->Register("accelerator", &g_accel); g_accelRegistered = true; } -// Mirror-unregisters the keyboard hook on teardown. void unregisterAccel() { if (!g_accelRegistered || !g_rec) return; g_rec->Register("-accelerator", &g_accel); g_accelRegistered = false; } +// --- Drag (move between regions/onto a tab) ----------------------------------- + +constexpr int kDragThreshold = 5; // px the pointer must move to begin a drag + +// Resolves the drop target under client (x, y) during a drag, updating dropKind / +// dropBankId. A drop onto the pool region -> the pool; onto a named tab -> that bank; +// anywhere else -> none. +void updateDropTarget(int x, int y) { + RECT cr{}; + GetClientRect(g_panel.hwnd, &cr); + const int w = cr.right - cr.left, h = cr.bottom - cr.top; + + g_panel.dropKind = DropKind::None; + g_panel.dropBankId.clear(); + + if (banksShown()) { + const RECT br = banksRegionRect(w, h); + const TabStripRect strip = banksTabStripRect(br); + const std::vector tabs = namedBanks(); + const TabHit hit = hitTestTabStrip(x, y, strip, + static_cast(tabs.size()), kTabSpec, + g_panel.tabScroll); + if (hit.kind == TabHitKind::Tab) { + g_panel.dropKind = DropKind::Tab; + g_panel.dropBankId = tabs[static_cast(hit.index)]->id; + return; + } + } + if (poolShown()) { + const RECT pr = poolRegionRect(w, h); + const RECT grid = regionGridRect(pr, false); + if (x >= grid.left && x < grid.right && y >= grid.top && y < grid.bottom) { + g_panel.dropKind = DropKind::PoolRegion; + return; + } + } +} + +void onMouseMove(int x, int y) { + if (g_panel.dragArmed && !g_panel.dragging) { + if (std::abs(x - g_panel.dragStartX) > kDragThreshold || + std::abs(y - g_panel.dragStartY) > kDragThreshold) { + // Threshold crossed — begin the drag. Snapshot the payload NOW. + g_panel.dragging = true; + g_panel.dragSourceBankId = bankIdForRegion(g_panel.dragSourceRegion); + g_panel.dragSampleIds = focusedSelectionIds(); + SetCapture(g_panel.hwnd); + } + } + if (g_panel.dragging) { + updateDropTarget(x, y); + invalidatePanel(); + } +} + +// Commits (or abandons) a drag on button-up. A drop onto a DIFFERENT bank moves the +// dragged samples there; a drop onto the source bank / dead space is a no-op. Ctrl +// held at drop = copy (the deliberate secondary), else move. +void onLBtnUp(int x, int y) { + if (g_panel.dragging) { + updateDropTarget(x, y); + std::string destId; + if (g_panel.dropKind == DropKind::PoolRegion) destId = std::string(kPoolBankId); + else if (g_panel.dropKind == DropKind::Tab) destId = g_panel.dropBankId; + + if (!destId.empty() && destId != g_panel.dragSourceBankId && + !g_panel.dragSampleIds.empty()) { + transferSamples(g_panel.dragSampleIds, g_panel.dragSourceBankId, destId, + /*copy=*/ctrlDown()); + } + if (GetCapture() == g_panel.hwnd) ReleaseCapture(); + } else if (g_panel.dragArmed) { + // Press-release on a selected cell with no drag: treat as a plain click that + // collapses the multi-selection to the pressed cell (standard behavior). + const BankIndex* idx = indexForRegion(g_panel.focusedRegion); + const int count = idx ? static_cast(idx->size()) : 0; + const int focus = g_panel.selection.focus; + if (focus >= 0) + g_panel.selection = applyClick(g_panel.selection, focus, false, false, count); + } + g_panel.dragArmed = false; + g_panel.dragging = false; + g_panel.dropKind = DropKind::None; + g_panel.dropBankId.clear(); + invalidatePanel(); +} + +// A right-click: on a named tab -> the tab management menu; on a grid cell of the +// focused region with a selection -> the move/copy menu. +void handleRightClick(int x, int y) { + RECT cr{}; + GetClientRect(g_panel.hwnd, &cr); + const int w = cr.right - cr.left, h = cr.bottom - cr.top; + + // Tab management menu. + if (banksShown()) { + const RECT br = banksRegionRect(w, h); + const TabStripRect strip = banksTabStripRect(br); + const std::vector tabs = namedBanks(); + const TabHit hit = hitTestTabStrip(x, y, strip, + static_cast(tabs.size()), kTabSpec, + g_panel.tabScroll); + if (hit.kind == TabHitKind::Tab) { + POINT pt{x, y}; + ClientToScreen(g_panel.hwnd, &pt); + showTabMenu(pt.x, pt.y, tabs[static_cast(hit.index)]->id); + return; + } + } + + // Grid selection menu (move/copy). Only when the right-click lands in the focused + // region's grid and there is a selection. + Region reg = Region::Pool; + if (regionAt(x, y, reg) && reg == g_panel.focusedRegion && + !g_panel.selection.empty()) { + POINT pt{x, y}; + ClientToScreen(g_panel.hwnd, &pt); + showSelectionMenu(pt.x, pt.y); + } +} + // --- Dialog proc + docking ---------------------------------------------------- WDL_DLGRET dlgProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) { @@ -883,22 +1568,25 @@ WDL_DLGRET dlgProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) { return 0; } case WM_LBUTTONDOWN: { - // Take keyboard focus so the accelerator hook routes arrows/audition - // keys to us, then resolve the click. Coordinates are client-relative - // signed shorts in lParam (SWELL sets these even though it omits the - // MK_* modifier bits in wParam — hence GetAsyncKeyState for modifiers). SetFocus(hwnd); - const int x = GET_X_LPARAM(lParam); - const int y = GET_Y_LPARAM(lParam); - handleClick(x, y); + handleClick(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)); return 0; } + case WM_MOUSEMOVE: + onMouseMove(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)); + return 0; + case WM_LBUTTONUP: + onLBtnUp(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)); + return 0; + case WM_RBUTTONDOWN: + SetFocus(hwnd); + handleRightClick(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)); + return 0; case WM_DESTROY: - // REAPER closed the dock (user X'd it). Stop any audition (window-close - // stop path — no preview may outlive the window) and reflect closed - // state so the toggle re-opens rather than reusing a dead HWND. + if (GetCapture() == hwnd) ReleaseCapture(); stopAudition(); g_panel.selection = Selection{}; + g_panel.dragArmed = g_panel.dragging = false; g_panel.hwnd = nullptr; g_panel.open = false; return 0; @@ -913,36 +1601,27 @@ void openPanel() { DockWindowActivate(g_panel.hwnd); return; } - // Create the dialog as a child (WS_CHILD in the template); REAPER's docker - // reparents it. lParam is unused (state lives in g_panel). - // Set up the preview register's lock ONCE before the window can audition. initPreview(); g_panel.hwnd = CreateDialogParam(g_hInst, MAKEINTRESOURCE(IDD_BANK_PANEL), GetMainHwnd(), dlgProc, 0); if (!g_panel.hwnd) return; - // Dock it. identstr is a stable per-window key REAPER uses to remember the - // dock position/state across sessions; FOREVER-STABLE like the action ids. - // allowShow=true asks REAPER to show the dock if hidden. DockWindowAddEx(g_panel.hwnd, "ReaSampler Bank", "reasampler_bank_panel", true); DockWindowActivate(g_panel.hwnd); g_panel.open = true; - // Start receiving arrow/audition keys while the panel is open. registerAccel(); - // Prime the fingerprint so the first timer tick doesn't count the initial - // bank as a "change" (it's already drawn on open). + reconcileShownBank(); refreshFingerprint(); } void closePanel() { - // Stop audition before the window goes away (window-close stop path). WM_DESTROY - // also stops, but stop here too so a DockWindowRemove that suppresses WM_DESTROY - // still tears the preview down (idempotent: stopAudition no-ops if not playing). + if (GetCapture() == g_panel.hwnd) ReleaseCapture(); stopAudition(); g_panel.selection = Selection{}; + g_panel.dragArmed = g_panel.dragging = false; unregisterAccel(); if (g_panel.hwnd) { DockWindowRemove(g_panel.hwnd); @@ -972,50 +1651,33 @@ bool bankPanelIsOpen() { } std::vector bankPanelSelectedSampleIds() { - std::vector ids; - const BankIndex* bank = g_panel.session ? &g_panel.session->bank() : nullptr; - if (!bank) return ids; - const std::vector& samples = bank->all(); - const int count = static_cast(samples.size()); - // selection.indices is sorted-ascending unique (bank_grid invariant), so the - // returned ids come out in bank order. Guard each index against the live count - // in case the selection outran a shrink the fingerprint pass hasn't cleared yet. - for (int idx : g_panel.selection.indices) { - if (idx >= 0 && idx < count) - ids.push_back(samples[static_cast(idx)].id); - } - return ids; + return focusedSelectionIds(); +} + +std::string bankPanelSelectedSourceBankId() { + // The focused region's displayed bank is the move/copy source. Default to the + // pool (a safe source) when nothing is selected / the panel never opened. + if (g_panel.selection.empty()) return std::string(kPoolBankId); + const std::string id = bankIdForRegion(g_panel.focusedRegion); + return id.empty() ? std::string(kPoolBankId) : id; } void bankPanelRefresh() { if (!g_panel.open || !g_panel.hwnd) return; - // Repaint only when the bank actually changed (generation bump). Cheap tick - // otherwise — just a fingerprint string compare. if (refreshFingerprint()) InvalidateRect(g_panel.hwnd, nullptr, FALSE); } TailSetting bankPanelTailSetting() { - // In-memory for the extension's lifetime (g_panel is static): the toggle's mode - // survives panel open/close and bank changes, and resets to the default None only - // on extension unload. Persistence across project reload is a noted follow-on. - // manualMs is clamped here so a caller always receives a within-cap length, even - // if a future fine-adjust UI stored an over-cap value. TailSetting s = g_panel.tail; s.manualMs = clampManualMs(s.manualMs); return s; } BankPanelFullHeight bankPanelFullHeight() { - // In-memory for the extension's lifetime (g_panel is static), like the tail - // setting: survives panel open/close and bank changes, resets to Split on unload. return g_panel.fullHeight; } -// Shared toggle body: enter `target` from any other state, or fall back to Split when -// already at `target` (a second press restores the split). Requests a repaint via the -// same InvalidateRect the refresh path uses, so an open panel reflects the change; a -// closed panel (hwnd null) simply stores the bit for B4 to render when it opens. static void setFullHeight(BankPanelFullHeight target) { g_panel.fullHeight = (g_panel.fullHeight == target) ? BankPanelFullHeight::Split : target; @@ -1031,8 +1693,8 @@ void bankPanelToggledBanksFullHeight() { } void bankPanelShutdown() { - closePanel(); // stops audition + destroys the window - deinitPreview(); // destroy the preview lock (after the last stop) + closePanel(); + deinitPreview(); g_panel.cache.clear(); g_panel.session = nullptr; } diff --git a/src/bank_panel.h b/src/bank_panel.h index 95405a8..294c59b 100644 --- a/src/bank_panel.h +++ b/src/bank_panel.h @@ -43,8 +43,23 @@ bool bankPanelIsOpen(); // Note: the panel's selection is cleared on a bank change (capture / project // load), so a returned id always names a sample present in the current bank at // the moment of the call; the caller still tolerates an absent id gracefully. +// +// Phase B4 (vertical split): the selection lives in whichever REGION the user last +// interacted with (the pool grid on top or a named-bank grid below), which is NOT +// necessarily the active/capture-target bank. The returned ids therefore name +// samples in the FOCUSED region's displayed bank — the bank the user visibly +// selected in. Pair with bankPanelSelectedSourceBankId() to know which bank those +// ids belong to (the move/copy source). std::vector bankPanelSelectedSampleIds(); +// The bank id the current selection belongs to — the displayed bank of the region +// the user last interacted with (pool region -> the pool id; named-banks region -> +// the shown tab's bank id). This is the SOURCE bank for a move/copy of the current +// selection, and it is distinct from the active/capture-target bank (active ≠ shown). +// Returns the pool id when nothing is selected or the panel has never opened (a safe +// default source). READ of panel state only; no mutation. +std::string bankPanelSelectedSourceBankId(); + // Requests a repaint if the bank changed since the last paint (generation bump). // Cheap when nothing changed. Driven by the timer so a capture / project load is // reflected without the panel diffing the bank itself. diff --git a/src/insert.cpp b/src/insert.cpp index 32153a3..509200b 100644 --- a/src/insert.cpp +++ b/src/insert.cpp @@ -125,7 +125,13 @@ InsertResult runInsert(ReaSamplerSession* session, const InsertRequest& request) const std::string projectDir = currentProjectDir(); if (projectDir.empty()) { result.status = InsertStatus::NoProject; return result; } - const BankIndex& bank = session->bank(); + // Resolve the id against the bank the SELECTION came from — under B4's vertical + // split the selection may live in the pool or a shown named bank, which is NOT + // necessarily the active/capture-target bank. Fall back to the active bank when + // the source id names no bank (defensive). + const std::string srcBankId = bankPanelSelectedSourceBankId(); + const BankIndex* srcIndex = session->book().index(srcBankId); + const BankIndex& bank = srcIndex ? *srcIndex : session->bank(); const Sample* sample = bank.query(id); if (!sample) { result.status = InsertStatus::NothingResolved; return result; } diff --git a/src/tab_strip.cpp b/src/tab_strip.cpp new file mode 100644 index 0000000..68c57a2 --- /dev/null +++ b/src/tab_strip.cpp @@ -0,0 +1,112 @@ +// tab_strip — pure implementation. See tab_strip.h. NO REAPER / SWELL / vendor. + +#include "tab_strip.h" + +#include + +namespace reasampler { + +TabStripLayout computeTabStripLayout(const TabStripRect& strip, int tabCount, + const TabStripSpec& spec, int scrollOffset) { + (void)scrollOffset; // layout depends on geometry only, not the current offset + TabStripLayout out; + if (tabCount <= 0 || strip.width <= 0) { + out.trackX = strip.x; + out.trackWidth = strip.width > 0 ? strip.width : 0; + return out; // nothing to lay out: track == strip, no overflow, no chevrons + } + + const int totalTabsWidth = tabCount * spec.tabWidth; + if (totalTabsWidth <= strip.width) { + // Everything fits: the whole strip is the track; no chevrons, no scroll. + out.overflow = false; + out.trackX = strip.x; + out.trackWidth = strip.width; + out.maxScroll = 0; + return out; + } + + // Overflow: reserve a chevron band at each end; the tabs live between them. + out.overflow = true; + out.leftChevron = true; + out.rightChevron = true; + out.trackX = strip.x + spec.chevronWidth; + out.trackWidth = strip.width - 2 * spec.chevronWidth; + if (out.trackWidth < 0) out.trackWidth = 0; + // The tab run exceeds the track by this many pixels; the strip may scroll exactly + // that far so the last tab's right edge reaches the track's right edge, no more. + out.maxScroll = totalTabsWidth - out.trackWidth; + if (out.maxScroll < 0) out.maxScroll = 0; + return out; +} + +int clampTabScroll(int desiredOffset, const TabStripLayout& layout) { + if (desiredOffset < 0) return 0; + if (desiredOffset > layout.maxScroll) return layout.maxScroll; + return desiredOffset; +} + +std::vector computeTabRects(const TabStripRect& strip, int tabCount, + const TabStripSpec& spec, int scrollOffset) { + std::vector rects; + if (tabCount <= 0 || strip.width <= 0) return rects; + + const TabStripLayout layout = + computeTabStripLayout(strip, tabCount, spec, scrollOffset); + const int offset = layout.overflow ? clampTabScroll(scrollOffset, layout) : 0; + const int trackLeft = layout.trackX; + const int trackRight = layout.trackX + layout.trackWidth; + + rects.reserve(static_cast(tabCount)); + for (int i = 0; i < tabCount; ++i) { + const int rawLeft = trackLeft + i * spec.tabWidth - offset; + const int rawRight = rawLeft + spec.tabWidth; + // Clip to the track: a partially-scrolled tab must not draw under a chevron + // or spill past the track. A tab whose clipped extent is empty is omitted. + int left = rawLeft < trackLeft ? trackLeft : rawLeft; + int right = rawRight > trackRight ? trackRight : rawRight; + if (right <= left) continue; // fully scrolled out of view either side + TabRect r; + r.index = i; + r.x = left; + r.y = strip.y; + r.width = right - left; + r.height = strip.height; + rects.push_back(r); + } + return rects; +} + +TabHit hitTestTabStrip(int px, int py, const TabStripRect& strip, int tabCount, + const TabStripSpec& spec, int scrollOffset) { + TabHit miss; // {None, -1} + if (tabCount <= 0 || strip.width <= 0 || strip.height <= 0) return miss; + + // Reject anything outside the strip band first (half-open bounds). + if (px < strip.x || px >= strip.x + strip.width || + py < strip.y || py >= strip.y + strip.height) + return miss; + + const TabStripLayout layout = + computeTabStripLayout(strip, tabCount, spec, scrollOffset); + + // Chevrons take precedence at the strip ends: a click in a reserved chevron band + // is a scroll, never a tab (the tab track excludes those bands). + if (layout.overflow) { + if (px < strip.x + spec.chevronWidth) + return TabHit{TabHitKind::ScrollLeft, -1}; + if (px >= strip.x + strip.width - spec.chevronWidth) + return TabHit{TabHitKind::ScrollRight, -1}; + } + + // Inside the track: find the visible tab whose clipped rect contains px. Reuse + // computeTabRects so the hit matches exactly what was drawn (clipping included). + const std::vector rects = + computeTabRects(strip, tabCount, spec, scrollOffset); + for (const TabRect& r : rects) { + if (px >= r.x && px < r.x + r.width) return TabHit{TabHitKind::Tab, r.index}; + } + return miss; // track dead space (no tab under the point) +} + +} // namespace reasampler diff --git a/src/tab_strip.h b/src/tab_strip.h new file mode 100644 index 0000000..7932070 --- /dev/null +++ b/src/tab_strip.h @@ -0,0 +1,135 @@ +#pragma once +// tab_strip — the REAPER-free layout + hit-test math behind the bank_panel's +// named-banks tab strip (Phase B, Wave 4 — B4). The named-banks region of the +// vertical-split bank window is a LICE-drawn tab strip (one tab per named bank, +// NOT a SWELL-native tab control), and — from the start — it must scroll when the +// tabs overflow the strip width (a naive fixed-width strip breaks down at ~8–12 +// tabs). What is NOT DAW-bound — how N fixed-width tabs tile a strip of a given +// pixel width, where the overflow chevrons sit, which tab/chevron a click lands in, +// and how far the strip may scroll — lives here so it is unit-tested outside the +// DAW (CLAUDE.md §load-bearing split). The panel shell (bank_panel.cpp) owns the +// SWELL window, LICE drawing, and the live BankBook read; it calls into this seam +// for every rect and every hit. Mirror of mode_switch / bank_grid. +// +// PURE MODULE: NO REAPER types, NO SWELL, NO vendor/ includes. Standard library +// only. Builds and unit-tests without REAPER. + +#include + +namespace reasampler { + +// The strip the tabs are drawn into, top-left origin (SWELL/LICE convention). +// (x, y) is the top-left corner; width/height are the strip extents. The panel +// reserves this as a fixed-height band at the top of the named-banks region. +struct TabStripRect { + int x = 0; + int y = 0; + int width = 0; + int height = 0; + + bool operator==(const TabStripRect& o) const { + return x == o.x && y == o.y && width == o.width && height == o.height; + } +}; + +// Fixed inputs that shape the strip. tabWidth is the pixel width of each tab (fixed +// so the strip reads as a uniform segmented control and overflow math stays simple — +// labels ellipsize within the tab, they do not resize it). chevronWidth is the width +// reserved at each end for the scroll affordance WHEN the tabs overflow; when they +// fit, no chevron is reserved and the tabs use the full strip width. +struct TabStripSpec { + int tabWidth = 96; + int chevronWidth = 20; +}; + +// One tab's pixel rectangle within the strip, top-left origin, ALREADY translated +// by the current scroll offset and clipped to the visible track. `index` is the +// tab's index in the caller's list (ordinal order) so the shell can label/​light it +// without re-deriving. A tab scrolled fully out of view is omitted from the result +// (the shell only draws what computeTabRects returns), so every returned rect is at +// least partially visible. +struct TabRect { + int index = 0; + int x = 0; + int y = 0; + int width = 0; + int height = 0; + + bool operator==(const TabRect& o) const { + return index == o.index && x == o.x && y == o.y && + width == o.width && height == o.height; + } +}; + +// The scrollable track's geometry: where the tabs may be drawn (between the +// chevrons when overflowing, or the whole strip when they fit) and whether each +// chevron is present. Derived once and shared by layout + hit-testing so both agree. +struct TabStripLayout { + bool overflow = false; // true iff N tabs at tabWidth exceed the track width + int trackX = 0; // left edge of the tab track (past the left chevron) + int trackWidth = 0; // width available to tabs (strip minus both chevrons) + int maxScroll = 0; // largest valid scroll offset (0 when no overflow) + bool leftChevron = false; // a left-scroll affordance is reserved this frame + bool rightChevron = false;// a right-scroll affordance is reserved this frame +}; + +// Computes the strip layout for `tabCount` tabs of `spec.tabWidth` in `strip`, +// given the current `scrollOffset`. Pure geometry: +// * No overflow (all tabs fit the strip width): overflow=false, no chevrons, the +// track IS the strip, maxScroll=0. +// * Overflow: both chevrons are reserved (chevronWidth each), the track is the +// strip minus both chevrons, and maxScroll is the pixels by which the tab run +// exceeds the track (so the last tab's right edge can reach the track's right +// edge but not scroll past it). Chevrons are always both present under overflow +// (a fixed affordance is simpler and unambiguous than hiding one at an end; +// clicking a chevron at a scroll limit is a harmless no-op the shell clamps). +// tabCount <= 0 or a non-positive strip width returns a zeroed layout (no overflow, +// track == strip, maxScroll 0). +TabStripLayout computeTabStripLayout(const TabStripRect& strip, int tabCount, + const TabStripSpec& spec, int scrollOffset); + +// Clamps a desired scroll offset into [0, maxScroll] for the given layout. The shell +// calls this after a chevron click / wheel so the strip never scrolls past either +// end. maxScroll is 0 when the tabs fit, so a fitting strip always clamps to 0. +int clampTabScroll(int desiredOffset, const TabStripLayout& layout); + +// Tiles `tabCount` fixed-width tabs left-to-right into the layout's track, shifted +// left by `scrollOffset`, and returns the rects that are at least partially visible +// (in tab-index order). Each tab i sits at trackX + i*tabWidth - scrollOffset; a tab +// whose visible extent is empty (fully left of or right of the track) is omitted. +// Returned rects are CLIPPED to the track horizontally so a partially-scrolled tab +// does not draw under a chevron. The caller passes the SAME scrollOffset it passed +// to computeTabStripLayout (the shell clamps once, then uses the clamped value for +// both). tabCount <= 0 -> empty. +std::vector computeTabRects(const TabStripRect& strip, int tabCount, + const TabStripSpec& spec, int scrollOffset); + +// What a point in the strip resolves to. +enum class TabHitKind { + None, // outside the strip, or in dead space between visible tabs + Tab, // a tab — `index` is the tab's index in the caller's list + ScrollLeft, // the left overflow chevron + ScrollRight, // the right overflow chevron +}; + +// The outcome of hit-testing a point against the strip. For Tab, `index` is the tab +// index; for the chevrons and None it is -1. +struct TabHit { + TabHitKind kind = TabHitKind::None; + int index = -1; + + bool operator==(const TabHit& o) const { + return kind == o.kind && index == o.index; + } +}; + +// Hit-tests a point (SWELL/LICE top-left client coords) against the strip laid out +// for `tabCount` tabs at `scrollOffset`. Chevrons take precedence over tabs at the +// strip ends (a click in the reserved chevron band is a scroll, never a tab), and a +// point outside the strip band, or in the track but not on any visible tab, is None. +// Half-open bounds match computeTabRects / the chevron bands so no pixel is claimed +// twice. The shell passes the SAME clamped scrollOffset it drew with. +TabHit hitTestTabStrip(int px, int py, const TabStripRect& strip, int tabCount, + const TabStripSpec& spec, int scrollOffset); + +} // namespace reasampler diff --git a/tests/test_bank_book.cpp b/tests/test_bank_book.cpp index 5c96e22..1058056 100644 --- a/tests/test_bank_book.cpp +++ b/tests/test_bank_book.cpp @@ -521,6 +521,123 @@ static void testActiveBankResolveAfterCorruptPersistedId() { CHECK(back && back->activeBankId() == std::string(kPoolBankId)); } +// --- B4 fold-in: deserialize coalesces duplicate folded display names -------- +// +// The in-model create/rename path enforces unique display names under the trimmed + +// case-insensitive fold, but a hand-edited .rpp blob can carry two banks whose names +// fold to the same key. deserialize must NOT reject the whole book (that would drop +// the user's entire library over one collision) — it AUTO-DISAMBIGUATES the later +// duplicate deterministically so the book loads intact with unique names, all banks +// and samples preserved, and ids untouched. + +// Rewrites the first occurrence of `from` in `s` to `to` (test helper: injects a +// colliding display name into a serialized blob to simulate a hand-edit). +static std::string replaceFirst(std::string s, const std::string& from, + const std::string& to) { + const auto pos = s.find(from); + if (pos != std::string::npos) s.replace(pos, from.size(), to); + return s; +} + +static void testDeserializeCoalescesDuplicateFoldedNames() { + // Build a real book with two distinctly-named banks each holding a sample, then + // corrupt the second bank's display name so it folds to the first's key + // (" drums " folds to "drums", same as "Drums"). This is exactly what a + // hand-edited blob would look like. + BankBook book; + CHECK(book.createBank("a", "Drums")); + CHECK(book.createBank("b", "Bass")); + CHECK(book.bank("a")->index.add(sampleWith("a1")) == AddResult::Added); + CHECK(book.bank("b")->index.add(sampleWith("b1")) == AddResult::Added); + + const std::string json = book.serialize(); + // Rename bank "b" from "Bass" to " drums " (folds to "drums") — a duplicate of "a". + const std::string corrupted = + replaceFirst(json, "\"displayName\":\"Bass\"", "\"displayName\":\" drums \""); + CHECK(corrupted != json); // the substitution landed + + auto back = BankBook::deserialize(corrupted); + CHECK(back.has_value()); + if (!back) return; + + // The book loaded intact: pool + 2 named banks, no bank lost. + CHECK(back->size() == 3); + // Ids are preserved (disambiguation touches names only, never ids). + CHECK(back->bank("a") != nullptr); + CHECK(back->bank("b") != nullptr); + // The FIRST bank to carry the folded key keeps its name; the later one is + // suffixed to a unique name. + CHECK(back->bank("a")->displayName == "Drums"); + CHECK(back->bank("b")->displayName != back->bank("a")->displayName); + + // The disambiguated names are genuinely unique under the model's own fold — the + // book can now round-trip through the in-model uniqueness invariant. Prove it by + // re-serializing and re-parsing: idempotent, no further renames. + const std::string json2 = back->serialize(); + auto back2 = BankBook::deserialize(json2); + CHECK(back2.has_value()); + if (back2) CHECK(back2->serialize() == json2); + + // No sample was lost across the coalesce. + CHECK(back->bank("a")->index.size() == 1); + CHECK(back->bank("b")->index.size() == 1); + CHECK(back->bank("a")->index.query("id-a1") != nullptr); + CHECK(back->bank("b")->index.query("id-b1") != nullptr); +} + +static void testDeserializeCoalescesMultipleCollisions() { + // Three banks all folding to the same key: the first keeps its name, the next two + // get distinct suffixes so all three end unique (no two disambiguate to the same). + BankBook book; + CHECK(book.createBank("a", "Drums")); + CHECK(book.createBank("b", "Bass")); + CHECK(book.createBank("c", "Keys")); + + std::string json = book.serialize(); + json = replaceFirst(json, "\"displayName\":\"Bass\"", "\"displayName\":\"drums\""); + json = replaceFirst(json, "\"displayName\":\"Keys\"", "\"displayName\":\"DRUMS\""); + + auto back = BankBook::deserialize(json); + CHECK(back.has_value()); + if (!back) return; + CHECK(back->size() == 4); // pool + 3, none lost + + // All three named banks carry distinct folded keys after coalesce. + const std::string na = back->bank("a")->displayName; + const std::string nb = back->bank("b")->displayName; + const std::string nc = back->bank("c")->displayName; + CHECK(na != nb); + CHECK(na != nc); + CHECK(nb != nc); + + // Re-parse proves the result satisfies the round-trip (unique keys throughout). + auto back2 = BankBook::deserialize(back->serialize()); + CHECK(back2.has_value()); + if (back2) CHECK(back2->serialize() == back->serialize()); +} + +static void testDeserializeNamedBankCollidingWithPoolIsDisambiguated() { + // A named bank whose name folds to the pool's reserved "Pool" key is renamed away + // from the pool (never the reverse — the pool's name is fixed and reserved). + BankBook book; + CHECK(book.createBank("a", "Drums")); + std::string json = book.serialize(); + json = replaceFirst(json, "\"displayName\":\"Drums\"", "\"displayName\":\"pool\""); + + auto back = BankBook::deserialize(json); + CHECK(back.has_value()); + if (!back) return; + CHECK(back->size() == 2); + // The pool keeps its authoritative name; the named bank is disambiguated off it. + CHECK(back->pool().displayName == std::string(kPoolBankName)); + CHECK(back->bank("a") != nullptr); + CHECK(back->bank("a")->displayName != std::string(kPoolBankName)); + // And it is not any case/space variant that would re-collide with "Pool". + auto back2 = BankBook::deserialize(back->serialize()); + CHECK(back2.has_value()); + if (back2) CHECK(back2->serialize() == back->serialize()); +} + int main() { testPoolSeededAndDefaults(); testPoolPrivileges(); @@ -547,6 +664,9 @@ int main() { testCycleUnknownActiveResolvesToFirst(); testCycleEmptyListYieldsEmpty(); testCycleMatchesBookOrdinalOrder(); + testDeserializeCoalescesDuplicateFoldedNames(); + testDeserializeCoalescesMultipleCollisions(); + testDeserializeNamedBankCollidingWithPoolIsDisambiguated(); if (g_fail == 0) std::printf("All tests passed.\n"); return g_fail ? 1 : 0; diff --git a/tests/test_tab_strip.cpp b/tests/test_tab_strip.cpp new file mode 100644 index 0000000..ab32613 --- /dev/null +++ b/tests/test_tab_strip.cpp @@ -0,0 +1,202 @@ +// Standalone tests for reasampler::tab_strip — no REAPER, no test framework. Same +// fast loop as the sibling pure tests (mode_switch / bank_grid et al.): assert the +// named-banks tab-strip layout, overflow/scroll math, and hit-testing directly. +// +// Covers (B4 brief §unit-test the pure seam): no-overflow tiling (tabs fit, no +// chevrons, track == strip); overflow (chevrons reserved, track shrinks, maxScroll +// = run - track); scroll clamping to [0, maxScroll]; clipped visible rects (a +// partially-scrolled tab is clipped to the track, a fully-scrolled-out tab is +// omitted); scrolling to the end surfaces the last tab; hit-testing (tab hit, +// left/right chevron precedence at the ends, dead space between visible tabs, +// outside the band above/below/left/right, half-open boundary pixels). + +#include "../src/tab_strip.h" + +#include +#include +#include + +using namespace reasampler; + +static int g_fail = 0; +#define CHECK(cond) do { if(!(cond)) { \ + std::printf("FAIL line %d: %s\n", __LINE__, #cond); ++g_fail; } } while(0) + +// --- No overflow: tabs fit --------------------------------------------------- + +// 3 tabs at 96px = 288 fit a 300-wide strip: no overflow, no chevrons, the whole +// strip is the track, maxScroll 0. +static void testFitsNoOverflow() { + TabStripRect strip{0, 0, 300, 24}; + TabStripSpec spec{96, 20}; + TabStripLayout layout = computeTabStripLayout(strip, 3, spec, 0); + CHECK(!layout.overflow); + CHECK(!layout.leftChevron && !layout.rightChevron); + CHECK(layout.trackX == 0); + CHECK(layout.trackWidth == 300); + CHECK(layout.maxScroll == 0); + + auto rects = computeTabRects(strip, 3, spec, 0); + CHECK(rects.size() == 3); + CHECK((rects[0] == TabRect{0, 0, 0, 96, 24})); + CHECK((rects[1] == TabRect{1, 96, 0, 96, 24})); + CHECK((rects[2] == TabRect{2, 192, 0, 96, 24})); +} + +// A fitting strip ignores a stray non-zero scroll offset (maxScroll 0 clamps it). +static void testFitStripIgnoresScroll() { + TabStripRect strip{0, 0, 300, 24}; + TabStripSpec spec{96, 20}; + auto rects = computeTabRects(strip, 3, spec, /*scrollOffset=*/500); + CHECK(rects.size() == 3); + CHECK(rects[0].x == 0); // offset was clamped to 0 +} + +// --- Overflow: chevrons reserved, track shrinks ------------------------------ + +// 10 tabs at 96 = 960 overflow a 300-wide strip. Chevrons (20 each) are reserved, +// so the track is [20, 280) = 260 wide; maxScroll = 960 - 260 = 700. +static void testOverflowReservesChevrons() { + TabStripRect strip{0, 0, 300, 24}; + TabStripSpec spec{96, 20}; + TabStripLayout layout = computeTabStripLayout(strip, 10, spec, 0); + CHECK(layout.overflow); + CHECK(layout.leftChevron && layout.rightChevron); + CHECK(layout.trackX == 20); + CHECK(layout.trackWidth == 260); + CHECK(layout.maxScroll == 700); +} + +// At scroll 0 the first tabs are visible from the track's left edge; a tab that +// straddles the right chevron is clipped to the track's right edge, and tabs fully +// past it are omitted. +static void testOverflowScrollZeroClipsRight() { + TabStripRect strip{0, 0, 300, 24}; + TabStripSpec spec{96, 20}; + auto rects = computeTabRects(strip, 10, spec, 0); + // Track is [20, 280). Tab 0 at [20,116), tab 1 [116,212), tab 2 [212,308) clipped + // to [212,280). Tabs 3.. start past 280 -> omitted. + CHECK(rects.size() == 3); + CHECK((rects[0] == TabRect{0, 20, 0, 96, 24})); + CHECK((rects[1] == TabRect{1, 116, 0, 96, 24})); + CHECK((rects[2] == TabRect{2, 212, 0, 68, 24})); // clipped at the track's right +} + +// Scrolling to maxScroll surfaces the LAST tab flush against the track's right edge +// and drops the earliest tabs off the left. This is the property that makes overflow +// usable: every tab is reachable by scrolling. +static void testScrollToEndSurfacesLastTab() { + TabStripRect strip{0, 0, 300, 24}; + TabStripSpec spec{96, 20}; + TabStripLayout layout = computeTabStripLayout(strip, 10, spec, 0); + auto rects = computeTabRects(strip, 10, spec, layout.maxScroll); + CHECK(!rects.empty()); + const TabRect& last = rects.back(); + CHECK(last.index == 9); // the last tab is visible + CHECK(last.x + last.width == layout.trackX + layout.trackWidth); // flush right (280) +} + +// --- Scroll clamping --------------------------------------------------------- + +static void testClampScroll() { + TabStripRect strip{0, 0, 300, 24}; + TabStripSpec spec{96, 20}; + TabStripLayout layout = computeTabStripLayout(strip, 10, spec, 0); + CHECK(clampTabScroll(-50, layout) == 0); + CHECK(clampTabScroll(0, layout) == 0); + CHECK(clampTabScroll(300, layout) == 300); + CHECK(clampTabScroll(layout.maxScroll, layout) == layout.maxScroll); + CHECK(clampTabScroll(layout.maxScroll + 999, layout) == layout.maxScroll); + + TabStripLayout fits = computeTabStripLayout(strip, 2, spec, 0); + CHECK(clampTabScroll(123, fits) == 0); // no overflow -> everything clamps to 0 +} + +// --- Hit-testing ------------------------------------------------------------- + +// No overflow: a point in a tab returns that tab; the gap-free tiling means every +// x in the strip band lands on some tab. +static void testHitFitStrip() { + TabStripRect strip{0, 0, 300, 24}; + TabStripSpec spec{96, 20}; + CHECK((hitTestTabStrip(10, 12, strip, 3, spec, 0) == TabHit{TabHitKind::Tab, 0})); + CHECK((hitTestTabStrip(100, 12, strip, 3, spec, 0) == TabHit{TabHitKind::Tab, 1})); + CHECK((hitTestTabStrip(250, 12, strip, 3, spec, 0) == TabHit{TabHitKind::Tab, 2})); + // Outside the band: above, below, left, right all miss. + CHECK((hitTestTabStrip(10, -1, strip, 3, spec, 0) == TabHit{TabHitKind::None, -1})); + CHECK((hitTestTabStrip(10, 24, strip, 3, spec, 0) == TabHit{TabHitKind::None, -1})); + CHECK((hitTestTabStrip(-1, 12, strip, 3, spec, 0) == TabHit{TabHitKind::None, -1})); + CHECK((hitTestTabStrip(300, 12, strip, 3, spec, 0) == TabHit{TabHitKind::None, -1})); +} + +// Overflow: the reserved chevron bands hit-test to the scroll affordances and take +// precedence over any tab that would otherwise sit there. +static void testHitChevrons() { + TabStripRect strip{0, 0, 300, 24}; + TabStripSpec spec{96, 20}; + // Left chevron band [0,20). + CHECK((hitTestTabStrip(5, 12, strip, 10, spec, 0) == + TabHit{TabHitKind::ScrollLeft, -1})); + CHECK((hitTestTabStrip(19, 12, strip, 10, spec, 0) == + TabHit{TabHitKind::ScrollLeft, -1})); + // Right chevron band [280,300). + CHECK((hitTestTabStrip(280, 12, strip, 10, spec, 0) == + TabHit{TabHitKind::ScrollRight, -1})); + CHECK((hitTestTabStrip(299, 12, strip, 10, spec, 0) == + TabHit{TabHitKind::ScrollRight, -1})); + // Just inside the track (x=20) is the first tab, not the left chevron. + CHECK((hitTestTabStrip(20, 12, strip, 10, spec, 0) == TabHit{TabHitKind::Tab, 0})); +} + +// A hit in the track matches the drawn (clipped) rects; a point in track dead space +// (no visible tab under it) is None. With overflow at scroll 0 the visible tabs are +// 0,1,2 (2 clipped to [212,280)); everything in [20,280) is covered here, so we test +// dead space by scrolling so a tab boundary leaves no gap — instead assert the hit +// agrees with computeTabRects for a mid-scroll offset. +static void testHitMatchesRectsMidScroll() { + TabStripRect strip{0, 0, 300, 24}; + TabStripSpec spec{96, 20}; + const int offset = 150; + auto rects = computeTabRects(strip, 10, spec, offset); + CHECK(!rects.empty()); + for (const TabRect& r : rects) { + // A point at the rect's left edge and one just inside its right edge both + // resolve to this tab (half-open bounds). + CHECK((hitTestTabStrip(r.x, 12, strip, 10, spec, offset) == + TabHit{TabHitKind::Tab, r.index})); + CHECK((hitTestTabStrip(r.x + r.width - 1, 12, strip, 10, spec, offset) == + TabHit{TabHitKind::Tab, r.index})); + } +} + +// --- Degenerate inputs ------------------------------------------------------- + +static void testDegenerate() { + TabStripRect strip{0, 0, 300, 24}; + TabStripSpec spec{96, 20}; + CHECK(computeTabRects(strip, 0, spec, 0).empty()); + CHECK((hitTestTabStrip(10, 12, strip, 0, spec, 0) == TabHit{TabHitKind::None, -1})); + + TabStripRect empty{0, 0, 0, 24}; + CHECK(computeTabRects(empty, 3, spec, 0).empty()); + TabStripLayout layout = computeTabStripLayout(empty, 3, spec, 0); + CHECK(!layout.overflow); + CHECK(layout.maxScroll == 0); +} + +int main() { + testFitsNoOverflow(); + testFitStripIgnoresScroll(); + testOverflowReservesChevrons(); + testOverflowScrollZeroClipsRight(); + testScrollToEndSurfacesLastTab(); + testClampScroll(); + testHitFitStrip(); + testHitChevrons(); + testHitMatchesRectsMidScroll(); + testDegenerate(); + + if (g_fail == 0) std::printf("tab_strip: all tests passed\n"); + else std::printf("tab_strip: %d FAILED\n", g_fail); + return g_fail == 0 ? 0 : 1; +} From cca738077727eb412f705603fea7e7c83d88bedc Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 25 Jul 2026 17:10:46 -0400 Subject: [PATCH 6/6] fix(bank_panel): null-guard uniformity, stale comment/focus nudge on delete, dragArmed on capture-loss, menuAppend comment --- src/bank_panel.cpp | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 4b2aef6..0db2cda 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -991,6 +991,7 @@ std::string mintBankId() { } void doCreateBank() { + if (!book()) return; std::string name; if (!promptText("ReaSampler: create bank", "Bank name:", "", name)) return; const std::string id = mintBankId(); @@ -1006,6 +1007,7 @@ void doCreateBank() { } void doRenameBank(const std::string& bankId) { + if (!book()) return; const Bank* bk = book()->bank(bankId); if (!bk || bk->isPool()) return; const std::string current = bk->displayName; // copy before any mutation @@ -1024,6 +1026,7 @@ void doRenameBank(const std::string& bankId) { // member count AND offers evacuate as the one-click alternative (Yes=delete anyway, // No=evacuate-then-keep, Cancel=abort) — richer than B3's basic YESNO. void doDeleteBank(const std::string& bankId) { + if (!book()) return; const Bank* bk = book()->bank(bankId); if (!bk || bk->isPool()) return; const std::size_t members = bk->index.size(); // read BEFORE any mutation @@ -1049,12 +1052,14 @@ void doDeleteBank(const std::string& bankId) { } if (!book()->deleteBank(bankId)) return; persistBook(); - // shownBankId is reconciled by the next fingerprint pass; nudge focus to pool if - // no named banks remain so the selection has a valid home. + // shownBankId is reconciled by the next fingerprint pass. If no named banks remain, + // nudge focus to the pool so the selection has a valid home. + if (namedBanks().empty()) g_panel.focusedRegion = Region::Pool; invalidatePanel(); } void doEvacuateBank(const std::string& bankId) { + if (!book()) return; const Bank* bk = book()->bank(bankId); if (!bk || bk->isPool()) return; if (!book()->evacuate(bankId)) return; @@ -1063,6 +1068,7 @@ void doEvacuateBank(const std::string& bankId) { } void doActivateBank(const std::string& bankId) { + if (!book()) return; if (!book()->setActiveBank(bankId)) return; // rejects an unknown id persistBook(); invalidatePanel(); @@ -1073,6 +1079,7 @@ void doActivateBank(const std::string& bankId) { void transferSamples(const std::vector& sampleIds, const std::string& srcBankId, const std::string& destBankId, bool copy) { + if (!book()) return; if (sampleIds.empty() || srcBankId == destBankId) return; if (!book()->bank(srcBankId) || !book()->bank(destBankId)) return; for (const std::string& sid : sampleIds) { @@ -1108,9 +1115,8 @@ std::vector focusedSelectionIds() { // hands the chosen id straight back, so no hookcommand routing is involved. // Appends a string item (id) to `menu` at its end. Portable over Win32/SWELL: both -// accept InsertMenu(menu, pos, MF_BYPOSITION|MF_STRING, id, text) with pos past the -// end appending. -1 as an unsigned position appends on Win32; SWELL clamps a large -// pos to the end. +// accept InsertMenu(menu, pos, MF_BYPOSITION|MF_STRING, id, text) with a negative +// position appending. Win32 and SWELL both treat pos < 0 as an append. void menuAppend(HMENU menu, unsigned int id, const char* text, bool grayed = false) { UINT flags = MF_BYPOSITION | MF_STRING; if (grayed) flags |= MF_GRAYED; @@ -1135,6 +1141,7 @@ enum : unsigned int { // Shows the right-click context menu for a named-bank TAB: activate / rename / delete // / evacuate that bank, plus a create entry. Drives the id-keyed ops. void showTabMenu(int screenX, int screenY, const std::string& bankId) { + if (!book()) return; const Bank* bk = book()->bank(bankId); if (!bk || bk->isPool()) return; const bool isActive = book()->activeBankId() == bankId; @@ -1582,6 +1589,14 @@ WDL_DLGRET dlgProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam) { SetFocus(hwnd); handleRightClick(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam)); return 0; + case WM_CAPTURECHANGED: + // Capture lost before a drag began (e.g. pointer left window pre-threshold + // and button released outside) — disarm so the state doesn't stay stale. + if (g_panel.dragArmed && !g_panel.dragging) { + g_panel.dragArmed = false; + invalidatePanel(); + } + return 0; case WM_DESTROY: if (GetCapture() == hwnd) ReleaseCapture(); stopAudition();