From 7cb671319e0f43f9d355ed79ffb05ff46a2de72f Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Thu, 5 May 2022 15:35:14 +0200
Subject: [PATCH] Properly define all predefined note expressions

This is apparently how you're supposed to do it in VST3. How would you
know? You'd ask Steinberg.
---
 src/wrapper/vst3/note_expressions.rs | 67 +++++++++++++++++++++-------
 src/wrapper/vst3/wrapper.rs          | 54 ++++++++++++++--------
 2 files changed, 85 insertions(+), 36 deletions(-)

diff --git a/src/wrapper/vst3/note_expressions.rs b/src/wrapper/vst3/note_expressions.rs
index aefa4721..714367e5 100644
--- a/src/wrapper/vst3/note_expressions.rs
+++ b/src/wrapper/vst3/note_expressions.rs
@@ -25,6 +25,42 @@ pub const EXPRESSION_EXPRESSION_ID: u32 = 4;
 /// `kBrightnessTypeID`
 pub const BRIGHTNESS_EXPRESSION_ID: u32 = 5;
 
+/// The note expressions we support. It's completely undocumented, but apparently VST3 plugins need
+/// to specifically define a custom note expression for the predefined note expressiosn for them to
+/// work.
+pub const KNOWN_NOTE_EXPRESSIONS: [NoteExpressionInfo; 6] = [
+    NoteExpressionInfo {
+        type_id: VOLUME_EXPRESSION_ID,
+        title: "Volume",
+        unit: "dB",
+    },
+    NoteExpressionInfo {
+        type_id: PAN_EXPRESSION_ID,
+        title: "Pan",
+        unit: "",
+    },
+    NoteExpressionInfo {
+        type_id: TUNING_EXPRESSION_ID,
+        title: "Tuning",
+        unit: "semitones",
+    },
+    NoteExpressionInfo {
+        type_id: VIBRATO_EXPRESSION_ID,
+        title: "Vibrato",
+        unit: "",
+    },
+    NoteExpressionInfo {
+        type_id: EXPRESSION_EXPRESSION_ID,
+        title: "Expression",
+        unit: "",
+    },
+    NoteExpressionInfo {
+        type_id: BRIGHTNESS_EXPRESSION_ID,
+        title: "Brightness",
+        unit: "",
+    },
+];
+
 /// VST3 has predefined note expressions just like CLAP, but unlike the other note events these
 /// expressions are identified only with a note ID. To account for that, we'll keep track of the
 /// most recent note IDs we've encountered so we can later map those IDs back to a note and channel
@@ -39,24 +75,21 @@ pub struct NoteExpressionController {
     note_ids_idx: usize,
 }
 
