Skip to content
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.

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

ピッチ編集機能を追加 #2003

Merged
merged 50 commits into from
Apr 25, 2024

Conversation

sigprogramming
Copy link
Contributor

@sigprogramming sigprogramming commented Apr 19, 2024

内容

以下を行います。

  • ピッチ編集機能を追加
    • ピッチ編集モードに移行後、左クリックでドロー、Ctrl+左クリックで消去
    • ひとまず実験的機能にしています
  • SequencerPitchの処理を変更
    • データ区間(データがある区間)のマップを生成し、データ区間ごとにピッチラインを描画
    • 元のピッチとユーザーの編集を表示
    • 元のピッチは有声区間のみを表示(無声子音の区間は非表示)
      • 無声区間は薄く表示でも良いかも
  • エディターのフレームレートを定義
  • 縦方向のズームの下限・上限を調整
  • リファクタリング

以下は未実装です。

  • ピッチ編集データの保存・読み込み
  • ピッチ編集のundo/redo(実装しました)

関連 Issue

close #1941

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

ピッチ編集

その他

  • 編集データのframeRateをどこで持つかで悩んでいます

Comment on lines 32 to 35
const originalPitchLineColor = new Color(177, 201, 181, 255);
const originalPitchLineWidth = 1.2;
const pitchEditLineColor = new Color(146, 214, 154, 255);
const pitchEditLineWidth = 2;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ノートの色とピッチラインの色、色々試しましたが良い組み合わせが見つかりませんでした…

@sigprogramming sigprogramming marked this pull request as ready for review April 19, 2024 21:56
@sigprogramming sigprogramming requested a review from a team as a code owner April 19, 2024 21:56
@sigprogramming sigprogramming requested review from y-chan and Hiroshiba and removed request for a team and y-chan April 19, 2024 21:56
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.

一旦UIに関してのコメントです!!
一言で言うと良さそうに思いました!!細かい部分について確認とメモを兼ねたコメントです。

  1. ピッチ編集時はノート選択できない感を出すためにマウスカーソルをペンか鉛筆に変更するといいかも
  2. ピッチ編集モードの時は、ノートの色を完全にグレーアウトしてあげても良さそう
    • 選択不可能だということを完全にわかるようにするため
  3. ピッチ編集結果ってトラックのグローバル情報としてデータ持つんですね!!
    • 既存ソフト見に行ったらそうなっててめちゃくちゃ驚きました
    • UX的にはたぶんノートに連動させる形が良さそう感がなきにしもあらずだけど、一旦この仕様で・・・!
  4. ピッチが急峻に変化したときの出音がノイジーに聞こえるかも?
    • スペクトログラムを見た感じ、ピッチが瞬時的に上下してる感じになってそう
    • エンジン(音声合成)側の課題?エディタ側の課題?
      • 補間かけてたらエディタ側の可能性もありそう
  5. ↑と同じような理由でなだらかなピッチであってもノイジーに聞こえることもある
    • ゆっくり線を引いても段々になるから・・・・かも・・・?
      • だとしたらもしかしたらエディタ側でなんとかできそう
      • 拡大するとこんな感じ
    • エンジン側のフレームレートまでピッチ編集分解能を上げてもこの現象が起きるならどうしようもなさそう

個人的には1のカーソル変更だけこのプルリクエストでもできそうならやっちゃって、他は後回しでも良いのかなと思ってます!

Copy link
Member

@sevenc-nanashi sevenc-nanashi left a comment

Choose a reason for hiding this comment

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

とりあえずコード見る限り。
マイグレーション処理大丈夫そうです?

src/sing/domain.ts Outdated Show resolved Hide resolved
@sigprogramming
Copy link
Contributor Author

保存・読み込みとundo/redoはまだ実装できてないです!
(すみませんPRの説明に書いていませんでした…)

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.

レビューしました!!
データ構造・設計・UI含め大枠良さそうに思いました!!


鉛筆マーク可愛くていいですね!!
消しゴムマークもあると楽しそうかもと思いました。(のちのちでも良さそう)

