clsid2 / mpc-hc

Media Player Classic
GNU General Public License v3.0
11.36k stars 499 forks source link

Hide Windowed Controls related issues #2966

Closed xLn2 closed 3 months ago

xLn2 commented 3 months ago
  1. Playlist with Hide on Fullscreen with combination Hide Windowed Controls cause gleam for around 3 seconds. (When MPCHC is switched Fullscreen to Windowed mode) // When Hide on Fullscreen disabled, there is no undesired gleam, might be related to this.

  2. Auto Hide Windowed Controls + Capture settings opening dropdown menu causes hiding capture panel

mpc-hc64_4IxJv3IbjG

  1. Auto Hide Windowed Controls + Capture settings opening Dialogbox via ... Button causes hiding capture panel

  2. When editing a Playlist item, the mouse leaves the relevant region, causing a visual defect.

mpc-hc64_p5ke5Wkhoh

  1. Control bar is not hiding when Rendering FPS is 0 (I guess). Related issue https://github.com/clsid2/mpc-hc/issues/2960

353243887-7670a598-2911-4fe7-99f7-be50336cf917

adipose commented 3 months ago
  1. https://github.com/clsid2/mpc-hc/pull/2977
  2. I'm not sure I care. Once you close the popup dialog, your mouse will be out of hover most likely, at which point it will again hide itself. So if we somehow keep it open while the dialog is open, it will close anyway.
adipose commented 3 months ago

@clsid2 , could use some guidance. The edits and buttons (checkboxes, too) can have focus when auto-hiding. They retain focus even while it is hidden. You can continue typing or use <space> to actually press buttons or change checkboxes.

My thought is, if there is a relevant focus (such as: you are typing in an edit field), do not hide the bar. A less relevant focus is probably: a button/checkbox/combobox is selected. In those cases I suggest we remove the focus to prevent accident keystrokes on a hidden window.

clsid2 commented 3 months ago

Yes, that sound logical.

xLn2 commented 3 months ago
  1. I'm not sure I care. Once you close the popup dialog, your mouse will be out of hover most likely, at which point it will again hide itself. So if we somehow keep it open while the dialog is open, it will close anyway.

I agree that it seems illogical, but I believe that it can be equally logical to keep it on the screen because it is a continuation of a feature of the panels. Even if it is not in the relevant context, it feels like a functional behavior that the Playlist continues to appear while the dialog box is on the screen when clicking Playlist > save list.

adipose commented 3 months ago
  1. fixed by https://github.com/clsid2/mpc-hc/pull/2979
adipose commented 3 months ago
  1. I'm not sure I care. Once you close the popup dialog, your mouse will be out of hover most likely, at which point it will again hide itself. So if we somehow keep it open while the dialog is open, it will close anyway.

I agree that it seems illogical, but I believe that it can be equally logical to keep it on the screen because it is a continuation of a feature of the panels. Even if it is not in the relevant context, it feels like a functional behavior that the Playlist continues to appear while the dialog box is on the screen when clicking Playlist > save list.

You're not wrong...but if I fix the issue it will just expose a different one. Perhaps a "better" solution is to turn off auto-hide for that toolbar when the filedialog is opened, and restore it after, because trying to detect a child file dialog may be a pain.

adipose commented 3 months ago
  1. https://github.com/clsid2/mpc-hc/pull/2979

It now keeps it open during file dialog, and sets the focus back to the file name edit afterwards. This is a bit of a hack to keep it open, because focused edits now keep it open. To do better, I'd need to have a way to flag it as needing to stay open and then clear it on mouse entry.

adipose commented 3 months ago
  1. Unable to reproduce this. I tried an audio file with art, but it didn't seem to have the same problem.
xLn2 commented 3 months ago
  1. Unable to reproduce this. I tried an audio file with art, but it didn't seem to have the same problem.

If you have a DVD5/9 sample file containing photo album, it can be reproducible. But I don't know origin of problem is same or not.

mpc-hc64_bXABRrbNb6

adipose commented 3 months ago

DVD5/9 sample file containing photo album

What is that?

clsid2 commented 3 months ago

It requires a repaint.

adipose commented 3 months ago

I tried dvd menu and mp3 with art but they all work fine.

Not sure what photo album on DVD is. I can try to download one.

