vcl/inc/win/saltimer.h | 24 ++--- vcl/win/app/salinst.cxx | 218 +++++++++++++++++++++++++++++++++++------------ vcl/win/app/saltimer.cxx | 52 ++++++----- 3 files changed, 204 insertions(+), 90 deletions(-)
New commits: commit 3185bf228cbeb8f716444930b04afb0924c60ef3 Author: Jan-Marek Glogowski <glo...@fbihome.de> AuthorDate: Thu Oct 12 10:20:17 2017 +0200 Commit: Thorsten Behrens <thorsten.behr...@cib.de> CommitDate: Wed Jan 16 19:33:12 2019 +0100 tdf#112975 WIN correctly handle VclInputFlags::OTHER On Windows we can just check the message queue for existing messages. But VclInputFlags::OTHER is used to check for any messages, which can't be explicitly checked. In the case of checking for VclInputFlags::OTHER while excluding an other message type, we have to make multiple PeekMessage calls and exclude all non-checked message ids. Change-Id: I1cedd4b76444769842c74228fc547f0d924f8b60 Reviewed-on: https://gerrit.libreoffice.org/43337 Tested-by: Jenkins <c...@libreoffice.org> Reviewed-by: Jan-Marek Glogowski <glo...@fbihome.de> Reviewed-on: https://gerrit.libreoffice.org/66451 Reviewed-by: Thorsten Behrens <thorsten.behr...@cib.de> Tested-by: Thorsten Behrens <thorsten.behr...@cib.de> diff --git a/vcl/inc/win/saltimer.h b/vcl/inc/win/saltimer.h index 37976bbfaf8b..68973e1cadc3 100644 --- a/vcl/inc/win/saltimer.h +++ b/vcl/inc/win/saltimer.h @@ -47,6 +47,7 @@ public: virtual void Stop() override; inline bool IsDirectTimeout() const; + inline bool HasTimerElapsed() const; }; inline bool WinSalTimer::IsDirectTimeout() const @@ -54,6 +55,11 @@ inline bool WinSalTimer::IsDirectTimeout() const return m_bDirectTimeout; } +inline bool WinSalTimer::HasTimerElapsed() const +{ + return m_bDirectTimeout || ExistsValidEvent(); +} + #endif /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx index 054602c46641..7bef41e8ac4c 100644 --- a/vcl/win/app/salinst.cxx +++ b/vcl/win/app/salinst.cxx @@ -793,10 +793,119 @@ LRESULT CALLBACK SalComWndProcW( HWND hWnd, UINT nMsg, WPARAM wParam, LPARAM lPa return nRet; } +struct MsgRange +{ + UINT nStart; + UINT nEnd; +}; + +static std::vector<MsgRange> GetOtherRanges( VclInputFlags nType ) +{ + assert( nType != VCL_INPUT_ANY ); + + // this array must be kept sorted! + const UINT nExcludeMsgIds[] = + { + 0, + + WM_MOVE, // 3 + WM_SIZE, // 5 + WM_PAINT, // 15 + WM_KEYDOWN, // 256 + WM_TIMER, // 275 + + WM_MOUSEFIRST, // 512 + 513, + 514, + 515, + 516, + 517, + 518, + 519, + 520, + WM_MOUSELAST, // 521 + + SAL_MSG_POSTMOVE, // WM_USER+136 + SAL_MSG_POSTCALLSIZE, // WM_USER+137 + + SAL_MSG_TIMER_CALLBACK, // WM_USER+162 + + UINT_MAX + }; + const unsigned MAX_EXCL = SAL_N_ELEMENTS( nExcludeMsgIds ); + + bool aExcludeMsgList[ MAX_EXCL ] = { false, }; + std::vector<MsgRange> aResult; + + // set the excluded values + if ( !(nType & VclInputFlags::MOUSE) ) + { + for ( unsigned i = 0; nExcludeMsgIds[ 6 + i ] <= WM_MOUSELAST; ++i ) + aExcludeMsgList[ 6 + i ] = true; + } + + if ( !(nType & VclInputFlags::KEYBOARD) ) + aExcludeMsgList[ 4 ] = true; + + if ( !(nType & VclInputFlags::PAINT) ) + { + aExcludeMsgList[ 1 ] = true; + aExcludeMsgList[ 2 ] = true; + aExcludeMsgList[ 3 ] = true; + aExcludeMsgList[ 16 ] = true; + aExcludeMsgList[ 17 ] = true; + } + + if ( !(nType & VclInputFlags::TIMER) ) + { + aExcludeMsgList[ 5 ] = true; + aExcludeMsgList[ 18 ] = true; + } + + // build the message ranges to check + MsgRange aRange = { 0, 0 }; + bool doEnd = true; + for ( unsigned i = 1; i < MAX_EXCL; ++i ) + { + if ( aExcludeMsgList[ i ] ) + { + if ( !doEnd ) + { + if ( nExcludeMsgIds[ i ] == aRange.nStart ) + ++aRange.nStart; + else + doEnd = true; + } + if ( doEnd ) + { + aRange.nEnd = nExcludeMsgIds[ i ] - 1; + aResult.push_back( aRange ); + doEnd = false; + aRange.nStart = aRange.nEnd + 2; + } + } + } + + if ( aRange.nStart != UINT_MAX ) + { + aRange.nEnd = UINT_MAX; + aResult.push_back( aRange ); + } + + return aResult; +} + bool WinSalInstance::AnyInput( VclInputFlags nType ) { MSG aMsg; + if ( nType & VclInputFlags::TIMER ) + { + const WinSalTimer* pTimer = static_cast<WinSalTimer*>( ImplGetSVData()->maSchedCtx.mpSalTimer ); + if ( pTimer && pTimer->HasTimerElapsed() ) + return true; + } + if ( (nType & VCL_INPUT_ANY) == VCL_INPUT_ANY ) { // revert bugfix for #108919# which never reported timeouts when called from the timer handler @@ -806,32 +915,52 @@ bool WinSalInstance::AnyInput( VclInputFlags nType ) } else { - if ( nType & VclInputFlags::MOUSE ) - { - // Test for mouse input - if ( PeekMessageW( &aMsg, 0, WM_MOUSEFIRST, WM_MOUSELAST, - PM_NOREMOVE | PM_NOYIELD ) ) - return true; - } + const bool bCheck_KEYBOARD (nType & VclInputFlags::KEYBOARD); + const bool bCheck_OTHER (nType & VclInputFlags::OTHER); - if ( nType & VclInputFlags::KEYBOARD ) + // If there is a modifier key event, it counts as OTHER + // Previously we were simply ignoring these events... + if ( bCheck_KEYBOARD || bCheck_OTHER ) { - // Test for key input - if ( PeekMessageW( &aMsg, 0, WM_KEYDOWN, WM_KEYDOWN, + if ( PeekMessageW( &aMsg, nullptr, WM_KEYDOWN, WM_KEYDOWN, PM_NOREMOVE | PM_NOYIELD ) ) { - if ( (aMsg.wParam == VK_SHIFT) || - (aMsg.wParam == VK_CONTROL) || - (aMsg.wParam == VK_MENU) ) - return false; - else + const bool bIsModifier = ( (aMsg.wParam == VK_SHIFT) || + (aMsg.wParam == VK_CONTROL) || (aMsg.wParam == VK_MENU) ); + if ( bCheck_KEYBOARD && !bIsModifier ) + return true; + if ( bCheck_OTHER && bIsModifier ) return true; } } + // Other checks for all messages not excluded. + // The less we exclude, the less ranges have to be checked! + if ( bCheck_OTHER ) + { + // TIMER and KEYBOARD are already handled, so always exclude them! + VclInputFlags nOtherType = nType & + ~VclInputFlags(VclInputFlags::KEYBOARD | VclInputFlags::TIMER); + + std::vector<MsgRange> aMsgRangeList( GetOtherRanges( nOtherType ) ); + for ( MsgRange aRange : aMsgRangeList ) + if ( PeekMessageW( &aMsg, nullptr, aRange.nStart, + aRange.nEnd, PM_NOREMOVE | PM_NOYIELD ) ) + return true; + + // MOUSE and PAINT already handled, so skip futher checks + return false; + } + + if ( nType & VclInputFlags::MOUSE ) + { + if ( PeekMessageW( &aMsg, nullptr, WM_MOUSEFIRST, WM_MOUSELAST, + PM_NOREMOVE | PM_NOYIELD ) ) + return true; + } + if ( nType & VclInputFlags::PAINT ) { - // Test for paint input if ( PeekMessageW( &aMsg, 0, WM_PAINT, WM_PAINT, PM_NOREMOVE | PM_NOYIELD ) ) return true; @@ -852,22 +981,6 @@ bool WinSalInstance::AnyInput( VclInputFlags nType ) PM_NOREMOVE | PM_NOYIELD ) ) return true; } - - if ( nType & VclInputFlags::TIMER ) - { - // Test for timer input - if ( PeekMessageW( &aMsg, 0, WM_TIMER, WM_TIMER, - PM_NOREMOVE | PM_NOYIELD ) ) - return true; - - } - - if ( nType & VclInputFlags::OTHER ) - { - // Test for any input - if ( PeekMessageW( &aMsg, 0, 0, 0, PM_NOREMOVE | PM_NOYIELD ) ) - return true; - } } return false; commit 59ef49b5b31e3338c8a3e7675d6c83424fe73244 Author: Jan-Marek Glogowski <glo...@fbihome.de> AuthorDate: Thu Oct 12 16:00:42 2017 +0200 Commit: Thorsten Behrens <thorsten.behr...@cib.de> CommitDate: Wed Jan 16 19:32:57 2019 +0100 WIN another system loop integration attempt This time we skip the intention to handle our Scheduler completely via the system event loop. Instead we basically guarantee to process a Scheduler event during each DoYield, if one is available. This way we won't block system events when busy in our event loop. Change-Id: I37094e61cbf928733151d9cc3299cdac76b3a5cd Reviewed-on: https://gerrit.libreoffice.org/43349 Tested-by: Jenkins <c...@libreoffice.org> Reviewed-by: Jan-Marek Glogowski <glo...@fbihome.de> Reviewed-on: https://gerrit.libreoffice.org/66450 Reviewed-by: Thorsten Behrens <thorsten.behr...@cib.de> Tested-by: Thorsten Behrens <thorsten.behr...@cib.de> diff --git a/vcl/inc/win/saltimer.h b/vcl/inc/win/saltimer.h index 06e77b171cb3..37976bbfaf8b 100644 --- a/vcl/inc/win/saltimer.h +++ b/vcl/inc/win/saltimer.h @@ -26,15 +26,18 @@ class WinSalTimer final : public SalTimer, protected VersionedEvent { // for access to Impl* functions friend LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, int& rDef ); - // for access to m_bPollForMessage - friend static void CALLBACK SalTimerProc( PVOID data, BOOLEAN ); + // for access to GetNextVersionedEvent + friend void CALLBACK SalTimerProc( PVOID data, BOOLEAN ); + // for access to ImplHandleElapsedTimer + friend bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents ); HANDLE m_nTimerId; ///< Windows timer id - bool m_bPollForMessage; ///< Run yield until a message is caught (most likely the 0ms timer) + bool m_bDirectTimeout; ///< timeout can be processed directly void ImplStart( sal_uIntPtr nMS ); void ImplStop(); - void ImplEmitTimerCallback(); + void ImplHandleTimerEvent( WPARAM aWPARAM ); + void ImplHandleElapsedTimer(); public: WinSalTimer(); @@ -43,19 +46,12 @@ public: virtual void Start(sal_uIntPtr nMS) override; virtual void Stop() override; - inline bool IsValidWPARAM( WPARAM wParam ) const; - - inline bool PollForMessage() const; + inline bool IsDirectTimeout() const; }; -inline bool WinSalTimer::IsValidWPARAM( WPARAM aWPARAM ) const -{ - return IsValidEventVersion( static_cast<sal_Int32>( aWPARAM ) ); -} - -inline bool WinSalTimer::PollForMessage() const +inline bool WinSalTimer::IsDirectTimeout() const { - return m_bPollForMessage; + return m_bDirectTimeout; } #endif diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx index 7d5bdde80f75..054602c46641 100644 --- a/vcl/win/app/salinst.cxx +++ b/vcl/win/app/salinst.cxx @@ -575,12 +575,11 @@ static void ImplSalDispatchMessage( MSG* pMsg ) ImplSalPostDispatchMsg( pMsg, lResult ); } -bool -ImplSalYield( bool bWait, bool bHandleAllCurrentEvents ) +bool ImplSalYield( bool bWait, bool bHandleAllCurrentEvents ) { static sal_uInt32 nLastTicks = 0; MSG aMsg; - bool bWasMsg = false, bOneEvent = false; + bool bWasMsg = false, bOneEvent = false, bWasTimeoutMsg = false; ImplSVData *const pSVData = ImplGetSVData(); WinSalTimer* pTimer = static_cast<WinSalTimer*>( pSVData->maSchedCtx.mpSalTimer ); @@ -595,6 +594,8 @@ ImplSalYield( bool bWait, bool bHandleAllCurrentEvents ) if ( bOneEvent ) { bWasMsg = true; + if ( !bWasTimeoutMsg ) + bWasTimeoutMsg = (SAL_MSG_TIMER_CALLBACK == aMsg.message); TranslateMessage( &aMsg ); ImplSalDispatchMessage( &aMsg ); if ( bHandleAllCurrentEvents @@ -603,23 +604,26 @@ ImplSalYield( bool bWait, bool bHandleAllCurrentEvents ) bHadNewerEvent = true; bOneEvent = !bHadNewerEvent; } - // busy loop to catch a message, eventually the 0ms timer. - // we don't need to loop, if we wait anyway. - if ( !bWait && !bWasMsg && pTimer && pTimer->PollForMessage() ) - { - SwitchToThread(); - continue; - } + if ( !(bHandleAllCurrentEvents && bOneEvent) ) break; } while( true ); + // 0ms timeouts are handled out-of-bounds to prevent busy-locking the + // event loop with timeout massages. + // We ensure we never handle more then one timeout per call. + // This way we'll always process a normal system message. + if ( !bWasTimeoutMsg && pTimer && pTimer->IsDirectTimeout() ) + { + pTimer->ImplHandleElapsedTimer(); + bWasMsg = true; + } + if ( bHandleAllCurrentEvents ) nLastTicks = nCurTicks; - // Also check that we don't wait when application already has quit - if ( bWait && !bWasMsg && !pSVData->maAppData.mbAppQuit ) + if ( bWait && !bWasMsg ) { if ( GetMessageW( &aMsg, 0, 0, 0 ) ) { @@ -645,6 +649,7 @@ bool WinSalInstance::DoYield(bool bWait, bool bHandleAllCurrentEvents, sal_uLong // so don't do anything dangerous before releasing it here sal_uLong const nCount = (nReleased != 0) ? nReleased : ImplSalReleaseYieldMutex(); + if ( !IsMainThread() ) { // #97739# A SendMessage call blocks until the called thread (here: the main thread) @@ -749,17 +754,7 @@ LRESULT CALLBACK SalComWndProc( HWND, UINT nMsg, WPARAM wParam, LPARAM lParam, i { WinSalTimer *const pTimer = static_cast<WinSalTimer*>( ImplGetSVData()->maSchedCtx.mpSalTimer ); assert( pTimer != nullptr ); - MSG aMsg; - bool bValidMSG = pTimer->IsValidWPARAM( wParam ); - // PM_QS_POSTMESSAGE is needed, so we don't process the SendMessage from DoYield! - while ( PeekMessageW(&aMsg, GetSalData()->mpFirstInstance->mhComWnd, SAL_MSG_TIMER_CALLBACK, - SAL_MSG_TIMER_CALLBACK, PM_REMOVE | PM_NOYIELD | PM_QS_POSTMESSAGE) ) - { - assert( !bValidMSG && "Unexpected non-last valid message" ); - bValidMSG = pTimer->IsValidWPARAM( aMsg.wParam ); - } - if ( bValidMSG ) - pTimer->ImplEmitTimerCallback(); + pTimer->ImplHandleTimerEvent( wParam ); break; } } diff --git a/vcl/win/app/saltimer.cxx b/vcl/win/app/saltimer.cxx index fff4cb65a2c7..f155de0d4ce8 100644 --- a/vcl/win/app/saltimer.cxx +++ b/vcl/win/app/saltimer.cxx @@ -28,7 +28,7 @@ #include <sehandler.hxx> #endif -static void CALLBACK SalTimerProc(PVOID pParameter, BOOLEAN bTimerOrWaitFired); +void CALLBACK SalTimerProc(PVOID pParameter, BOOLEAN bTimerOrWaitFired); // See http://msdn.microsoft.com/en-us/library/windows/desktop/ms687003%28v=vs.85%29.aspx // (and related pages) for details about the Timer Queues. @@ -48,16 +48,11 @@ void WinSalTimer::ImplStop() return; m_nTimerId = nullptr; + m_bDirectTimeout = false; DeleteTimerQueueTimer( nullptr, hTimer, INVALID_HANDLE_VALUE ); - // Keep both after DeleteTimerQueueTimer, because they are set in SalTimerProc + // Keep InvalidateEvent after DeleteTimerQueueTimer, because the event id + // is set in SalTimerProc, which DeleteTimerQueueTimer will finish or cancel. InvalidateEvent(); - m_bPollForMessage = false; - - // remove as many pending SAL_MSG_TIMER_CALLBACK messages as possible - // PM_QS_POSTMESSAGE is needed, so we don't process the SendMessage from DoYield! - MSG aMsg; - while ( PeekMessageW(&aMsg, pInst->mhComWnd, SAL_MSG_TIMER_CALLBACK, - SAL_MSG_TIMER_CALLBACK, PM_REMOVE | PM_NOYIELD | PM_QS_POSTMESSAGE) ); } void WinSalTimer::ImplStart( sal_uLong nMS ) @@ -72,20 +67,21 @@ void WinSalTimer::ImplStart( sal_uLong nMS ) // cannot change a one-shot timer, so delete it and create a new one ImplStop(); - // keep the yield running, if a 0ms Idle is scheduled - m_bPollForMessage = ( 0 == nMS ); + // directly indicate an elapsed timer + m_bDirectTimeout = ( 0 == nMS ); // probably WT_EXECUTEONLYONCE is not needed, but it enforces Period // to be 0 and should not hurt; also see // https://www.microsoft.com/msj/0499/pooling/pooling.aspx - CreateTimerQueueTimer(&m_nTimerId, nullptr, SalTimerProc, - reinterpret_cast<void*>( - sal_IntPtr(GetNextEventVersion())), - nMS, 0, WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE); + if ( !m_bDirectTimeout ) + CreateTimerQueueTimer(&m_nTimerId, nullptr, SalTimerProc, this, + nMS, 0, WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE); + // We don't need any wakeup message, as this code can just run in the + // main thread! } WinSalTimer::WinSalTimer() : m_nTimerId( nullptr ) - , m_bPollForMessage( false ) + , m_bDirectTimeout( false ) { } @@ -124,7 +120,7 @@ void WinSalTimer::Stop() * This gets invoked from a Timer Queue thread. * Don't acquire the SolarMutex to avoid deadlocks. */ -static void CALLBACK SalTimerProc(PVOID data, BOOLEAN) +void CALLBACK SalTimerProc(PVOID data, BOOLEAN) { #if defined ( __MINGW32__ ) && !defined ( _WIN64 ) jmp_buf jmpbuf; @@ -136,11 +132,10 @@ static void CALLBACK SalTimerProc(PVOID data, BOOLEAN) __try { #endif - // always post message when the timer fires, we will remove the ones - // that happened during execution of the callback later directly from - // the message queue - BOOL const ret = PostMessageW(GetSalData()->mpFirstInstance->mhComWnd, - SAL_MSG_TIMER_CALLBACK, (WPARAM) data, 0); + WinSalTimer *pTimer = reinterpret_cast<WinSalTimer*>( data ); + BOOL const ret = PostMessageW( + GetSalData()->mpFirstInstance->mhComWnd, SAL_MSG_TIMER_CALLBACK, + static_cast<WPARAM>(pTimer->GetNextEventVersion()), 0 ); #if OSL_DEBUG_LEVEL > 0 if (0 == ret) // SEH prevents using SAL_WARN here? fputs("ERROR: PostMessage() failed!\n", stderr); @@ -158,15 +153,24 @@ static void CALLBACK SalTimerProc(PVOID data, BOOLEAN) } -void WinSalTimer::ImplEmitTimerCallback() +void WinSalTimer::ImplHandleElapsedTimer() { // Test for MouseLeave SalTestMouseLeave(); - m_bPollForMessage = false; + m_bDirectTimeout = false; ImplSalYieldMutexAcquireWithWait(); CallCallback(); ImplSalYieldMutexRelease(); } +void WinSalTimer::ImplHandleTimerEvent( const WPARAM aWPARAM ) +{ + assert( aWPARAM <= SAL_MAX_INT32 ); + if ( !IsValidEventVersion( static_cast<sal_Int32>( aWPARAM ) ) ) + return; + + ImplHandleElapsedTimer(); +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits