From 0044bc6b6085bfbbc014bac4fa7794e6808784f8 Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Sun, 24 Jan 2021 14:44:42 +0100
Subject: [PATCH] Allow passing null pointers to IPlugView::setFrame

---
 src/common/logging/vst3.cpp                     |  8 +++++++-
 .../serialization/vst3/plug-view/plug-view.h    | 11 +++++++++--
 .../bridges/vst3-impls/plug-view-proxy.cpp      | 12 ++++++++----
 src/wine-host/bridges/vst3.cpp                  | 17 +++++++++++------
 4 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/common/logging/vst3.cpp b/src/common/logging/vst3.cpp
index a8c197a7..b4dad673 100644
--- a/src/common/logging/vst3.cpp
+++ b/src/common/logging/vst3.cpp
@@ -629,7 +629,13 @@ bool Vst3Logger::log_request(bool is_host_vst,
                              const YaPlugView::SetFrame& request) {
     return log_request_base(is_host_vst, [&](auto& message) {
         message << request.owner_instance_id
-                << ": IPlugView::setFrame(frame = <IPlugFrame*>)";
+                << ": IPlugView::setFrame(frame = ";
+        if (request.plug_frame_args) {
+            message << "<IPlugFrame*>";
+        } else {
+            message << "<nullptr>";
+        }
+        message << ")";
     });
 }
 
diff --git a/src/common/serialization/vst3/plug-view/plug-view.h b/src/common/serialization/vst3/plug-view/plug-view.h
index a964c0e0..d6e88ad3 100644
--- a/src/common/serialization/vst3/plug-view/plug-view.h
+++ b/src/common/serialization/vst3/plug-view/plug-view.h
@@ -17,6 +17,7 @@
 #pragma once
 
 #include <pluginterfaces/gui/iplugview.h>
+#include "bitsery/ext/std_optional.h"
 
 #include "../../common.h"
 #include "../base.h"
@@ -291,12 +292,18 @@ class YaPlugView : public Steinberg::IPlugView {
 
         native_size_t owner_instance_id;
 
-        Vst3PlugFrameProxy::ConstructArgs plug_frame_args;
+        /**
+         * Some hosts will pass a null pointer to explicitly destroy the
+         * `IPlugFrame` instance before freeing the plugin's `IPlugView`
+         * instance. This also happens in the SDK, so this seems like valid
+         * behaviour we should support.
+         */
+        std::optional<Vst3PlugFrameProxy::ConstructArgs> plug_frame_args;
 
         template <typename S>
         void serialize(S& s) {
             s.value8b(owner_instance_id);
-            s.object(plug_frame_args);
+            s.ext(plug_frame_args, bitsery::ext::StdOptional{});
         }
     };
 
diff --git a/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp b/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp
index dbce87d6..31727625 100644
--- a/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp
+++ b/src/plugin/bridges/vst3-impls/plug-view-proxy.cpp
@@ -200,7 +200,8 @@ tresult PLUGIN_API Vst3PlugViewProxyImpl::onFocus(TBool state) {
 
 tresult PLUGIN_API
 Vst3PlugViewProxyImpl::setFrame(Steinberg::IPlugFrame* frame) {
-    // TODO: Null pointers are valid here, should we pass them through?
+    // Null pointers are valid here going from the reference implementations in
+    // the SDK
     if (frame) {
         // We'll store the pointer for when the plugin later makes a callback to
         // this component handler
@@ -229,9 +230,12 @@ Vst3PlugViewProxyImpl::setFrame(Steinberg::IPlugFrame* frame) {
             .plug_frame_args = Vst3PlugFrameProxy::ConstructArgs(
                 plug_frame, owner_instance_id())});
     } else {
-        bridge.logger.log(
-            "WARNING: Null pointer passed to 'IPlugView::setFrame()'");
-        return Steinberg::kInvalidArgument;
+        plug_frame.reset();
+        run_loop_tasks.reset();
+
+        return send_mutually_recursive_message(
+            YaPlugView::SetFrame{.owner_instance_id = owner_instance_id(),
+                                 .plug_frame_args = std::nullopt});
     }
 }
 
diff --git a/src/wine-host/bridges/vst3.cpp b/src/wine-host/bridges/vst3.cpp
index f691d278..e612d7c5 100644
--- a/src/wine-host/bridges/vst3.cpp
+++ b/src/wine-host/bridges/vst3.cpp
@@ -737,13 +737,18 @@ void Vst3Bridge::run() {
             },
             [&](YaPlugView::SetFrame& request)
                 -> YaPlugView::SetFrame::Response {
-                // We'll create a proxy object for the `IPlugFrame` object and
-                // pass that to the `setFrame()` function. The lifetime of this
-                // object is tied to that of the actual `IPlugFrame` object
-                // we're passing this proxy to.
+                // If the host passed a valid `IPlugFrame*`, then We'll create a
+                // proxy object for the `IPlugFrame` object and pass that to the
+                // `setFrame()` function. The lifetime of this object is tied to
+                // that of the actual `IPlugFrame` object we're passing this
+                // proxy to. IF the host passed a null pointer (which seems to
+                // be common when terminating plugins) we'll do the same thing
+                // here.
                 object_instances[request.owner_instance_id].plug_frame_proxy =
-                    Steinberg::owned(new Vst3PlugFrameProxyImpl(
-                        *this, std::move(request.plug_frame_args)));
+                    request.plug_frame_args
+                        ? Steinberg::owned(new Vst3PlugFrameProxyImpl(
+                              *this, std::move(*request.plug_frame_args)))
+                        : nullptr;
 
                 // This likely doesn't have to be run from the GUI thread, but
                 // since 80% of the `IPlugView` functions have to be we'll do it