Aleksoid1978 / VideoRenderer

Внешний видео-рендерер
GNU General Public License v3.0
997 stars 110 forks source link

Information tab is cleared when pressing Esc #100

Closed Rikk closed 8 months ago

Rikk commented 8 months ago
  1. Open MPCVR settings and click Information tab.
  2. While info text is selected, press Esc.
  3. This will make the text area disappear instead of closing the dialog.

I tested in MPC-HC.

Aleksoid1978 commented 8 months ago

I can't reproduce in MPC-BE.

clsid2 commented 8 months ago

@adipose

Happens with Dark theme only

v0lt commented 8 months ago

The EDITTEXT dialog with the ES_MULTILINE property behaves very unusually. Therefore, you may experience strange tab behavior in which such a dialog is present. And the text disappearing when you press Esc is the least of the problems (you wanted to close the window anyway).

adipose commented 8 months ago

I will take a look.

v0lt commented 8 months ago

I found my commit on this topic - https://github.com/Aleksoid1978/VideoRenderer/commit/c414764fde67118a628b2d33d6502dfc0ad8d0cb I don't remember the details, but without this change it was worse. :-)

adipose commented 8 months ago

I can reproduce it...

adipose commented 8 months ago

I debugged the situation and found that the code below never activates during the ESC. Or in other words, it never gets the close message.

    if (uMsg == WM_CLOSE) {
        // fixed Esc handling when EDITTEXT control has ES_MULTILINE property and is in focus
        return (LRESULT)1;
    }

I can solve this issue pretty easily in MPC-HC. Inside the subclass, I can trap the escape message. The issue seems to derive from this one:

https://forums.codeguru.com/showthread.php?346327-Multiline-Edit-cancels-Dialog-with-ESC

However, I do not quite understand why it only happens when I subclass the edit control (even a trivial subclass from CEdit causes the problem).

adipose commented 8 months ago

https://github.com/clsid2/mpc-hc/pull/2293

This causes it to behave the same as the light theme, now. However, it is worth considering making it work properly (as in, treat multi-line the same as everything else).

What I found was that the 2 invisible dialog boxes that get created with propertypages were inheriting CDialog::PreTranslateMessage (due to the theme inheriting from CDialog), even though they apparently should not. I am guessing those dialogs are purely for layout and don't need to process any messages. In any case it would seem they aren't supposed to do what CDialog does, which is process the code here:

BOOL CDialog::PreTranslateMessage(MSG* pMsg)
{
    // for modeless processing (or modal)
    ASSERT(m_hWnd != NULL);

    // allow tooltip messages to be filtered
    if (CWnd::PreTranslateMessage(pMsg))
        return TRUE;

    // don't translate dialog messages when in Shift+F1 help mode
    CFrameWnd* pFrameWnd = GetTopLevelFrame();
    if (pFrameWnd != NULL && pFrameWnd->m_bHelpMode)
        return FALSE;

    // fix around for VK_ESCAPE in a multiline Edit that is on a Dialog
    // that doesn't have a cancel or the cancel is disabled.
    if (pMsg->message == WM_KEYDOWN &&
        (pMsg->wParam == VK_ESCAPE || pMsg->wParam == VK_CANCEL) &&
        (::GetWindowLong(pMsg->hwnd, GWL_STYLE) & ES_MULTILINE) &&
        _AfxCompareClassName(pMsg->hwnd, _T("Edit")))
    {
        HWND hItem = ::GetDlgItem(m_hWnd, IDCANCEL);
        if (hItem == NULL || ::IsWindowEnabled(hItem))
        {
            SendMessage(WM_COMMAND, IDCANCEL, 0);
            return TRUE;
        }
    }
    // filter both messages to dialog and from children
    return PreTranslateInput(pMsg);
}

Note this sends a IDCANCEL command to itself, which destroys the dialog.

adipose commented 8 months ago

https://github.com/Aleksoid1978/VideoRenderer/pull/101

Here is a PR that actually closes the dialog when ESC is hit, as it would work if it weren't multi-line.

v0lt commented 8 months ago

Check it out in version 0.7.0.2151.

Rikk commented 8 months ago

Here is a PR that actually closes the dialog when ESC is hit, as it would work if it weren't multi-line.