-impl NoteExpressionController {
-    /// Returns `true` if the VST3 expression type ID is one of the predefiend types we support.
-    /// This is only needed as a workaround for a Bitwig bug where it only sends us note expressions
-    /// if we explicitly return `kResultOk` on
-    /// `INoteExpressionController::get_note_expression_info()` with malformed expression index
-    /// arguments.
-    pub const fn known_expression_type_id(type_id: u32) -> bool {
-        matches!(
-            type_id,
-            VOLUME_EXPRESSION_ID
-                | PAN_EXPRESSION_ID
-                | TUNING_EXPRESSION_ID
-                | VIBRATO_EXPRESSION_ID
-                | EXPRESSION_EXPRESSION_ID
-                | BRIGHTNESS_EXPRESSION_ID
-        )
-    }
+/// This is used to register a (predefined) note expression in the `INoteExpressionController`. The
+/// data is kept in this module to keep everything related to VST3 note expressions in one place.
+///
+/// This does not contain value descriptions because those are also predefined as normalized `[0,
+/// 1]` values.
+pub struct NoteExpressionInfo {
+    /// The predefined VST3 note expression type ID for this note expression.
+    pub type_id: u32,
+    /// The title for the note expression. Also used for the short title because why not.
+    pub title: &'static str,
+    /// The unit for the note expression.
+    pub unit: &'static str,
+}
 
+impl NoteExpressionController {
     /// Register the note ID from a note on event so it can later be retrieved when handling a note
     /// expression value event.
     pub fn register_note(&mut self, event: &NoteOnEvent) {
diff --git a/src/wrapper/vst3/wrapper.rs b/src/wrapper/vst3/wrapper.rs
index ea3a6af7..b8c7635f 100644
--- a/src/wrapper/vst3/wrapper.rs
+++ b/src/wrapper/vst3/wrapper.rs
@@ -8,10 +8,11 @@ use vst3_sys::base::{kInvalidArgument, kNoInterface, kResultFalse, kResultOk, tr
 use vst3_sys::base::{IBStream, IPluginBase};
 use vst3_sys::utils::SharedVstPtr;
 use vst3_sys::vst::{
-    kNoProgramListId, kRootUnitId, Event, EventTypes, IAudioProcessor, IComponent, IEditController,
-    IEventList, IMidiMapping, INoteExpressionController, IParamValueQueue, IParameterChanges,
-    IUnitInfo, LegacyMidiCCOutEvent, NoteExpressionTypeInfo, NoteOffEvent, NoteOnEvent,
-    ParameterFlags, PolyPressureEvent, ProgramListInfo, TChar, UnitInfo,
+    kNoParamId, kNoParentUnitId, kNoProgramListId, kRootUnitId, Event, EventTypes, IAudioProcessor,
+    IComponent, IEditController, IEventList, IMidiMapping, INoteExpressionController,
+    IParamValueQueue, IParameterChanges, IUnitInfo, LegacyMidiCCOutEvent, NoteExpressionTypeInfo,
+    NoteExpressionValueDescription, NoteOffEvent, NoteOnEvent, ParameterFlags, PolyPressureEvent,
+    ProgramListInfo, TChar, UnitInfo,
 };
 use vst3_sys::VST3;
 use widestring::U16CStr;
@@ -29,7 +30,7 @@ use crate::util::permit_alloc;
 use crate::wrapper::state;
 use crate::wrapper::util::process_wrapper;
 use crate::wrapper::vst3::inner::ProcessEvent;
-use crate::wrapper::vst3::note_expressions::NoteExpressionController;
+use crate::wrapper::vst3::note_expressions::{self, NoteExpressionController};
 use crate::wrapper::vst3::util::{VST3_MIDI_CHANNELS, VST3_MIDI_PARAMS_END};
 
 // Alias needed for the VST3 attribute macro
@@ -1435,10 +1436,9 @@ impl<P: Vst3Plugin> IUnitInfo for Wrapper<P> {
 
 impl<P: Vst3Plugin> INoteExpressionController for Wrapper<P> {
     unsafe fn get_note_expression_count(&self, bus_idx: i32, _channel: i16) -> i32 {
-        // NOTE: We don't have any custom note expressions. But Bitwig won't send us the predefined
-        //       note expressions unless we pretend we do.
+        // Apparently you need to define the predefined note expressions. Thanks VST3.
         if P::MIDI_INPUT >= MidiConfig::Basic && bus_idx == 0 {
-            1
+            note_expressions::KNOWN_NOTE_EXPRESSIONS.len() as i32
         } else {
             0
         }
@@ -1451,22 +1451,38 @@ impl<P: Vst3Plugin> INoteExpressionController for Wrapper<P> {
         note_expression_idx: i32,
         info: *mut NoteExpressionTypeInfo,
     ) -> tresult {
-        if P::MIDI_INPUT < MidiConfig::Basic || bus_idx != 0 {
+        if P::MIDI_INPUT < MidiConfig::Basic
+            || bus_idx != 0
+            || !(0..note_expressions::KNOWN_NOTE_EXPRESSIONS.len() as i32)
+                .contains(&note_expression_idx)
+        {
             return kInvalidArgument;
         }
 
         check_null_ptr!(info);
 
-        // NOTE: As mentioned above, this is only a workaround for a Bitwig bug. We don't have any
-        //       custom note expressions, and the IDs passed to this function are the type IDs which
-        //       is incorrect. We won't even bother filling in the information. This argument is of
-        //       course also incorrect, as these expression indices are supposed to be linear
-        //       indices starting at 0, while Bitwig queries this with 0, 1, 2, and 5.
-        if NoteExpressionController::known_expression_type_id(note_expression_idx as u32) {
-            kResultOk
-        } else {
-            kResultFalse
-        }
+        *info = mem::zeroed();
+
+        let info = &mut *info;
+        let note_expression_info =
+            &note_expressions::KNOWN_NOTE_EXPRESSIONS[note_expression_idx as usize];
+        info.type_id = note_expression_info.type_id;
+        u16strlcpy(&mut info.title, note_expression_info.title);
+        u16strlcpy(&mut info.short_title, note_expression_info.title);
+        u16strlcpy(&mut info.units, note_expression_info.unit);
+        info.unit_id = kNoParentUnitId;
+        // This should not be needed since they're predefined, but then again you'd think you also
+        // wouldn't need to define predefined note expressions now do you?
+        info.value_desc = NoteExpressionValueDescription {
+            default_value: 0.5,
+            min: 0.0,
+            max: 1.0,
+            step_count: 0,
+        };
+        info.id = kNoParamId;
+        info.flags = 1 << 2; // kIsAbsolute
+
+        kResultOk
     }
 
     unsafe fn get_note_expression_string_by_value(