src/type/preload.ts Show resolved Hide resolved
src/components/Dialog/SettingDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/SettingDialog.vue Outdated Show resolved Hide resolved
src/components/Sing/ScoreSequencer.vue Outdated Show resolved Hide resolved
src/components/Sing/ScoreSequencer.vue Show resolved Hide resolved
src/sing/viewHelper.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/singing.ts Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
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です!!
あとAsnycTaskRunnerのとこだけかなと!!
実際使ってみてもだいぶ良い感じだなと思いました、もろもろの調整ありがとうございます!!


ピッチ編集が無声部分を特殊な扱いするの、とても良いのですがそういう仕様なことを忘れそうですね。。
覚えてたら使い方に書いとこうと思います!


あ、これは全然関係ないのですが。
ScoreSequencer.vueがかなり巨大化してきた印象です。ちょっと経験が浅くてわからないのですが、メンテ不能に陥るリスクが生じ始めている気がします。
解体の機運が高まっているのですが、この辺切り崩せそうと直感がある部分ってあったりされますか・・・?

とりあえずissueを作ろうかと思っていて、そのためにはある程度見通しがあると嬉しいのでお聞きした次第です!
レンダリング周りはWebGL化するときに抜本的に変わる可能性があるのでさておき、例えばダブルクリック判定部分とかは外に出せそうな気がしています。

src/sing/graphics/lineStrip.ts Show resolved Hide resolved
src/sing/utility.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
Comment on lines +1700 to +1704
.cursor-draw {
cursor:
url("/draw-cursor.png") 2 30,
auto;
}
Copy link
Member

@Hiroshiba Hiroshiba Apr 24, 2024

Choose a reason for hiding this comment

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

消してるときは消しゴムマークにしても良いかも。
(もちろん後回しでも!)

:class="{ 'rect-selecting': shiftKey }"
:class="{
'rect-selecting': editTarget === 'NOTE' && shiftKey,
'cursor-draw': editTarget === 'PITCH' && !ctrlKey,
Copy link
Member

Choose a reason for hiding this comment

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

とても細かいけど、スクロールバー上にあるときもカーソルがeditになってるのは違和感あるかもでした!
(解決できるんだろうか・・・。)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

レイヤーが整理できていないので、一旦整理してイベントを受け取るレイヤーを作った後に、こちら行った方が良いかもです。

src/sing/viewHelper.ts Outdated Show resolved Hide resolved
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!!

お疲れ様でした!!!
これで今マージしても大丈夫な状態でしょうか?

マージ後にここに書いていたことをissueにしていきたいと思います!
#2003 (review)

@Hiroshiba
Copy link
Member

Hiroshiba commented Apr 25, 2024

テストが落ちてますね。。。。。。。なんでだろう。。。。処理が重くなった・・・?
とりあえずスキップさせる・・・・・?

追記:3日前にmacosのGithub Actions環境が変わったっぽみでした!!
とりあえず落ちるテストだけskipさせるのが良さそう。

@Hiroshiba
Copy link
Member

テストが落ちる件に関しては今のmainブランチをマージすれば大丈夫だと思います!
マージして大丈夫であればマージします・・・!! @sigprogramming

@sigprogramming
Copy link
Contributor Author

sigprogramming commented Apr 25, 2024

リファクタリングissue作っても良いかも 1 2

pushとspliceを使用する形に変更しました!(レビュー後ですみません…)

@sigprogramming
Copy link
Contributor Author

元のピッチの色、少し暗いと感じたので、彩度を12から15に変更しました!
(色はとりあえずで設定しているので、後で調整必要かもです)

@sigprogramming
Copy link
Contributor Author

@Hiroshiba
問題なければマージしていただいて大丈夫です…!

src/components/Sing/ScoreSequencer.vue Outdated Show resolved Hide resolved
src/components/Sing/ScoreSequencer.vue Outdated Show resolved Hide resolved
@Hiroshiba
Copy link
Member

マージします!!
ついでいろんなissueを作ったりしようと思います!

@Hiroshiba Hiroshiba merged commit 8d2e0eb into VOICEVOX:main Apr 25, 2024
8 checks passed
@sigprogramming
Copy link
Contributor Author

ScoreSequencer.vueがかなり巨大化してきた印象です。ちょっと経験が浅くてわからないのですが、メンテ不能に陥るリスクが生じ始めている気がします。
解体の機運が高まっているのですが、この辺切り崩せそうと直感がある部分ってあったりされますか・・・?

以下を考えています! @Hiroshiba

