Samillion / ModernZ

A sleek and modern OSC for mpv designed to enhance functionality by adding more features, all while preserving the core standards of mpv's OSC.
GNU Lesser General Public License v2.1
53 stars 5 forks source link

Minor change to the close button #191

Closed Keith94 closed 1 week ago

Keith94 commented 1 week ago

The width of the close (X) button is such that if you throw your mouse to the right edge/corner of the screen, the button is not clickable.

I found that changing the width from w = 40 to w = 49 improves on that to behave more like Windows.

https://github.com/Samillion/ModernZ/blob/aa862c2802016e24f67524feffd30890aaf87b20/modernz.lua#L1489

Let me know your thoughts.

Samillion commented 1 week ago

Really appreciate this report, it made me realize there are a few issues to fix with window controls. Each height and width should be 50, and the window controls width should be 150, not 138.

Will fix.

Samillion commented 1 week ago

Could you please confirm that it is fixed here: https://github.com/Samillion/ModernZ/blob/dev_windowcontrols_fixdim/modernz.lua

Changes:

Keith94 commented 1 week ago

Awesome thanks man. Will update you in ~4ish hours when I'm home.

Samillion commented 1 week ago

Take all the time you need.

Keith94 commented 1 week ago

Using scalewindowed=0.9 with maximized window there's one pixel where the buttons are hovered at the same time.

image

Samillion commented 1 week ago

Hmmm, are you sure scalewindowed is the cause? I can't reproduce

https://github.com/user-attachments/assets/aa4bf5d7-3a38-4fda-9410-aa47e9d2ba01

Keith94 commented 1 week ago

Wait sorry it has nothing to do with scalewindowed because it happens with default settings.

Sadly I don't know why it happens for me. I can open a new issue if I find out.

However on a positive note, the original issue is fixed now. Can merge. 👍

Samillion commented 1 week ago

No worries, we can keep it in the same issue since it's about fixing window controls.

I'm wondering if screen resolution is the issue. I run at 1920x1080. Do you run a 2k/4k+?

Keith94 commented 1 week ago

Yup I run at 1920x1080 too at 100% scale.

Samillion commented 1 week ago

Strange. If you ever have the time, could you fiddle around with the values here to see? It's probably a 1 pixel difference.

    local second_geo = {x = controlbox_left + 75, y = button_y, an = 5, w = 50, h = wc_geo.h}

Specifically the x = controlbox_left + 75, try changing the 75 to 76

Until I figure out what's causing the discrepancy

Keith94 commented 1 week ago

Changing it to 76 basically creates a gap between the two. I can play around with it more another day.

It also happens between the restore and close buttons. Oddly when the window is restored, the buttons don't act like this.

https://github.com/user-attachments/assets/aa17d7d5-960d-45e5-89e1-9955ef91bccd

Samillion commented 1 week ago

This is most likely because the window un-maximize is about 2 pixels bigger than maximize.

https://github.com/Samillion/ModernZ/blob/dev_windowcontrols_fixdim/modernz.lua

Keith94 commented 1 week ago

49 didn't work well, but after a few attempts these values work optimally for my case

    local first_geo = {x = controlbox_left + 25, y = button_y, an = 5, w = 50, h = wc_geo.h}
    local second_geo = {x = controlbox_left + 75, y = button_y, an = 5, w = 49, h = wc_geo.h}
    local third_geo = {x = controlbox_left + 125, y = button_y, an = 5, w = 51, h = wc_geo.h}
Samillion commented 1 week ago

In theory, this would work as well?

    local first_geo = {x = controlbox_left + 25, y = button_y, an = 5, w = 50, h = wc_geo.h}
    local second_geo = {x = controlbox_left + 75, y = button_y, an = 5, w = 48, h = wc_geo.h}
    local third_geo = {x = controlbox_left + 125, y = button_y, an = 5, w = 50, h = wc_geo.h}
Keith94 commented 1 week ago

Not ideally.. with that change there is always a 1 pixel gap between the middle and outer buttons.

Samillion commented 1 week ago

Lets account for both shapes of the maximized button in all states, how about this:

    local second_geo = {x = controlbox_left + 75, y = button_y, an = 5, w = (state.maximized or state.fullscreen) and 48 or 50, h = wc_geo.h}
Samillion commented 1 week ago

Or, if there is still a gap on fullscreen/maximized, make it 49 instead of 48

    local second_geo = {x = controlbox_left + 75, y = button_y, an = 5, w = (state.maximized or state.fullscreen) and 49 or 50, h = wc_geo.h}

Let me know which one works better, whenever you can, please. Take all the time you need.

Keith94 commented 1 week ago

48 or 50 looks like [min] (gap) [restore] (gap) [close]

49 or 50 looks like [min] [restore] (gap) [close]

So there is just that one pesky gap remaining lol.

Samillion commented 1 week ago

That's what's puzzling me. The gap is because it's not 50 lol. But when you do make it 50, for you it overlaps for some reason.

Since it's a 1 pixel gap, is the 49/50 the best scenario here?

Keith94 commented 1 week ago

Wanna know the weirdest thing?

When I click on the actual maximize button, the bug doesn't happen. But if I double click on the client to toggle fullscreen, the bug happens.

Any possible chance you can reproduce that? Using simple values of 50,50,50

Keith94 commented 1 week ago

I have a feeling this is due to how the scaling works in fullscreen vs nonfullscreen? Because when I toggle fullscreen the elements look janky when they're getting "resized/scaled" into place:

https://github.com/user-attachments/assets/a767df3e-940e-4984-96bf-9705c0d6dae2

But when maximizing/restoring it, elements look completely smooth. No jank with window controls either.

Samillion commented 1 week ago

Very strange. Not sure what's the best route here, I can merge for now until we figure out the actual cause, since it's not a function breaking issue, just a 1 pixel gap.

However, I can't decide whether to leave them all 50

Or make them 50, except for maximize would be 49/50

Keith94 commented 1 week ago

I would say leave them all 50 for now.

Samillion commented 1 week ago

When I click on the actual maximize button, the bug doesn't happen. But if I double click on the client to toggle fullscreen, the bug happens.

I bet this is a win32 bug in mpv like the one we faced before with the maximize behavior adding two pixels. This one is adding one in that specific state. I'll mention it to kasper later on.

Keith94 commented 1 week ago

It could be the case honestly. Thanks for mentioning it.

Stock window controls don't change color on hover so it's hard to say. I don't have access to other platforms either; my testing is limited :(