From 94587e87badca17e45e8bc0007883a56993fe683 Mon Sep 17 00:00:00 2001 From: Andrew Belt Date: Wed, 16 Sep 2020 18:25:12 -0400 Subject: [PATCH] Add WeakPtr. Use WeakPtr in ModuleWidget context menu. Fix crash when displaying preset menu when the preset folder does not exist. --- include/weakptr.hpp | 104 ++++++++++++++++++++++++++++++++++++++ include/widget/Widget.hpp | 3 +- src/app/ModuleWidget.cpp | 84 ++++++++++++++++++++---------- 3 files changed, 162 insertions(+), 29 deletions(-) create mode 100644 include/weakptr.hpp diff --git a/include/weakptr.hpp b/include/weakptr.hpp new file mode 100644 index 00000000..b854fcbd --- /dev/null +++ b/include/weakptr.hpp @@ -0,0 +1,104 @@ +#pragma once +#include +#include + + +namespace rack { + + +struct WeakHandle { + void* ptr; + std::atomic count{0}; + WeakHandle(void* ptr) : ptr(ptr) {} +}; + + +/** Base class for classes that allow WeakPtrs to be used. +*/ +struct WeakBase { + WeakHandle* weakHandle = nullptr; + + ~WeakBase() { + if (weakHandle) { + weakHandle->ptr = nullptr; + } + } + size_t getWeakCount() { + if (!weakHandle) + return 0; + return weakHandle->count; + } +}; + + +/** A weak pointer to a subclass of WeakBase. +Weak pointers behave like raw pointers but can be accessed safely (returning nullptr) after the pointed object is deleted. + +Example: + + Foo* foo = new Foo; + WeakPtr weakFoo = foo; + weakFoo.get(); // returns foo + delete foo; + weakFoo.get(); // returns nullptr + +Caveat: In multithreaded environments, the object pointed to by the WeakPtr could be deleted at any time after obtaining its pointer from WeakPtr. +*/ +template +struct WeakPtr { + WeakHandle* weakHandle = NULL; + + WeakPtr() {} + WeakPtr(T* ptr) { + set(ptr); + } + WeakPtr& operator=(const WeakPtr& other) { + set(other.get()); + return *this; + } + ~WeakPtr() { + set(nullptr); + } + void set(T* ptr) { + // Release handle + if (weakHandle) { + // Decrement and check handle reference count + if (--weakHandle->count == 0) { + // Remove handle from object and delete it + T* oldPtr = reinterpret_cast(weakHandle->ptr); + if (oldPtr) { + oldPtr->weakHandle = nullptr; + } + delete weakHandle; + } + weakHandle = nullptr; + } + // Obtain handle + if (ptr) { + if (!ptr->weakHandle) { + // Create new handle for object + ptr->weakHandle = new WeakHandle(ptr); + } + weakHandle = ptr->weakHandle; + // Increment handle reference count + weakHandle->count++; + } + } + T* get() const { + if (!weakHandle) + return nullptr; + return reinterpret_cast(weakHandle->ptr); + } + T& operator*() const { + return *get(); + } + T* operator->() const { + return get(); + } + explicit operator bool() const { + return get(); + } +}; + + +} // namespace rack diff --git a/include/widget/Widget.hpp b/include/widget/Widget.hpp index bd631f00..76a55ca8 100644 --- a/include/widget/Widget.hpp +++ b/include/widget/Widget.hpp @@ -6,6 +6,7 @@ #include #include #include +#include namespace rack { @@ -20,7 +21,7 @@ namespace widget { The bounding box of a Widget is a rectangle specified by `box` relative to their parent. The appearance is defined by overriding `draw()`, and the behavior is defined by overriding `step()` and `on*()` event handlers. */ -struct Widget { +struct Widget : WeakBase { /** Stores position and size */ math::Rect box = math::Rect(math::Vec(), math::Vec(INFINITY, INFINITY)); /** Automatically set when Widget is added as a child to another Widget */ diff --git a/src/app/ModuleWidget.cpp b/src/app/ModuleWidget.cpp index 68eed5d0..4b915919 100644 --- a/src/app/ModuleWidget.cpp +++ b/src/app/ModuleWidget.cpp @@ -151,73 +151,91 @@ struct ModuleInfoItem : ui::MenuItem { struct ModuleDisconnectItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->disconnectAction(); } }; struct ModuleResetItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->resetAction(); } }; struct ModuleRandomizeItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->randomizeAction(); } }; struct ModuleCopyItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->copyClipboard(); } }; struct ModulePasteItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->pasteClipboardAction(); } }; struct ModuleSaveItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->saveDialog(); } }; struct ModuleSaveTemplateItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->saveTemplate(); } }; struct ModuleLoadItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->loadDialog(); } }; struct ModulePresetPathItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; std::string presetPath; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; try { moduleWidget->loadAction(presetPath); } @@ -229,8 +247,10 @@ struct ModulePresetPathItem : ui::MenuItem { struct ModulePresetItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; ui::Menu* createChildMenu() override { + if (!moduleWidget) + return NULL; ui::Menu* menu = new ui::Menu; ModuleCopyItem* copyItem = new ModuleCopyItem; @@ -264,21 +284,23 @@ struct ModulePresetItem : ui::MenuItem { auto createPresetItems = [&](std::string presetDir) { bool hasPresets = false; // Note: This is not cached, so opening this menu each time might have a bit of latency. - for (const std::string& presetPath : system::getEntries(presetDir)) { - if (system::getExtension(presetPath) != ".vcvm") - continue; - hasPresets = true; - - std::string presetName = system::getStem(presetPath); - // Remove "1_", "42_", "001_", etc at the beginning of preset filenames - std::regex r("^\\d*_"); - presetName = std::regex_replace(presetName, r, ""); - - ModulePresetPathItem* presetItem = new ModulePresetPathItem; - presetItem->text = presetName; - presetItem->presetPath = presetPath; - presetItem->moduleWidget = moduleWidget; - menu->addChild(presetItem); + if (system::isDirectory(presetDir)) { + for (const std::string& presetPath : system::getEntries(presetDir)) { + if (system::getExtension(presetPath) != ".vcvm") + continue; + hasPresets = true; + + std::string presetName = system::getStem(presetPath); + // Remove "1_", "42_", "001_", etc at the beginning of preset filenames + std::regex r("^\\d*_"); + presetName = std::regex_replace(presetName, r, ""); + + ModulePresetPathItem* presetItem = new ModulePresetPathItem; + presetItem->text = presetName; + presetItem->presetPath = presetPath; + presetItem->moduleWidget = moduleWidget; + menu->addChild(presetItem); + } } if (!hasPresets) { menu->addChild(createMenuLabel("(None)")); @@ -301,24 +323,30 @@ struct ModulePresetItem : ui::MenuItem { struct ModuleCloneItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->cloneAction(); } }; struct ModuleBypassItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->bypassAction(); } }; struct ModuleDeleteItem : ui::MenuItem { - ModuleWidget* moduleWidget; + WeakPtr moduleWidget; void onAction(const event::Action& e) override { + if (!moduleWidget) + return; moduleWidget->removeAction(); } };