Window dragging setting

[MvRens]

Instead of creating a ticket for this personal minor annoyance I had a go at compiling and contributing to MPC-HC. Not sure if it's interesting enough for the main release, but here's a pull request in case you feel it is up to standard.

The changes:
As I'm always accidentally moving the window instead of seeking, which also restores it from it's maximized (non-fullscreen) state, I've added a setting under Advanced which controls the ability to drag the main window on client area controls (video panel, toolbar, infobar, statusbar). It's enabled by default to be backwards compatible.

[XhmikosR]

Thanks for the PR!

We are in the process of making a new team from the people who stepped up.

/CC @mpc-hc/developers-new for review

PS. The resource files need to be updated too.

[MvRens]

Thanks, I found the mpcresources BuildAll step I missed before. Was building it and committing the changed po's all that's required?

[matthewmauger]

Looks pretty reasonable to me.

Without wishing to be pedantic bClientDragWindow is quite confusing, can we call it bDisableClientDragOnWindow or similar and have it default to false - would that interfere with backwards compatibility in any way?

@XhmikosR - can you confirm the resource stuff is done correctly?

[clsid2]

The inverse option might be a bit less confusing. Something like
bDragWindowTitlebarOnly

Resource stuff looks ok.

Perhaps the window drag code segment in CPlayerSeekBar::OnLButtonDown could be removed entirely? I am not able to trigger a drag in the seekbar area.

There have been suggestions in the past to make the bottom part of the video surface insensitive to mouse clicks/drags. That would also solve your problem. It would also solve accidental pause (something that happens to me).

[XhmikosR]

The resource changes should be good, yeah. Just make sure the build runs fine for the main program and translations too.

[MvRens]

I do like the idea of having a bit of the bottom part of the video excluded. Maybe just having that and the toolbar (or the top bit of it) is enough to prevent most of the accidental pauses and drags, instead of my nuclear option of disabling it entirely.

As for the naming, it's never pedantic in my opinion as code is read far more often than it's written :-). It can indeed use a few more words in the variable to be clear. As for why I chose to use "enable" with a default of true instead of "disable": I accounted for this eventually being a proper checkbox instead of an advanced setting, where some UX guidelines prefer positive wording if not confusing (plus it avoids a double negative in code as well).

I don't have any time to work on this in the upcoming week, but I'd be happy to experiment with the insensitivity idea after that if nobody beats me to it!

[clsid2]

No hurry ;)

XhmikosR, what do you think about the insensitivity idea? Imo, that could be enabled by default. An option might not even be needed initially and could always be added at later stage if there are user requests.

A 'dead' area of about max 1,5x or 2x seekbar height (DPI scaled) would probably be enough. When implementing, don't forget to test full screen, as seekbar is positioned differently in that case.

[XhmikosR]

I'm not sure I understand the issue completely to be honest. A test build or a video demonstrating the issue would help me a lot.

Note that I have many things in my plate and I'm trying to find time for everything and I don't have as much time as I had before for mpc-hc. That is why I want to find time to organize things a bit and get them rolling.

[kasper93]

A 'dead' area of about max 1,5x or 2x seekbar height (DPI scaled) would probably be enough. When implementing, don't forget to test full screen, as seekbar is positioned differently in that case.

This might work, but maybe seek bar is just too small? @PsychoMark Could you play around with SAFE_ZONE and tell what value fixes the issue for you.

diff --git a/src/mpc-hc/MainFrm.cpp b/src/mpc-hc/MainFrm.cpp
index 174b86a7e..8958d6191 100644
--- a/src/mpc-hc/MainFrm.cpp
+++ b/src/mpc-hc/MainFrm.cpp
@@ -16213,6 +16213,18 @@ LRESULT CMainFrame::WindowProc(UINT message, WPARAM wParam, LPARAM lParam)
         return 0;
     }
 
