search

AudioKit / Keyboard

SwiftUI music keyboard
MIT License
137 stars 25 forks source link

Allow customizing piano spacing #21

Closed josephktcheung closed 1 year ago

josephktcheung commented 1 year ago

This PR is to allow library user to define their own piano key spacing.

This is achieved by adding initialSpacerRatio, spacerRatio and relativeBlackKeyWidth to customize the piano key spacing.

josephktcheung commented 1 year ago

@aure please review, thanks.

wtholliday commented 1 year ago

Does this need to be a protocol? Could it simply be an array or dictionary of offsets instead?

Should the PianoSpacerProtocol be in a separate file? (that might improve the diff too)

josephktcheung commented 1 year ago

Does this need to be a protocol? Could it simply be an array or dictionary of offsets instead?

We need to reuse some existing methods / computed properties to help calculating spacing e.g. pitchRangeBoundedByNaturals.

Besides space(pitch: Pitch) -> CGFloat is a function https://github.com/AudioKit/Keyboard/pull/21/files#diff-ad016a649df2351705bd46700b71cb2b10df7ecd735faafdaca811d5f2a23673R46-R54, which I think may not be that easy to replace it with dictionary of offsets. User may want to specify space per pitch, and they may want to have their own way to calculate it using the pitch parameter.

josephktcheung commented 1 year ago

Should the PianoSpacerProtocol be in a separate file? (that might improve the diff too)

I've moved it to a new file. Please check.

wtholliday commented 1 year ago

Besides space(pitch: Pitch) -> CGFloat is a function https://github.com/AudioKit/Keyboard/pull/21/files#diff-ad016a649df2351705bd46700b71cb2b10df7ecd735faafdaca811d5f2a23673R46-R54, which I think may not be that easy to replace it with dictionary of offsets. User may want to specify space per pitch, and they may want to have their own way to calculate it using the pitch parameter.

There are only 128 pitches (MIDI note numbers), so assuming this function is a pure function of pitch, then it can be represented by a table of 128 values. It seems the protocol could just be a struct which is generated by some function.

josephktcheung commented 1 year ago

Besides space(pitch: Pitch) -> CGFloat is a function https://github.com/AudioKit/Keyboard/pull/21/files#diff-ad016a649df2351705bd46700b71cb2b10df7ecd735faafdaca811d5f2a23673R46-R54, which I think may not be that easy to replace it with dictionary of offsets. User may want to specify space per pitch, and they may want to have their own way to calculate it using the pitch parameter.

There are only 128 pitches (MIDI note numbers), so assuming this function is a pure function of pitch, then it can be represented by a table of 128 values. It seems the protocol could just be a struct which is generated by some function.

@wtholliday, I've updated API to further limit the scope end user can modify. They can pass different ratio dictionaries to piano and vertical piano layout to change the spacing. Please check thanks.