Piano: Use LibDSP::Keyboard for all keyboard-playing logic

The only major functional change is that the Track now needs to know
whether it's active or not, in order to listen to the keyboard (or not).

There are some bugs exposed/created by this, mainly:
* KeysWidget sometimes shows phantom notes. Those do not actually exist
  as far as debugging has revealed and do not play in the synth.
* The keyboard can lock up Piano when rapidly pressing keys. This
  appears to be a HashMap bug; I invested significant time in bugfixing
  but got nowhere.

Userland/Applications/Piano/KeysWidget.cpp

@@ -7,53 +7,32 @@
  */
 
 #include "KeysWidget.h"
+#include "LibDSP/Keyboard.h"
 #include "TrackManager.h"
 #include <AK/Array.h>
 #include <AK/StringView.h>
 #include <LibGUI/Painter.h>
 
-KeysWidget::KeysWidget(TrackManager& track_manager)
+KeysWidget::KeysWidget(NonnullRefPtr<LibDSP::Keyboard> keyboard)
-    : m_track_manager(track_manager)
+    : m_keyboard(move(keyboard))
 {
     set_fill_with_background_color(true);
 }
 
 int KeysWidget::mouse_note() const
 {
-    if (m_mouse_down && m_mouse_note + m_track_manager.octave_base() < note_count)
+    if (m_mouse_down && m_mouse_note + m_keyboard->virtual_keyboard_octave_base() < note_count)
         return m_mouse_note; // Can be -1.
     else
         return -1;
 }
 
-void KeysWidget::set_key(int key, Switch switch_key)
+void KeysWidget::set_key(i8 key, LibDSP::Keyboard::Switch switch_note)
 {
-    if (key == -1 || key + m_track_manager.octave_base() >= note_count)
+    m_keyboard->set_keyboard_note_in_active_octave(key, switch_note);
-        return;
-
-    if (switch_key == On) {
-        ++m_key_on[key];
-    } else {
-        if (m_key_on[key] >= 1)
-            --m_key_on[key];
-    }
-    VERIFY(m_key_on[key] <= 2);
-
-    m_track_manager.set_keyboard_note(key + m_track_manager.octave_base(), switch_key);
 }
 
