search

ch-nry / VCV_nozori

port of nozori eurorack module for VCV Rack
GNU General Public License v3.0
15 stars 2 forks source link

Fix sample rate warning in Mac OSX Catalina #12

Closed joshdavenport closed 3 years ago

joshdavenport commented 3 years ago

Fixes https://github.com/ch-nry/VCV_nozori/issues/11 and hopefully allows this to work everywhere too.

ch-nry commented 3 years ago

wow, thanks Josh. This look very similar to version 1.1.4. Is the main difference : lights[TEXT_LIGHT_48].setBrightness(0.f); instead of : lights[TEXT_LIGHT_48].setBrightness(0.); ?

joshdavenport commented 3 years ago

Happy to help :) Can't wait to tinker with these modules now!

I think they're equivalient (0. and 0.f) in this case actually. I'm just used to writing with the .f as I believe the VCV plugin docs do that in their example code, and that's my only real experience with VCV modules and C++ generally to be completely honest! I don't think that is the main fix here.

As my C++ is really poor, I'm not exactly sure where things were breaking before. I essentially took apart the bits that were trying to make this work and put it together again and found a solution that worked. My strongest guess though is that it had something to do with how VCV sets color.a and how the modules here were controlling brightness to show/hide the warning.

No idea why that would be different across platforms either! Sorry, I know that's not too helpful.

ch-nry commented 3 years ago

hum, so I'll never know what was not working in my initial code! I made a new version, the VCV team have to review and test it. The update should be automatically available in few days. thanks again.

joshdavenport commented 3 years ago

My pleasure @ch-nry, thank you for contributing to the VCV community, there's going to be many people out there excited to try out your modules I'm sure.

As an experiment I took the codebase at its 1.1.4 state and looked for the smallest possible change to get it working in that state. This commit demonstrates that, ignoring plugin.json which is changed for quicker make install: https://github.com/joshdavenport/VCV_nozori/commit/c701225d4323192a5e45339212ebea30eaa18fb5.

With the ADSR module in that state, I see no warning @ 96kHz and the warning only for a few seconds @ 88.2kHz as intended.

I think the version that I initially got when first acquiring these modules had the changes you made with the 48/96 needed variables. My instinct here is perhaps there were some logic issues with those too that were muddying the waters further, not exactly sure about that.

Interesting in any case!

ch-nry commented 3 years ago

hum, I'm afraid I understand. The 1st issue was open the 9th may. module version was 1.1.5, but the logic did not really change since 1.1.0 Few days ago, I was working to make a 1.1.6 version with the warning less visible because I was not able to fix this issue. I also add this variables aiming at optimized things (not changing the light at every DSP tick) Just before asking for VCV team for a review, I received your email about your fix. So I made the version 1.1.7 and version 1.1.6 was never released. Version 1.1.5 is the version in the VCV rack library. The one you where using when you had this issue.

So, what your are saying is that things works when you compile yourself, but the same version did not work when it's compiled by the VCV team.

Let's wait few days so you can get the 1.1.7 from VCV. I bet it will not work for you. If so, I'll fill a bug report on VCV...