xLn2 commented 3 months ago

It doesn't reproducible on mp3 with cover. DVD with photo album that has still image (no animation anywhere on video frame) need some triggers (like resizing window or hover on buttons) to refresh video frame and mpchc bars/panels. In that case minimizing the player window makes video frame all black color and needs some --repaint-- triggers. I think that the reason filters like MPC-Image source appear to process at 0.5 fps may be to prevent such behavior.

adipose commented 3 months ago

It doesn't reproducible on mp3 with cover. DVD with photo album that has still image (no animation anywhere on video frame) need some triggers (like resizing window or hover on buttons) to refresh video frame and mpchc bars/panels. In that case minimizing the player window makes video frame all black color and needs some --repaint-- triggers. I think that the reason filters like MPC-Image source appear to process at 0.5 fps may be to prevent such behavior.

I am not sure what is meant by "photo album" but DVD menus often are static but I couldn't reproduce.

xLn2 commented 3 months ago

"Photo album or Picture Gallery" is a DVD menu, which is usually considered extra content, contains some still images from the film and sometimes behind-the-scenes footage, and is easy to navigate using auxiliary buttons such as the left and right arrows, which are placed statically.

Unless there is something that needs to be drawn, the renderer will not render the frame, so with auto hidden panels leaving a black smear of panels overlaying the video area.

Reproducible results can be obtained from the file in the link at my previous comments. If you have trouble downloading, I can upload it to my google drive, but it may take some time.

adipose commented 3 months ago

Thanks, I can reproduce it now, you can remove your base64.

Please close this issue unless another problem remains, as this is not a docking toolbar issue.

adipose commented 3 months ago

https://github.com/clsid2/mpc-hc/issues/2985

xLn2 commented 3 months ago

The main issue in # 5 is that the control bar does not try to hide like the playlist panel. It is better to examine it in a separate issue since there are other related problems.

Only issue at # 1 remains..

adipose commented 3 months ago

If using mpcvr, does issue 5 disappear?

xLn2 commented 3 months ago

If using mpcvr, does issue 5 disappear?

Negative.

adipose commented 3 months ago

If using mpcvr, does issue 5 disappear?

Negative.

Can you redo your video for 5 with mpcvr?

xLn2 commented 3 months ago

I can redo resim

adipose commented 3 months ago

Well, when I switch to MPC-VR the problem completely goes away. Not sure why it would be different for you.

adipose commented 3 months ago

Can you use a different audio renderer than sanear as well? The dvd issue we had before was related to audio.

xLn2 commented 3 months ago

Resetting MPC-VR settings probably solve the issue. Even every setting looks same, it solved my issue.

adipose commented 3 months ago

For issue 1 can you do a video as well. Maybe using a phone if necessary.

adipose commented 3 months ago

Resetting MPC-VR settings probably solve the issue. Even every setting looks same, it solved my issue.

Hmm DX9 v DX11?

xLn2 commented 3 months ago
DirectX 11
Graphics adapter: Intel(R) HD Graphics 630 (8086:591B)
VideoProcessor  : D3D11, RateConversion_0
DeinterlaceTech.: Blend, Bob, Adaptive, Motion Compensation, Inverse Telecine

For issue 1 can you do a video as well. Maybe using a phone if necessary.

I probably can grab with screen recorder, i'll upload it soon

xLn2 commented 3 months ago

Issue at # 1 While swithing fullscreen to windowed, Playlist appear a period of time and disappears.

firefox_lqiKPuRHUU

adipose commented 3 months ago

Does it depend on mouse position at all?

xLn2 commented 3 months ago

Nope, I can also reproduce it with F11.

adipose commented 3 months ago

Ok and mouse all the way to the left looks the same? I will check later.

clsid2 commented 3 months ago

Another issue: If fullscreen is on separate monitor, and you make a dragging motion on video, then control window moves. Edit: CMouse::IsOnFullscreenWindow() is wrongly returning false

adipose commented 3 months ago

Issue at # 1 While swithing fullscreen to windowed, Playlist appear a period of time and disappears.

For this issue please set fullscreen delay to 1. It is intended to deal with all sorts of window resizing animations that make a mess during a big resize like this. A resize requires a repaint, but by default Windows does not actually wait for a repaint before showing the resized window.

