bitfieldaudio / OTTO

Sampler, Sequencer, Multi-engine synth and effects - in a box! [WIP]
https://bitfieldaudio.com
Other
2.63k stars 144 forks source link

Initial port of Chorus FX #139

Closed jpjamipark closed 4 years ago

jpjamipark commented 4 years ago

Running into some bugs with the encoder values disappearing immediately + not updating properly until notes are pressed. Unsure if this is b/c of the port. Just putting this initial PR up for feedback since I'm new.

jmidt commented 4 years ago

I will take a closer look later, but I think you need to reset the envelopes in the actionhandlers in the screen. With a env.resetSoft()

jmidt commented 4 years ago

Yes, that sounds fine for now!

man. 2. dec. 2019 02.35 skrev jpjamipark notifications@github.com:

@jpjamipark commented on this pull request.

In src/engines/fx/chorus/screen.cpp https://github.com/OTTO-project/OTTO/pull/139#discussion_r352392602:

  • ctx.fillText(fmt::format("{}", std::round(100 * delay_)), x_pad, y_pad + number_shift);
  • //Heads
  • constexpr float spacing_constant = 10;
  • constexpr int num_heads = 10;
  • float spacing = spacingconstant * delay + 10;
  • Point start = {120 - delay_ * 50, 165};
  • OTTOASSERT(phase != nullptr);
  • for (int i=num_heads; i>=1; i--) {
  • float head_height = waveheight gam::scl::sinP9(gam::scl::wrap(phase - 0.2f*(float)i, 1.f, -1.f));
  • //This brightness calculation is HORRIBLY over-engineered...
  • float brightness = powf(cosh(abs(feedback_)1.5)0.4, 0.3 * float(i));

It looks like brightness & head_height also rely on the i value based on the head - so should I create an array of size num_heads and store brightness/head_height for each head ?

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/OTTO-project/OTTO/pull/139?email_source=notifications&email_token=AHY3NVQQCLZAONEUWPCOUBTQWRQ7XA5CNFSM4JTFHUB2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCNPNXYQ#discussion_r352392602, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHY3NVQ6OTL6Q5KNCMJQS23QWRQ7XANCNFSM4JTFHUBQ .

jmidt commented 4 years ago

Merging this. I will finish this now.