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.

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

refactor: gridInfoではなくnumMeasuresをinjectするようにする #2605

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+123 −95
Open

refactor: gridInfoではなくnumMeasuresをinjectするようにする #2605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ada1de
gridInfoではなくnumMeasuresをinjectするように
sigprogramming Mar 7, 2025
52700c3
Merge branch 'main' into refactor_around_num_measures
Hiroshiba Mar 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 37 additions & 35 deletions src/components/Sing/ScoreSequencer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,8 @@
import { ComputedRef } from "vue";
import type { InjectionKey } from "vue";
export const gridInfoInjectionKey: InjectionKey<{
gridWidth: ComputedRef<number>;
gridHeight: ComputedRef<number>;
export const numMeasuresInjectionKey: InjectionKey<{
numMeasures: ComputedRef<number>;
}> = Symbol();
</script>

Expand All @@ -242,36 +241,37 @@ import {
watch,
provide,
} from "vue";
import SequencerParameterPanel from "./SequencerParameterPanel.vue";
import SequencerGridSpacer from "./SequencerGridSpacer.vue";
import SequencerParameterPanel from "@/components/Sing/SequencerParameterPanel.vue";
import SequencerGridSpacer from "@/components/Sing/SequencerGridSpacer.vue";
import ContextMenu, {
ContextMenuItemData,
} from "@/components/Menu/ContextMenu/Container.vue";
import { useStore } from "@/store";
import { Note } from "@/store/type";
import {
getEndTicksOfPhrase,
getMeasureDuration,
getNoteDuration,
getStartTicksOfPhrase,
getTimeSignaturePositions,
noteNumberToFrequency,
tickToMeasureNumber,
tickToSecond,
} from "@/sing/domain";
import {
tickToBaseX,
baseXToTick,
noteNumberToBaseY,
baseYToNoteNumber,
keyInfos,
ZOOM_X_MIN,
ZOOM_X_MAX,
ZOOM_X_STEP,
ZOOM_Y_MIN,
ZOOM_Y_MAX,
ZOOM_Y_STEP,
PREVIEW_SOUND_DURATION,
getKeyBaseHeight,
SEQUENCER_MIN_NUM_MEASURES,
} from "@/sing/viewHelper";
import { getLast } from "@/sing/utility";
import SequencerGrid from "@/components/Sing/SequencerGrid/Container.vue";
import SequencerRuler from "@/components/Sing/SequencerRuler/Container.vue";
import SequencerKeys from "@/components/Sing/SequencerKeys.vue";
Expand All @@ -292,21 +292,23 @@ const { warn } = createLogger("ScoreSequencer");
const store = useStore();
const state = store.state;
// トラック、TPQN、テンポ、ノーツ
// トラック、TPQN、テンポ、拍子、ノーツ
const tpqn = computed(() => state.tpqn);
const tempos = computed(() => state.tempos);
const timeSignatures = computed(() => store.state.timeSignatures);
const tracks = computed(() => store.state.tracks);
const selectedTrackId = computed(() => store.getters.SELECTED_TRACK_ID);
const notesInSelectedTrack = computed(() => store.getters.SELECTED_TRACK.notes);
const notesInOtherTracks = computed(() =>
[...store.state.tracks.entries()].flatMap(([trackId, track]) =>
[...tracks.value.entries()].flatMap(([trackId, track]) =>
trackId === selectedTrackId.value ? [] : track.notes,
),
);
const overlappingNoteIdsInSelectedTrack = computed(() =>
store.getters.OVERLAPPING_NOTE_IDS(selectedTrackId.value),
);
const selectedNotes = computed(() =>
store.getters.SELECTED_TRACK.notes.filter((note) =>
notesInSelectedTrack.value.filter((note) =>
selectedNoteIds.value.has(note.id),
),
);
Expand Down Expand Up @@ -362,35 +364,35 @@ const snapTicks = computed(() => {
});
// 小節の数
// NOTE: スコア長(曲長さ)が決まっていないため、無限スクロール化する or 最後尾に足した場合は伸びるようにするなど？
// NOTE: いったん最後尾に足した場合は伸びるようにする
const numMeasures = computed(() => {
return store.getters.SEQUENCER_NUM_MEASURES;
});
const tsPositions = getTimeSignaturePositions(
timeSignatures.value,
tpqn.value,
);
const notes = [...tracks.value.values()].flatMap((track) => track.notes);
const noteEndPositions = notes.map((note) => note.position + note.duration);
let maxTicks = 0;
const lastTsPosition = tsPositions[tsPositions.length - 1];
maxTicks = Math.max(maxTicks, lastTsPosition);
// グリッド関係の値
const gridWidth = computed(() => {
const timeSignatures = store.state.timeSignatures;
const gridCellWidth = tickToBaseX(snapTicks.value, tpqn.value) * zoomX.value;
let numOfGridColumns = 0;
for (const [i, timeSignature] of timeSignatures.entries()) {
const nextTimeSignature = timeSignatures[i + 1];
const nextMeasureNumber =
nextTimeSignature?.measureNumber ?? numMeasures.value + 1;
const beats = timeSignature.beats;
const beatType = timeSignature.beatType;
const measureDuration = getMeasureDuration(beats, beatType, tpqn.value);
numOfGridColumns +=
Math.round(measureDuration / snapTicks.value) *
(nextMeasureNumber - timeSignature.measureNumber);
const lastTempoPosition = getLast(tempos.value).position;
maxTicks = Math.max(maxTicks, lastTempoPosition);
for (const noteEndPosition of noteEndPositions) {
maxTicks = Math.max(maxTicks, noteEndPosition);
}
return gridCellWidth * numOfGridColumns;
});
const gridHeight = computed(() => {
const gridCellHeight = getKeyBaseHeight() * zoomY.value;
return gridCellHeight * keyInfos.length;
return Math.max(
SEQUENCER_MIN_NUM_MEASURES,
tickToMeasureNumber(maxTicks, timeSignatures.value, tpqn.value) + 8,
);
});
provide(gridInfoInjectionKey, { gridWidth, gridHeight });
provide(numMeasuresInjectionKey, { numMeasures });
// スクロール位置
const scrollX = ref(0);
Expand Down
10 changes: 3 additions & 7 deletions src/components/Sing/SequencerGrid/Container.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
:sequencerZoomY
:sequencerSnapType
:numMeasures
:gridWidth
:gridHeight
:offsetX="props.offsetX"
:offsetY="props.offsetY"
/>
Expand All @@ -16,9 +14,9 @@
<script setup lang="ts">
import { computed } from "vue";
import { inject } from "vue";
import { gridInfoInjectionKey } from "../ScoreSequencer.vue";
import Presentation from "./Presentation.vue";
import { useStore } from "@/store";
import { numMeasuresInjectionKey } from "@/components/Sing/ScoreSequencer.vue";
defineOptions({
name: "SequencerGrid",
Expand All @@ -29,12 +27,11 @@ const props = defineProps<{
offsetY: number;
}>();
const injectedValue = inject(gridInfoInjectionKey);
const injectedValue = inject(numMeasuresInjectionKey);
if (injectedValue == undefined) {
throw new Error("injectedValue is undefined.");
}
const { gridWidth, gridHeight } = injectedValue;
const { numMeasures } = injectedValue;
const store = useStore();
Expand All @@ -43,5 +40,4 @@ const timeSignatures = computed(() => store.state.timeSignatures);
const sequencerZoomX = computed(() => store.state.sequencerZoomX);
const sequencerZoomY = computed(() => store.state.sequencerZoomY);
const sequencerSnapType = computed(() => store.state.sequencerSnapType);
const numMeasures = computed(() => store.getters.SEQUENCER_NUM_MEASURES);
</script>
29 changes: 25 additions & 4 deletions src/components/Sing/SequencerGrid/Presentation.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,15 @@

<script setup lang="ts">
import { computed, toRef } from "vue";
import { getKeyBaseHeight, keyInfos, tickToBaseX } from "@/sing/viewHelper";
import {
getKeyBaseHeight,
getNumKeys,
keyInfos,
tickToBaseX,
} from "@/sing/viewHelper";
import { TimeSignature } from "@/store/type";
import { useSequencerGrid } from "@/composables/useSequencerGridPattern";
import { getNoteDuration } from "@/sing/domain";
import { getNoteDuration, measureNumberToTick } from "@/sing/domain";
const props = defineProps<{
tpqn: number;
Expand All @@ -124,12 +129,28 @@ const props = defineProps<{
sequencerZoomY: number;
sequencerSnapType: number;
numMeasures: number;
gridWidth: number;
gridHeight: number;
offsetX: number;
offsetY: number;
}>();
const endMeasureNumber = computed(() => props.numMeasures + 1);
const endTicks = computed(() => {
return measureNumberToTick(
endMeasureNumber.value,
props.timeSignatures,
props.tpqn,
);
});
const gridWidth = computed(() => {
return tickToBaseX(endTicks.value, props.tpqn) * props.sequencerZoomX;
});
const gridHeight = computed(() => {
return getKeyBaseHeight() * getNumKeys() * props.sequencerZoomY;
});
const gridCellWidth = computed(() => {
const snapTicks = getNoteDuration(props.sequencerSnapType, props.tpqn);
return tickToBaseX(snapTicks, props.tpqn) * props.sequencerZoomX;
Expand Down
2 changes: 0 additions & 2 deletions src/components/Sing/SequencerGrid/index.stories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ const meta: Meta<typeof Presentation> = {
tpqn: 480,
sequencerSnapType: 16,
numMeasures: 32,
gridWidth: 3840,
gridHeight: 2880,
offsetX: 0,
offsetY: 0,
},
Expand Down
35 changes: 31 additions & 4 deletions src/components/Sing/SequencerGridSpacer.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,42 @@
</template>
Copy link
Member

Choose a reason for hiding this comment

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

（ただのコメントです）

このコンポーネント今思ったらなくせそうですね！！
widhtとheight指定してdiv作ってるだけなので、ScoreSequencer.vue側でこんな感じで書けば良さそう？

<div style={
  width: `${gridWidth}px`,
  height: `${gridHeight}px`,
  "pointer-events": "none",
}/>

あるいはコンポーネント化するにしても、GridSpacerではなくただのSpacerとできそうだな～と。


<script setup lang="ts">
import { inject } from "vue";
import { gridInfoInjectionKey } from "./ScoreSequencer.vue";
import { computed, inject } from "vue";
import { getKeyBaseHeight, getNumKeys, tickToBaseX } from "@/sing/viewHelper";
import { useStore } from "@/store";
import { measureNumberToTick } from "@/sing/domain";
import { numMeasuresInjectionKey } from "@/components/Sing/ScoreSequencer.vue";
const injectedValue = inject(gridInfoInjectionKey);
const injectedValue = inject(numMeasuresInjectionKey);
if (injectedValue == undefined) {
throw new Error("injectedValue is undefined.");
}
const { numMeasures } = injectedValue;
const { gridWidth, gridHeight } = injectedValue;
const store = useStore();
const tpqn = computed(() => store.state.tpqn);
const timeSignatures = computed(() => store.state.timeSignatures);
const zoomX = computed(() => store.state.sequencerZoomX);
const zoomY = computed(() => store.state.sequencerZoomY);
const endMeasureNumber = computed(() => numMeasures.value + 1);
const endTicks = computed(() => {
return measureNumberToTick(
endMeasureNumber.value,
timeSignatures.value,
tpqn.value,
);
});
const gridWidth = computed(() => {
return tickToBaseX(endTicks.value, tpqn.value) * zoomX.value;
});
const gridHeight = computed(() => {
return getKeyBaseHeight() * getNumKeys() * zoomY.value;
});
Comment on lines +25 to +41
Copy link
Member

Choose a reason for hiding this comment

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

コードがコピペになっているのが気になりました！
https://github.com/sigprogramming/voicevox/blob/52700c33c963878c99e863f49d8d5bc1eeb0a1e4/src/components/Sing/SequencerGridSpacer.vue#L25-L41
https://github.com/sigprogramming/voicevox/blob/52700c33c963878c99e863f49d8d5bc1eeb0a1e4/src/components/Sing/SequencerGrid/Presentation.vue#L136-L153
この辺り多分整合性取れなくなりうるので、provideで与えるかどうかともかく、DRY原則に従って共通化していくのが良いかなと思いました！

ちなみにgridWidthを以前共通化を提案したときのコメントはこんな感じでした。
#2472 (comment)

Copy link
Contributor Author

@sigprogramming sigprogramming Mar 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

（まず前提として：Storybookでプロパティの値を変更してもSequencerGridが壊れないようにする）

データの流れ（依存関係）は
numMeasures→endMeasureNumber→endTicks→gridWidth
となっていて、numMeasuresが決まればgridWidthは一意に決まるので、整合性が取れなくなる（仕様がずれる）ことはたぶんないと思いますが、おっしゃる通りコードが重複しているので、endTicksまでを親コンポーネントで計算するのも良いかもです。
その場合、endTIcksのみをprovideする形になり、SequencerGridのnumMeasuresへの依存を無くす（描画ロジックを変更する）作業が必要になりそうです。
もしくはnumMeasures→endMeasureNumber→endTicksまでの算出値をオブジェクトでまとめて、そのオブジェクトを子コンポーネント（SequencerGridなど）に渡すという形もあると思います。

整理すると：

  • endTicksまでを親で計算し、子はendTicksを受け取る形
    • 算出ロジックを共通化できる
    • SequencerGridの描画ロジックを変更する必要がある
    • 子はnumMeasuresに依存しないように（endTicksに依存するように）しないといけない
  • endTicksまでを親で計算し、子はnumMeasures→endTicksまでの算出値（オブジェクト）を受け取る形
    • 算出ロジックを共通化できる
    • SequencerGridの描画ロジックの変更は必要ない
    • 子はnumMeasures（を含むオブジェクト）に依存できる
    • オブジェクト内の値の依存関係が分からなくなりそう
  • numMeasuresまでを親で計算し、子はnumMeasuresを受け取る形（このPR）
    • 変更されやすいのは、おそらくnumMeasures→gridWidthの変換処理ではなくnumMeasuresの算出処理なので、numMeasuresをprovideして、比較的安定している（関数化されている）numMeasures→gridWidthの変換処理を子コンポーネントで行う（少しの重複は許容する）という考え

</script>

<style scoped lang="scss">
Expand Down
43 changes: 22 additions & 21 deletions src/sing/domain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,9 @@ export function getTimeSignaturePositions(
return tsPositions;
}

/**
* tick位置に対応する小節番号（整数）を計算する。
*/
export function tickToMeasureNumber(
ticks: number,
timeSignatures: TimeSignature[],
Expand All @@ -210,6 +213,25 @@ export function tickToMeasureNumber(
return ts.measureNumber + Math.floor(ticksWithinTs / measureDuration);
}

/**
* 小節番号に対応するtick位置（小節の開始位置）を計算する。
*/
export function measureNumberToTick(
measureNumber: number,
timeSignatures: TimeSignature[],
tpqn: number,
) {
const tsPositions = getTimeSignaturePositions(timeSignatures, tpqn);
const tsIndex = timeSignatures.findLastIndex((value) => {
return measureNumber >= value.measureNumber;
});
const ts = timeSignatures[tsIndex];
const tsPosition = tsPositions[tsIndex];
const measureOffset = measureNumber - ts.measureNumber;
const measureDuration = getMeasureDuration(ts.beats, ts.beatType, tpqn);
return tsPosition + measureOffset * measureDuration;
}

// NOTE: 戻り値の単位はtick
export function getMeasureDuration(
beats: number,
Expand Down Expand Up @@ -256,26 +278,6 @@ export const ticksToMeasuresBeats = (
return { measures, beats };
};

export function getNumMeasures(
notes: Note[],
tempos: Tempo[],
timeSignatures: TimeSignature[],
tpqn: number,
) {
const tsPositions = getTimeSignaturePositions(timeSignatures, tpqn);
let maxTicks = 0;
const lastTsPosition = tsPositions[tsPositions.length - 1];
const lastTempoPosition = tempos[tempos.length - 1].position;
maxTicks = Math.max(maxTicks, lastTsPosition);
maxTicks = Math.max(maxTicks, lastTempoPosition);
if (notes.length > 0) {
const noteEndPositions = notes.map((note) => note.position + note.duration);
const lastNoteEndPosition = Math.max(...noteEndPositions);
maxTicks = Math.max(maxTicks, lastNoteEndPosition);
}
return tickToMeasureNumber(maxTicks, timeSignatures, tpqn);
}

// NOTE: 戻り値の単位はtick
export function getNoteDuration(noteType: number, tpqn: number) {
return (tpqn * 4) / noteType;
Expand Down Expand Up @@ -332,7 +334,6 @@ export const DEFAULT_TPQN = 480;
export const DEFAULT_BPM = 120;
export const DEFAULT_BEATS = 4;
export const DEFAULT_BEAT_TYPE = 4;
export const SEQUENCER_MIN_NUM_MEASURES = 32;

// マルチエンジン対応のために将来的に廃止予定で、利用は非推奨
export const DEPRECATED_DEFAULT_EDITOR_FRAME_RATE = 93.75;
Expand Down
5 changes: 5 additions & 0 deletions src/sing/viewHelper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { isMac } from "@/helpers/platform";
const BASE_X_PER_QUARTER_NOTE = 120;
const BASE_Y_PER_SEMITONE = 30;

export const SEQUENCER_MIN_NUM_MEASURES = 32;
export const ZOOM_X_MIN = 0.15;
export const ZOOM_X_MAX = 2;
export const ZOOM_X_STEP = 0.05;
Expand Down Expand Up @@ -101,6 +102,10 @@ export const keyInfos = Array.from({ length: 128 }, (_, noteNumber) => {
};
}).toReversed();

export const getNumKeys = () => {
return keyInfos.length;
};

export const getStyleDescription = (style: StyleInfo) => {
const description: string[] = [];
if (style.styleType === "talk") {
Expand Down
Loading
Loading