Switch to vst3 crate #29

davemollen commented 2026年05月27日 15:37:07 +02:00

I have moved this PR from the nih-plug repository to here. I have gone through Micah's comments over there.

I think a thorough review of these changes is still needed. A big chunk of this is from Micah's draft PR. All I did was try and get that to the finish line.

Some additional comments:

• VirtualKeyCode parsing was recently added with this PR: #9. The vst3 crate also defines VirtualKeyCodes_ and KeyModifier_, and these are automatically generated. So I think it would be cleaner to use these bindings instead. But I left this out because changing this would probably be a breaking change.
• The RunLoopEventHandler struct contained this comment:

/// Allow handling tasks on the host's GUI thread on Linux. This doesn't need to be a separate
/// struct, but vst3-sys does not let us implement interfaces conditionally and the interface is
/// only exposed when compiling on Linux. The struct will register itself when calling
/// [`RunLoopEventHandler::new()`] and it will unregister itself when it gets dropped.

According to this comment this struct could be redundant because we replaced the vst3-sys crate. I kept it in because I think it's actually nice to have this scoped to a separate struct. Since it's a linux only thing, and only then deals with the IEventHandler. But this comment might still be something to consider.

I have moved this PR from the nih-plug repository to here. I have gone through [Micah's comments over there](https://github.com/robbert-vdh/nih-plug/pull/263).
 I think a thorough review of these changes is still needed. A big chunk of this is from [Micah's draft PR](https://github.com/robbert-vdh/nih-plug/pull/243). All I did was try and get that to the finish line. Some additional comments: * VirtualKeyCode parsing was recently added with this PR: #9. The vst3 crate also defines `VirtualKeyCodes_` and `KeyModifier_`, and these are automatically generated. So I think it would be cleaner to use these bindings instead. But I left this out because changing this would probably be a breaking change. * The RunLoopEventHandler struct contained this comment: ``` /// Allow handling tasks on the host's GUI thread on Linux. This doesn't need to be a separate /// struct, but vst3-sys does not let us implement interfaces conditionally and the interface is /// only exposed when compiling on Linux. The struct will register itself when calling /// [`RunLoopEventHandler::new()`] and it will unregister itself when it gets dropped. ``` According to this comment this struct could be redundant because we replaced the vst3-sys crate. I kept it in because I think it's actually nice to have this scoped to a separate struct. Since it's a linux only thing, and only then deals with the IEventHandler. But this comment might still be something to consider.

BillyDM commented 2026年05月27日 19:22:29 +02:00

So I think it would be cleaner to use these bindings instead. But I left this out because changing this would probably be a breaking change.

Actually it's necessary to have our own definitions of VirtualKeyCodes and KeyModifier because nice-plug needs to still be able to compile without the "vst3" feature enabled.

> So I think it would be cleaner to use these bindings instead. But I left this out because changing this would probably be a breaking change. Actually it's necessary to have our own definitions of `VirtualKeyCodes` and `KeyModifier` because nice-plug needs to still be able to compile without the "vst3" feature enabled.

davemollen commented 2026年05月27日 20:10:55 +02:00

True, I was thinking we could expose/export the vst3 VirtualKeyCodes and KeyModifier types only when the "vst3" feature is enabled. But thinking about it some more the vst3 crate types are not really suited for this use case anyway. So it's fine to have some "duplication" then

True, I was thinking we could expose/export the vst3 `VirtualKeyCodes` and `KeyModifier` types only when the "vst3" feature is enabled. But thinking about it some more the vst3 crate types are not really suited for this use case anyway. So it's fine to have some "duplication" then

Sekwah commented 2026年05月27日 21:43:21 +02:00

Works fine for me on Mac, tested most of the example plugins and couldn't find any issues.

Works fine for me on Mac, tested most of the example plugins and couldn't find any issues.

BillyDM commented 2026年05月30日 02:33:01 +02:00

Alright, I got Windows 11 dual boot set up and testing the plugins. They all worked correctly in the DAWs I tested! (FL, Reaper, Bitwig).

I also ran the plugins through pluginval on both Linux and Mac, and they all pass.

Could one of you please run the plugins through pluginval on MacOS?

https://github.com/Tracktion/pluginval/blob/develop/docs/Testing%20plugins%20with%20pluginval.md

Alright, I got Windows 11 dual boot set up and testing the plugins. They all worked correctly in the DAWs I tested! (FL, Reaper, Bitwig). I also ran the plugins through pluginval on both Linux and Mac, and they all pass. Could one of you please run the plugins through pluginval on MacOS? https://github.com/Tracktion/pluginval/blob/develop/docs/Testing%20plugins%20with%20pluginval.md

BillyDM commented 2026年05月30日 02:50:40 +02:00

Running cargo clippy produces quite a few warnings. Could you fix those please?

Running `cargo clippy` produces quite a few warnings. Could you fix those please?

BillyDM commented 2026年05月30日 03:00:26 +02:00

Though I imagine that the "unecessary cast" warnings from clippy might actually be platform-specific (as in the integer types might be different on different platforms). So you could slap #[allow(clippy::unnecessary_cast)] on those if that's the case.

Though I imagine that the "unecessary cast" warnings from clippy might actually be platform-specific (as in the integer types might be different on different platforms). So you could slap `#[allow(clippy::unnecessary_cast)]` on those if that's the case.

davemollen commented 2026年05月30日 17:31:52 +02:00

Clippy warnings have been fixed. Some casts were actually unnecessary. And in some cases it's like you said; the integer types are sometimes different on different platforms.

Clippy warnings have been fixed. Some casts were actually unnecessary. And in some cases it's like you said; [the integer types are sometimes different on different platforms](https://github.com/coupler-rs/vst3-rs/blob/master/src/support.rs#L208-L212).

davemollen commented 2026年05月30日 17:53:16 +02:00

I ran pluginval on macOS for all the plugins in this repository. There is no regression.

But there are some issues that are also present on the main branch. I will list them briefly here. I can open some separate issues and add more detailed descriptions there. The issues are:

• byo_gui_gl crashes immediately on load
• byo_gui_softbuffer doesn't look right compared to byo_gui_wgpu. But or is it supposed to look like this? See the attached screenshots.

I ran pluginval on macOS for all the plugins in this repository. There is no regression. But there are some issues that are also present on the main branch. I will list them briefly here. I can open some separate issues and add more detailed descriptions there. The issues are: - byo_gui_gl crashes immediately on load - byo_gui_softbuffer doesn't look right compared to byo_gui_wgpu. But or is it supposed to look like this? See the attached screenshots.

BillyDM commented 2026年05月30日 20:14:19 +02:00

Yes, the softbuffer example is supposed to look like that (I pulled the example rainbow gradient from the "hello world" example in the softbuffer repository.) I should probably add some screenshots of those examples so people know what it's supposed to look like.

What happens when you run byo_gui_gl in standalone mode? (cargo run --package byo_gui_gl)

Yes, the softbuffer example is supposed to look like that (I pulled the example rainbow gradient from the "hello world" example in the softbuffer repository.) I should probably add some screenshots of those examples so people know what it's supposed to look like. What happens when you run `byo_gui_gl` in standalone mode? (`cargo run --package byo_gui_gl`)

BillyDM commented 2026年05月30日 20:31:25 +02:00

But yeah, byo_gui_gl crashing could be a separate issue.

I'm happy to merge this PR if everyone else is!

But yeah, byo_gui_gl crashing could be a separate issue. I'm happy to merge this PR if everyone else is!

davemollen commented 2026年05月31日 14:43:04 +02:00

@BillyDM wrote in #29 (comment):

Yes, the softbuffer example is supposed to look like that (I pulled the example rainbow gradient from the "hello world" example in the softbuffer repository.) I should probably add some screenshots of those examples so people know what it's supposed to look like.

What happens when you run byo_gui_gl in standalone mode? (cargo run --package byo_gui_gl)

Alright, good to know the softbuffer example looks like it's supposed to. I assumed it would be the same UI as the wpgu example. Never assume...

Running cargo run --package byo_gui_gl gives the same error. I opened this issue for it.

@BillyDM wrote in https://codeberg.org/RustAudio/nice-plug/pulls/29#issuecomment-16201091: > Yes, the softbuffer example is supposed to look like that (I pulled the example rainbow gradient from the "hello world" example in the softbuffer repository.) I should probably add some screenshots of those examples so people know what it's supposed to look like. > > What happens when you run `byo_gui_gl` in standalone mode? (`cargo run --package byo_gui_gl`) Alright, good to know the softbuffer example looks like it's supposed to. I assumed it would be the same UI as the wpgu example. Never assume... Running `cargo run --package byo_gui_gl` gives the same error. I opened [this issue](https://codeberg.org/RustAudio/nice-plug/issues/32) for it.

BillyDM commented 2026年06月02日 21:06:22 +02:00

Ok, I'm thinking it's good to merge now. Thanks for all your help!

Ok, I'm thinking it's good to merge now. Thanks for all your help!