From e4ca520b64b55809cc43b844a3c639b54b3118b0 Mon Sep 17 00:00:00 2001 From: Robbert van der Helm Date: Thu, 20 May 2021 00:53:48 +0200 Subject: [PATCH] :boom: Redo all higher order template functions This does what we did for a few functions in the last few commits for every function. We now use either the `std::invocable` concept or our own `invocable_returning` concept wherever possible to make sure we pass function types to these template functions, since constraint errors are a lot more readable than template deduction errors. And instead of having to specify the return type as a template argument, we now just use `std::invoke_result_t` instead. The VST3 message handling functions are still using the good old `typename F` since those are overloaded polymorphic functions. This was also a good moment to modify `AdHocSocketHandler::send()` to allow functions returning void (this got rid of an old fixme where we had to return some dummy value from a function instead of just not returning anything). --- src/common/communication/common.h | 67 ++++++++++--------- src/common/communication/vst2.h | 19 ++---- src/common/communication/vst3.h | 23 +++---- src/common/mutual-recursion.h | 8 +-- src/plugin/bridges/common.h | 9 ++- .../bridges/vst3-impls/plug-view-proxy.h | 13 ++-- src/plugin/bridges/vst3.cpp | 4 +- src/plugin/bridges/vst3.h | 2 +- src/plugin/utils.h | 6 +- src/wine-host/bridges/vst3.cpp | 22 +++--- src/wine-host/bridges/vst3.h | 11 ++- 11 files changed, 91 insertions(+), 93 deletions(-) diff --git a/src/common/communication/common.h b/src/common/communication/common.h index ebc09594..980d6c90 100644 --- a/src/common/communication/common.h +++ b/src/common/communication/common.h @@ -435,8 +435,8 @@ class SocketHandler { * @see read_object * @see SocketHandler::receive_single */ - template - void receive_multi(F callback) { + template &> F> + void receive_multi(F&& callback) { std::vector buffer{}; while (true) { try { @@ -566,13 +566,17 @@ class AdHocSocketHandler { * @param callback A function that will be called with a reference to a * socket. This is either the primary `socket`, or a new ad hock socket if * this function is currently being called from another thread. - * - * @tparam T The return value of F. - * @tparam F A function in the form of - * `T(boost::asio::local::stream_protocol::socket&)`. */ - template - T send(F callback) { + template F> + std::invoke_result_t send( + F&& callback) { + // A bit of template and constexpr nastiness to allow us to either + // return a value from the callback (for when writing the response to a + // new object) or to return void (when we deserialize into an existing + // object) + constexpr bool returns_void = std::is_void_v>; + // XXX: Maybe at some point we should benchmark how often this // ad hoc socket spawning mechanism gets used. If some hosts // for instance consistently and repeatedly trigger this then @@ -582,10 +586,15 @@ class AdHocSocketHandler { // This was used to always block when sending the first message, // because the other side may not be listening for additional // connections yet - auto result = callback(socket); - sent_first_event = true; + if constexpr (returns_void) { + callback(socket); + sent_first_event = true; + } else { + auto result = callback(socket); + sent_first_event = true; - return result; + return result; + } } else { try { boost::asio::local::stream_protocol::socket secondary_socket( @@ -609,10 +618,15 @@ class AdHocSocketHandler { if (!sent_first_event) { std::lock_guard lock(write_mutex); - auto result = callback(socket); - sent_first_event = true; + if constexpr (returns_void) { + callback(socket); + sent_first_event = true; + } else { + auto result = callback(socket); + sent_first_event = true; - return result; + return result; + } } else { // Rethrow the exception if the sockets we're not // handling the specific case described above @@ -636,15 +650,12 @@ class AdHocSocketHandler { * an incoming connection on a secondary socket. This would often do the * same thing as `primary_callback`, but secondary sockets may need some * different handling. - * - * @tparam F A function type in the form of - * `void(boost::asio::local::stream_protocol::socket&)`. - * @tparam G The same as `F`. */ - template + template F, + std::invocable G> void receive_multi(std::optional> logger, - F primary_callback, - G secondary_callback) { + F&& primary_callback, + G&& secondary_callback) { // We use this flag to have the `close()` function wait for the this // function to exit, to prevent use-after-frees when destroying this // object from another thread. @@ -726,10 +737,10 @@ class AdHocSocketHandler { * * @overload */ - template + template F> void receive_multi(std::optional> logger, - F callback) { - receive_multi(logger, callback, callback); + F&& callback) { + receive_multi(logger, callback, std::forward(callback)); } private: @@ -742,16 +753,12 @@ class AdHocSocketHandler { * @param logger A logger instance for logging connection errors. This * should only be passed on the plugin side. * @param callback A function that handles the new socket connection. - * - * @tparam F A function in the form - * `void(boost::asio::local::stream_protocol::socket)` to handle a new - * incoming connection. */ - template + template F> void accept_requests( boost::asio::local::stream_protocol::acceptor& acceptor, std::optional> logger, - F callback) { + F&& callback) { acceptor.async_accept( [&, logger, callback]( const boost::system::error_code& error, diff --git a/src/common/communication/vst2.h b/src/common/communication/vst2.h index 628f37f9..051fd174 100644 --- a/src/common/communication/vst2.h +++ b/src/common/communication/vst2.h @@ -20,6 +20,7 @@ #include "../logging/vst2.h" #include "../serialization/vst2.h" +#include "../utils.h" #include "common.h" /** @@ -184,7 +185,7 @@ class EventHandler : public AdHocSocketHandler { // messages from arriving out of order. `AdHocSocketHandler::send()` // will either use a long-living primary socket, or if that's currently // in use it will spawn a new socket for us. - EventResult response = this->template send( + const EventResult response = this->send( [&](boost::asio::local::stream_protocol::socket& socket) { write_object(socket, event); return read_object(socket); @@ -219,16 +220,12 @@ class EventHandler : public AdHocSocketHandler { * @param callback The function used to generate a response out of an event. * See the definition of `F` for more information. * - * @tparam F A function type in the form of `EventResponse(Event, bool)`. - * The boolean flag is `true` when this event was received on the main - * socket, and `false` otherwise. - * * @relates EventHandler::send_event * @relates passthrough_event */ - template + template F> void receive_events(std::optional> logging, - F callback) { + F&& callback) { // Reading, processing, and writing back event data from the sockets // works in the same way regardless of which socket we're using const auto process_event = @@ -384,16 +381,14 @@ class Vst2Sockets : public Sockets { * @param callback The function to call with the arguments received from the * socket, either `AEffect::dispatcher()` or `audioMasterCallback()`. * - * @tparam F A function with the same signature as `AEffect::dispatcher` or - * `audioMasterCallback`. - * * @return The result of the operation. If necessary the `DataConverter` will * unmarshall the payload again and write it back. * * @relates EventHandler::receive_events */ -template -EventResult passthrough_event(AEffect* plugin, F callback, Event& event) { +template < + invocable_returning F> +EventResult passthrough_event(AEffect* plugin, F&& callback, Event& event) { // This buffer is used to write strings and small objects to. We'll // initialize the beginning with null values to both prevent it from being // read as some arbitrary C-style string, and to make sure that diff --git a/src/common/communication/vst3.h b/src/common/communication/vst3.h index 00ce9bf4..964b911a 100644 --- a/src/common/communication/vst3.h +++ b/src/common/communication/vst3.h @@ -138,15 +138,10 @@ class Vst3MessageHandler : public AdHocSocketHandler { // messages from arriving out of order. `AdHocSocketHandler::send()` // will either use a long-living primary socket, or if that's currently // in use it will spawn a new socket for us. - this->template send( - [&](boost::asio::local::stream_protocol::socket& socket) { - write_object(socket, Request(object), buffer); - read_object(socket, response_object, buffer); - // FIXME: We have to return something here, and ML was not yet - // invented when they came up with C++ so void is not - // valid here - return std::monostate{}; - }); + this->send([&](boost::asio::local::stream_protocol::socket& socket) { + write_object(socket, Request(object), buffer); + read_object(socket, response_object, buffer); + }); if (should_log_response) { auto [logger, is_host_vst] = *logging; @@ -205,7 +200,7 @@ class Vst3MessageHandler : public AdHocSocketHandler { */ template void receive_messages(std::optional> logging, - F callback) { + F&& callback) { // Reading, processing, and writing back the response for the requests // we receive works in the same way regardless of which socket we're // using @@ -377,12 +372,15 @@ class Vst3Sockets : public Sockets { * Wine plugin host is even listening on it. * @param cb An overloaded function that can take every type `T` in the * `AudioProcessorRequest` variant and then returns `T::Response`. + * + * @tparam F A function type in the form of `T::Response(T)` for every `T` + * in `AudioProcessorRequest::Payload`. */ template void add_audio_processor_and_listen( size_t instance_id, std::promise& socket_listening_latch, - F&& cb) { + F&& callback) { { std::lock_guard lock(audio_processor_sockets_mutex); audio_processor_sockets.try_emplace( @@ -400,7 +398,8 @@ class Vst3Sockets : public Sockets { // receiving buffers for all calls. This slightly reduces the amount of // allocations in the audio processing loop. audio_processor_sockets.at(instance_id) - .template receive_messages(std::nullopt, std::forward(cb)); + .template receive_messages(std::nullopt, + std::forward(callback)); } /** diff --git a/src/common/mutual-recursion.h b/src/common/mutual-recursion.h index be44e8c9..3f6c1cd0 100644 --- a/src/common/mutual-recursion.h +++ b/src/common/mutual-recursion.h @@ -74,8 +74,8 @@ class MutualRecursionHelper { * * @return The return value of `fn`. */ - template F> - std::invoke_result_t fork(F fn) { + template + std::invoke_result_t fork(F&& fn) { using Result = std::invoke_result_t; // This IO context will accept incoming calls from `handle()` and @@ -135,7 +135,7 @@ class MutualRecursionHelper { * * @tparam F Some callable function that doesn't take any parameters. */ - template F> + template std::invoke_result_t handle(F&& fn) { // If we're not currently engaged in some mutually recursive calling // sequence, then we'll execute the function on this thread @@ -154,7 +154,7 @@ class MutualRecursionHelper { * * @see handle */ - template F> + template std::optional> maybe_handle(F&& fn) { using Result = std::invoke_result_t; diff --git a/src/plugin/bridges/common.h b/src/plugin/bridges/common.h index 155fd1c9..818f229a 100644 --- a/src/plugin/bridges/common.h +++ b/src/plugin/bridges/common.h @@ -51,14 +51,13 @@ class PluginBridge { * Using a lambda here feels wrong, but I can't think of a better * solution right now. * - * @tparam F A `TSockets(boost::asio::io_context&, const PluginInfo&)` - * function to create the `TSockets` instance. - * * @throw std::runtime_error Thrown when the Wine plugin host could not be * found, or if it could not locate and load a VST3 module. */ - template - PluginBridge(PluginType plugin_type, F create_socket_instance) + template F> + PluginBridge(PluginType plugin_type, F&& create_socket_instance) // This is still correct for VST3 plugins because we can configure an // entire directory (the module's bundle) at once : config(load_config_for(get_this_file_location())), diff --git a/src/plugin/bridges/vst3-impls/plug-view-proxy.h b/src/plugin/bridges/vst3-impls/plug-view-proxy.h index 6bd944a7..9d77d1b2 100644 --- a/src/plugin/bridges/vst3-impls/plug-view-proxy.h +++ b/src/plugin/bridges/vst3-impls/plug-view-proxy.h @@ -136,12 +136,11 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { * right now. * * @see Vst3HostBridge::send_mutually_recursive_message - * - * TODO: As mentioned elsewhere, refactor these functions to use - * `std::invoke_result_t` */ - template - T run_gui_task(F fn) { + template + std::invoke_result_t run_gui_task(F&& fn) { + using Result = std::invoke_result_t; + // If `Vst3Bridge::send_mutually_recursive_message()` is currently being // called (because the host is calling one of `IPlugView`'s methods from // its UGI thread), then we'll call `fn` from that same thread. @@ -154,8 +153,8 @@ class Vst3PlugViewProxyImpl : public Vst3PlugViewProxy { } if (run_loop_tasks) { - std::packaged_task do_call(std::move(fn)); - std::future do_call_response = do_call.get_future(); + std::packaged_task do_call(std::forward(fn)); + std::future do_call_response = do_call.get_future(); run_loop_tasks->schedule(std::move(do_call)); diff --git a/src/plugin/bridges/vst3.cpp b/src/plugin/bridges/vst3.cpp index 07ebae90..5b9f8370 100644 --- a/src/plugin/bridges/vst3.cpp +++ b/src/plugin/bridges/vst3.cpp @@ -219,7 +219,7 @@ Vst3PluginBridge::Vst3PluginBridge() // loop or else it will likely segfault at some point return plugin_proxies.at(request.owner_instance_id) .get() - .last_created_plug_view->run_gui_task([&]() { + .last_created_plug_view->run_gui_task([&]() -> tresult { return plugin_proxies.at(request.owner_instance_id) .get() .context_menus.at(request.context_menu_id) @@ -285,7 +285,7 @@ Vst3PluginBridge::Vst3PluginBridge() // REAPER requires this to be run from its provided event // loop or else it will likely segfault at some point - return plug_view->run_gui_task([&]() { + return plug_view->run_gui_task([&]() -> tresult { return plug_view->plug_frame->resizeView( plug_view, &request.new_size); }); diff --git a/src/plugin/bridges/vst3.h b/src/plugin/bridges/vst3.h index 11166089..ba52d7fc 100644 --- a/src/plugin/bridges/vst3.h +++ b/src/plugin/bridges/vst3.h @@ -165,7 +165,7 @@ class Vst3PluginBridge : PluginBridge> { * * @see Vst3PlugViewProxyImpl::run_gui_task */ - template + template std::optional> maybe_run_on_mutual_recursion_thread( F&& fn) { return mutual_recursion.maybe_handle(std::forward(fn)); diff --git a/src/plugin/utils.h b/src/plugin/utils.h index 4545ace5..05f68c2d 100644 --- a/src/plugin/utils.h +++ b/src/plugin/utils.h @@ -22,6 +22,7 @@ #include "../common/configuration.h" #include "../common/plugins.h" +#include "../common/utils.h" /** * Marker struct for when we use the default Wine prefix. @@ -275,11 +276,12 @@ Configuration load_config_for(const boost::filesystem::path& yabridge_path); * @return The path to the *file* found, or `std::nullopt` if the file could not * be found. */ -template +template F = + bool(const boost::filesystem::path&)> std::optional find_dominating_file( const std::string& filename, boost::filesystem::path starting_dir, - F predicate = boost::filesystem::exists) { + F&& predicate = boost::filesystem::exists) { while (starting_dir != "") { const boost::filesystem::path candidate = starting_dir / filename; if (predicate(candidate)) { diff --git a/src/wine-host/bridges/vst3.cpp b/src/wine-host/bridges/vst3.cpp index 6c278915..ecb44b69 100644 --- a/src/wine-host/bridges/vst3.cpp +++ b/src/wine-host/bridges/vst3.cpp @@ -226,7 +226,7 @@ void Vst3Bridge::run() { // as well do the same thing with `setState()`. See below. // NOTE: We also try to handle mutual recursion here, in case // this happens during a resize - return do_mutual_recursion_on_gui_thread([&]() { + return do_mutual_recursion_on_gui_thread([&]() -> tresult { // This same function is defined in both `IComponent` and // `IEditController`, so the host is calling one or the // other @@ -246,7 +246,7 @@ void Vst3Bridge::run() { // NOTE: This also requires mutual recursion because REAPER will // call `getState()` while opening a popup menu const tresult result = - do_mutual_recursion_on_gui_thread([&]() { + do_mutual_recursion_on_gui_thread([&]() -> tresult { // This same function is defined in both `IComponent` // and `IEditController`, so the host is calling one or // the other @@ -350,7 +350,7 @@ void Vst3Bridge::run() { // much slower in Ardour, but there's no other non-hacky // solution for this (and bypassing Ardour's connection // proxies sort of goes against the idea behind yabridge) - return do_mutual_recursion_on_gui_thread([&]() { + return do_mutual_recursion_on_gui_thread([&]() -> tresult { return object_instances[request.instance_id] .connection_point->notify( request.message_ptr.get_original()); @@ -435,7 +435,7 @@ void Vst3Bridge::run() { // mutually recursive because the host will immediately // relay the parameter change the plugin has just // announced. - return do_mutual_recursion_on_off_thread([&]() { + return do_mutual_recursion_on_off_thread([&]() -> tresult { return object_instances[request.instance_id] .edit_controller->setParamNormalized(request.id, request.value); @@ -782,7 +782,7 @@ void Vst3Bridge::run() { // not run from the GUI thread Steinberg::ViewRect size{}; const tresult result = - do_mutual_recursion_on_gui_thread([&]() { + do_mutual_recursion_on_gui_thread([&]() -> tresult { return object_instances[request.owner_instance_id] .plug_view_instance->plug_view->getSize(&size); }); @@ -800,7 +800,7 @@ void Vst3Bridge::run() { // code on the same thread that's currently waiting for a // response to the message it sent. See the docstring of // this function for more information on how this works. - return do_mutual_recursion_on_gui_thread([&]() { + return do_mutual_recursion_on_gui_thread([&]() -> tresult { return object_instances[request.owner_instance_id] .plug_view_instance->plug_view->onSize( &request.new_size); @@ -847,7 +847,7 @@ void Vst3Bridge::run() { -> YaPlugView::CanResize::Response { // To prevent weird behaviour we'll perform all size related // functions from the GUI thread, including this one - return do_mutual_recursion_on_gui_thread([&]() { + return do_mutual_recursion_on_gui_thread([&]() -> tresult { return object_instances[request.owner_instance_id] .plug_view_instance->plug_view->canResize(); }); @@ -855,7 +855,7 @@ void Vst3Bridge::run() { [&](YaPlugView::CheckSizeConstraint& request) -> YaPlugView::CheckSizeConstraint::Response { const tresult result = - do_mutual_recursion_on_gui_thread([&]() { + do_mutual_recursion_on_gui_thread([&]() -> tresult { return object_instances[request.owner_instance_id] .plug_view_instance->plug_view->checkSizeConstraint( &request.rect); @@ -1032,7 +1032,7 @@ void Vst3Bridge::run() { // plugins (like TEOTE) require this to be called from the // same thread when that happens. const tresult result = - do_mutual_recursion_on_off_thread([&]() { + do_mutual_recursion_on_off_thread([&]() -> tresult { return object_instances[request.instance_id] .unit_info->getProgramName( request.list_id, request.program_index, name); @@ -1325,8 +1325,8 @@ size_t Vst3Bridge::register_object_instance( // handled from the same thread to prevent // deadlocks caused by mutually recursive function // calls. - return do_mutual_recursion_on_off_thread( - [&]() { + return do_mutual_recursion_on_off_thread( + [&]() -> tresult { return object_instances[request.instance_id] .component->setActive(request.state); }); diff --git a/src/wine-host/bridges/vst3.h b/src/wine-host/bridges/vst3.h index 852359c2..03e793d6 100644 --- a/src/wine-host/bridges/vst3.h +++ b/src/wine-host/bridges/vst3.h @@ -294,12 +294,9 @@ class Vst3Bridge : public HostBridge { * run on the UI thread through `main_context` as usual. * * @see Vst3Bridge::send_mutually_recursive_message - * - * TODO: Refactor these two functions, `run_gui_task()`, and - * `main_context.run_in_context` to use `std::invocation_result_t` */ - template - T do_mutual_recursion_on_gui_thread(F&& fn) { + template + std::invoke_result_t do_mutual_recursion_on_gui_thread(F&& fn) { // If the above function is currently being called from some thread, // then we'll call `fn` from that same thread. Otherwise we'll just // submit it to the main IO context. @@ -317,8 +314,8 @@ class Vst3Bridge : public HostBridge { * * @see Vst3Bridge::do_mutual_recursion_on_gui_thread */ - template - T do_mutual_recursion_on_off_thread(F&& fn) { + template + std::invoke_result_t do_mutual_recursion_on_off_thread(F&& fn) { return mutual_recursion.handle(std::forward(fn)); }