Skip to content

feat: ソング：カーソルをシーケンサーの端に持っていったときにスクロールする機能を追加（#2319） #2583

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 23 commits into from
Mar 24, 2025
Merged

feat: ソング：カーソルをシーケンサーの端に持っていったときにスクロールする機能を追加（#2319） #2583

merged 23 commits into from
Mar 24, 2025

Conversation

terapotan
Copy link
Contributor

内容

カーソルをシーケンサーの端に持っていったときにスクロールする機能を暫定で追加してみました。
暫定ですので、以下の機能がまだ実装されていません。

  1. カーソルを持っていってシーケンサーが動くのは右端のみ。左端は動かない。
  2. ずっと右端にスクロールし続けるには、ノートを動かし続けなければならない。

特に2番に関して、マウスカーソルがシーケンサーの範囲外にあったときに、常に移動するように実装するには、previewMove以外のイベントに上記スクロール機能を実装する必要があり、現状どのイベントに実装すればいいか分かっていない状況です。

詳細な仕様含め、一度話し合ったほうがいいかなと思っています。

関連 Issue

ref #2319

スクリーンショット・動画など

スクロール端動くやつ

その他

@voicevox-preview-pages
Copy link

voicevox-preview-pages bot commented Mar 2, 2025 •
edited
Loading

🚀 プレビュー用ページを作成しました 🚀

更新時点でのコミットハッシュ：fa80ebf

@Hiroshiba
Copy link
Member

draft PRありがとうございます！
ちょっとコンフリクトの理由と対処法について詳しくコメントまで！

ステートマシンが実装されてコードが移動しました！

たぶん今回の変更箇所は moveNoteState.ts 辺りに移していただくことになるかなと･･･！
一旦コメントまで！

@sigprogramming
Copy link
Contributor

sigprogramming commented Mar 2, 2025 •
edited
Loading

PRありがとうございます！
ちょっと設計に関してコメントです。

スクロールはHTML要素に対して（scrollToで）行う必要がありますが、ステートマシン内でHTML要素を扱うと入出力が把握しにくくなるかもです。（HTML要素にはたくさんのプロパティ、メソッド、副作用があるので）

なので、「マウスドラッグ中のオートスクロールを有効にするかどうか」のフラグをステートマシンで扱うように（MoveNoteStateのonEnterでフラグを立てるなど）して、そのフラグが立っているときにScoreSequencer.vue側でオートスクロールの処理を行うようにするのが良いかもです。

@terapotan
Copy link
Contributor Author

Stateパターンの修正点について、確認してますので少々お待ちください………
不明点あったら@sigprogramming さんに質問するかもです。

@terapotan
Copy link
Contributor Author

@sigprogramming @Hiroshiba
コンフリクトの解消を行いました。
暫定となっている点は、前に示したのと変わりません。

@terapotan
Copy link
Contributor Author

上下左右に対応+マウスを動かさなくても自動スクロールするようにした修正版を展開いたします。
ご確認いただけると幸いです。下記は動作確認時の動画です。
スクロール端動くやつ_完成版

Hiroshiba
Hiroshiba reviewed Mar 3, 2025
View reviewed changes
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コードはまだ見れてないのですが、挙動は良い感じに思いました！！！

setIntervalではなくrequestAnimationFrameを使うとより良いかもです！
こっちはコールバックの第一引数にtimestampが来るので、実際にかかった時間を計算できるはず。
なので速度×時間で移動量を割り出せるので、ちょっとラグい環境でも使いやすい･･･かも･･･？
（setIntervalのように何度も呼んでくれないので再帰呼び出しが必要になりますが。VOICEVOXのコード内でも結構requestAnimationFrame使ってる箇所あるので参考になるかもです！）

sigprogramming
sigprogramming reviewed Mar 3, 2025
View reviewed changes
Copy link
Contributor

@sigprogramming sigprogramming left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コンフリクト解消＆修正ありがとうございます！

@terapotan
指摘事項修正
66dc90d 
- IntervaliImerの代わりにrequestAnimationFrameを使用するよう修正
- 他レビュー指摘事項修正
@terapotan
Copy link
Contributor Author

@sigprogramming @Hiroshiba
レビューありがとうございます！
レビュー指摘を受け、コードを全面的に見直したものを展開します。

以下は、修正後の動作確認の様子です。
スクロール端動くやつ_完成版

sigprogramming
sigprogramming reviewed Mar 4, 2025
View reviewed changes
Copy link
Contributor

@sigprogramming sigprogramming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

変更ありがとうございます！

試してみて、スクロールスピードはもう少し速くても良いかもと感じました！
端の判定も、もう少し広くとっても良いかも！

@terapotan
指摘事項修正
a5c1b74 
・スクロールスピード、スレッショルド値変更
・sequencerBodyがundefinedのとき、Exceptionをthrowするよう変更
・他指摘事項修正
@terapotan
Copy link
Contributor Author

@sigprogramming
再レビューありがとうございます！
再度修正したものを展開します。スクロールスピードを速くして、スレッショルドの値を大きくしてみました。
以下が動作確認時の動画です。

端スクロール修正版_2nd

sigprogramming
sigprogramming reviewed Mar 6, 2025
View reviewed changes
Copy link
Contributor

@sigprogramming sigprogramming left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

変更ありがとうございます！
スクロールスピードはあと少し早くても良いかもです、試してみて0.3くらいが良さそうに感じました！
また、ノート追加時とノートリサイズ時も自動スクロールされると便利かもです！
（moveNoteState.tsと同様に、addNoteState.ts、resizeNoteLeftState.ts、resizeNoteRightState.tsを変更することで実現できると思います）

