From 20f2ebae4b5ae0f52ac776a214bf0a9b0a9af98f Mon Sep 17 00:00:00 2001 From: Adrien Destugues Date: Thu, 7 May 2020 21:25:38 +0200 Subject: [PATCH] Remove MouseDownThread and its usages This code comes from an old Be Newsletter and since then the API received the addition of SetMouseEventMask. In several places the MouseDownThread was misused: it would spawn a new thread on every mouse click and not clear the previous one. This could for example lead to BSpinner skipping values if you clicked it at the right speed. There are functional changes in BSpinner, before it updated for the first time 100ms after mouse down, and then as you moved the mouse around the button, now it activates immediately on first click and then every 200ms (which may be a bit short). In other places, no functional changes intended. Change-Id: Ie600dc68cbb87d1e237633953e5189918bf36575 Reviewed-on: https://review.haiku-os.org/c/haiku/+/2599 Reviewed-by: waddlesplash --- headers/os/interface/MenuField.h | 3 - headers/private/shared/Thread.h | 151 ------------------------- src/apps/deskbar/ExpandoMenuBar.cpp | 68 +++++------ src/apps/deskbar/ExpandoMenuBar.h | 3 - src/kits/interface/AbstractSpinner.cpp | 60 +++++----- src/kits/interface/MenuField.cpp | 18 +-- src/kits/tracker/DialogPane.cpp | 75 ++++++------ src/kits/tracker/DialogPane.h | 5 +- 8 files changed, 102 insertions(+), 281 deletions(-) diff --git a/headers/os/interface/MenuField.h b/headers/os/interface/MenuField.h index fac2aec127..c91796a287 100644 --- a/headers/os/interface/MenuField.h +++ b/headers/os/interface/MenuField.h @@ -141,9 +141,6 @@ private: float _MenuBarOffset() const; float _MenuBarWidth() const; - void _DoneTracking(BPoint point); - void _Track(BPoint point, uint32); - private: char* fLabel; BMenu* fMenu; diff --git a/headers/private/shared/Thread.h b/headers/private/shared/Thread.h index 7a47213dd9..5ab9898d0e 100644 --- a/headers/private/shared/Thread.h +++ b/headers/private/shared/Thread.h @@ -45,46 +45,6 @@ All rights reserved. namespace BPrivate { -class MessengerAutoLocker { - // move this into AutoLock.h - public: - MessengerAutoLocker(BMessenger* messenger) - : fMessenger(messenger), - fHasLock(messenger->LockTarget()) - {} - - ~MessengerAutoLocker() - { - Unlock(); - } - - bool operator!() const - { - return !fHasLock; - } - - bool IsLocked() const - { - return fHasLock; - } - - void Unlock() - { - if (fHasLock) { - BLooper* looper; - fMessenger->Target(&looper); - if (looper) - looper->Unlock(); - fHasLock = false; - } - } - - private: - BMessenger* fMessenger; - bool fHasLock; -}; - - class SimpleThread { // this should only be used as a base class, // subclass needs to add proper locking mechanism @@ -313,117 +273,6 @@ LaunchInNewThread(const char* name, int32 priority, } -template -class MouseDownThread { -public: - static void TrackMouse(View* view, void (View::*)(BPoint), - void (View::*)(BPoint, uint32) = 0, - bigtime_t pressingPeriod = 100000); - -protected: - MouseDownThread(View* view, void (View::*)(BPoint), - void (View::*)(BPoint, uint32), bigtime_t pressingPeriod); - - virtual ~MouseDownThread(); - - void Go(); - virtual void Track(); - - static status_t TrackBinder(void*); - -private: - BMessenger fOwner; - void (View::*fDonePressing)(BPoint); - void (View::*fPressing)(BPoint, uint32); - bigtime_t fPressingPeriod; - volatile thread_id fThreadID; -}; - - -template -void -MouseDownThread::TrackMouse(View* view, - void(View::*donePressing)(BPoint), - void(View::*pressing)(BPoint, uint32), bigtime_t pressingPeriod) -{ - (new MouseDownThread(view, donePressing, pressing, pressingPeriod))->Go(); -} - - -template -MouseDownThread::MouseDownThread(View* view, - void (View::*donePressing)(BPoint), - void (View::*pressing)(BPoint, uint32), bigtime_t pressingPeriod) - : fOwner(view, view->Window()), - fDonePressing(donePressing), - fPressing(pressing), - fPressingPeriod(pressingPeriod) -{ -} - - -template -MouseDownThread::~MouseDownThread() -{ -} - - -template -void -MouseDownThread::Go() -{ - fThreadID = spawn_thread(&MouseDownThread::TrackBinder, - "MouseTrackingThread", B_NORMAL_PRIORITY, this); - - if (fThreadID <= 0 || resume_thread(fThreadID) != B_OK) - // didn't start, don't leak self - delete this; -} - - -template -status_t -MouseDownThread::TrackBinder(void* castToThis) -{ - MouseDownThread* self = static_cast(castToThis); - self->Track(); - // dead at this point - return B_OK; -} - - -template -void -MouseDownThread::Track() -{ - for (;;) { - MessengerAutoLocker lock(&fOwner); - if (!lock) - break; - - BLooper* looper; - View* view = dynamic_cast(fOwner.Target(&looper)); - if (!view) - break; - - uint32 buttons; - BPoint location; - view->GetMouse(&location, &buttons, false); - if (!buttons) { - (view->*fDonePressing)(location); - break; - } - if (fPressing) - (view->*fPressing)(location, buttons); - - lock.Unlock(); - snooze(fPressingPeriod); - } - - delete this; -} - - } // namespace BPrivate using namespace BPrivate; diff --git a/src/apps/deskbar/ExpandoMenuBar.cpp b/src/apps/deskbar/ExpandoMenuBar.cpp index 038080ba2b..d92e91c917 100644 --- a/src/apps/deskbar/ExpandoMenuBar.cpp +++ b/src/apps/deskbar/ExpandoMenuBar.cpp @@ -324,8 +324,8 @@ TExpandoMenuBar::MouseDown(BPoint where) && item->ExpanderBounds().Contains(where)) { // start the animation here, finish on mouse up fLastClickedItem = item; - MouseDownThread::TrackMouse(this, - &TExpandoMenuBar::_DoneTracking, &TExpandoMenuBar::_Track); + item->SetArrowDirection(BControlLook::B_RIGHT_DOWN_ARROW); + SetMouseEventMask(B_POINTER_EVENTS, B_NO_POINTER_HISTORY); Invalidate(item->ExpanderBounds()); return; // absorb the message @@ -360,6 +360,23 @@ TExpandoMenuBar::MouseMoved(BPoint where, uint32 code, const BMessage* message) // force a cleanup _FinishedDrag(); + if (buttons != 0) { + TTeamMenuItem* lastItem + = dynamic_cast(fLastClickedItem); + if (lastItem != NULL) { + if (lastItem->ExpanderBounds().Contains(where)) + lastItem->SetArrowDirection( + BControlLook::B_RIGHT_DOWN_ARROW); + else { + lastItem->SetArrowDirection(lastItem->IsExpanded() + ? BControlLook::B_DOWN_ARROW + : BControlLook::B_RIGHT_ARROW); + } + } + + Invalidate(lastItem->ExpanderBounds()); + } + switch (code) { case B_INSIDE_VIEW: { @@ -471,6 +488,15 @@ TExpandoMenuBar::MouseUp(BPoint where) // absorb the message } + TTeamMenuItem* lastItem = dynamic_cast(fLastClickedItem); + if (lastItem != NULL && lastItem->ExpanderBounds().Contains(where)) { + lastItem->ToggleExpandState(true); + lastItem->SetArrowDirection(lastItem->IsExpanded() + ? BControlLook::B_DOWN_ARROW + : BControlLook::B_RIGHT_ARROW); + + Invalidate(lastItem->ExpanderBounds()); + } BMenuBar::MouseUp(where); } @@ -1101,41 +1127,3 @@ TExpandoMenuBar::_FinishedDrag(bool invoke) if (!invoke && fBarView != NULL && fBarView->Dragging()) fBarView->DragStop(true); } - - -void -TExpandoMenuBar::_DoneTracking(BPoint where) -{ - TTeamMenuItem* lastItem = dynamic_cast(fLastClickedItem); - if (lastItem == NULL) - return; - - if (!lastItem->ExpanderBounds().Contains(where)) - return; - - lastItem->ToggleExpandState(true); - lastItem->SetArrowDirection(lastItem->IsExpanded() - ? BControlLook::B_DOWN_ARROW - : BControlLook::B_RIGHT_ARROW); - - Invalidate(lastItem->ExpanderBounds()); -} - - -void -TExpandoMenuBar::_Track(BPoint where, uint32) -{ - TTeamMenuItem* lastItem = dynamic_cast(fLastClickedItem); - if (lastItem == NULL) - return; - - if (lastItem->ExpanderBounds().Contains(where)) - lastItem->SetArrowDirection(BControlLook::B_RIGHT_DOWN_ARROW); - else { - lastItem->SetArrowDirection(lastItem->IsExpanded() - ? BControlLook::B_DOWN_ARROW - : BControlLook::B_RIGHT_ARROW); - } - - Invalidate(lastItem->ExpanderBounds()); -} diff --git a/src/apps/deskbar/ExpandoMenuBar.h b/src/apps/deskbar/ExpandoMenuBar.h index 8ed2bbd5e1..372c9e5ce3 100644 --- a/src/apps/deskbar/ExpandoMenuBar.h +++ b/src/apps/deskbar/ExpandoMenuBar.h @@ -113,9 +113,6 @@ private: void _FinishedDrag(bool invoke = false); - void _DoneTracking(BPoint where); - void _Track(BPoint where, uint32); - bool CheckForSizeOverrunVertical(); bool CheckForSizeOverrunHorizontal(); diff --git a/src/kits/interface/AbstractSpinner.cpp b/src/kits/interface/AbstractSpinner.cpp index 75ac67c12a..02216b7ac0 100644 --- a/src/kits/interface/AbstractSpinner.cpp +++ b/src/kits/interface/AbstractSpinner.cpp @@ -28,14 +28,13 @@ #include #include #include +#include #include #include #include #include #include -#include "Thread.h" - static const float kFrameMargin = 2.0f; @@ -170,20 +169,18 @@ public: virtual void MouseUp(BPoint where); virtual void MouseMoved(BPoint where, uint32 transit, const BMessage* message); + virtual void MessageReceived(BMessage* message); bool IsEnabled() const { return fIsEnabled; } virtual void SetEnabled(bool enable) { fIsEnabled = enable; }; private: - void _DoneTracking(BPoint where); - void _Track(BPoint where, uint32); - spinner_direction fSpinnerDirection; BAbstractSpinner* fParent; bool fIsEnabled; bool fIsMouseDown; bool fIsMouseOver; - bigtime_t fRepeatDelay; + BMessageRunner* fRepeater; }; @@ -309,13 +306,14 @@ SpinnerButton::SpinnerButton(BRect frame, const char* name, fIsEnabled(true), fIsMouseDown(false), fIsMouseOver(false), - fRepeatDelay(100000) + fRepeater(NULL) { } SpinnerButton::~SpinnerButton() { + delete fRepeater; } @@ -448,10 +446,14 @@ SpinnerButton::MouseDown(BPoint where) { if (fIsEnabled) { fIsMouseDown = true; + fSpinnerDirection == SPINNER_INCREMENT + ? fParent->Increment() + : fParent->Decrement(); Invalidate(); - fRepeatDelay = 100000; - MouseDownThread::TrackMouse(this, - &SpinnerButton::_DoneTracking, &SpinnerButton::_Track); + BMessage repeatMessage('rept'); + SetMouseEventMask(B_POINTER_EVENTS, B_NO_POINTER_HISTORY); + fRepeater = new BMessageRunner(BMessenger(this), repeatMessage, + 200000); } BView::MouseDown(where); @@ -470,8 +472,6 @@ SpinnerButton::MouseMoved(BPoint where, uint32 transit, uint32 buttons; GetMouse(&where, &buttons); fIsMouseOver = Bounds().Contains(where) && buttons == 0; - if (!fIsMouseDown) - Invalidate(); break; } @@ -491,38 +491,32 @@ void SpinnerButton::MouseUp(BPoint where) { fIsMouseDown = false; + delete fRepeater; + fRepeater = NULL; Invalidate(); BView::MouseUp(where); } -// #pragma mark - SpinnerButton private methods - - void -SpinnerButton::_DoneTracking(BPoint where) +SpinnerButton::MessageReceived(BMessage* message) { - if (fIsMouseDown || !Bounds().Contains(where)) - fIsMouseDown = false; -} + switch (message->what) { + case 'rept': + { + if (fIsMouseDown && fRepeater != NULL) { + fSpinnerDirection == SPINNER_INCREMENT + ? fParent->Increment() + : fParent->Decrement(); + } + break; + } -void -SpinnerButton::_Track(BPoint where, uint32) -{ - if (fParent == NULL || !Bounds().Contains(where)) { - fIsMouseDown = false; - return; + default: + BView::MessageReceived(message); } - fIsMouseDown = true; - - fSpinnerDirection == SPINNER_INCREMENT - ? fParent->Increment() - : fParent->Decrement(); - - snooze(fRepeatDelay); - fRepeatDelay = 10000; } diff --git a/src/kits/interface/MenuField.cpp b/src/kits/interface/MenuField.cpp index 6f68f36a26..cdec6c07c6 100644 --- a/src/kits/interface/MenuField.cpp +++ b/src/kits/interface/MenuField.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include #include @@ -484,8 +483,7 @@ BMenuField::MouseDown(BPoint where) if (fMouseDownFilter->Looper() == NULL) Window()->AddCommonFilter(fMouseDownFilter); - MouseDownThread::TrackMouse(this, &BMenuField::_DoneTracking, - &BMenuField::_Track); + SetMouseEventMask(B_POINTER_EVENTS, B_NO_POINTER_HISTORY); } } @@ -557,6 +555,7 @@ BMenuField::MouseMoved(BPoint point, uint32 code, const BMessage* message) void BMenuField::MouseUp(BPoint where) { + Window()->RemoveCommonFilter(fMouseDownFilter); BView::MouseUp(where); } @@ -1403,19 +1402,6 @@ BMenuField::_MenuBarWidth() const } -void -BMenuField::_DoneTracking(BPoint point) -{ - Window()->RemoveCommonFilter(fMouseDownFilter); -} - - -void -BMenuField::_Track(BPoint point, uint32) -{ -} - - // #pragma mark - BMenuField::LabelLayoutItem diff --git a/src/kits/tracker/DialogPane.cpp b/src/kits/tracker/DialogPane.cpp index 93b1440ccf..c3908016b7 100644 --- a/src/kits/tracker/DialogPane.cpp +++ b/src/kits/tracker/DialogPane.cpp @@ -38,7 +38,6 @@ All rights reserved. #include #include -#include "Thread.h" #include "Utilities.h" #include "Window.h" @@ -139,12 +138,53 @@ PaneSwitch::MouseDown(BPoint) return; fPressing = true; - MouseDownThread::TrackMouse(this, &PaneSwitch::DoneTracking, - &PaneSwitch::Track); + SetMouseEventMask(B_POINTER_EVENTS, B_NO_POINTER_HISTORY); Invalidate(); } +void +PaneSwitch::MouseMoved(BPoint point, uint32 code, const BMessage* message) +{ + int32 buttons; + BMessage* currentMessage = Window()->CurrentMessage(); + if (currentMessage == NULL + || currentMessage->FindInt32("buttons", &buttons) != B_OK) { + buttons = 0; + } + + if (buttons != 0) { + BRect bounds(Bounds()); + bounds.InsetBy(-3, -3); + + bool newPressing = bounds.Contains(point); + if (newPressing != fPressing) { + fPressing = newPressing; + Invalidate(); + } + } + + BControl::MouseMoved(point, code, message); +} + + +void +PaneSwitch::MouseUp(BPoint point) +{ + BRect bounds(Bounds()); + bounds.InsetBy(-3, -3); + + fPressing = false; + Invalidate(); + if (bounds.Contains(point)) { + SetValue(!Value()); + Invoke(); + } + + BControl::MouseUp(point); +} + + void PaneSwitch::GetPreferredSize(float* _width, float* _height) { @@ -213,35 +253,6 @@ PaneSwitch::SetLabels(const char* labelOn, const char* labelOff) } -void -PaneSwitch::DoneTracking(BPoint point) -{ - BRect bounds(Bounds()); - bounds.InsetBy(-3, -3); - - fPressing = false; - Invalidate(); - if (bounds.Contains(point)) { - SetValue(!Value()); - Invoke(); - } -} - - -void -PaneSwitch::Track(BPoint point, uint32) -{ - BRect bounds(Bounds()); - bounds.InsetBy(-3, -3); - - bool newPressing = bounds.Contains(point); - if (newPressing != fPressing) { - fPressing = newPressing; - Invalidate(); - } -} - - void PaneSwitch::DrawInState(PaneSwitch::State state) { diff --git a/src/kits/tracker/DialogPane.h b/src/kits/tracker/DialogPane.h index 4598956b1f..3198625cee 100644 --- a/src/kits/tracker/DialogPane.h +++ b/src/kits/tracker/DialogPane.h @@ -57,6 +57,8 @@ public: virtual void Draw(BRect updateRect); virtual void MouseDown(BPoint where); + virtual void MouseMoved(BPoint where, uint32 code, const BMessage*); + virtual void MouseUp(BPoint where); virtual void GetPreferredSize(float* _width, float* _height); @@ -69,9 +71,6 @@ public: const char* labelOff); protected: - void DoneTracking(BPoint where); - void Track(BPoint where, uint32); - enum State { kCollapsed, kPressed,