Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SlicerT: improve mouse pointer logic with related visual enhancement #7711

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dirk-de-bugger
Copy link

Hello,

I provide this patch in hope of it being usefull.

The loop in SlicerT's SlicerTWaveform::updateClosest currently picks the last slice point in range of the mouse pointer. This change makes it pick the closest one. Since (as fr as I observed) the list is sorted the loop can be exited if the distance starts to get bigger again.

I also find it hard to see the difference between light gray and white I also changed the highlight color to a more distinct one. (I need to figure out how to detect when the mouse leaves the area to clear the highlight.)

Please excuse any indentation issues. I made more changes (currently for my own benefit) and created this branch/PR with the Github online editor.

Regards,
Dirk

@qnebra qnebra self-requested a review February 20, 2025 21:29
Copy link
Collaborator

@qnebra qnebra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Color change looks good and have more contrast than previous color, despite worries of author it properly clears to default.

Regardless changes in workflow, I don't have issues and slices at cursor centre are higlighted, not those on right edge of cursor. So, it fixes minor annoyance with SlicerT, got better colors.

So, approved after functional review. Will leave code review to someone with better understanding of code.

@RoxasKH
Copy link
Contributor

RoxasKH commented Feb 20, 2025

Can we have a screenshot? Just for reference
Looking at the code i'm assuming the bar gets yellow when selecting a slice marker

Honestly in the design i made at that time i didn't even know there was a marker selected state

@qnebra
Copy link
Collaborator

qnebra commented Feb 20, 2025

Can we have a screenshot? Just for reference Looking at the code i'm assuming the bar gets yellow when selecting a slice marker

It is red. QColor use RGB values, judging from code and highlighted line being red in test.

@dirk-de-bugger
Copy link
Author

Thank you for considering my change.
This is what it looks like:
grafik

@RoxasKH
Copy link
Contributor

RoxasKH commented Feb 21, 2025

Can we have a screenshot? Just for reference Looking at the code i'm assuming the bar gets yellow when selecting a slice marker

It is red. QColor use RGB values, judging from code and highlighted line being red in test.

My bad, i meant red, i misread it as 255, 255, 0 someway. That red is quite a strong color (just like any "ms paint" color) and out of LMMS palette (mentioned here #3534). Not that the purple tones i used in SlicerT are, but it might be a good idea to tone it down a bit, while maintaning a good contrast, or find some other color that fits.

Examples of a slightly toned down red or orange. I've tried shades of green/blue too but they just don't match well and yellow is definitely too bright so the intuition here is right.

Red Orange
slicer selected red slicer selected

Honestly speaking tho, it's the first time i think i see a selected marker state on a slicer. I believe usually the slice is the subject of selection, i'm wondering what's the need for the last indicator that has been touched to be recognizable.

@RoxasKH
Copy link
Contributor

RoxasKH commented Feb 21, 2025

Thank you for considering my change. This is what it looks like: grafik

Whoops, thanks a lot for the screenshot, i was writing my answer while you posted yours, so i missed it, i appreciate it

@dirk-de-bugger
Copy link
Author

I made the red a little darker
static QColor s_sliceHighlightColor = QColor(200, 50, 50);
and added handling of the leaveEvent, so to remove the highlight when the mouse pointer leaves the waveform area:

Bildschirmaufnahme.2025-02-22.221929.mp4

Should I add the changes to my branch of this PR or create a new PR?

void SlicerTWaveform::leaveEvent(QEvent *event)
{
    m_closestObject = UIObjects::Nothing;
    m_closestSlice = -1;
    updateCursor();
    drawSeeker();
    drawEditor();
    update();
}

@qnebra
Copy link
Collaborator

qnebra commented Feb 23, 2025

Or just add as new commit, then maybe change pull request name to something more general but still about changes in SlicerT.
I don't know, something like maybe
"SlicerT: visual fixes, mouse pointer logic changes"

@dirk-de-bugger dirk-de-bugger changed the title Fix: SlicerT selects last slice point in range of mouse pointer not the closest to it SlicerT: improve mouse pointer logic with related visual enhancement Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants