No longer return a lambda in passthrough_event()

Now that it's no longer used directly this is no longer needed, and it's
much clearer without the lambda.

src/common/communication.h

@@ -565,10 +565,10 @@ class EventHandler {
      * then a blocking loop that handles events from the primary `socket`.
      *
      * The specified function will be used to create an `EventResult` from an
-     * `Event`. This is almost always a wrapper around `passthrough_event()`,
+     * `Event`. This is almost uses `passthrough_event()`, which converts a
-     * which converts the `EventPayload` into a format used by VST2, calls
+     * `EventPayload` into the format used by VST2, calls either `dispatch()` or
-     * either `dispatch()` or `audioMaster()` depending on the socket, and then
+     * `audioMaster()` depending on the context, and then serializes the result
-     * serializes the result back into an `EventResultPayload`.
+     * back into an `EventResultPayload`.
      *
      * This function will also be used separately for receiving MIDI data, as
      * some plugins will need pointers to received MIDI data to stay alive until
@@ -920,44 +920,38 @@ class Sockets {
 boost::filesystem::path generate_endpoint_base(const std::string& plugin_name);
 
 /**
- * Create a callback function that takes an `Event` object, decodes the data
+ * Unmarshall an `EventPayload` back to the representation used by VST2, pass
- * into the expected format for VST2 function calls, calls the given function
+ * that value to a callback function (either `AEffect::dispatcher()` for host ->
- * (either `AEffect::dispatcher()` for host -> plugin events or `audioMaster()`
+ * plugin events or `audioMaster()` for plugin -> host events), and then
- * for plugin -> host events), and serializes the results back into an
+ * serialize the results back into an `EventResult`.
- * `EventResult` object. I'd rather not get too Haskell-y in my C++, but this is
- * the cleanest solution for this problem.
  *
  * This is the receiving analogue of the `*DataCovnerter` objects.
  *
- * TODO: Now that `EventHandler::receive_events` replaced `receive_event()`,
- *       refactor this to just handle the event directly rather than returning a
- *       lambda
- *
  * @param plugin The `AEffect` instance that should be passed to the callback
- *   function.
+ *   function. During `WantsAEffect` we'll send back a copy of this, and when we
+ *   get sent an `AEffect` instance (e.g. during `audioMasterIOChanged()`) we'll
+ *   write the updated values to this isntance.
  * @param callback The function to call with the arguments received from the
- *   socket.
+ *   socket, either `AEffect::dispatcher()` or `audioMasterCallback()`.
  *
- * @tparam A function with the same signature as `AEffect::dispatcher` or
+ * @tparam F A function with the same signature as `AEffect::dispatcher` or
  *   `audioMasterCallback`.
  *
- * @return A `EventResult(Event)` callback function that can be passed to
+ * @return The result of the operation. If necessary the `DataConverter` will
- * `EditorHandler::receive_events()`.
+ *   unmarshall the payload again and write it back.
  *
  * @relates EventHandler::receive_events
  */
 template <typename F>
-auto passthrough_event(AEffect* plugin, F callback) {
+EventResult passthrough_event(AEffect* plugin, F callback, Event& event) {
-    return [=](Event& event) -> EventResult {
     // This buffer is used to write strings and small objects to. We'll
-        // initialize the beginning with null values to both prevent it from
+    // initialize the beginning with null values to both prevent it from being
-        // being read as some arbitrary C-style string, and to make sure that
+    // read as some arbitrary C-style string, and to make sure that
-        // `*static_cast<void**>(string_buffer.data)` will be a null pointer if
+    // `*static_cast<void**>(string_buffer.data)` will be a null pointer if the
-        // the plugin is supposed to write a pointer there but doesn't (such as
+    // plugin is supposed to write a pointer there but doesn't (such as with
-        // with `effEditGetRect`/`WantsVstRect`).
+    // `effEditGetRect`/`WantsVstRect`).
     std::array<char, max_string_length> string_buffer;
-        std::fill(string_buffer.begin(), string_buffer.begin() + sizeof(size_t),
+    std::fill(string_buffer.begin(), string_buffer.begin() + sizeof(size_t), 0);
-                  0);
 
     auto read_payload_fn = overload{
         [&](const std::nullptr_t&) -> void* { return nullptr; },
@@ -969,13 +963,12 @@ auto passthrough_event(AEffect* plugin, F callback) {
         },
         [&](native_size_t& window_handle) -> void* {
             // This is the X11 window handle that the editor should reparent
-                // itself to. We have a special wrapper around the dispatch
+            // itself to. We have a special wrapper around the dispatch function
-                // function that intercepts `effEditOpen` events and creates a
+            // that intercepts `effEditOpen` events and creates a Win32 window
-                // Win32 window and then finally embeds the X11 window Wine
+            // and then finally embeds the X11 window Wine created into this
-                // created into this wnidow handle. Make sure to convert the
+            // wnidow handle. Make sure to convert the window ID first to
-                // window ID first to `size_t` in case this is the 32-bit host.
+            // `size_t` in case this is the 32-bit host.
-                return reinterpret_cast<void*>(
+            return reinterpret_cast<void*>(static_cast<size_t>(window_handle));
-                    static_cast<size_t>(window_handle));
         },
         [&](const AEffect&) -> void* { return nullptr; },
         [&](DynamicVstEvents& events) -> void* {
@@ -985,11 +978,10 @@ auto passthrough_event(AEffect* plugin, F callback) {
             return &speaker_arrangement.as_c_speaker_arrangement();
         },
         [&](WantsAEffectUpdate&) -> void* {
-                // The host will never actually ask for an updated `AEffect`
+            // The host will never actually ask for an updated `AEffect` object
-                // object since that should not be a thing. This is purely a
+            // since that should not be a thing. This is purely a meant as a
-                // meant as a workaround for plugins that initialize their
+            // workaround for plugins that initialize their `AEffect` object
-                // `AEffect` object after the plugin has already finished
+            // after the plugin has already finished initializing.
-                // initializing.
             return nullptr;
         },
         [&](WantsChunkBuffer&) -> void* { return string_buffer.data(); },
@@ -1000,9 +992,9 @@ auto passthrough_event(AEffect* plugin, F callback) {
         [&](const WantsVstTimeInfo&) -> void* { return nullptr; },
         [&](WantsString&) -> void* { return string_buffer.data(); }};
 
-        // Almost all events pass data through the `data` argument. There are
+    // Almost all events pass data through the `data` argument. There are two
-        // two events, `effSetParameter` and `effGetParameter` that also pass
+    // events, `effSetParameter` and `effGetParameter` that also pass data
-        // data through the value argument.
+    // through the value argument.
     void* data = std::visit(read_payload_fn, event.payload);
     intptr_t value = event.value;
     if (event.value_payload) {
@@ -1010,11 +1002,10 @@ auto passthrough_event(AEffect* plugin, F callback) {
             std::visit(read_payload_fn, *event.value_payload));
     }
 
-        const intptr_t return_value = callback(
+    const intptr_t return_value =
-            plugin, event.opcode, event.index, value, data, event.option);
+        callback(plugin, event.opcode, event.index, value, data, event.option);
 
-        // Only write back data when needed, this depends on the event payload
+    // Only write back data when needed, this depends on the event payload type
-        // type
     auto write_payload_fn = overload{
         [&](auto) -> EventResultPayload { return nullptr; },
         [&](const AEffect& updated_plugin) -> EventResultPayload {
@@ -1034,8 +1025,7 @@ auto passthrough_event(AEffect* plugin, F callback) {
             // return value from the event determines how much data the
             // plugin has written
             const uint8_t* chunk_data = *static_cast<uint8_t**>(data);
-                return std::vector<uint8_t>(chunk_data,
+            return std::vector<uint8_t>(chunk_data, chunk_data + return_value);
-                                            chunk_data + return_value);
         },
         [&](VstIOProperties& props) -> EventResultPayload { return props; },
         [&](VstMidiKeyName& key_name) -> EventResultPayload {
@@ -1046,11 +1036,10 @@ auto passthrough_event(AEffect* plugin, F callback) {
         },
         [&](WantsAEffectUpdate&) -> EventResultPayload { return *plugin; },
         [&](WantsVstRect&) -> EventResultPayload {
-                // The plugin should have written a pointer to a VstRect struct
+            // The plugin should have written a pointer to a VstRect struct into
-                // into the data pointer. I haven't seen this fail yet, but
+            // the data pointer. I haven't seen this fail yet, but since some
-                // since some hosts will call `effEditGetRect()` before
+            // hosts will call `effEditGetRect()` before `effEditOpen()` I can
-                // `effEditOpen()` I can assume there are plugins that don't
+            // assume there are plugins that don't handle this correctly.
-                // handle this correctly.
             VstRect* editor_rect = *static_cast<VstRect**>(data);
             if (!editor_rect) {
                 return nullptr;
@@ -1059,11 +1048,10 @@ auto passthrough_event(AEffect* plugin, F callback) {
             return *editor_rect;
         },
         [&](WantsVstTimeInfo&) -> EventResultPayload {
-                // Not sure why the VST API has twenty different ways of
+            // Not sure why the VST API has twenty different ways of returning
-                // returning structs, but in this case the value returned from
+            // structs, but in this case the value returned from the callback
-                // the callback function is actually a pointer to a
+            // function is actually a pointer to a `VstTimeInfo` struct! It can
-                // `VstTimeInfo` struct! It can also be a null pointer if the
+            // also be a null pointer if the host doesn't support this.
-                // host doesn't support this.
             const auto time_info =
                 reinterpret_cast<const VstTimeInfo*>(return_value);
             if (!time_info) {
@@ -1077,11 +1065,10 @@ auto passthrough_event(AEffect* plugin, F callback) {
         }};
 
     // As mentioned about, the `effSetSpeakerArrangement` and
-        // `effGetSpeakerArrangement` events are the only two events that use
+    // `effGetSpeakerArrangement` events are the only two events that use the
-        // the value argument as a pointer to write data to. Additionally, the
+    // value argument as a pointer to write data to. Additionally, the
-        // `effGetSpeakerArrangement` expects the plugin to write its own data
+    // `effGetSpeakerArrangement` expects the plugin to write its own data to
-        // to this value. Hence why we need to encode the response here
+    // this value. Hence why we need to encode the response here separately.
-        // separately.
     const EventResultPayload response_data =
         std::visit(write_payload_fn, event.payload);
     std::optional<EventResultPayload> value_response_data = std::nullopt;
@@ -1095,5 +1082,4 @@ auto passthrough_event(AEffect* plugin, F callback) {
                          .value_payload = value_response_data};
 
     return response;
-    };
 }

src/plugin/plugin-bridge.cpp

@@ -141,8 +141,8 @@ PluginBridge::PluginBridge(audioMasterCallback host_callback)
 
                     return response;
                 } else {
-                    return passthrough_event(&plugin,
+                    return passthrough_event(&plugin, host_callback_function,
-                                             host_callback_function)(event);
+                                             event);
                 }
             });
     });

src/wine-host/bridges/vst2.cpp

@@ -152,8 +152,8 @@ Vst2Bridge::Vst2Bridge(MainContext& main_context,
                     DynamicVstEvents& events =
                         next_audio_buffer_midi_events.back();
 
-                    // Exact same handling as in `passthrough_event`, apart from
+                    // Exact same handling as in `passthrough_event()`, apart
-                    // making a copy of the events first
+                    // from making a copy of the events first
                     const intptr_t return_value = plugin->dispatcher(
                         plugin, event.opcode, event.index, event.value,
                         &events.as_c_events(), event.option);
@@ -173,8 +173,10 @@ Vst2Bridge::Vst2Bridge(MainContext& main_context,
                     // Maybe this should just be a hard error instead, since it
                     // should never happen
                     return passthrough_event(
-                        plugin, std::bind(&Vst2Bridge::dispatch_wrapper, this,
+                        plugin,
-                                          _1, _2, _3, _4, _5, _6))(event);
+                        std::bind(&Vst2Bridge::dispatch_wrapper, this, _1, _2,
+                                  _3, _4, _5, _6),
+                        event);
                 }
             });
     });
@@ -320,9 +322,6 @@ bool Vst2Bridge::should_skip_message_loop() const {
 void Vst2Bridge::handle_dispatch() {
     sockets.host_vst_dispatch.receive_events(
         std::nullopt, [&](Event& event, bool /*on_main_thread*/) {
-            // TODO: As per the TODO in `passthrough_event`, this can use a
-            //       round of refactoring now that we never use its returned
-            //       lambda directly anymore
             return passthrough_event(
                 plugin,
                 [&](AEffect* plugin, int opcode, int index, intptr_t value,
@@ -346,7 +345,8 @@ void Vst2Bridge::handle_dispatch() {
                         return dispatch_wrapper(plugin, opcode, index, value,
                                                 data, option);
                     }
-                })(event);
+                },
+                event);
         });
 }
 
@@ -449,7 +449,7 @@ class HostCallbackDataConverter : DefaultDataConverter {
             case audioMasterIOChanged:
                 // This is a helpful event that indicates that the VST
                 // plugin's `AEffect` struct has changed. Writing these
-                // results back is done inside of `passthrough_event`.
+                // results back is done inside of `passthrough_event()`.
                 return AEffect(*plugin);
                 break;
             case audioMasterProcessEvents: