diff --git a/Cargo.lock b/Cargo.lock index 0c19a2c0..8cc8d914 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -300,6 +300,7 @@ dependencies = [ [[package]] name = "egui-baseview" version = "0.0.0" +source = "git+https://github.com/robbert-vdh/egui-baseview.git?branch=fix/update-dependencies#161c831ded1289eb63d6cd530ac4574e6d8f351a" dependencies = [ "baseview", "copypasta", @@ -533,6 +534,7 @@ dependencies = [ "egui", "egui-baseview", "nih_plug", + "parking_lot", ] [[package]] diff --git a/nih_plug_egui/Cargo.toml b/nih_plug_egui/Cargo.toml index 5ad2d6a4..e0233c3f 100644 --- a/nih_plug_egui/Cargo.toml +++ b/nih_plug_egui/Cargo.toml @@ -15,3 +15,4 @@ crossbeam = "0.8" egui = "0.16" # Upstream doesn't work with the current baseview and egui versions egui-baseview = { git = "https://github.com/robbert-vdh/egui-baseview.git", branch = "fix/update-dependencies" } +parking_lot = "0.12" diff --git a/nih_plug_egui/src/lib.rs b/nih_plug_egui/src/lib.rs index 766c0546..60c59e0a 100644 --- a/nih_plug_egui/src/lib.rs +++ b/nih_plug_egui/src/lib.rs @@ -21,7 +21,8 @@ use baseview::{Size, WindowHandle, WindowOpenOptions, WindowScalePolicy}; use egui::CtxRef; use egui_baseview::{EguiWindow, RenderSettings, Settings}; -use nih_plug::{Editor, EditorWindowHandle}; +use nih_plug::{Editor, ParamSetter, ParentWindowHandle}; +use parking_lot::RwLock; use std::sync::Arc; /// Re-export for convenience. @@ -30,88 +31,110 @@ pub use egui; /// Create an [Editor] instance using an [::egui] GUI. Using the state is optional, but it can be /// useful for keeping track of some temporary GUI-only settings. See the `gui_gain` example for -/// more information on how to use this. The size passed to this function is the GUI's intiial size, -/// and this is kept in sync whenever the GUI gets resized. You should return the same size value in -/// your plugin' [nih_plug::Plugin::editor_size()] implementation. +/// more information on how to use this. The size passed to this function is the GUI's intitial +/// size, and this is kept in sync whenever the GUI gets resized. If you want this size to be +/// persisted when restoring a plugin instance, then you can store it in a `#[persist]` field on +/// your parameters struct. // // TODO: DPI scaling, this needs to be implemented on the framework level pub fn create_egui_editor( - parent: EditorWindowHandle, size: Arc>, initial_state: T, - mut update: U, + update: U, ) -> Option> where - T: 'static + Send, - U: FnMut(&CtxRef, &mut T) + 'static + Send, + T: 'static + Send + Sync, + U: Fn(&CtxRef, &ParamSetter, &mut T) + 'static + Send + Sync, { - let (width, height) = size.load(); - let window = EguiWindow::open_parented( - &parent, - Settings { - window: WindowOpenOptions { - title: String::from("egui window"), - size: Size::new(width as f64, height as f64), - // TODO: What happens when we use the system scale factor here? I'd assume this - // would work everywhere, even if the window may be tiny in some cases. - scale: WindowScalePolicy::ScaleFactor(1.0), - }, - render_settings: RenderSettings { - version: (3, 2), - red_bits: 8, - blue_bits: 8, - green_bits: 8, - // If the window was not created with the correct visual, then specifying 8 bits - // here will cause creating the context to fail - alpha_bits: 0, - depth_bits: 24, - stencil_bits: 8, - samples: None, - srgb: true, - double_buffer: true, - vsync: true, - ..Default::default() - }, - }, - initial_state, - |_, _, _| {}, - move |egui_ctx, queue, state| { - // For now, just always redraw. Most plugin GUIs have meters, and those almost always - // need a redraw. Later we can try to be a bit more sophisticated about this. Without - // this we would also have a blank GUI when it gets first opened because most DAWs open - // their GUI while the window is still unmapped. - // TODO: Are there other useful parts of this queue we could pass to thep lugin? - queue.request_repaint(); - update(egui_ctx, state); - }, - ); - - // There's no error handling here, so let's just pray it worked - if window.is_open() { - Some(Box::new(EguiEditor { window, size })) - } else { - None - } + Some(Box::new(EguiEditor { + size, + state: Arc::new(RwLock::new(initial_state)), + update: Arc::new(update), + })) } /// An [Editor] implementation that calls an egui draw loop. -pub struct EguiEditor { - window: WindowHandle, +struct EguiEditor { size: Arc>, + /// The plugin's state. This is kept in between editor openenings. + state: Arc>, + update: Arc, } -/// The window handle enum stored within 'WindowHandle' contains raw pointers. Is there a way around -/// having this requirement? -unsafe impl Send for EguiEditor {} -unsafe impl Sync for EguiEditor {} +impl Editor for EguiEditor +where + T: 'static + Send + Sync, +{ + fn spawn( + &self, + parent: ParentWindowHandle, + context: Arc, + ) -> Box { + let update = self.update.clone(); + let state = self.state.clone(); + + let (width, height) = self.size.load(); + let window = EguiWindow::open_parented( + &parent, + Settings { + window: WindowOpenOptions { + title: String::from("egui window"), + size: Size::new(width as f64, height as f64), + // TODO: What happens when we use the system scale factor here? I'd assume this + // would work everywhere, even if the window may be tiny in some cases. + scale: WindowScalePolicy::ScaleFactor(1.0), + }, + render_settings: RenderSettings { + version: (3, 2), + red_bits: 8, + blue_bits: 8, + green_bits: 8, + // If the window was not created with the correct visual, then specifying 8 bits + // here will cause creating the context to fail + alpha_bits: 0, + depth_bits: 24, + stencil_bits: 8, + samples: None, + srgb: true, + double_buffer: true, + vsync: true, + ..Default::default() + }, + }, + state, + |_, _, _| {}, + move |egui_ctx, queue, state| { + let setter = ParamSetter::new(context.as_ref()); + + // For now, just always redraw. Most plugin GUIs have meters, and those almost always + // need a redraw. Later we can try to be a bit more sophisticated about this. Without + // this we would also have a blank GUI when it gets first opened because most DAWs open + // their GUI while the window is still unmapped. + // TODO: Are there other useful parts of this queue we could pass to thep lugin? + queue.request_repaint(); + (update)(egui_ctx, &setter, &mut state.write()); + }, + ); + + Box::new(EguiEditorHandle { window }) + } -impl Editor for EguiEditor { fn size(&self) -> (u32, u32) { self.size.load() } } -impl Drop for EguiEditor { +/// The window handle used for [EguiEditor]. +struct EguiEditorHandle { + window: WindowHandle, +} + +/// The window handle enum stored within 'WindowHandle' contains raw pointers. Is there a way around +/// having this requirement? +unsafe impl Send for EguiEditorHandle {} +unsafe impl Sync for EguiEditorHandle {} + +impl Drop for EguiEditorHandle { fn drop(&mut self) { // XXX: This should automatically happen when the handle gets dropped, but apparently not self.window.close(); diff --git a/plugins/examples/gain-gui/src/lib.rs b/plugins/examples/gain-gui/src/lib.rs index 88682fdb..8ce09513 100644 --- a/plugins/examples/gain-gui/src/lib.rs +++ b/plugins/examples/gain-gui/src/lib.rs @@ -19,8 +19,8 @@ extern crate nih_plug; use atomic_float::AtomicF32; use nih_plug::{ - formatters, util, Buffer, BufferConfig, BusConfig, Editor, EditorWindowHandle, GuiContext, - ParamSetter, Plugin, ProcessContext, ProcessStatus, Vst3Plugin, + formatters, util, Buffer, BufferConfig, BusConfig, Editor, Plugin, ProcessContext, + ProcessStatus, Vst3Plugin, }; use nih_plug::{FloatParam, Param, Params, Range, Smoother, SmoothingStyle}; use nih_plug_egui::{create_egui_editor, egui, AtomicCell}; @@ -97,20 +97,13 @@ impl Plugin for Gain { self.params.as_ref() } - fn create_editor( - &self, - parent: EditorWindowHandle, - context: Arc, - ) -> Option> { + fn editor(&self) -> Option> { let params = self.params.clone(); let peak_meter = self.peak_meter.clone(); create_egui_editor( - parent, self.editor_size.clone(), (), - move |egui_ctx, _state| { - let setter = ParamSetter::new(context.as_ref()); - + move |egui_ctx, setter, _state| { egui::CentralPanel::default().show(egui_ctx, |ui| { ui.allocate_space(egui::Vec2::splat(3.0)); ui.label("Gain"); @@ -153,10 +146,6 @@ impl Plugin for Gain { ) } - fn editor_size(&self) -> Option<(u32, u32)> { - Some(self.editor_size.load()) - } - fn accepts_bus_config(&self, config: &BusConfig) -> bool { // This works with any symmetrical IO layout config.num_input_channels == config.num_output_channels && config.num_input_channels > 0 diff --git a/src/lib.rs b/src/lib.rs index 412e1cb8..6d27c397 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,7 +34,7 @@ pub use param::range::Range; pub use param::smoothing::{Smoother, SmoothingStyle}; pub use param::{BoolParam, FloatParam, IntParam, Param}; pub use plugin::{ - BufferConfig, BusConfig, Editor, EditorWindowHandle, NoteEvent, Plugin, ProcessStatus, + BufferConfig, BusConfig, Editor, NoteEvent, ParentWindowHandle, Plugin, ProcessStatus, Vst3Plugin, }; diff --git a/src/plugin.rs b/src/plugin.rs index 010475f2..50a31750 100644 --- a/src/plugin.rs +++ b/src/plugin.rs @@ -15,6 +15,7 @@ // along with this program. If not, see . use raw_window_handle::{HasRawWindowHandle, RawWindowHandle}; +use std::any::Any; use std::pin::Pin; use std::sync::Arc; @@ -66,31 +67,12 @@ pub trait Plugin: Default + Send + Sync + 'static { /// plugin receives an update. fn params(&self) -> Pin<&dyn Params>; - /// Create an editor for this plugin and embed it in the parent window. A plugin editor will - /// likely want to interact with the plugin's parameters, so the idea is that you take a - /// reference to your [Params] object in your editor as well as the [GuiContext] that's passed - /// to this function. You can then read the parameter values directly from your [Params] object, - /// and modifying the values can be done using the functions on [GuiContext::setter()]. When you - /// change a parameter value that way it will be broadcasted to the host and also updated in - /// your [Params] struct. - // - // TODO: Think of how this would work with the event loop. On Linux the wrapper must provide a - // timer using VST3's `IRunLoop` interface, but on Window and macOS the window would - // normally register its own timer. Right now we just ignore this because it would - // otherwise be basically impossible to have this still be GUI-framework agnostic. Any - // callback that deos involve actual GUI operations will still be spooled to the IRunLoop - // instance. - fn create_editor( - &self, - parent: EditorWindowHandle, - context: Arc, - ) -> Option> { - None - } - - /// Return the current size of the plugin's editor, if it has one. This is also used to check - /// whether the plugin has an editor without creating one. - fn editor_size(&self) -> Option<(u32, u32)> { + /// The plugin's editor, if it has one. The actual editor instance is created in + /// [Editor::spawn]. A plugin editor likely wants to interact with the plugin's parameters and + /// other shared data, so you'll need to move [Arc] pointing to any data you want to access into + /// the editor. You can later modify the parameters through the [GuiContext] and [ParamSetter] + /// after the editor GUI has been created. + fn editor(&self) -> Option> { None } @@ -146,8 +128,26 @@ pub trait Vst3Plugin: Plugin { const VST3_CATEGORIES: &'static str; } -/// An editor for a [Plugin]. This object gets dropped when the host closes the editor. -pub trait Editor { +/// An editor for a [Plugin]. +pub trait Editor: Send + Sync { + /// Create an instance of the plugin's editor and embed it in the parent window. As explained in + /// [Plugin::editor], you can then read the parameter values directly from your [Params] object, + /// and modifying the values can be done using the functions on the [ParamSetter]. When you + /// change a parameter value that way it will be broadcasted to the host and also updated in + /// your [Params] struct. + /// + /// This function should return a handle to the editor, which will be dropped when the editor + /// gets closed. Implement the [Drop] trait on the returned handle if you need to explicitly + /// handle the editor's closing behavior. + // + // TODO: Think of how this would work with the event loop. On Linux the wrapper must provide a + // timer using VST3's `IRunLoop` interface, but on Window and macOS the window would + // normally register its own timer. Right now we just ignore this because it would + // otherwise be basically impossible to have this still be GUI-framework agnostic. Any + // callback that deos involve actual GUI operations will still be spooled to the IRunLoop + // instance. + fn spawn(&self, parent: ParentWindowHandle, context: Arc) -> Box; + /// Return the (currnent) size of the editor in pixels as a `(width, height)` pair. fn size(&self) -> (u32, u32); @@ -160,11 +160,11 @@ pub trait Editor { } /// A raw window handle for platform and GUI framework agnostic editors. -pub struct EditorWindowHandle { +pub struct ParentWindowHandle { pub handle: RawWindowHandle, } -unsafe impl HasRawWindowHandle for EditorWindowHandle { +unsafe impl HasRawWindowHandle for ParentWindowHandle { fn raw_window_handle(&self) -> RawWindowHandle { self.handle } diff --git a/src/wrapper/vst3.rs b/src/wrapper/vst3.rs index 04707cdb..3edc9833 100644 --- a/src/wrapper/vst3.rs +++ b/src/wrapper/vst3.rs @@ -22,6 +22,7 @@ use crossbeam::atomic::AtomicCell; use lazy_static::lazy_static; use parking_lot::{RwLock, RwLockWriteGuard}; use raw_window_handle::RawWindowHandle; +use std::any::Any; use std::cmp; use std::collections::{HashMap, VecDeque}; use std::ffi::{c_void, CStr}; @@ -51,7 +52,7 @@ use crate::plugin::{ }; use crate::wrapper::state::{ParamValue, State}; use crate::wrapper::util::{hash_param_id, process_wrapper, strlcpy, u16strlcpy}; -use crate::EditorWindowHandle; +use crate::ParentWindowHandle; // Alias needed for the VST3 attribute macro use vst3_sys as vst3_com; @@ -104,7 +105,11 @@ macro_rules! check_null_ptr_msg { /// its own struct. struct WrapperInner { /// The wrapped plugin instance. - plugin: Box>, + plugin: RwLock

, + /// The plugin's editor, if it has one. This object does not do anything on its own, but we need + /// to instantiate this in advance so we don't need to lock the entire [Plugin] object when + /// creating an editor. + editor: Option>, /// The host's `IComponentHandler` instance, if passed through /// `IEditController::set_component_handler`. @@ -173,7 +178,8 @@ pub(crate) struct Wrapper { #[VST3(implements(IPlugView))] struct WrapperView { inner: Arc>, - editor: RwLock>>, + editor: Arc, + editor_handle: RwLock>>, } /// A [ProcessContext] implementation for the wrapper. This is a separate object so it can hold on @@ -298,8 +304,12 @@ impl WrapperInner

{ // confused by all of these vtables #[allow(unused_unsafe)] pub fn new() -> Arc { + let plugin = RwLock::new(P::default()); + let editor = plugin.read().editor().map(Arc::from); + let mut wrapper = Self { - plugin: Box::new(RwLock::default()), + plugin, + editor, component_handler: RwLock::new(None), @@ -425,8 +435,8 @@ impl Wrapper

{ } impl WrapperView

{ - pub fn new(inner: Arc>) -> Box { - Self::allocate(inner, RwLock::new(None)) + pub fn new(inner: Arc>, editor: Arc) -> Box { + Self::allocate(inner, editor, RwLock::new(None)) } } @@ -977,10 +987,9 @@ impl IEditController for Wrapper

{ unsafe fn create_view(&self, _name: vst3_sys::base::FIDString) -> *mut c_void { // Without specialization this is the least redundant way to check if the plugin has an // editor. The default implementation returns a None here. - match self.inner.plugin.read().editor_size() { - Some(_) => { - Box::into_raw(WrapperView::

::new(self.inner.clone())) as *mut vst3_sys::c_void - } + match &self.inner.editor { + Some(editor) => Box::into_raw(WrapperView::new(self.inner.clone(), editor.clone())) + as *mut vst3_sys::c_void, None => ptr::null_mut(), } } @@ -1275,7 +1284,7 @@ impl IAudioProcessor for Wrapper

{ } } - let mut plugin = self.inner.plugin.write(); + let plugin = &mut *self.inner.plugin.data_ptr(); let mut context = self.inner.make_process_context(); match plugin.process(&mut output_buffer, &mut context) { ProcessStatus::Error(err) => { @@ -1336,8 +1345,8 @@ impl IPlugView for WrapperView

{ } unsafe fn attached(&self, parent: *mut c_void, type_: vst3_sys::base::FIDString) -> tresult { - let mut editor = self.editor.write(); - if editor.is_none() { + let mut editor_handle = self.editor_handle.write(); + if editor_handle.is_none() { let type_ = CStr::from_ptr(type_); let handle = match type_.to_str() { #[cfg(all(target_family = "unix", not(target_os = "macos")))] @@ -1364,15 +1373,10 @@ impl IPlugView for WrapperView

{ } }; - // FIXME: On second thought, this needs some reworking. Needing a read lock on the - // plugin's object means that the process call (which requires a write lock) will - // be blocked. The better API will be to move the create function to the `Editor` - // struct, so we can already fetch that during initialization. - *editor = self - .inner - .plugin - .read() - .create_editor(EditorWindowHandle { handle }, self.inner.clone()); + *editor_handle = Some( + self.editor + .spawn(ParentWindowHandle { handle }, self.inner.clone()), + ); kResultOk } else { kResultFalse @@ -1380,9 +1384,9 @@ impl IPlugView for WrapperView

{ } unsafe fn removed(&self) -> tresult { - let mut editor = self.editor.write(); - if editor.is_some() { - *editor = None; + let mut editor_handle = self.editor_handle.write(); + if editor_handle.is_some() { + *editor_handle = None; kResultOk } else { kResultFalse @@ -1416,20 +1420,9 @@ impl IPlugView for WrapperView

{ unsafe fn get_size(&self, size: *mut vst3_sys::gui::ViewRect) -> tresult { check_null_ptr!(size); - // If the editor is already open, then take the size from the editor in case the plugin - // updates one but not the other - let (width, height) = match self.editor.read().as_ref() { - Some(editor) => editor.size(), - None => self - .inner - .plugin - .read() - .editor_size() - .expect("Wait, this returned a Some just now!?"), - }; - *size = mem::zeroed(); + let (width, height) = self.editor.size(); let size = &mut *size; size.left = 0; size.right = width as i32;