Real control surface plugin instead of hidden control surface

[Schroedingers-Cat]

The docs from pub fn plugin_register_add_csurf_inst state that this command registers a hidden control surface. I guess that is the reason why the registered control surface isn't being shown in REAPER's control surface preferences.

Does reaper-rs support registering proper control surfaces or creating proper control surface plugins? If that's not possible ATM, what missing functionality of reaper-rs needs to be implemented in order to support proper REAPER control surface plugins?

[helgoboss]

Yes, it's hidden. I use dogfooding to develop reaper-rs, so I add only the functions that I need for my own projects. Didn't need control surfaces yet that show up in the control surface list.

I think if you want it to show up, you need to use "csurf" instead of "csurf_inst" when registering the surface.

[Schroedingers-Cat]

I think if you want it to show up, you need to use "csurf" instead of "csurf_inst" when registering the surface.

Are you referring to the following section of reaper-rs?

reaper-rs/main/medium/src/misc_enums.rs

Lines 791 to 794 in 781adc7

	CsurfInst(inst) => PluginRegistration {
	key: reaper_str!("csurf_inst").into(),
	value: inst.as_ptr() as _,
	},

Where the string csurf should be used as key instead of csurf_inst?

[Schroedingers-Cat]

Made a first attempt at this, which actually does create an entry in REAPER's control surface plugin list: Schroedingers-Cat@ccb57fe

The descriptive name of the entry however is a garbage string, likely caused by uninitialized memory. My guess is that https://github.com/Schroedingers-Cat/reaper-rs/blob/master/main/medium/src/misc_enums.rs#L686 uses https://github.com/Schroedingers-Cat/reaper-rs/blob/ccb57fee694ea451246e00f58b3cf6329b2d0aef/main/low/src/bindings.rs#L1296-L1307 which does not represent the reaper_csurf_reg_t struct from C++ reaper_plugin.h, so that would explain the uninitialized memory. Since the bindings.rs file is generated (/* automatically generated by rust-bindgen */), I'm surprised to not find anything that could resemble the reaper_csurf_reg_t struct.

Any idea where I could find it or how I could pass the necessary data struct for properly registering a real control surface?

[helgoboss]

You need to add reaper_csurf_reg_t to bindgen in reaper-low's build.rs (via whitelist_type() method) and then regenerate via cargo build --features generate followed by cargo fmt. I chose to only generate what's actually used.

[Schroedingers-Cat]

I tried that but whenever I run cargo build --features generate or cargo build --features generate -stage-one, cargo build --features generate-stage-two or cargo build --all-features directly from reaper-rsor reaper-rs/main/low, the only change I see in git is something formatting related in medium/src/misc_enums.rs. I also tried removing things from the build.rs whitelist but nothing seems to change the resulting binding.rs.

[helgoboss]

Keep in mind you need to do it on Linux or macOS (eventually has to be Linux though, that's my convention). I think I disabled it for Windows.

[helgoboss]

If you are on Windows, the easiest thing it to use WSL.

[Schroedingers-Cat]

Oh thanks, I didn't know that. No problem, I have all operating systems, will try it on another one.

[Schroedingers-Cat]

Generating on Linux worked just fine!

I also added an alternative constructor to CppToRustControlSurface that takes reaper_csurf_reg_t as second parameter (with alternative functions up to reaper_session.rs in medium that send the csurf registration struct down to cpp).

At the moment, an instance of reaper_csurf_reg_t is constructed within the csurf (without inst) alternative function plugin_register_add_csurf in medium reaper_session.rs, but I wonder if this is the best place to do that? I'm especially wondering how to assign the function pointers for create and ShowConfig since they're declared in C++ land, but I'm sure it would be nice to be able to write these functions in Rust, too. Do you have a suggestion on how this should be approached?

[helgoboss]

Generating on Linux worked just fine!

Great!

I also added an alternative constructor to CppToRustControlSurface that takes reaper_csurf_reg_t as second parameter (with alternative functions up to reaper_session.rs in medium that send the csurf registration struct down to cpp).

Is it necessary to do that? Usually, I only go to C++ land for two reasons: Either vtables or variadics. reaper_csurf_reg_t is just a simple struct which doesn't have any of that, so it could exist purely in Rust land.

At the moment, an instance of reaper_csurf_reg_t is constructed within the csurf (without inst) alternative function plugin_register_add_csurf in medium reaper_session.rs, but I wonder if this is the best place to do that? I'm especially wondering how to assign the function pointers for create and ShowConfig since they're declared in C++ land, but I'm sure it would be nice to be able to write these functions in Rust, too. Do you have a suggestion on how this should be approached?

The generated bindings should contain extern "C" function pointer fields for create and ShowConfig (wrapped in an Option because pointers are always nullable). These should be assignable in Rust. You can write the function in Rust (prefixed with extern "C") and assign it wrapped in a Some().

I think the best would be to do this thing in two steps: First, getting the low-level API right (although I think this only involves adding the generated reaper_csurf_reg_t, which you have done already). Second, building the more convenient medium-level API on top of that. This is the harder part because of my conventions ;) You need to add a wrapper for the raw reaper_csurf_reg_t struct which lifts it to medium-level "quality". E.g. translate all the const char* stuff to a &ReaperStr and translating errStats to a enumflags value. Let me know if you need help. Alternatively, you can also just show me the working example on your repo and I try to translate it to my conventions.