-bool KeysWidget::note_is_set(int note) const
+i8 KeysWidget::key_code_to_key(int key_code)
-{
-    if (note < m_track_manager.octave_base())
-        return false;
-
-    if (note >= m_track_manager.octave_base() + note_count)
-        return false;
-
-    return m_key_on[note - m_track_manager.octave_base()] != 0;
-}
-
-int KeysWidget::key_code_to_key(int key_code) const
 {
     switch (key_code) {
     case Key_A:
@@ -170,7 +149,7 @@ void KeysWidget::paint_event(GUI::PaintEvent& event)
     int i = 0;
     for (;;) {
         Gfx::IntRect rect(x, 0, white_key_width, frame_inner_rect().height());
-        painter.fill_rect(rect, m_key_on[note] ? note_pressed_color : Color::White);
+        painter.fill_rect(rect, m_keyboard->is_pressed_in_active_octave(note) ? note_pressed_color : Color::White);
         painter.draw_rect(rect, Color::Black);
         if (i < white_key_labels_count) {
             rect.set_height(rect.height() * 1.5);
@@ -181,7 +160,7 @@ void KeysWidget::paint_event(GUI::PaintEvent& event)
         x += white_key_width;
         ++i;
 
-        if (note + m_track_manager.octave_base() >= note_count)
+        if (note + m_keyboard->virtual_keyboard_octave_base() >= note_count)
             break;
         if (x >= frame_inner_rect().width())
             break;
@@ -192,7 +171,7 @@ void KeysWidget::paint_event(GUI::PaintEvent& event)
     i = 0;
     for (;;) {
         Gfx::IntRect rect(x, 0, black_key_width, black_key_height);
-        painter.fill_rect(rect, m_key_on[note] ? note_pressed_color : Color::Black);
+        painter.fill_rect(rect, m_keyboard->is_pressed_in_active_octave(note) ? note_pressed_color : Color::Black);
         painter.draw_rect(rect, Color::Black);
         if (i < black_key_labels_count) {
             rect.set_height(rect.height() * 1.5);
@@ -203,7 +182,7 @@ void KeysWidget::paint_event(GUI::PaintEvent& event)
         x += black_key_offsets[i % black_keys_per_octave];
         ++i;
 
-        if (note + m_track_manager.octave_base() >= note_count)
+        if (note + m_keyboard->virtual_keyboard_octave_base() >= note_count)
             break;
         if (x >= frame_inner_rect().width())
             break;
@@ -274,7 +253,7 @@ void KeysWidget::mousedown_event(GUI::MouseEvent& event)
 
     m_mouse_note = note_for_event_position(event.position());
 
-    set_key(m_mouse_note, On);
+    set_key(m_mouse_note, LibDSP::Keyboard::Switch::On);
     update();
 }
 
@@ -285,7 +264,7 @@ void KeysWidget::mouseup_event(GUI::MouseEvent& event)
 
     m_mouse_down = false;
 
-    set_key(m_mouse_note, Off);
+    set_key(m_mouse_note, LibDSP::Keyboard::Switch::Off);
     update();
 }
 
@@ -299,8 +278,8 @@ void KeysWidget::mousemove_event(GUI::MouseEvent& event)
     if (m_mouse_note == new_mouse_note)
         return;
 
-    set_key(m_mouse_note, Off);
+    set_key(m_mouse_note, LibDSP::Keyboard::Switch::Off);
-    set_key(new_mouse_note, On);
+    set_key(new_mouse_note, LibDSP::Keyboard::Switch::On);
     update();
 
     m_mouse_note = new_mouse_note;

Userland/Applications/Piano/KeysWidget.h

@@ -9,6 +9,8 @@
 #pragma once
 
 #include "Music.h"
+#include <AK/NonnullRefPtr.h>
+#include <LibDSP/Keyboard.h>
 #include <LibGUI/Frame.h>
 
 class TrackManager;
@@ -18,14 +20,11 @@ class KeysWidget final : public GUI::Frame {
 public:
     virtual ~KeysWidget() override = default;
 
-    int key_code_to_key(int key_code) const;
+    static i8 key_code_to_key(int key_code);
     int mouse_note() const;
 
-    void set_key(int key, Switch);
-    bool note_is_set(int note) const;
-
 private:
-    explicit KeysWidget(TrackManager&);
+    KeysWidget(NonnullRefPtr<LibDSP::Keyboard>);
 
     virtual void paint_event(GUI::PaintEvent&) override;
     virtual void mousedown_event(GUI::MouseEvent&) override;
@@ -34,9 +33,9 @@ private:
 
     int note_for_event_position(Gfx::IntPoint const&) const;
 
-    TrackManager& m_track_manager;
+    void set_key(i8 key, LibDSP::Keyboard::Switch);
 
-    u8 m_key_on[note_count] { 0 };
+    NonnullRefPtr<LibDSP::Keyboard> m_keyboard;
 
     bool m_mouse_down { false };
     int m_mouse_note { -1 };

Userland/Applications/Piano/KnobsWidget.cpp

@@ -35,7 +35,7 @@ KnobsWidget::KnobsWidget(TrackManager& track_manager, MainWidget& main_widget)
     m_values_container->set_fixed_height(10);
 
     m_volume_value = m_values_container->add<GUI::Label>(String::number(m_track_manager.current_track().volume()));
-    m_octave_value = m_values_container->add<GUI::Label>(String::number(m_track_manager.octave()));
+    m_octave_value = m_values_container->add<GUI::Label>(String::number(m_track_manager.keyboard()->virtual_keyboard_octave()));
 
     m_knobs_container = add<GUI::Widget>();
     m_knobs_container->set_layout<GUI::HorizontalBoxLayout>();
@@ -57,13 +57,13 @@ KnobsWidget::KnobsWidget(TrackManager& track_manager, MainWidget& main_widget)
     m_octave_knob = m_knobs_container->add<GUI::VerticalSlider>();
     m_octave_knob->set_tooltip("Z: octave down, X: octave up");
     m_octave_knob->set_range(octave_min - 1, octave_max - 1);
-    m_octave_knob->set_value((octave_max - 1) - (m_track_manager.octave() - 1));
+    m_octave_knob->set_value((octave_max - 1) - (m_track_manager.keyboard()->virtual_keyboard_octave() - 1));
     m_octave_knob->set_step(1);
     m_octave_knob->on_change = [this](int value) {
         int new_octave = octave_max - value;
         if (m_change_underlying)
             m_main_widget.set_octave_and_ensure_note_change(new_octave);
-        VERIFY(new_octave == m_track_manager.octave());
+        VERIFY(new_octave == m_track_manager.keyboard()->virtual_keyboard_octave());
         m_octave_value->set_text(String::number(new_octave));
     };
 
@@ -117,7 +117,7 @@ void KnobsWidget::update_knobs()
     m_change_underlying = false;
 
     m_volume_knob->set_value(volume_max - m_track_manager.current_track().volume());
-    m_octave_knob->set_value(octave_max - m_track_manager.octave());
+    m_octave_knob->set_value(octave_max - m_track_manager.keyboard()->virtual_keyboard_octave());
 
     m_change_underlying = true;
 }

Userland/Applications/Piano/MainWidget.cpp

@@ -46,7 +46,7 @@ MainWidget::MainWidget(TrackManager& track_manager, AudioPlayerLoop& loop)
     m_keys_and_knobs_container->set_fixed_height(130);
     m_keys_and_knobs_container->set_fill_with_background_color(true);
 
-    m_keys_widget = m_keys_and_knobs_container->add<KeysWidget>(track_manager);
+    m_keys_widget = m_keys_and_knobs_container->add<KeysWidget>(track_manager.keyboard());
 
     m_knobs_widget = m_keys_and_knobs_container->add<KnobsWidget>(track_manager, *this);
 
@@ -85,7 +85,7 @@ void MainWidget::keydown_event(GUI::KeyEvent& event)
     else
         m_keys_pressed[event.key()] = true;
 
-    note_key_action(event.key(), On);
+    note_key_action(event.key(), LibDSP::Keyboard::Switch::On);
     special_key_action(event.key());
     m_keys_widget->update();
 }
@@ -94,24 +94,26 @@ void MainWidget::keyup_event(GUI::KeyEvent& event)
 {
     m_keys_pressed[event.key()] = false;
 
-    note_key_action(event.key(), Off);
+    note_key_action(event.key(), LibDSP::Keyboard::Switch::Off);
     m_keys_widget->update();
 }
 
-void MainWidget::note_key_action(int key_code, Switch switch_note)
+void MainWidget::note_key_action(int key_code, LibDSP::Keyboard::Switch switch_note)
 {
-    int key = m_keys_widget->key_code_to_key(key_code);
+    auto key = m_keys_widget->key_code_to_key(key_code);
-    m_keys_widget->set_key(key, switch_note);
+    if (key == -1)
+        return;
+    m_track_manager.keyboard()->set_keyboard_note_in_active_octave(key, switch_note);
 }
 
 void MainWidget::special_key_action(int key_code)
 {
     switch (key_code) {
     case Key_Z:
-        set_octave_and_ensure_note_change(Down);
+        set_octave_and_ensure_note_change(LibDSP::Keyboard::Direction::Down);
         break;
     case Key_X:
-        set_octave_and_ensure_note_change(Up);
+        set_octave_and_ensure_note_change(LibDSP::Keyboard::Direction::Up);
         break;
     case Key_C:
         m_knobs_widget->cycle_waveform();
@@ -124,36 +126,38 @@ void MainWidget::special_key_action(int key_code)
 
 void MainWidget::turn_off_pressed_keys()
 {
-    m_keys_widget->set_key(m_keys_widget->mouse_note(), Off);
+    if (m_keys_widget->mouse_note() != -1)
+        m_track_manager.keyboard()->set_keyboard_note_in_active_octave(m_keys_widget->mouse_note(), LibDSP::Keyboard::Switch::Off);
     for (int i = 0; i < key_code_count; ++i) {
         if (m_keys_pressed[i])
-            note_key_action(i, Off);
+            note_key_action(i, LibDSP::Keyboard::Switch::Off);
     }
 }
 
 void MainWidget::turn_on_pressed_keys()
 {
-    m_keys_widget->set_key(m_keys_widget->mouse_note(), On);
+    if (m_keys_widget->mouse_note() != -1)
+        m_track_manager.keyboard()->set_keyboard_note_in_active_octave(m_keys_widget->mouse_note(), LibDSP::Keyboard::Switch::On);
     for (int i = 0; i < key_code_count; ++i) {
         if (m_keys_pressed[i])
-            note_key_action(i, On);
+            note_key_action(i, LibDSP::Keyboard::Switch::On);
     }
 }
 
 void MainWidget::set_octave_and_ensure_note_change(int octave)
 {
     turn_off_pressed_keys();
-    m_track_manager.set_octave(octave);
+    MUST(m_track_manager.keyboard()->set_virtual_keyboard_octave(octave));
     turn_on_pressed_keys();
 
     m_knobs_widget->update_knobs();
     m_keys_widget->update();
 }
 
-void MainWidget::set_octave_and_ensure_note_change(Direction direction)
+void MainWidget::set_octave_and_ensure_note_change(LibDSP::Keyboard::Direction direction)
 {
     turn_off_pressed_keys();
-    m_track_manager.set_octave(direction);
+    m_track_manager.keyboard()->change_virtual_keyboard_octave(direction);
     turn_on_pressed_keys();
 
     m_knobs_widget->update_knobs();

Userland/Applications/Piano/MainWidget.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include "Music.h"
+#include <LibDSP/Keyboard.h>
 #include <LibGUI/Widget.h>
 
 class AudioPlayerLoop;
@@ -28,7 +29,7 @@ public:
 
     void add_track_actions(GUI::Menu&);
 
-    void set_octave_and_ensure_note_change(Direction);
+    void set_octave_and_ensure_note_change(LibDSP::Keyboard::Direction);
     void set_octave_and_ensure_note_change(int);
 
 private:
@@ -38,7 +39,7 @@ private:
     virtual void keyup_event(GUI::KeyEvent&) override;
     virtual void custom_event(Core::CustomEvent&) override;
 
-    void note_key_action(int key_code, Switch);
+    void note_key_action(int key_code, LibDSP::Keyboard::Switch);
     void special_key_action(int key_code);
 
     void turn_off_pressed_keys();
@@ -56,5 +57,6 @@ private:
     RefPtr<KnobsWidget> m_knobs_widget;
     RefPtr<PlayerWidget> m_player_widget;
 
+    // Not the piano keys, but the computer keyboard keys!
     bool m_keys_pressed[key_code_count] { false };
 };

Userland/Applications/Piano/Music.h

@@ -33,11 +33,6 @@ constexpr double sample_rate = 44100;
 // Headroom for the synth
 constexpr double volume_factor = 0.8;
 
-enum Switch {
-    Off,
-    On,
-};
-
 enum Direction {
     Down,
     Up,

Userland/Applications/Piano/RollWidget.cpp

@@ -126,7 +126,7 @@ void RollWidget::paint_event(GUI::PaintEvent& event)
             int distance_to_next_x = next_x_pos - x_pos;
             Gfx::IntRect rect(x_pos, y_pos, distance_to_next_x, note_height);
 
-            if (keys_widget() && keys_widget()->note_is_set(note))
+            if (m_track_manager.keyboard()->is_pressed(note))
                 painter.fill_rect(rect, note_pressed_color.with_alpha(128));
         }
     }

Userland/Applications/Piano/Track.cpp

@@ -16,10 +16,11 @@
 #include <LibDSP/Music.h>
 #include <math.h>
 
-Track::Track(NonnullRefPtr<LibDSP::Transport> transport)
+Track::Track(NonnullRefPtr<LibDSP::Transport> transport, NonnullRefPtr<LibDSP::Keyboard> keyboard)
     : m_transport(move(transport))
     , m_delay(make_ref_counted<LibDSP::Effects::Delay>(m_transport))
     , m_synth(make_ref_counted<LibDSP::Synthesizers::Classic>(m_transport))
+    , m_keyboard(move(keyboard))
 {
     set_volume(volume_max);
 }
@@ -29,17 +30,22 @@ void Track::fill_sample(Sample& sample)
     auto playing_notes = LibDSP::RollNotes {};
 
     for (size_t i = 0; i < note_count; ++i) {
+        bool has_roll_notes = false;
         auto& notes_at_pitch = m_roll_notes[i];
         for (auto& note : notes_at_pitch) {
-            if (note.is_playing(m_transport->time()))
+            if (note.is_playing(m_transport->time())) {
+                has_roll_notes = true;
                 playing_notes.set(i, note);
+            }
+        }
+        if (m_is_active_track) {
+            auto key_at_pitch = m_keyboard->note_at(i);
+            if (key_at_pitch.has_value() && key_at_pitch.value().is_playing(m_transport->time()))
+                playing_notes.set(i, key_at_pitch.release_value());
+            // If there are roll notes playing, don't stop them when we lift a keyboard key.
+            else if (!has_roll_notes)
+                playing_notes.remove(i);
         }
-        auto& key_at_pitch = m_keyboard_notes[i];
-        if (key_at_pitch.has_value() && key_at_pitch.value().is_playing(m_transport->time()))
-            playing_notes.set(i, key_at_pitch.value());
-        // No need to keep non-playing keyboard notes around.
-        else
-            m_keyboard_notes[i] = {};
     }
 
     auto synthesized_sample = LibDSP::Signal { FixedArray<Audio::Sample>::must_create_but_fixme_should_propagate_errors(1) };
@@ -105,24 +111,6 @@ void Track::set_roll_note(int note, u32 on_sample, u32 off_sample)
     sync_roll(note);
 }
 
-void Track::set_keyboard_note(int note, Switch state)
-{
-    VERIFY(note >= 0 && note < note_count);
-    if (state == Switch::Off) {
-        // If the note is playing, we need to start releasing it, otherwise just delete
-        if (auto& maybe_roll_note = m_keyboard_notes[note]; maybe_roll_note.has_value()) {
-            auto& roll_note = maybe_roll_note.value();
-            if (roll_note.is_playing(m_transport->time()))
-                roll_note.off_sample = m_transport->time();
-            else
-                m_keyboard_notes[note] = {};
-        }
-    } else
-        // FIXME: The end time needs to be far in the future.
-        m_keyboard_notes[note]
-            = RollNote { m_transport->time(), m_transport->time() + static_cast<u32>(sample_rate) * 10'000, static_cast<u8>(note), 0 };
-}
-
 void Track::set_volume(int volume)
 {
     VERIFY(volume >= 0);

Userland/Applications/Piano/Track.h

@@ -14,6 +14,7 @@
 #include <AK/NonnullRefPtr.h>
 #include <AK/SinglyLinkedList.h>
 #include <LibDSP/Effects.h>
+#include <LibDSP/Keyboard.h>
 #include <LibDSP/Music.h>
 #include <LibDSP/Synthesizers.h>
 #include <LibDSP/Transport.h>
@@ -26,7 +27,7 @@ class Track {
     AK_MAKE_NONMOVABLE(Track);
 
 public:
-    Track(NonnullRefPtr<LibDSP::Transport>);
+    Track(NonnullRefPtr<LibDSP::Transport>, NonnullRefPtr<LibDSP::Keyboard>);
     ~Track() = default;
 
     Vector<Audio::Sample> const& recorded_sample() const { return m_recorded_sample; }
@@ -39,8 +40,8 @@ public:
     void reset();
     String set_recorded_sample(StringView path);
     void set_roll_note(int note, u32 on_sample, u32 off_sample);
-    void set_keyboard_note(int note, Switch state);
     void set_volume(int volume);
+    void set_active(bool active) { m_is_active_track = active; }
 
 private:
     Audio::Sample recorded_sample(size_t note);
@@ -57,5 +58,6 @@ private:
 
     SinglyLinkedList<RollNote> m_roll_notes[note_count];
     RollIter m_roll_iterators[note_count];
-    Array<Optional<RollNote>, note_count> m_keyboard_notes;
+    NonnullRefPtr<LibDSP::Keyboard> m_keyboard;
+    bool m_is_active_track { false };
 };

Userland/Applications/Piano/TrackManager.cpp

@@ -13,8 +13,10 @@
 
 TrackManager::TrackManager()
     : m_transport(make_ref_counted<LibDSP::Transport>(120, 4))
+    , m_keyboard(make_ref_counted<LibDSP::Keyboard>(m_transport))
 {
     add_track();
+    m_tracks[m_current_track]->set_active(true);
 }
 
 void TrackManager::time_forward(int amount)
@@ -59,36 +61,16 @@ void TrackManager::reset()
 
     m_transport->set_time(0);
 
-    for (auto& track : m_tracks)
+    for (auto& track : m_tracks) {
         track->reset();
-}
+        track->set_active(false);
-
-void TrackManager::set_keyboard_note(int note, Switch note_switch)
-{
-    m_tracks[m_current_track]->set_keyboard_note(note, note_switch);
-}
-
-void TrackManager::set_octave(Direction direction)
-{
-    if (direction == Up) {
-        if (m_octave < octave_max)
-            ++m_octave;
-    } else {
-        if (m_octave > octave_min)
-            --m_octave;
-    }
-}
-
-void TrackManager::set_octave(int octave)
-{
-    if (octave <= octave_max && octave >= octave_min) {
-        m_octave = octave;
     }
+    m_tracks[m_current_track]->set_active(true);
 }
 
 void TrackManager::add_track()
 {
-    m_tracks.append(make<Track>(m_transport));
+    m_tracks.append(make<Track>(m_transport, m_keyboard));
 }
 
 int TrackManager::next_track_index() const

Userland/Applications/Piano/TrackManager.h

@@ -9,6 +9,8 @@
 
 #pragma once
 
+#include "AK/NonnullRefPtr.h"
+#include "LibDSP/Keyboard.h"
 #include "Music.h"
 #include "Track.h"
 #include <AK/Array.h>
@@ -26,30 +28,32 @@ public:
 
     Track& current_track() { return *m_tracks[m_current_track]; }
     Span<const Sample> buffer() const { return m_current_front_buffer; }
-    int octave() const { return m_octave; }
-    int octave_base() const { return (m_octave - octave_min) * 12; }
     int track_count() { return m_tracks.size(); };
     void set_current_track(size_t track_index)
     {
         VERIFY((int)track_index < track_count());
+        auto old_track = m_current_track;
         m_current_track = track_index;
+        m_tracks[old_track]->set_active(false);
+        m_tracks[m_current_track]->set_active(true);
     }
 
     NonnullRefPtr<LibDSP::Transport> transport() const { return m_transport; }
+    NonnullRefPtr<LibDSP::Keyboard> keyboard() const { return m_keyboard; }
     // Legacy API, do not add new users.
     void time_forward(int amount);
 
     void fill_buffer(Span<Sample>);
     void reset();
-    void set_keyboard_note(int note, Switch note_switch);
+    void set_keyboard_note(int note, LibDSP::Keyboard::Switch note_switch);
     void set_should_loop(bool b) { m_should_loop = b; }
-    void set_octave(Direction);
-    void set_octave(int octave);
     void add_track();
     int next_track_index() const;
 
 private:
     Vector<NonnullOwnPtr<Track>> m_tracks;
+    NonnullRefPtr<LibDSP::Transport> m_transport;
+    NonnullRefPtr<LibDSP::Keyboard> m_keyboard;
     size_t m_current_track { 0 };
 
     Array<Sample, sample_count> m_front_buffer;
@@ -57,9 +61,5 @@ private:
     Span<Sample> m_current_front_buffer { m_front_buffer.span() };
     Span<Sample> m_current_back_buffer { m_back_buffer.span() };
 
-    int m_octave { 4 };
-
-    NonnullRefPtr<LibDSP::Transport> m_transport;
-
     bool m_should_loop { true };
 };