From 51b2682374efdb8f83a68337aa4d5ebbdeabb5ef Mon Sep 17 00:00:00 2001 From: Paul Walker Date: Wed, 25 Sep 2024 19:19:49 -0400 Subject: [PATCH] An unacceptable workflow which allows missing sample resolution 1. If samples are missing at load time the screen has a 'resolve first' button which lets you pick the first in the list. If two are missing, press the button again after you resolve the first. obviously bad ui. But 2. If you do that the sample gets resolved and replaced and remapped 3. To do this added a path hash to the sample id and also did quite a bit of work cleaning up aliasing, loading, purging, api etc to make sure the right stuff happens at the right time 4. Along the way make the find-missing to be all samples in the sample db which are missing and not purged as opposed to traversing tree looking for attachment to same --- .../SCXTEditorResponseHandlers.cpp | 15 ++++--- .../MissingResolutionScreen.cpp | 45 ++++++++++++++++++- .../MissingResolutionScreen.h | 5 +++ src/engine/engine.cpp | 8 ++-- src/engine/missing_resolution.cpp | 33 +++----------- src/engine/missing_resolution.h | 4 +- src/engine/zone.cpp | 5 ++- src/json/missing_resolution_traits.h | 12 +++-- src/json/utils_traits.h | 4 +- src/messaging/client/client_serial.h | 2 + .../client/missing_resolution_messages.h | 42 +++++++++++++++++ src/sample/sample.cpp | 1 + src/sample/sample_manager.cpp | 25 ++++++++--- src/sample/sample_manager.h | 11 ++++- src/utils.h | 13 ++++-- 15 files changed, 163 insertions(+), 62 deletions(-) diff --git a/src-ui/app/editor-impl/SCXTEditorResponseHandlers.cpp b/src-ui/app/editor-impl/SCXTEditorResponseHandlers.cpp index 8a9189bb..ee9402bc 100644 --- a/src-ui/app/editor-impl/SCXTEditorResponseHandlers.cpp +++ b/src-ui/app/editor-impl/SCXTEditorResponseHandlers.cpp @@ -423,15 +423,18 @@ void SCXTEditor::onMissingResolutionWorkItemList( for (const auto &wi : items) { SCLOG("Missing resolution work item"); - SCLOG(" path : " << wi.path.u8string()); - SCLOG(" zone : " << wi.address); - SCLOG(" var : " << wi.variant); - SCLOG(" md5 : " << wi.md5sum); + SCLOG(" path : " << wi.path.u8string()); + SCLOG(" id : " << wi.missingID.to_string()); } - if (!items.empty()) + missingResolutionScreen->setWorkItemList(items); + + if (items.empty()) + { + missingResolutionScreen->setVisible(false); + } + else { - missingResolutionScreen->setWorkItemList(items); missingResolutionScreen->setBounds(getLocalBounds()); missingResolutionScreen->setVisible(true); missingResolutionScreen->toFront(true); diff --git a/src-ui/app/missing-resolution/MissingResolutionScreen.cpp b/src-ui/app/missing-resolution/MissingResolutionScreen.cpp index e2654b55..77956eff 100644 --- a/src-ui/app/missing-resolution/MissingResolutionScreen.cpp +++ b/src-ui/app/missing-resolution/MissingResolutionScreen.cpp @@ -37,7 +37,7 @@ namespace scxt::ui::app::missing_resolution struct Contents : juce::Component, HasEditor { MissingResolutionScreen *parent{nullptr}; - std::unique_ptr okButton; + std::unique_ptr okButton, resolveZeroButton; Contents(MissingResolutionScreen *p, SCXTEditor *e) : parent(p), HasEditor(e) { okButton = std::make_unique(); @@ -48,6 +48,16 @@ struct Contents : juce::Component, HasEditor w->setVisible(false); }); addAndMakeVisible(*okButton); + + resolveZeroButton = std::make_unique(); + resolveZeroButton->setLabel("Resolve First"); + resolveZeroButton->setOnCallback([w = juce::Component::SafePointer(parent)]() { + if (!w) + return; + + w->resolveItem(0); + }); + addAndMakeVisible(*resolveZeroButton); } void resized() override @@ -55,6 +65,8 @@ struct Contents : juce::Component, HasEditor auto b = getLocalBounds(); b = b.withTrimmedTop(b.getHeight() - 22).withTrimmedLeft(b.getWidth() - 100); okButton->setBounds(b); + + resolveZeroButton->setBounds(b.translated(-110, 0)); } void paint(juce::Graphics &g) override { @@ -69,7 +81,7 @@ struct Contents : juce::Component, HasEditor g.setFont(editor->themeApplier.interMediumFor(12)); g.setColour(editor->themeColor(theme::ColorMap::generic_content_medium)); - g.drawText(i.md5sum, bd, juce::Justification::bottomLeft); + g.drawText(i.missingID.md5, bd, juce::Justification::bottomLeft); g.setColour(editor->themeColor(theme::ColorMap::generic_content_low)); g.drawRect(bx, 1); @@ -94,4 +106,33 @@ void MissingResolutionScreen::resized() contentsArea->setBounds(getLocalBounds().reduced(100, 80)); } +void MissingResolutionScreen::resolveItem(int idx) +{ + SCLOG("Resoving item " << idx); + const auto &wi = workItems[idx]; + + fileChooser = + std::make_unique("Resolve sample " + wi.path.filename().u8string()); + fileChooser->launchAsync( + juce::FileBrowserComponent::canSelectFiles | juce::FileBrowserComponent::openMode, + [idx, w = juce::Component::SafePointer(this)](const juce::FileChooser &c) { + auto result = c.getResults(); + if (result.isEmpty() || result.size() > 1) + { + return; + } + if (!w) + return; + auto p = fs::path(fs::u8path(result[0].getFullPathName().toStdString())); + w->applyResolution(idx, p); + }); +} + +void MissingResolutionScreen::applyResolution(int idx, const fs::path &toThis) +{ + SCLOG("Resolving item " << idx << " with " << toThis.u8string()); + sendToSerialization( + scxt::messaging::client::ResolveSample({workItems[idx], toThis.u8string()})); +} + } // namespace scxt::ui::app::missing_resolution \ No newline at end of file diff --git a/src-ui/app/missing-resolution/MissingResolutionScreen.h b/src-ui/app/missing-resolution/MissingResolutionScreen.h index 043d0326..3558097e 100644 --- a/src-ui/app/missing-resolution/MissingResolutionScreen.h +++ b/src-ui/app/missing-resolution/MissingResolutionScreen.h @@ -55,6 +55,11 @@ struct MissingResolutionScreen : juce::Component, HasEditor repaint(); } + void resolveItem(int idx); + void applyResolution(int idx, const fs::path &toThis); + + std::unique_ptr fileChooser; + std::vector workItems; JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR(MissingResolutionScreen); }; diff --git a/src/engine/engine.cpp b/src/engine/engine.cpp index 2ae7b350..99453fdb 100644 --- a/src/engine/engine.cpp +++ b/src/engine/engine.cpp @@ -1086,11 +1086,9 @@ void Engine::sendFullRefreshToClient() const getSelectionManager()->sendOtherTabsSelectionToClient(); auto missing = collectMissingResolutionWorkItems(*this); - if (!missing.empty()) - { - serializationSendToClient(messaging::client::s2c_send_missing_resolution_workitem_list, - missing, *(getMessageController())); - } + // send missing even if empty. An empty missing flags dont show dialog + serializationSendToClient(messaging::client::s2c_send_missing_resolution_workitem_list, missing, + *(getMessageController())); } void Engine::clearAll() diff --git a/src/engine/missing_resolution.cpp b/src/engine/missing_resolution.cpp index 3afa90ba..53d6f21a 100644 --- a/src/engine/missing_resolution.cpp +++ b/src/engine/missing_resolution.cpp @@ -33,35 +33,16 @@ std::vector collectMissingResolutionWorkItems(const E { std::vector res; - int pidx{0}; - for (const auto &p : *(e.getPatch())) + const auto &sm = e.getSampleManager(); + auto it = sm->samplesBegin(); + while (it != sm->samplesEnd()) { - int gidx{0}; - for (const auto &g : *p) + const auto &[id, s] = *it; + if (s->isMissingPlaceholder) { - int zidx{0}; - for (const auto &z : *g) - { - int idx{0}; - for (const auto &v : z->variantData.variants) - { - if (v.active && z->samplePointers[idx]->isMissingPlaceholder) - { - auto &sm = z->samplePointers[idx]; - MissingResolutionWorkItem wf; - wf.address = {pidx, gidx, zidx}; - wf.variant = idx; - wf.path = sm->mFileName; - wf.md5sum = sm->md5Sum; - res.push_back(wf); - } - idx++; - } - zidx++; - } - gidx++; + res.push_back({id, s->mFileName}); } - pidx++; + it++; } return res; diff --git a/src/engine/missing_resolution.h b/src/engine/missing_resolution.h index 81d09bff..221bbc92 100644 --- a/src/engine/missing_resolution.h +++ b/src/engine/missing_resolution.h @@ -35,10 +35,8 @@ namespace scxt::engine { struct MissingResolutionWorkItem { - selection::SelectionManager::ZoneAddress address; - int16_t variant; + SampleID missingID; fs::path path; - std::string md5sum; }; std::vector collectMissingResolutionWorkItems(const Engine &e); diff --git a/src/engine/zone.cpp b/src/engine/zone.cpp index 46f6fb65..7fab8f96 100644 --- a/src/engine/zone.cpp +++ b/src/engine/zone.cpp @@ -202,7 +202,8 @@ void Zone::setupOnUnstream(const engine::Engine &e) if (nid != oid) { - SCLOG("Relabeling zone on change : " << oid.to_string() << " -> " << nid.to_string()); + SCLOG(getName() << " : Relabeling zone on change : " << oid.to_string() << " -> " + << nid.to_string()); variantData.variants[i].sampleID = nid; } @@ -442,6 +443,8 @@ int16_t Zone::missingSampleCount() const if (sv.active) { auto smp = samplePointers[idx]; + // SCLOG("Checking missing sample at " << sv.sampleID.to_string() << " " << smp.get()); + if (smp && smp->isMissingPlaceholder) { ct++; diff --git a/src/json/missing_resolution_traits.h b/src/json/missing_resolution_traits.h index 2122dab5..0f57853f 100644 --- a/src/json/missing_resolution_traits.h +++ b/src/json/missing_resolution_traits.h @@ -40,15 +40,13 @@ namespace scxt::json { SC_STREAMDEF(scxt::engine::MissingResolutionWorkItem, SC_FROM({ - v = {{"a", from.address}, - {"v", from.variant}, - {"p", from.path.u8string()}, - {"md5", from.md5sum}}; + v = { + {"i", from.missingID}, + {"p", from.path.u8string()}, + }; }), SC_TO({ - findIf(v, "a", to.address); - findIf(v, "v", to.variant); - findIf(v, "md5", to.md5sum); + findIf(v, "i", to.missingID); std::string fp; findIf(v, "p", fp); to.path = fs::path(fs::u8path(fp)); diff --git a/src/json/utils_traits.h b/src/json/utils_traits.h index 965be600..9c27d849 100644 --- a/src/json/utils_traits.h +++ b/src/json/utils_traits.h @@ -71,13 +71,15 @@ template struct scxt_traits> } }; -SC_STREAMDEF(scxt::SampleID, SC_FROM(v = {{"m", from.md5}, {"a", from.multiAddress}}); +SC_STREAMDEF(scxt::SampleID, + SC_FROM(v = {{"m", from.md5}, {"a", from.multiAddress}, {"p", from.pathHash}}); , SC_TO( auto vs = v.find("m"); if (vs) { std::string m5; vs->to(m5); strncpy(to.md5, m5.c_str(), scxt::SampleID::md5len + 1); findIf(v, "a", to.multiAddress); + findIf(v, "p", to.pathHash); } else { std::string legType{"t"}, legID{"i"}; if (SC_UNSTREAMING_FROM_PRIOR_TO(0x2024'08'06)) diff --git a/src/messaging/client/client_serial.h b/src/messaging/client/client_serial.h index 8ac5fe2f..7f63c22b 100644 --- a/src/messaging/client/client_serial.h +++ b/src/messaging/client/client_serial.h @@ -152,6 +152,8 @@ enum ClientToSerializationMessagesIds c2s_send_full_part_config, + c2s_resolve_sample, + num_clientToSerializationMessages }; diff --git a/src/messaging/client/missing_resolution_messages.h b/src/messaging/client/missing_resolution_messages.h index 5c98ab17..7a04111e 100644 --- a/src/messaging/client/missing_resolution_messages.h +++ b/src/messaging/client/missing_resolution_messages.h @@ -40,5 +40,47 @@ namespace scxt::messaging::client { SERIAL_TO_CLIENT(SendMissingResolutionWorkItemList, s2c_send_missing_resolution_workitem_list, std::vector, onMissingResolutionWorkItemList); + +using resolveSamplePayload_t = std::tuple; +inline void doResolveSample(const resolveSamplePayload_t &payload, engine::Engine &e, + messaging::MessageController &cont) +{ + auto mwi = std::get<0>(payload); + auto p = fs::path(fs::u8path(std::get<1>(payload))); + auto smp = e.getSampleManager()->loadSampleByPath(p); + SCLOG("Resolving : " << p.u8string()); + SCLOG(" Was : " << mwi.missingID.to_string()); + SCLOG(" To : " << smp->to_string()); + if (smp.has_value()) + { + for (auto &p : *(e.getPatch())) + { + for (auto &g : *p) + { + for (auto &z : *g) + { + int vidx{0}; + for (auto &v : z->variantData.variants) + { + if (v.active && v.sampleID == mwi.missingID) + { + SCLOG(" Zone : " << z->getName()); + v.sampleID = *smp; + z->attachToSampleAtVariation( + *(e.getSampleManager()), *smp, vidx, + engine::Zone::SampleInformationRead::ENDPOINTS); + } + vidx++; + } + } + } + } + } + e.getSampleManager()->purgeUnreferencedSamples(); + e.sendFullRefreshToClient(); } +CLIENT_TO_SERIAL(ResolveSample, c2s_resolve_sample, resolveSamplePayload_t, + doResolveSample(payload, engine, cont)); + +} // namespace scxt::messaging::client #endif // MISSING_RESOLUTION_MESSAGES_H diff --git a/src/sample/sample.cpp b/src/sample/sample.cpp index 5756fa87..3ecdb652 100644 --- a/src/sample/sample.cpp +++ b/src/sample/sample.cpp @@ -52,6 +52,7 @@ bool Sample::load(const fs::path &path) return false; md5Sum = infrastructure::createMD5SumFromFile(path); + id.setPathHash(path.u8string().c_str()); // If you add a type here add it in Browser::isLoadableFile also to stay in sync if (extensionMatches(path, ".wav")) diff --git a/src/sample/sample_manager.cpp b/src/sample/sample_manager.cpp index 7d9b5ed7..781894a5 100644 --- a/src/sample/sample_manager.cpp +++ b/src/sample/sample_manager.cpp @@ -70,9 +70,7 @@ void SampleManager::restoreFromSampleAddressesAndIDs(const sampleAddressesAndIds { if (*nid != id) { - SCLOG("Adding Alias ID [" << id.to_string() << "] -> [" << nid->to_string() - << "]"); - idAliases[id] = *nid; + addIdAlias(id, *nid); } } } @@ -154,6 +152,7 @@ std::optional SampleManager::loadSampleFromSF2(const fs::path &p, sf2: sp->md5Sum = sf2MD5ByPath[p.u8string()]; assert(!sp->md5Sum.empty()); sp->id.setAsMD5WithAddress(sp->md5Sum, preset, instrument, region); + sp->id.setPathHash(p); SCLOG("Loading : " << p.u8string()); SCLOG(" : " << sp->id.to_string()); @@ -169,6 +168,7 @@ std::optional SampleManager::setupSampleFromMultifile(const fs::path & { auto sp = std::make_shared(); sp->id.setAsMD5WithAddress(md5, idx, -1, -1); + sp->id.setPathHash(p); sp->parse_riff_wave(data, dataSize); sp->type = Sample::MULTISAMPLE_FILE; @@ -221,6 +221,11 @@ void SampleManager::purgeUnreferencedSamples() { SCLOG("Purging : " << b->second->mFileName.u8string()); SCLOG(" : " << b->first.to_string()); + if (b->second->isMissingPlaceholder) + { + SCLOG(" : Missing Placeholder"); + } + b = samples.erase(b); } else @@ -231,7 +236,8 @@ void SampleManager::purgeUnreferencedSamples() if (samples.size() != preSize) { - SCLOG_WFUNC("PostPurge : Purged " << (preSize - samples.size())); + SCLOG_WFUNC("PostPurge : Purged " << (preSize - samples.size()) << " Remaining " + << samples.size()); } updateSampleMemory(); } @@ -271,11 +277,18 @@ void SampleManager::addSampleAsMissing(const SampleID &id, const Sample::SampleF { auto ms = Sample::createMissingPlaceholder(f); ms->id = id; + ms->id.setPathHash(f.path); SCLOG("Missing : " << f.path.u8string()); - SCLOG(" : " << id.to_string()); + SCLOG(" : " << ms->id.to_string()); - samples[id] = ms; + samples[ms->id] = ms; + + if (ms->id != id) + { + // Path change so + addIdAlias(id, ms->id); + } } } diff --git a/src/sample/sample_manager.h b/src/sample/sample_manager.h index 8147bfb7..8c0f673b 100644 --- a/src/sample/sample_manager.h +++ b/src/sample/sample_manager.h @@ -141,15 +141,24 @@ struct SampleManager : MoveableOnly { auto ap = idAliases.find(a); if (ap == idAliases.end()) + { return a; + } assert(ap->second != a); return resolveAlias(ap->second); } + using sampleMap_t = std::unordered_map>; + + // You can do a const interation but not a non-const one + sampleMap_t::const_iterator samplesBegin() const { return samples.cbegin(); } + sampleMap_t::const_iterator samplesEnd() const { return samples.cend(); } + private: void updateSampleMemory(); std::unordered_map idAliases; - std::unordered_map> samples; + + sampleMap_t samples; std::unordered_map, std::unique_ptr>> diff --git a/src/utils.h b/src/utils.h index 7e46f3c3..3e5e866f 100644 --- a/src/utils.h +++ b/src/utils.h @@ -98,6 +98,7 @@ struct SampleID char md5[md5len + 1]{}; static constexpr int unusedAddress{-1}; + size_t pathHash{0}; std::array multiAddress{unusedAddress, unusedAddress, unusedAddress}; SampleID() { setAsInvalid(); } @@ -106,16 +107,17 @@ struct SampleID { std::string hd{"SampleID"}; if (multiAddress[0] == unusedAddress) - return fmt::format("{}:{}", hd, std::string(md5)); + return fmt::format("{}:{}-{:016x}", hd, std::string(md5), pathHash); else - return fmt::format("{}:{}@{}/{}/{}", hd, std::string(md5), multiAddress[0], - multiAddress[1], multiAddress[2]); + return fmt::format("{}:{}-{:016x}@{}/{}/{}", hd, std::string(md5), pathHash, + multiAddress[0], multiAddress[1], multiAddress[2]); } bool operator==(const SampleID &other) const { return strncmp(md5, other.md5, md5len) == 0 && multiAddress[0] == other.multiAddress[0] && - multiAddress[1] == other.multiAddress[1] && multiAddress[2] == other.multiAddress[2]; + pathHash == other.pathHash && multiAddress[1] == other.multiAddress[1] && + multiAddress[2] == other.multiAddress[2]; } bool operator!=(const SampleID &other) const { return !(other == *this); } @@ -137,6 +139,7 @@ struct SampleID auto ls = fmt::format("lgcy({})", oldId); setAsMD5(ls); } + void setPathHash(const fs::path &p) { pathHash = std::hash()(p.u8string()); } void setAsMD5WithAddress(const std::string &m, int a0, int a1, int a2) { @@ -339,6 +342,7 @@ inline std::string humanReadableVersion(uint64_t v) { return fmt::format("{:04x}-{:02x}-{:02x}", (v >> 16) & 0xFFFF, (v >> 8) & 0xFF, v & 0xFF); } + } // namespace scxt // Make the ID hashable so we can use it as a map key @@ -352,6 +356,7 @@ template <> struct std::hash size_t operator()(const scxt::SampleID &v) const { auto mh = std::hash()(v.md5); + mh ^= 0x9e3779b9 + (v.pathHash << 6) + (v.pathHash >> 2); for (int i = 0; i < 3; ++i) { if (v.multiAddress[i] >= 0)