DOM

  • グリッド部分をコンポーネント(SequencerGrid)として切り出せそう
  • SequencerNoteから、SequencerNotesにできそう
  • v-for1つでノートを描画しても重くならないかも
    • v-forを1つにすると、ダブルクリック判定の処理も無くせるはず
  • スクロールバーをコンポーネント化(SequencerScroll)した方が良いかも

処理

  • 以下の関数は処理を抽象化して別ファイルに移動できそう(これだけでも結構行数減ると思います)
    • previewAdd
    • previewMove
    • previewResizeRight
    • previewResizeLeft
    • previewDrawPitch
    • previewErasePitch
    • preview
    • startPreview
    • endPreview
  • onMouseDownonMouseMoveonMouseUpの処理も抽象化できそう(ifを減らせそう)
  • rectSelectはプレビュー処理(previewAddなど)と同じように扱った方が良いかも
  • 以下の関数はviewHelper.tsに置いても良いかも
    • getXInBorderBox
    • getYInBorderBox
  • onNoteLyricMouseDownは要らないかも

@Hiroshiba
Copy link
Member

@sigprogramming 詳しくありがとうございます!!!

すみません、お手数おかけしてしまうのですがそちらの内容でissue作成をお願いしても良いでしょうか 🙇
と言っても僕がコピペすればよさそうなのですが、追加とかあった時に @sigprogramming さんが編集できた方がいいのかなと思い・・・!

お時間なかったらコピペになってしまいますがこちらで作ってしまおうと思います!!

sevenc-nanashi pushed a commit to sevenc-nanashi/voicevox that referenced this pull request Apr 28, 2024
* ピッチ編集機能を追加

* 修正

* 色を調整、UIを修正

* ペンの画像を追加、ピッチ編集時のカーソルをペンに変更

* TODOコメントを追加

* ピッチ編集をundo/redoできるようにした

* 有声区間のみピッチ編集を適用するように変更

* ピッチ編集データを平滑化してからセットするようにした

* Update src/components/Dialog/SettingDialog.vue

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

* Update src/components/Sing/ScoreSequencer.vue

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

* 例外を使用して網羅性チェックを行うようにした

* pitchEdit.dataのところにコメントを追加、データが無いことを表す値を定数で定義

* prevCursorFrameとprevCursorFrequencyをまとめてprevCursorPosに変更、コメントを追加

* 修正

* ピッチ編集機能が無効になったときに行う処理をSingEditorに移動

* 1フレームのピッチ変更を適用しない理由をコメントで説明

* 編集モードの判定をcomputed内で行うようにした

* プレビュー時に実行される関数の処理を説明するコメントを追加

* コメントを追加

* EditModeをEditTargetに変更

* DataSectionのハッシュの型をブランド型にした

* データ区間のマップを更新する関数の処理を説明するコメントを追加、コメントを修正

* フレームレート周りを一旦変更

* Colorクラスの値の範囲をコメントで書いた

* FramewiseDataSectionに変更

* watchのimmediateをtrueに設定

* データ区間のマップを生成する処理を関数に切り出した

* concatからpushに変更、spliceを使用してデータを書き換える形に変更

* SET_PITCH_EDIT_DATAのところにコメントを追加

* ピッチ編集を適用する処理のところにコメントを追加

* 修正

* DataSectionMapの更新処理が開始順で完了せずピッチの表示が更新されないことがある不具合を修正

* 修正

* Revert "concatからpushに変更、spliceを使用してデータを書き換える形に変更"

This reverts commit a0c3260.

* ExhaustiveErrorを移動

* NOTEコメントを追加

* 修正

* structuredCloneを使用してディープコピーするようにした

* asyncProcessで例外が発生したときにisRunningとisRunRequestedがfalseにならないのを修正

* 非同期処理の実行部分を変更

* AsyncLockを使用する形にした

* DEPRECATEDにしてコメントを追加

* CLEAR_PITCH_EDIT_DATAを削除

* 縦方向のズームの下限を調整

* Update src/type/utility.ts

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

* Update src/sing/utility.ts

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>

* 元のピッチの色の彩度を12から15にした

* concatからpushに変更、spliceを使用してデータを書き換える形に変更

* macでピッチの消去ができるように修正

* to mouseButton

---------

Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
@sigprogramming
Copy link
Contributor Author

issue作成します!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ソング:ピッチの編集を反映できるようにする
3 participants