jacquesh / foo_openlyrics

An open-source lyric display panel for foobar2000
MIT License
442 stars 26 forks source link

Option to disable autoscroll for non-synced lyrics #86

Open kamenminkov opened 2 years ago

kamenminkov commented 2 years ago

I think this should come by default, but it'd be nice to have a toggle for it. As it is now, if Scroll type is set to Automatic, even non-synced lyrics are being scrolled, which isn't an issue if the panel is tall enough, but if it's not (i.e. being just tall enough to fit the whole text), it would start off with the start in the middle and gradually scroll to the top, resulting in that not the whole text is visible.

Now that I think of it, it would make sense even for synced lyrics to stay fixed if there is enough height.

anemochore commented 2 years ago

+1. Lyric Show 3 provides manual scrolling by drag-and-drop (regarlessly with synced) and we want the feature.

jacquesh commented 2 years ago

Just to be sure, the behaviour you want is just for it to not scroll if all the lyrics would fit on the panel at once, is that correct?

What would you want to happen if the lyrics are 1 line too tall (assuming vertical scrolling) for synced lyrics? Would it scroll just one line over the course of the entire track so that the first line is right at the top of the panel at the start of the track when its highlighted, and then the text scrolls by very slowly just enough for the bottom line to be right at the bottom of the panel at the end of the track when its highlighted? Or would it switch to being centered as it is now?

kamenminkov commented 2 years ago

To preface the problem, I've only set Scroll type: Automatic because this seems to be the only way to highlight the current line for synced lyrics (setting Scroll type: Manual disables the highlighting as well unfortunately). This should probably go into another issue (unless you're aware of it already).

My main concern is when there's enough height in the pane and yet non-synced lyrics insist on scrolling. Say you have 60 lines of lyrics and the pane is tall enough to contain 70. What happens is that if you enable automatic scrolling, at the beginning of the song there's only the first half of the text visible, and towards the end of the song only the second half is visible. Screenshots to illustrate (with an intentionally small window that barely fits the lyrics - but this also happens with larger windows and longer lyrics) - first one with Scroll type: Automatic, second one - Manual.

Screenshot 2022-01-20 111752 Screenshot 2022-01-20 111812

jacquesh commented 2 years ago

Yeah ok that's what I thought you meant.

Its come up before, I'll look into it. The implementation is complicated a little by the fact that whatever we do needs to make sense in the larger context of other options and its not necessarily clear how this change maps to synced lyrics (as mentioned above). It'd also need to apply to horizontal scrolling, although presumably there can be an analogous change there (don't scroll if you don't need to).

I suppose we could look to LSP3/ESLyric for inspiration. Do you know what they do?

kamenminkov commented 2 years ago

LSP3

Yeah, I used to use it - and I'm pretty happy with its behaviour regarding what we discuss here - but it lacks in automatic searching, sources and so on (it seems to be out of active development, one of the reasons I tried OpenLyrics to begin with).

Shamtam commented 2 years ago

LSP3 would still allow manual scrolling "override," even when set to automatic scroll mode.

My preferred behavior would be the following: if scrolling is initiated when in automatic scroll mode, "temporarily" change to manual scroll mode (this would be reset when loading a new track/lyrics, or if requested via e.g. context menu or pressing Esc). From my memory, this is more or less how LSP3 worked. This solves the main problem for me: when I get lyrics that are not sync'd, then I can still scroll to where I want to be without needing to open up the options menu and switch to manual scrolling.

The other option I could think of is to independently choose scroll type based on whether lyrics are sync'd or not.