[Schroedingers-Cat]

Is it necessary to do that?

You're right. I had wrong assumptions about this that got cleared up by reading some texts about generating bindings with Rust.

I think the best would be to do this thing in two steps: First, getting the low-level API right [...]. Second, building the more convenient medium-level API on top of that.

I'm struggling with something in between. The bindings are generated (Schroedingers-Cat@4c59b20) and exposed (Schroedingers-Cat@36ecad5). Before trying to add the medium abstraction, I wanted to get a very simple but hacky approach working first, so I added an instance of the csurf_reg_t in a copy of plugin_register_add_csurf (Schroedingers-Cat@ee65931). However, it turns out to be tricky to get the extern "c" fn call from C++ for creating the IReaperControlSurface instance in Rust to reference self (which is necessary for creating the IReaperControlSurface in Rust). I sketched something out for that (Schroedingers-Cat@b60e8c5) but it doesn't even compile.
I'm wondering how to approach this as the C++ side needs to get a pointer to a function on the Rust side but that function has to use self to create the IReaperControlSurface while it isn't allowed to use self. The only idea I got was to create a data structure, probably a hash map, that uses the value of the type_string pointer parameter to identify the control surface instance that REAPER is handling with a specific call. Maybe there'd be a more idiomatic way?

[helgoboss]

I think reaper_csurf_reg_t.create is not designed to take self, so there's no way to magically access self without building some kind of abstraction on top of that (e.g. using a static hash map). The medium-level API shouldn't contain such abstractions because they are too opinionated. Therefore I suggest to stay very close to the original API.

Just for comparison, there's a struct in reaper_plugin.h called audio_hook_register_t which is designed in a similar way. However, this one explicitly allows to pass arbitrary user data (*userdata1 and *userdata2) to the callback OnAudioBuffer, which I use to pass a pointer to self. But reaper_csurf_reg_t just doesn't provide this possibility, it can only operate in a static context.

[Schroedingers-Cat]

The medium-level API shouldn't contain such abstractions because they are too opinionated.

So the static hash map should happen in reaper-rs high, then.

Therefore I suggest to stay very close to the original API.

Sounds like the non-inst version of pub fn plugin_register_add_csurf_inst<T> should just take the reaper_csurf_reg_t as a parameter and developers using reaper-rs medium should provide the implementation themselves.
However, there's still the problem that with a non-inst csurf, Reaper is calling the create function itself and wants the resulting IReaperControlSurface from that function while the plugin_register_addfunctions have to return a RegistrationHandle to the IReaperControlSurface before Reaper will actually call the create function. Both requirements seem contradictory to me and the only workaround I can think of (creating before Reaper calls it and simply return the already created IReaperControlSurface from a hash map when Reaper calls the create function) should be in reaper-rs high.

I'm also struggling with the creation of the extern "C" fn that returns the IReaperControlSurface. Rust either wants me to return dyn IReaperControlSurface because trait objects must include dyn or complains about the return type's unknown size at compile time when I do include the dyn keyword. I'm unsure how to meet both requirements.

[Schroedingers-Cat]

Restarted this again on a new branch from the current main branch to get a cleaner history. The reaper_csurf_reg_t has been whitelisted, generated and the trait has been added to low. I also added the csurf_inst equivalent functionality to medium except for the non-inst variant of plugin_register_add_csurf_inst():
master...Schroedingers-Cat:reaper-rs:feature/add-real-csurf

Creating a non-inst variant of the plugin_register_add_csurf_inst() function is where I'm stuck as I'm not fully understanding what's happening in the original function. What I understand is that from the given box an adapter, a registration handle and more boxes are being created, inserted into a hashmap and finally the plugin is being registered with the handle being returned by the function.

I guess the non-inst variant of that function would have the same signature but should be restricted to where T: reaper_csurf_reg_t + 'static,?

I'm not sure how the ControlSurfaceAdapter would look like in that case.

Also, the double_boxed_low_cs and following parts that require it like the handler insertion into the hash map force the creation of a plugin instance at that moment while we're actually just about to register a control surface plugin. This seems to be in contrast with the design of reaper_csurf_reg_t which is build around defining the construction code that's being handed to REAPER without any control over when REAPER is going to call that construction code. How should this be handled in medium?

[helgoboss]

@Schroedingers-Cat So sorry, I can't handle larger reaper-rs PRs/questions at the moment. Too much other stuff going on. This topic is unfortunately a bit more time-consuming. I would have to look into it in very much detail but at the same time I don't need that functionality right now so it would be a big deviation from my Playtime/ReaLearn plans.