PKBeam / Edda

A beatmap editor for the VR rhythm game Ragnarock
GNU General Public License v3.0
34 stars 8 forks source link

Spectrogram UX improvements #40

Closed Brollyy closed 1 year ago

Brollyy commented 1 year ago

24

Added a way to resize the column with spectrogram to get both a better look at the image and to allow users to drag it closer to the note editor to check their alignment.

Added caching for spectrogram to make sure we don't re-do expensive calculations unnecessarily when opening a map again or resizing the spectrogram.

Added user settings for customizing the spectrogram, including algorithm used, frequency range and colormap (previously it was always MelScale, 16kHZ, Blues). I've set the defaults to Standard, 11kHz, which seems about the same settings that are used by MMA2 for their spectrogram. I've left Blues as the default colormap for theming, but after testing a few I actually found I prefer Lopora more and I saw that MMA2 uses Plasma.

image

White bar allows user to change the width of the spectrogram by dragging it: image

New section in user settings (only displays after user enables spectrogram): image

Using Standard with 11kHz takes a little longer to load initially, but provides much higher resolution, especially with the ability to resize: image

Brollyy commented 1 year ago

I still have to test some edge cases for performance (e.g. rapidly changing settings) and waiting on feedback from Waatsu after the last change with the flip. Will let you know once it's ready to be merged.

Nytilde commented 1 year ago

it's no good practise to add something lager than one line within a try-catch code block. just as my recommendation: i would either outsource those parts into a function or apply try-catch only to the part where the error can actually occur.

Brollyy commented 1 year ago

I've implemented option to load the spectrogram in chunks to support longer maps and done some code refactoring for readability in VorbisSpectrogramGenerator.

I've tested all of those changes myself, so it should be fine to merge if you don't see any issues - if anything comes up during beta that I didn't find, I'll fix it then.