From 815442b2b5edb441dac0d4478990d89fb66e2992 Mon Sep 17 00:00:00 2001 From: Florian Kaiser Date: Mon, 6 Feb 2023 20:46:41 +0100 Subject: [PATCH] Piano: Fix insertion and deletion of notes On mouse move the pressed button is not present in the event argument which causes the corresponding code to never fire. Instead it now stores the original mouse down event and acts according to that on mouse move. --- Userland/Applications/Piano/RollWidget.cpp | 88 +++++++++++----------- Userland/Applications/Piano/RollWidget.h | 7 +- Userland/Libraries/LibDSP/Clip.cpp | 9 +++ Userland/Libraries/LibDSP/Clip.h | 2 + Userland/Libraries/LibDSP/Track.cpp | 10 +++ Userland/Libraries/LibDSP/Track.h | 1 + 6 files changed, 71 insertions(+), 46 deletions(-) diff --git a/Userland/Applications/Piano/RollWidget.cpp b/Userland/Applications/Piano/RollWidget.cpp index 91eabadb8b..fa7c1db56b 100644 --- a/Userland/Applications/Piano/RollWidget.cpp +++ b/Userland/Applications/Piano/RollWidget.cpp @@ -185,61 +185,63 @@ void RollWidget::mousedown_event(GUI::MouseEvent& event) if (!widget_inner_rect().contains(event.x(), event.y())) return; - m_note_drag_start = event.position(); + if (event.button() == GUI::MouseButton::Secondary) { + auto const time = roll_length * (static_cast(get_note_for_x(event.x())) / m_num_notes); + auto const note = m_track_manager.current_track()->note_at(time, get_pitch_for_y(event.y())); - int y = (m_note_drag_start.value().y() + vertical_scrollbar().value()) - frame_thickness(); - y /= note_height; - m_drag_note = (note_count - 1) - y; + if (note.has_value()) { + m_track_manager.current_track()->remove_note(note.value()); + update(); + } + return; + } + m_mousedown_event = event; +} + +void RollWidget::mouseup_event(GUI::MouseEvent& event) +{ mousemove_event(event); + m_mousedown_event = {}; +} + +u8 RollWidget::get_pitch_for_y(int y) const +{ + return (note_count - 1) - ((y + vertical_scrollbar().value()) - frame_thickness()) / note_height; +} + +int RollWidget::get_note_for_x(int x) const +{ + // There's a case where we can't just use x / m_note_width. For example, if + // your m_note_width is 3.1 you will have a rect starting at 3. When that + // leftmost pixel of the rect is clicked you will do 3 / 3.1 which is 0 + // and not 1. We can avoid that case by shifting x by 1 if m_note_width is + // fractional, being careful not to shift out of bounds. + x = (x + horizontal_scrollbar().value()) - frame_thickness(); + bool const note_width_is_fractional = m_note_width - static_cast(m_note_width) != 0; + bool const x_is_not_last = x != widget_inner_rect().width() - 1; + if (note_width_is_fractional && x_is_not_last) + ++x; + x /= m_note_width; + return clamp(x, 0, m_num_notes - 1); } void RollWidget::mousemove_event(GUI::MouseEvent& event) { - if (!m_note_drag_start.has_value()) + if (!m_mousedown_event.has_value()) return; - if (m_note_drag_location.has_value()) { - // Clear previous note - m_track_manager.current_track()->remove_note(m_note_drag_location.value()); - } + if (m_mousedown_event.value().button() == GUI::MouseButton::Primary) { + int const x_start = get_note_for_x(m_mousedown_event.value().x()); + int const x_end = get_note_for_x(event.x()); - // Right-Click deletes notes - if (event.button() == GUI::MouseButton::Secondary) { + const u32 on_sample = round(roll_length * (static_cast(min(x_start, x_end)) / m_num_notes)); + const u32 off_sample = round(roll_length * (static_cast(max(x_start, x_end) + 1) / m_num_notes)) - 1; + auto const note = RollNote { on_sample, off_sample, get_pitch_for_y(m_mousedown_event.value().y()), 127 }; + + m_track_manager.current_track()->set_note(note); update(); - return; } - - auto get_note_x = [&](int x0) { - // There's a case where we can't just use x / m_note_width. For example, if - // your m_note_width is 3.1 you will have a rect starting at 3. When that - // leftmost pixel of the rect is clicked you will do 3 / 3.1 which is 0 - // and not 1. We can avoid that case by shifting x by 1 if m_note_width is - // fractional, being careful not to shift out of bounds. - int x = (x0 + horizontal_scrollbar().value()) - frame_thickness(); - bool note_width_is_fractional = m_note_width - static_cast(m_note_width) != 0; - bool x_is_not_last = x != widget_inner_rect().width() - 1; - if (note_width_is_fractional && x_is_not_last) - ++x; - x /= m_note_width; - return clamp(x, 0, m_num_notes - 1); - }; - - int x0 = get_note_x(m_note_drag_start.value().x()); - int x1 = get_note_x(event.x()); - - u32 on_sample = roll_length * (static_cast(min(x0, x1)) / m_num_notes); - u32 off_sample = (roll_length * (static_cast(max(x0, x1) + 1) / m_num_notes)) - 1; - m_note_drag_location = RollNote { on_sample, off_sample, (u8)m_drag_note, 127 }; - m_track_manager.current_track()->set_note(m_note_drag_location.value()); - - update(); -} - -void RollWidget::mouseup_event([[maybe_unused]] GUI::MouseEvent& event) -{ - m_note_drag_start = {}; - m_note_drag_location = {}; } // FIXME: Implement zoom and horizontal scroll events in LibGUI, not here. diff --git a/Userland/Applications/Piano/RollWidget.h b/Userland/Applications/Piano/RollWidget.h index bc7a8e9129..d28add12be 100644 --- a/Userland/Applications/Piano/RollWidget.h +++ b/Userland/Applications/Piano/RollWidget.h @@ -43,12 +43,13 @@ private: double m_note_width { 0.0 }; int m_zoom_level { 1 }; - Optional m_note_drag_start; - Optional m_note_drag_location; - int m_drag_note; + Optional m_mousedown_event; RefPtr m_background; int m_prev_zoom_level { m_zoom_level }; int m_prev_scroll_x { horizontal_scrollbar().value() }; int m_prev_scroll_y { vertical_scrollbar().value() }; + + u8 get_pitch_for_y(int y) const; + int get_note_for_x(int x) const; }; diff --git a/Userland/Libraries/LibDSP/Clip.cpp b/Userland/Libraries/LibDSP/Clip.cpp index 1a3464b863..032b97dc64 100644 --- a/Userland/Libraries/LibDSP/Clip.cpp +++ b/Userland/Libraries/LibDSP/Clip.cpp @@ -14,6 +14,15 @@ Sample AudioClip::sample_at(u32 time) return m_samples[time]; } +Optional NoteClip::note_at(u32 time, u8 pitch) const +{ + for (auto& note : m_notes) { + if (time >= note.on_sample && time <= note.off_sample && pitch == note.pitch) + return note; + } + return {}; +} + void NoteClip::set_note(RollNote note) { m_notes.remove_all_matching([&](auto const& other) { diff --git a/Userland/Libraries/LibDSP/Clip.h b/Userland/Libraries/LibDSP/Clip.h index 977b089920..32c7994715 100644 --- a/Userland/Libraries/LibDSP/Clip.h +++ b/Userland/Libraries/LibDSP/Clip.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include #include @@ -50,6 +51,7 @@ public: { } + Optional note_at(u32 time, u8 pitch) const; void set_note(RollNote note); // May do nothing; that's fine. void remove_note(RollNote note); diff --git a/Userland/Libraries/LibDSP/Track.cpp b/Userland/Libraries/LibDSP/Track.cpp index 557db34d52..66179d7246 100644 --- a/Userland/Libraries/LibDSP/Track.cpp +++ b/Userland/Libraries/LibDSP/Track.cpp @@ -146,6 +146,16 @@ void AudioTrack::compute_current_clips_signal() TODO(); } +Optional NoteTrack::note_at(u32 time, u8 pitch) const +{ + for (auto& clip : m_clips) { + if (time >= clip.start() && time <= clip.end()) + return clip.note_at(time, pitch); + } + + return {}; +} + void NoteTrack::set_note(RollNote note) { for (auto& clip : m_clips) { diff --git a/Userland/Libraries/LibDSP/Track.h b/Userland/Libraries/LibDSP/Track.h index b3267079f0..63c6d73716 100644 --- a/Userland/Libraries/LibDSP/Track.h +++ b/Userland/Libraries/LibDSP/Track.h @@ -80,6 +80,7 @@ public: bool check_processor_chain_valid() const override; ReadonlySpan> notes() const { return m_clips.span(); } + Optional note_at(u32 time, u8 pitch) const; void set_note(RollNote note); void remove_note(RollNote note);