From f6041789cdca0a7793b63449e06ee4ab33a43d13 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Tue, 7 Mar 2023 21:22:15 +0100 Subject: [PATCH] Use computed sizes for ViziaState --- CHANGELOG.md | 12 +++ nih_plug_vizia/src/editor.rs | 5 ++ nih_plug_vizia/src/lib.rs | 73 ++++++++++--------- nih_plug_vizia/src/widgets.rs | 21 +++++- plugins/crisp/src/editor.rs | 2 +- plugins/diopser/src/editor.rs | 2 +- plugins/examples/gain_gui_vizia/src/editor.rs | 2 +- plugins/spectral_compressor/src/editor.rs | 2 +- 8 files changed, 78 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e34bbc05..21c58ed4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,11 +14,23 @@ state is to list breaking changes. This document is now also used to keep track of non-breaking changes. +### Breaking changes + +- The way window sizes work in `ViziaState` has been reworked to be more + predictable and reliable. Instead of creating a `ViziaState` with a predefined + size and then tracking the window's current size in that object, `ViziaState` + now takes a callback that returns the window's current logical size. This can + be used to compute the window's current size based on the plugin's state. The + result is that window sizes always match the plugin's current state and + recalling an old incorrect size is no longer possible. + ### Added - Debug builds now include debug assertions that detect incorrect use of the `GuiContext`'s parameter setting methods. +### Changed + ## [2023-02-28] ### Breaking changes diff --git a/nih_plug_vizia/src/editor.rs b/nih_plug_vizia/src/editor.rs index 63ed4fa1..b326ea9f 100644 --- a/nih_plug_vizia/src/editor.rs +++ b/nih_plug_vizia/src/editor.rs @@ -67,9 +67,14 @@ impl Editor for ViziaEditor { // And we'll link `WindowEvent::ResizeWindow` and `WindowEvent::SetScale` events to our // `ViziaState`. We'll notify the host when any of these change. + let current_inner_window_size = cx.window_size(); widgets::WindowModel { context: context.clone(), vizia_state: vizia_state.clone(), + last_inner_window_size: AtomicCell::new(( + current_inner_window_size.width, + current_inner_window_size.height, + )), } .build(cx); diff --git a/nih_plug_vizia/src/lib.rs b/nih_plug_vizia/src/lib.rs index 28729668..b478210e 100644 --- a/nih_plug_vizia/src/lib.rs +++ b/nih_plug_vizia/src/lib.rs @@ -7,6 +7,7 @@ use crossbeam::atomic::AtomicCell; use nih_plug::params::persist::PersistentField; use nih_plug::prelude::{Editor, GuiContext}; use serde::{Deserialize, Serialize}; +use std::fmt::Debug; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use vizia::prelude::*; @@ -81,13 +82,14 @@ pub enum ViziaTheming { Custom, } -/// State for an `nih_plug_vizia` editor. The scale factor can be manipulated at runtime by changing -/// `cx.user_scale_factor`. -#[derive(Debug, Serialize, Deserialize)] +/// State for an `nih_plug_vizia` editor. The scale factor can be manipulated at runtime using +/// `cx.set_user_scale_factor()`. +#[derive(Serialize, Deserialize)] pub struct ViziaState { - /// The window's size in logical pixels before applying `scale_factor`. - #[serde(with = "nih_plug::params::persist::serialize_atomic_cell")] - size: AtomicCell<(u32, u32)>, + /// A function that returns the window's current size in logical pixels, before any sort of + /// scaling is applied. This size can be computed based on the plugin's current state. + #[serde(skip, default = "empty_size_fn")] + size_fn: Box (u32, u32) + Send + Sync>, /// A scale factor that should be applied to `size` separate from from any system HiDPI scaling. /// This can be used to allow GUIs to be scaled uniformly. #[serde(with = "nih_plug::params::persist::serialize_atomic_cell")] @@ -95,17 +97,27 @@ pub struct ViziaState { /// Whether the editor's window is currently open. #[serde(skip)] open: AtomicBool, +} - /// Whether the size should be saved. If the window's size is always scaled uniformly, then this - /// is not needed and can only result in problems. - should_save_size: bool, +/// A default implementation for `size_fn` needed to be able to derive the `Deserialize` trait. +fn empty_size_fn() -> Box (u32, u32) + Send + Sync> { + Box::new(|| (0, 0)) +} + +impl Debug for ViziaState { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let (width, height) = (self.size_fn)(); + + f.debug_struct("ViziaState") + .field("size_fn", &format!(" ({}, {})", width, height)) + .field("scale_factor", &self.scale_factor) + .field("open", &self.open) + .finish() + } } impl<'a> PersistentField<'a, ViziaState> for Arc { fn set(&self, new_value: ViziaState) { - if self.should_save_size { - self.size.store(new_value.size.load()); - } self.scale_factor.store(new_value.scale_factor.load()); } @@ -119,42 +131,35 @@ impl<'a> PersistentField<'a, ViziaState> for Arc { impl ViziaState { /// Initialize the GUI's state. This value can be passed to [`create_vizia_editor()`]. The - /// window size is in logical pixels, so before it is multiplied by the DPI scaling factor. - /// - /// Setting `should_save_size` to `false` may be useful when the size is supposed to be fixed - /// and only the scaling factor changes. This allows the object to be persisted in a `Params` - /// object without accidentally restoring old sizes after the window's logical size has changed - /// in a plugin update. - pub fn from_size(width: u32, height: u32, should_save_size: bool) -> Arc { + /// callback always returns the window's current size is in logical pixels, so before it is + /// multiplied by the DPI scaling factor. This size can be computed based on the plugin's + /// current state. + pub fn new(size_fn: impl Fn() -> (u32, u32) + Send + Sync + 'static) -> Arc { Arc::new(ViziaState { - size: AtomicCell::new((width, height)), + size_fn: Box::new(size_fn), scale_factor: AtomicCell::new(1.0), open: AtomicBool::new(false), - should_save_size, }) } - /// The same as [`from_size()`][Self::from_size()], but with a separate initial scale factor. - /// This scale factor gets applied on top of any HiDPI scaling, and it can be modified at - /// runtime by changing `cx.user_scale_factor`. - pub fn from_size_with_scale( - width: u32, - height: u32, - scale_factor: f64, - should_save_size: bool, + /// The same as [`new()`][Self::new()], but with a separate initial scale factor. This scale + /// factor gets applied on top of any HiDPI scaling, and it can be modified at runtime by + /// changing `cx.set_user_scale_factor()`. + pub fn new_with_default_scale_factor( + size_fn: impl Fn() -> (u32, u32) + Send + Sync + 'static, + default_scale_factor: f64, ) -> Arc { Arc::new(ViziaState { - size: AtomicCell::new((width, height)), - scale_factor: AtomicCell::new(scale_factor), + size_fn: Box::new(size_fn), + scale_factor: AtomicCell::new(default_scale_factor), open: AtomicBool::new(false), - should_save_size, }) } /// Returns a `(width, height)` pair for the current size of the GUI in logical pixels, after /// applying the user scale factor. pub fn scaled_logical_size(&self) -> (u32, u32) { - let (logical_width, logical_height) = self.size.load(); + let (logical_width, logical_height) = self.inner_logical_size(); let scale_factor = self.scale_factor.load(); ( @@ -166,7 +171,7 @@ impl ViziaState { /// Returns a `(width, height)` pair for the current size of the GUI in logical pixels before /// applying the user scale factor. pub fn inner_logical_size(&self) -> (u32, u32) { - self.size.load() + (self.size_fn)() } /// Get the non-DPI related uniform scaling factor the GUI's size will be multiplied with. This diff --git a/nih_plug_vizia/src/widgets.rs b/nih_plug_vizia/src/widgets.rs index 31db997f..613c29e0 100644 --- a/nih_plug_vizia/src/widgets.rs +++ b/nih_plug_vizia/src/widgets.rs @@ -5,6 +5,8 @@ //! None of these widgets are finalized, and their sizes or looks can change at any point. Feel free //! to copy the widgets and modify them to your personal taste. +use crossbeam::atomic::AtomicCell; +use nih_plug::nih_debug_assert_eq; use nih_plug::prelude::{GuiContext, Param, ParamPtr}; use std::sync::Arc; use vizia::prelude::*; @@ -78,6 +80,10 @@ pub(crate) struct ParamModel { pub(crate) struct WindowModel { pub context: Arc, pub vizia_state: Arc, + + /// The last known unscaled logical window size. Used to prevent sending duplicate resize + /// requests. + pub last_inner_window_size: AtomicCell<(u32, u32)>, } impl Model for ParamModel { @@ -104,8 +110,16 @@ impl Model for WindowModel { event.map(|window_event, _| { if let WindowEvent::GeometryChanged { .. } = window_event { let logical_size = (cx.window_size().width, cx.window_size().height); + // `self.vizia_state.inner_logical_size()` should match `logical_size`. Since it's + // computed we need to store the last logical size on this object. + nih_debug_assert_eq!( + logical_size, + self.vizia_state.inner_logical_size(), + "The window size set on the vizia context does not match the size returned by \ + 'ViziaState::size_fn'" + ); let old_logical_size @ (old_logical_width, old_logical_height) = - self.vizia_state.size.load(); + self.last_inner_window_size.load(); let scale_factor = cx.user_scale_factor(); let old_user_scale_factor = self.vizia_state.scale_factor.load(); @@ -117,13 +131,14 @@ impl Model for WindowModel { // Our embedded baseview window will have already been resized. If the host does not // accept our new size, then we'll try to undo that - self.vizia_state.size.store(logical_size); + self.last_inner_window_size.store(logical_size); self.vizia_state.scale_factor.store(scale_factor); if !self.context.request_resize() { - self.vizia_state.size.store(old_logical_size); + self.last_inner_window_size.store(old_logical_size); self.vizia_state.scale_factor.store(old_user_scale_factor); // This will cause the window's size to be reverted on the next event loop + // NOTE: Is resizing back the correct behavior now that the size is computed? cx.set_window_size(WindowSize { width: old_logical_width, height: old_logical_height, diff --git a/plugins/crisp/src/editor.rs b/plugins/crisp/src/editor.rs index 19fa39c9..28db42ed 100644 --- a/plugins/crisp/src/editor.rs +++ b/plugins/crisp/src/editor.rs @@ -31,7 +31,7 @@ impl Model for Data {} // Makes sense to also define this here, makes it a bit easier to keep track of pub(crate) fn default_state() -> Arc { - ViziaState::from_size(400, 390, false) + ViziaState::new(|| (400, 390)) } pub(crate) fn create( diff --git a/plugins/diopser/src/editor.rs b/plugins/diopser/src/editor.rs index f47cc33c..348d3ff8 100644 --- a/plugins/diopser/src/editor.rs +++ b/plugins/diopser/src/editor.rs @@ -60,7 +60,7 @@ impl Model for Data {} // Makes sense to also define this here, makes it a bit easier to keep track of pub(crate) fn default_state() -> Arc { - ViziaState::from_size(EDITOR_WIDTH, EDITOR_HEIGHT, false) + ViziaState::new(|| (EDITOR_WIDTH, EDITOR_HEIGHT)) } pub(crate) fn create(editor_data: Data, editor_state: Arc) -> Option> { diff --git a/plugins/examples/gain_gui_vizia/src/editor.rs b/plugins/examples/gain_gui_vizia/src/editor.rs index 3bbf7c74..9ca20905 100644 --- a/plugins/examples/gain_gui_vizia/src/editor.rs +++ b/plugins/examples/gain_gui_vizia/src/editor.rs @@ -19,7 +19,7 @@ impl Model for Data {} // Makes sense to also define this here, makes it a bit easier to keep track of pub(crate) fn default_state() -> Arc { - ViziaState::from_size(200, 150, false) + ViziaState::new(|| (200, 150)) } pub(crate) fn create( diff --git a/plugins/spectral_compressor/src/editor.rs b/plugins/spectral_compressor/src/editor.rs index c312ac80..09148e55 100644 --- a/plugins/spectral_compressor/src/editor.rs +++ b/plugins/spectral_compressor/src/editor.rs @@ -36,7 +36,7 @@ impl Model for Data {} // Makes sense to also define this here, makes it a bit easier to keep track of pub(crate) fn default_state() -> Arc { - ViziaState::from_size(680, 535, false) + ViziaState::new(|| (680, 535)) } pub(crate) fn create(