+#define SAFE_ZONE 5
+    if (message == WM_NCLBUTTONDOWN) {
+        CRect r;
+        CPoint pt;
+        m_wndSeekBar.GetWindowRect(r);
+        POINTSTOPOINT(pt, lParam);
+        r.InflateRect(SAFE_ZONE, SAFE_ZONE);
+
+        if (r.PtInRect(pt))
+            return 0;
+    }
+
     LRESULT ret = 0;
     bool bCallOurProc = true;
     if (m_pMVRSR) {
diff --git a/src/mpc-hc/PlayerInfoBar.cpp b/src/mpc-hc/PlayerInfoBar.cpp
index 250d7a727..c3e60bc44 100644
--- a/src/mpc-hc/PlayerInfoBar.cpp
+++ b/src/mpc-hc/PlayerInfoBar.cpp
@@ -250,7 +250,7 @@ void CPlayerInfoBar::OnLButtonDown(UINT nFlags, CPoint point)
 {
     CMainFrame* pFrame = ((CMainFrame*)GetParentFrame());
     if (!pFrame->m_fFullScreen) {
-        MapWindowPoints(pFrame, &point, 1);
+        ClientToScreen(&point);
         pFrame->PostMessage(WM_NCLBUTTONDOWN, HTCAPTION, MAKELPARAM(point.x, point.y));
     }
 }
diff --git a/src/mpc-hc/PlayerSeekBar.cpp b/src/mpc-hc/PlayerSeekBar.cpp
index 57c1a5476..f4c9a8b5b 100644
--- a/src/mpc-hc/PlayerSeekBar.cpp
+++ b/src/mpc-hc/PlayerSeekBar.cpp
@@ -549,7 +549,7 @@ void CPlayerSeekBar::OnLButtonDown(UINT nFlags, CPoint point)
         VERIFY(SetTimer(TIMER_HOVER_CAPTURED, HOVER_CAPTURED_TIMEOUT, nullptr));
     } else {
         if (!m_pMainFrame->m_fFullScreen) {
-            MapWindowPoints(m_pMainFrame, &point, 1);
+            ClientToScreen(&point);
             m_pMainFrame->PostMessage(WM_NCLBUTTONDOWN, HTCAPTION, MAKELPARAM(point.x, point.y));
         }
     }
diff --git a/src/mpc-hc/PlayerStatusBar.cpp b/src/mpc-hc/PlayerStatusBar.cpp
index 3f4404980..98ac6b572 100644
--- a/src/mpc-hc/PlayerStatusBar.cpp
+++ b/src/mpc-hc/PlayerStatusBar.cpp
@@ -437,9 +437,10 @@ void CPlayerStatusBar::OnLButtonDown(UINT nFlags, CPoint point)
     } else if (!pFrame->m_fFullScreen && wp.showCmd != SW_SHOWMAXIMIZED) {
         CRect r;
         GetClientRect(r);
+        ClientToScreen(&r);
         CPoint p = point;
 
-        MapWindowPoints(pFrame, &point, 1);
+        ClientToScreen(&point);
 
         pFrame->PostMessage(WM_NCLBUTTONDOWN,
                             (p.x >= r.Width() - r.Height() && !pFrame->IsCaptionHidden()) ? HTBOTTOMRIGHT :
diff --git a/src/mpc-hc/PlayerToolBar.cpp b/src/mpc-hc/PlayerToolBar.cpp
index ac408e253..bf8d9a168 100644
--- a/src/mpc-hc/PlayerToolBar.cpp
+++ b/src/mpc-hc/PlayerToolBar.cpp
@@ -387,7 +387,7 @@ void CPlayerToolBar::OnLButtonDown(UINT nFlags, CPoint point)
     int i = getHitButtonIdx(point);
 
     if (!m_pMainFrame->m_fFullScreen && (i < 0 || (GetButtonStyle(i) & (TBBS_SEPARATOR | TBBS_DISABLED)))) {
-        MapWindowPoints(m_pMainFrame, &point, 1);
+        ClientToScreen(&point);
         m_pMainFrame->PostMessage(WM_NCLBUTTONDOWN, HTCAPTION, MAKELPARAM(point.x, point.y));
     } else {
         __super::OnLButtonDown(nFlags, point);

[clsid2]

A small increase in seekbar height would be fine with me. Personally, when I miss-click it is usually just a few pixels above it.

[kasper93]

Or instead could snap mouse pointer to seekbar if someone click close enough.

[clsid2]

That would be even better.

[kasper93]

Any news? We could marge current settings and work on better solution in the future if someone wants to work on it. It is hard for me to find best solution because I'm not affected by the problem.

[MvRens]

Just got back and tested your safe_zone modification @kasper93; it's definitely better with 5 and completely resolved for me with a safe zone of 15.

I personally prefer not responding at all (this safe/dead zone) to just having a larger seekbar area, because I'll eventually drift down (due to low mouse sensitivity and my arm moving away from me) and end up with the same problem. When it's not responding it's a natural cue to move up a bit. And snapping should only be done when the cursor is hidden imo.

So as far as I'm concerned I prefer the safe zone to my changes!