fix: prevent album art rendering when modal is open

[soifou]

Problem:

• Album art rerendering could overlap open modals, making content unreadable
• This occurred with events like UiEvent::SongChanged, UiEvent::Reconnected,
  and AppEvent::Resized while a modal was open on the queue list

Solution:

• Add an is_modal_open flag to AppContext to track modal state
• Implement checks to prevent album art rendering when a modal is open
• Defer album art updates until modal is closed

[soifou]

I've not found an easy way to know whether a modal is open hence the new flag in the context. I've tried to keep it simple while refactoring the match arms a bit (those context.render() calls were unnecessary, right?).

There's room for optimization here. Now, rmpc attempts to fetch the album art every time the modal closes, as the song might have changed in the meantime (potentially bringing up a new album). Instead of immediately rendering the previous album art, we could consider implementing a more efficient approach.

Do you see any other areas where we could optimize the album art handling?
Thought?

[mierak]

Its always something with the album arts...

I've not found an easy way to know whether a modal is open hence the new flag in the context. I've tried to keep it simple while refactoring the match arms a bit (those context.render() calls were unnecessary, right?).

Cant we simply track it inside the album pane struct instead of context? The rest of the application does not care about modal being opened I think and we can always move it back to the context if needed.

Otherwise it looks fine to me, thanks!

Now, rmpc attempts to fetch the album art every time the modal closes, as the song might have changed in the meantime (potentially bringing up a new album). Instead of immediately rendering the previous album art, we could consider implementing a more efficient approach.

One thing that comes to mind is keeping track of whether the song changed while the modal was opened. If it did not we can render the old image, otherwise fetch the new one.

edit:

those context.render() calls were unnecessary, right?

Theres a bunch of those, yeah. They are a no-op when render was already requested for that frame so I am pretty liberal with them.

[soifou]

Cant we simply track it inside the album pane struct instead of context?

Absolutely, done.

One thing that comes to mind is keeping track of whether the song changed while the modal was opened. If it did not we can render the old image, otherwise fetch the new one.

I thought about this too but did not want to overcomplicated the logic at first, done now.

I am pretty liberal with them.

Ok fine to know :) I thought it was an oversight

[mierak]

merged, thanks!