Fix empty MIDI clip drawing + overhaul mute/selection logic

[yohannd1]

Fixes #8460, by drawing empty MIDI clips the same way as non-empty MIDI clips. It was a matter of properly checking if the empty MIDI clip is in the Pattern Editor or the Song Editor. Additionally, this PR also does:

• rewriting of the calculation for the background (and sometimes foreground);
• minor code refactoring near the changed logic;
• add a virtual method Clip::isEmpty to signal when a clip is empty;

Before (left group unselected, right group selected):

After:

Suggested future work (noticed while working on this PR):

• fixedClips() should be properly documented, it's very confusing right now; didn't do this here because I didn't quite understand if its sole purpose is for clips in the Pattern Editor, but I'm guessing that is at least the main purpose, since we have bool displayPattern = fixedClips() || drawLegacyBB;.
• in ClipView::getColorForDisplay, there is an algorithm for calculating the selected color when using custom colors, but for the default clip color there is a dedicated selectedColor property that is read instead. Is that really needed? I imagine just using the algorithm for every case is just fine.
• some style properties might be unused now (namely selected/muted color).
• the mutedBackgroundColor property is only used as an... overlay?

[yohannd1]

The changes inside this if statement are not necessary, I'm just doing it because it's close enough to where I was touching and it was begging for a refactor :)

[yohannd1]

Okay I think this is done!

[Itreza2]

Tested and it works well !

The same coloring mechanism could be applied to the background of sample clips, for consistency.

[yohannd1]

The same coloring mechanism could be applied to the background of sample clips, for consistency.

Oh, I forgot about these! I thought I had tested them properly, but both them and automation clips are not working as I intended. I'll take a look into this.

[yohannd1]

I think it's okay now? Sample clips work very differently from others, so I didn't try to get as close, just I guess "semantically close".

[regulus79]

I tested it out, and I like it. I noticed the selected automation clip color and the default automation clip color feel almost too similar. Or maybe it's fine. The selection colors are definitely more consistent across clip types now, which is great.

[yohannd1]

Been experimenting with a darker overlay color. Maybe this is better? I've been trying it out and it seems to look fine.

[bratpeki]

Why was isEmpty added?

[bratpeki]

Why is there no implementation here?

[bratpeki]

My guess is that MIDI clips are never empty but turn into Melody clips (or whatever they're called) once empty? So it just implements the top abstract method which returns false.

Correct me if I'm wrong though!

[yohannd1]

Why is there no implementation here?

The implementation lives at MidiClip.cpp (it used to be the empty() method, but I renamed it... and I honestly don't remember why I did that; I think it was a byproduct of the way I changed things since the signature is slightly different and a vague memory of having two separate empty methods in an earlier implementation).

Correct me if I'm wrong though!

The implementation that always returns false is a "default" on the Clip in case any subclass does not want to implement it.

For example, PatternClip at the moment does not have its own dedicated isEmpty implementation. I think I made this because I don't quite know how all of them work, so I opted for a "has something until proven otherwise" approach. Plus, it seems there's code that uses the Clip class directly, and in that case an implementation for it is also needed.

"Proof" for the above note

Making these changes still yields an error. Since it's at link time I can't quite figure out where it's happening though.

diff --git a/include/Clip.h b/include/Clip.h
index b31da8663..c20e61a25 100644
--- a/include/Clip.h
+++ b/include/Clip.h
@@ -105,7 +105,7 @@ public:
        //! @brief Whether the clip is empty.
        //
        // Classes that inherit this one should override if they want to signal a clip is empty.
-       virtual bool isEmpty() const { return false; }
+       virtual bool isEmpty() const;

        //! @brief Set whether a clip has been resized yet by the user or the knife tool.
        //!
diff --git a/include/PatternClip.h b/include/PatternClip.h
index 0be217407..680b0ef53 100644
--- a/include/PatternClip.h
+++ b/include/PatternClip.h
@@ -40,6 +40,8 @@ public:
        PatternClip(Track* track);
        ~PatternClip() override = default;

+       bool isEmpty() const override { return false; };
+
        void saveSettings( QDomDocument & _doc, QDomElement & _parent ) override;
        void loadSettings( const QDomElement & _this ) override;
        inline QString nodeName() const override

[yohannd1]

Plus, it seems there's code that uses the Clip class directly, and in that case an implementation for it is also needed.

Somehow I forgot that the code that uses Clip::isEmpty is the one that I added here lol

I forgot to add = 0 to the default isEmpty(). I tested it here and it does compile but I don't know, but is it a good idea to make Clip an abstract class then?

[yohannd1]

Thanks CI for dying while installing packages <3

[yohannd1]

Also pushing two new color-related fixes here!
(Edit: part of the changes are right after this comment; I forgot to commit everything at once...)

1. Fix loading some old projects with int-based colors

Main example here is "Spoken" by unfa. See relevant discord discussion.

2. Improve visibility when using pitch-black clips

This was messed up because QColor::lighter is essentially a no-op when you have pitch black colors. This is a rare case but well, I already made the fix before thinking about it.

Before:

After (notice the outline):

[bratpeki]

Why was isEmpty added?

Asking again

[yohannd1]

Sorry! I think I forgot to reply?
It's used here to "darken" the clip when it's empty. More specifically, what used to refer to muted clips now refers to "muted OR empty" clips.

I honestly wish there was a better way to distinguish the two but I couldn't think of any, and this looks good enough for me.