Hi @adipose Does it need a new build of MPC-HC with https://github.com/clsid2/mpc-hc/commit/0620cf6fb422c60ebeb6d26832d1babef70d68bb for this to work? 0.7.0.2151 and MPC-HC 2.1.0 behaves the same regarding this issue.

I see, however, a different behavior in 0.7.0.2151 (vs 0.7.0.2148) in the relation between DXVA2 and DX11. 2148 keeps DXVA2 active in LAV and uses DX9 in MPCVR, automatically. 2151 makes DXVA2 inactive in LAV and uses DX11 in MPCVR. In order for 2151 to behave the same as 2148 I need to manually uncheck 'Use D3D 11' option.

clsid2 commented 8 months ago

My MPCVR build has one commit reverted, allowing to use DXVA2 decoding with D3D11. But better is to use D3D11 decoding.

You need to wait for new MPC-HC build.

Rikk commented 8 months ago

But better is to use D3D11 decoding.

My GPU (old Intel HD 3000) works best with DXVA2n.

My MPCVR build has one commit reverted, allowing to use DXVA2 decoding with D3D11.

I'll resume below my tests with D3D11 option and what happens in MPCVR and LAV regarding DXVA2:

Version 'Use Direct3D 11' MPCVR info LAV info
0.7.0.2148 clsid2
  • [x]
DirectX 9
VideoProcessor : DXVA2
Active decoder: dxva2n
0.7.0.2148 clsid2
  • [ ]
DirectX 9
VideoProcessor : DXVA2
Active decoder: dxva2n
0.7.0.2151 nightly
  • [x]
DirectX 11
VideoProcessor : D3D11
Active decoder: avcodec [^1]
0.7.0.2151 nightly
  • [ ]
DirectX 9
VideoProcessor : DXVA2
Active decoder: dxva2n
0.6.9.2117 release
  • [x]
DirectX 11
VideoProcessor : D3D11
Active decoder: dxva2n
0.6.9.2117 release
  • [ ]
DirectX 9
VideoProcessor : DXVA2
Active decoder: dxva2n

[^1]: you know, avcodec = no h/w acceleration

0.7.0.2151 has an unfriendly behavior, imo.

adipose commented 8 months ago

Here is a PR that actually closes the dialog when ESC is hit, as it would work if it weren't multi-line.

Hi @adipose Does it need a new build of MPC-HC with https://github.com/clsid2/mpc-hc/commit/0620cf6fb422c60ebeb6d26832d1babef70d68bb for this to work? 0.7.0.2151 and MPC-HC 2.1.0 behaves the same regarding this issue.

The fix for mpc-hc allows the themed widgets to behave more like native ones with regards to keys passed up the hierarchy.

However, the patch to mpcvr should change the behavior of the edit to avoid the Windows 2.0 "feature" entirely. The mpc-hc fix shouldn't be necessary, in theory.

Edit: in testing, it wasn't true--the edit control behavior was overridden by the CMPCTheme edit control.

v0lt commented 8 months ago

https://github.com/Aleksoid1978/VideoRenderer/blob/master/history.txt

0.7.0 dev

The DXVA2 decoder is no longer supported in DirectX 11 mode. ...

Rikk commented 8 months ago

Yes, but couldn't it be changed so it downgrades automatically to DX9 instead of disabling LAV acceleration (this is what clsid2 build seems to do)? Maybe some king of notice would make the change less obscure to users.

Btw, was there some kind of instability with D3D11 in VR and DXVA2 in LAV? It seems to work ok here.

Ok, after some tests I've found a combination that works better with MPCVR 0.7.0.2151: Set LAV to D3D11 (auto) and MPCVR at default; LAV will decode with D3D11 Native active. However:

  1. clsid2 build always outputs as DX9 w/ DXVA2 and LAV decodes at D3D11 CB Direct.
  2. When using any other renderer (EVR, VMR, Sync), LAV also decodes at D3D11 CB. That's why I'm used to use LAV set to DXVA2 Native.
adipose commented 8 months ago

Hi @adipose Does it need a new build of MPC-HC with clsid2/mpc-hc@0620cf6 for this to work? 0.7.0.2151 and MPC-HC 2.1.0 behaves the same regarding this issue.

Ignore my previous comment. PreTranslateMessage happens in CDialog before the keydown is sent to the edit, so the MPCVR fix will not work until the PreTranslateMessage fix is in place. You need an updated mpc-hc.