@terapotan
Copy link
Contributor Author

また、ノート追加時とノートリサイズ時も自動スクロールされると便利かもです！
（moveNoteState.tsと同様に、addNoteState.ts、resizeNoteLeftState.ts、resizeNoteRightState.tsを変更することで実現できると思います）

確かに便利そうですね！　追加してみます

@terapotan
Copy link
Contributor Author

@sigprogramming
再度修正したものを展開します！
自動スクロールタイミング追加版

sigprogramming
sigprogramming approved these changes Mar 8, 2025
View reviewed changes
Copy link
Contributor

@sigprogramming sigprogramming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM！
動作も問題なさそうです！

@terapotan terapotan marked this pull request as ready for review March 8, 2025 14:00
@terapotan terapotan requested a review from a team as a code owner March 8, 2025 14:00
@terapotan terapotan removed the request for review from a team March 8, 2025 14:00
@sigprogramming
Copy link
Contributor

sigprogramming commented Mar 22, 2025 •
edited
Loading

以下を行いました。

  • 点と矩形の距離を計算する処理（calcMinimumDistanceVectorRectAndPoint）を変更
  • スクロール量を計算する処理を変更・整理
    • 計算式自体は変わっていません、/ 10などを無くして、値を調整しました。
  • TriggerRectクラスを無くして、Vector2DクラスとcalcMinimumDistanceVectorRectAndPoint関数をsing/utility.tsに移動
  • 自動スクロールの処理をコンポーザブル化

Hiroshiba
Hiroshiba approved these changes Mar 23, 2025
View reviewed changes
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM！！

細かい点をコメントしました！
マージボタン押せると思うので良い感じになったら押していただければ･･･！
（変更した後心配でしたらコメントなどいただければ 🙏 ）

@terapotan さんもありがとうございました！！

@Hiroshiba Hiroshiba changed the title ソング：カーソルをシーケンサーの端に持っていったときにスクロールする機能を追加【暫定】（#2319） feat: ソング：カーソルをシーケンサーの端に持っていったときにスクロールする機能を追加（#2319） Mar 23, 2025
@terapotan
Copy link
Contributor Author

@sigprogramming
作業引き継いで頂きありがとうございます！

@sigprogramming
Copy link
Contributor

少し調整を行ってからマージします！

@sigprogramming
Copy link
Contributor

sigprogramming commented Mar 23, 2025 •
edited
Loading

@Hiroshiba
mount周りを変更したので、レビューよろしくお願いします！

@Hiroshiba Hiroshiba mentioned this pull request Mar 23, 2025
Closed
Hiroshiba
Hiroshiba approved these changes Mar 23, 2025
View reviewed changes
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM！！

mountedのコンポーザブル作ってみました！　よかったら適用後マージいただければ！！

@sigprogramming
Copy link
Contributor

@terapotan
問題なければ terapotan#2 をマージしていただけると…！

@Hiroshiba
Copy link
Member

あっそうか @tarepan さんの方にPR行ってしまいますね。。すみません！！！

お忙しいところご迷惑かけてしまうのも申し訳ないので、別でvoicevoxに向けてPR作ろうと思います！

ということでこのPRはマージさせていただきます！！
お二人ともありがとうございました！！

@Hiroshiba Hiroshiba enabled auto-merge March 24, 2025 10:23
@Hiroshiba Hiroshiba added this pull request to the merge queue Mar 24, 2025
@terapotan
Copy link
Contributor Author

@Hiroshiba
（terapanさんは別の方かと……！ 私はterapotanです！）

@Hiroshiba
Copy link
Member

あっっっ間違えてしまいました。。お二人ともすみません。。。🙇🙇🙇

Merged via the queue into VOICEVOX:main with commit e02396d Mar 24, 2025
11 checks passed
@terapotan terapotan deleted the AddScrollFunction branch March 24, 2025 10:32
@Hiroshiba Hiroshiba mentioned this pull request Mar 24, 2025
Merged
@Hiroshiba
Copy link
Member

ちょっと相談が！

ニーズのある機能が実装されたときにSNSで言及しておりまして、今回の機能が実装されたこともポストしたいと思っています。
https://x.com/search?q=%23VOICEVOX%E9%96%8B%E7%99%BA%E7%8A%B6%E6%B3%81
もしよかったらそこで @terapotan さんのXアカウントをツイート文に含めて紹介させていただきたいのですが、いかがでしょうか･･･？
アカウントがなければ、@は無しでそのまま terapotan と記載したいと思います 🙏

ちなみにこんな感じを予定しています･･･！

#VOICEVOX開発状況 
ソングで編集時、カーソルが画面端に近づくと自動でスクロールするようになりました 🎉（今後のアップデートで実装されます。）
【開発者: @~~~】
https://github.com/VOICEVOX/voicevox/pull/2583

@terapotan
Copy link
Contributor Author

@Hiroshiba
ツイート文紹介ありがとうございます！　はい！　問題ないです、大丈夫です！　むしろ嬉しいです！
https://x.com/terapotan
↑が私のXアカウントですので、そちらにメンション飛ばしていただければ……！

@Hiroshiba
Copy link
Member

ポストさせていただきました！！　ありがとうございました！！ 🙏
https://x.com/voicevox_pj/status/1904172637712273486

@sigprogramming sigprogramming mentioned this pull request Apr 1, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sigprogramming sigprogramming sigprogramming approved these changes

@Hiroshiba Hiroshiba Hiroshiba approved these changes

@romot-co romot-co Awaiting requested review from romot-co

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@terapotan @Hiroshiba @sigprogramming