From da461da6f676322c77e6bc466d79a05607f1054e Mon Sep 17 00:00:00 2001 From: antoniou79 Date: Fri, 18 Jun 2021 14:27:36 +0300 Subject: [PATCH] GUI: Fix tooltip behavior to be less interruptive The tooltip will show only if mouse cursor was moved but not on a hovered focused editText field Changes and implications in this commit: - Fix _lastMousePosition coordinates and time being updated upon giving focus to the tooltip - Check for mouse cursor movement first in the decision for showing the tooltip, then check if sufficient time for mouse resting position has passed (kTooltipDelay). - Prevents showing a tooltip for a widget if it is an editable field (editText) and has both mouse hovering above it and the current focus. This is so that typing text is not interrupted / slowed down by a periodical display of the tooltip, if the mouse is hovering over the same text field that the user is editing. - If mouse cursor is moved but lands on the same widget as the one that had its tooltip shown last, then show the tooltip but after a different (larger) delay kTooltipSameWidgetDelay. - Still shows tooltip for other widgets, including editText ones, if the mouse is hovered over them and they are not the current focused editText widget. The bug was mentioned for the Android port on the forums here: https://forums.scummvm.org/viewtopic.php?p=95531#p95531 However, it is not Android specific, even though the slowdown is a lot more noticeable on an Android device. --- gui/Tooltip.cpp | 2 ++ gui/dialog.cpp | 2 ++ gui/dialog.h | 8 ++++++++ gui/gui-manager.cpp | 44 ++++++++++++++++++++++++++++++++++++-------- gui/gui-manager.h | 8 ++++++++ gui/widget.h | 2 ++ 6 files changed, 58 insertions(+), 8 deletions(-) diff --git a/gui/Tooltip.cpp b/gui/Tooltip.cpp index 04498287aeb..49c4e1aba9c 100644 --- a/gui/Tooltip.cpp +++ b/gui/Tooltip.cpp @@ -42,6 +42,8 @@ void Tooltip::setup(Dialog *parent, Widget *widget, int x, int y) { _parent = parent; + setMouseUpdatedOnFocus(false); + _maxWidth = g_gui.xmlEval()->getVar("Globals.Tooltip.MaxWidth", 100); _xdelta = g_gui.xmlEval()->getVar("Globals.Tooltip.XDelta", 0); _ydelta = g_gui.xmlEval()->getVar("Globals.Tooltip.YDelta", 0); diff --git a/gui/dialog.cpp b/gui/dialog.cpp index 57612f7f092..4f825b1e6f6 100644 --- a/gui/dialog.cpp +++ b/gui/dialog.cpp @@ -49,6 +49,7 @@ Dialog::Dialog(int x, int y, int w, int h) // started a 640x480 game with a non 1x scaler. g_gui.checkScreenChange(); + _mouseUpdatedOnFocus = true; _result = -1; } @@ -66,6 +67,7 @@ Dialog::Dialog(const Common::String &name) // and bug #2903: "SCUMM: F5 crashes game (640x480)" g_gui.checkScreenChange(); + _mouseUpdatedOnFocus = true; _result = -1; } diff --git a/gui/dialog.h b/gui/dialog.h index bd9fb36f4e0..c78a7823696 100644 --- a/gui/dialog.h +++ b/gui/dialog.h @@ -57,6 +57,11 @@ protected: Widget *_dragWidget; Widget *_tickleWidget; bool _visible; + // _mouseUpdatedOnFocus instructs gui-manager whether + // its lastMousePosition (time and x,y coordinates) + // should be updated, when this Dialog acquires focus. + // Default value is true. + bool _mouseUpdatedOnFocus; ThemeEngine::DialogBackground _backgroundType; @@ -71,6 +76,8 @@ public: bool isVisible() const override { return _visible; } + bool isMouseUpdatedOnFocus() const { return _mouseUpdatedOnFocus; } + void releaseFocus() override; void setFocusWidget(Widget *widget); Widget *getFocusWidget() { return _focusedWidget; } @@ -111,6 +118,7 @@ protected: Widget *findWidget(const char *name); void removeWidget(Widget *widget) override; + void setMouseUpdatedOnFocus(bool mouseUpdatedOnFocus) { _mouseUpdatedOnFocus = mouseUpdatedOnFocus; } void setDefaultFocusedWidget(); void setResult(int result) { _result = result; } diff --git a/gui/gui-manager.cpp b/gui/gui-manager.cpp index 08f1907644c..b6607ae62f0 100644 --- a/gui/gui-manager.cpp +++ b/gui/gui-manager.cpp @@ -53,7 +53,8 @@ namespace GUI { enum { kDoubleClickDelay = 500, // milliseconds kCursorAnimateDelay = 250, - kTooltipDelay = 1250 + kTooltipDelay = 1250, + kTooltipSameWidgetDelay = 7000 }; // Constructor @@ -440,13 +441,38 @@ void GuiManager::runLoop() { ++it; } - if (_lastMousePosition.time + kTooltipDelay < _system->getMillis(true)) { + // Handle tooltip for the widget under the mouse cursor. + // 1. Only try to show a tooltip if the mouse cursor was actually moved + // and sufficient time (kTooltipDelay) passed since mouse cursor rested in-place. + // Note, Dialog objects acquiring or losing focus lead to a _lastMousePosition update, + // which may lead to a change of its time and x,y coordinate values. + // See: GuiManager::giveFocusToDialog() + // We avoid updating _lastMousePosition when giving focus to the Tooltip object + // by having the Tooltip objects set a false value for their (inherited) member + // var _mouseUpdatedOnFocus (in Tooltip::setup()). + // However, when the tooltip loses focus, _lastMousePosition will be updated. + // If the mouse had stayed in the same position in the meantime, + // then at the time of the tooltip losing focus + // the _lastMousePosition.time will be new, but the x,y cordinates + // will be the same as the stored ones in _lastTooltipShown. + // 2. If the mouse was moved but ended on the same (tooltip enabled) widget, + // then delay showing the tooltip based on the value of kTooltipSameWidgetDelay. + uint32 systemMillisNowForTooltipCheck = _system->getMillis(true); + if ((_lastTooltipShown.x != _lastMousePosition.x || _lastTooltipShown.y != _lastMousePosition.y) + && _lastMousePosition.time + kTooltipDelay < systemMillisNowForTooltipCheck) { Widget *wdg = activeDialog->findWidget(_lastMousePosition.x, _lastMousePosition.y); - if (wdg && wdg->hasTooltip() && !(wdg->getFlags() & WIDGET_PRESSED)) { - Tooltip *tooltip = new Tooltip(); - tooltip->setup(activeDialog, wdg, _lastMousePosition.x, _lastMousePosition.y); - tooltip->runModal(); - delete tooltip; + if (wdg && wdg->hasTooltip() && !(wdg->getFlags() & WIDGET_PRESSED) + && (_lastTooltipShown.wdg != wdg || _lastTooltipShown.time + kTooltipSameWidgetDelay < systemMillisNowForTooltipCheck)) { + _lastTooltipShown.time = systemMillisNowForTooltipCheck; + _lastTooltipShown.wdg = wdg; + _lastTooltipShown.x = _lastMousePosition.x; + _lastTooltipShown.y = _lastMousePosition.y; + if (wdg->getType() != kEditTextWidget || activeDialog->getFocusWidget() != wdg) { + Tooltip *tooltip = new Tooltip(); + tooltip->setup(activeDialog, wdg, _lastMousePosition.x, _lastMousePosition.y); + tooltip->runModal(); + delete tooltip; + } } } @@ -690,7 +716,9 @@ void GuiManager::giveFocusToDialog(Dialog *dialog) { int16 dialogX = _globalMousePosition.x - dialog->_x; int16 dialogY = _globalMousePosition.y - dialog->_y; dialog->receivedFocus(dialogX, dialogY); - setLastMousePos(dialogX, dialogY); + if (dialog->isMouseUpdatedOnFocus()) { + setLastMousePos(dialogX, dialogY); + } } void GuiManager::setLastMousePos(int16 x, int16 y) { diff --git a/gui/gui-manager.h b/gui/gui-manager.h index a2f0efabe98..30ff6329379 100644 --- a/gui/gui-manager.h +++ b/gui/gui-manager.h @@ -30,6 +30,7 @@ #include "common/list.h" #include "gui/ThemeEngine.h" +#include "gui/widget.h" class OSystem; @@ -166,6 +167,13 @@ protected: int count; // How often was it already pressed? } _lastClick, _lastMousePosition, _globalMousePosition; + struct TooltipData { + TooltipData() : x(-1), y(-1) { time = 0; wdg = nullptr; } + uint32 time; // Time + Widget *wdg; // Widget that had its tooltip shown + int16 x, y; // Position of mouse before tooltip was focused + } _lastTooltipShown; + // mouse cursor state int _cursorAnimateCounter; int _cursorAnimateTimer; diff --git a/gui/widget.h b/gui/widget.h index ee92a972a0f..01c933c6348 100644 --- a/gui/widget.h +++ b/gui/widget.h @@ -153,6 +153,8 @@ public: void lostFocus() { _hasFocus = false; lostFocusWidget(); } virtual bool wantsFocus() { return false; } + uint32 getType() const { return _type; } + void setFlags(int flags); void clearFlags(int flags); int getFlags() const { return _flags; }