xLn2 commented 3 months ago

Setting fullscreen delay to 1sec or 2sec doesn't help. It is weird that if I disable Hide on Fullscreen there is no issue. resim

adipose commented 3 months ago

Setting fullscreen delay to 1sec or 2sec doesn't help. It is weird that if I disable Hide on Fullscreen there is no issue.

resim

Are you referring to the issue on entering fullscreen, or exiting?

xLn2 commented 3 months ago

Exiting from Fullscreen

adipose commented 3 months ago

While swithing fullscreen to windowed, Playlist appear a period of time and disappears.

Is this the only issue you are asking to fix? I can reproduce that, but I thought you wanted to fix a "gleam" which I took to mean a flicker or flash of light.

xLn2 commented 3 months ago

I chose that word for the reference to appearing and disappearing on the screen for a moment, but it's a word that can also mean something else. Yes, I mean the playlist bar that appears briefly when you switch from fullscreen to window mode.

adipose commented 3 months ago
  1. https://github.com/clsid2/mpc-hc/pull/2992
clsid2 commented 3 months ago

All of xLn2 reported issues fixed now I think?

adipose commented 3 months ago

Except the one which I made a new issue for, I think so, yes.

clsid2 commented 3 months ago

Another issue: If fullscreen is on separate monitor, and you make a dragging motion on video, then control window moves. Edit: CMouse::IsOnFullscreenWindow() is wrongly returning false

I haven't had time to dig into this one yet. Feel free to have a look at it.

clsid2 commented 3 months ago

Perhaps it might be due the window not getting focus on first click? Or issue with mouse capture.

When in fullscreen in separate window and I double click, these events fire: InternalOnLButtonDown InternalOnLButtonDown InternalOnLButtonUp

So a LeftUp is missing (and being send to another window?)

If I adjust CMouse::IsOnFullscreenWindow() to always return true, it it correctly processes a Up/Down/Up/Down sequence.

adipose commented 3 months ago

I know the problem but I don't know when it started.

The mouse capture isn't happening causing the events to bubble up to the parent window. However, I am not sure why, perhaps the toolwindow has special rules.

In pretranslatemsg the events are seen by the FS window. But they don't arrive at windowproc.

clsid2 commented 3 months ago

CMouse::TestDrag returns true and causes the problem. m_beginDragPoint has incorrect value

But dragging should be disabled in fullscreen. Problem circles back to IsOnFullscreenWindow() not working correctly.

adipose commented 3 months ago

Right. What's weird is that window is meant to track mouse events but the activate behavior is "broken" (I assume intentionally). But I don't quite know what about this command is achieving the goal in the comment:

    // allow the mainframe to keep focus
    bool ret = !!m_pDedicatedFSVideoWnd->CreateEx(WS_EX_TOPMOST | WS_EX_TOOLWINDOW, _T(""), ResStr(IDS_MAINFRM_136), WS_POPUP|WS_VISIBLE, monitorRect, nullptr, 0);
clsid2 commented 3 months ago

If I press F11 to go fullscreen, then double-click works good in fullscreen. If I double-click to go fullscreen, then first click in fullscreen has wrong point. So for second click (on same spot) CMouse::PointEqualsImprecise returns false. If I click once to (un)pause in fullscreen, then a double-click after that works fine.

I tried SW_SHOW instead of SW_SHOWNOACTIVATE, but made no difference.

clsid2 commented 3 months ago

If I added this to CMouse::InternalOnLButtonDown

CWnd& wnd1 = GetWnd();
TRACE(L"hwnd = %x\n", wnd1.m_hWnd);

Then the logged hwnd is same in both windowed and separate fullscreen. It should be different?

Edit: If I add this directly after creating the window then it logs different hwnd: m_pDedicatedFSVideoWnd->SetCapture(); But goes back to hwnd of main window after first click.

adipose commented 3 months ago

Then the logged hwnd is same in both windowed and separate fullscreen. It should be different?

It should be different, yes. That's the problem. It is allowing the messages to get back to the main view. This isn't a disaster, except that lbutton has a behavior that doesn't check where it is actually clicking. We could fix it with a pointinrect probably. But I'm wondering why it can't seem to get mouse events over there consistently.