GRAPHICS: MACGUI: Slowdown in Macintosh menu when moving cursor over menu bar

[eriktorbjorn]

Something I've noticed on my (no longer very new) computer with the Macintosh menus, e.g. in the Macintosh versions of Loom and Indy 3:

If I open a menu, then move the mouse to the right of the last menu in the menu bar (while still keeping the mouse within the menu bar), ScummVM starts using excessive amounts of CPU to the point where it doesn't even have the resources to keep the mouse cursor updated. It appears to freeze.

I have tentatively traced this to how MacMenu::processEvent() is called.

MacWindowManager::processEvent() calls MacMenu::processEvent(). That's all well and good.

MacMenu::processEvent() handles the EVENT_MOUSEMOVE event, which calls MacMenu::mouseMove(). That function will call MacMenu::mouseClick(), perhaps so that sub menus are automatically opened? (At first I thought the problem was that it calls draw() for every processEvent() which, when including mouse movement, could be excessive. But apparently draw is well-behaved, and doesn't bog down unnecessarily.)

In kWMModalMenuMode, MacMenu::mouseClick() will call MacMenu::eventLoop(), which then - recursively - calls MacMenu::processEvent(), and that's where my eyes start to glaze over because I don't know what it's doing at this point. If I remove the call to eventLoop(), the problem goes away for me but I have no idea what else I've broken in that process.

As far as I can tell, sev wrote this code and the call to eventLoop() was part of the "Initial code for truly modal MacMenu".

[eriktorbjorn]

I've attached a video demonstrating just how laggy the mouse movement gets on my computer.

As I said, it's getting a bit long in the tooth but I doubt mine is the weakest hardware that ScummVM is going to run on.

[eriktorbjorn]

Random observation:

While moving the mouse within the defined menus, MacMenu::mouseClick() returns early because it hits the (uint)_activeItem == i case near the start of the function. Only when I move the mouse to parts of the menu bar that don't have any menus defined does it proceed to reach the recursive call.

So while moving around the defined menus, I seem to get like one recursive call each time the active menu changes. But when I leave the menus behind, I get one recursive call for each mouse movement event.

[eriktorbjorn]

Incidentally, I see the exact same thing in Director games like Spaceship Warlock, so it's not unique to how Loom and Indy 3 uses the Mac menus.

[eriktorbjorn]

I'm at a computer where I cant reproduce the problem right now (maybe compiling ScummVM in Linux, running in Windows 10 isn't the ideal test environment! :-), but my theory is that the problem happens because the recursion depth easily reaches dozens or even hundreds of levels.

When that call stack unwinds, MacMenu::eventLoop() probably ends up calling delayMillis(10) once for each level.

I won't be able to test it until tomorrow, but perhaps we could simply move the call to eventLoop() a bit:

diff --git a/graphics/macgui/macmenu.cpp b/graphics/macgui/macmenu.cpp
index ab5fa3a81be..7507bc4641d 100644
--- a/graphics/macgui/macmenu.cpp
+++ b/graphics/macgui/macmenu.cpp
@@ -1285,16 +1285,6 @@ bool MacMenu::mouseClick(int x, int y) {
 			_wm->activateMenu();
 
 		setActive(true);
-
-		if (_wm->_mode & kWMModalMenuMode) {
-			draw(_wm->_screen);
-			eventLoop();
-
-			// Do not do full refresh as we took care of restoring
-			// the screen. WM is not even aware we were drawing.
-			_wm->setFullRefresh(false);
-		}
-
 		return true;
 	}
 
diff --git a/graphics/macgui/macwindowmanager.cpp b/graphics/macgui/macwindowmanager.cpp
index c039bed5d30..4e1d4d3c6d1 100644
--- a/graphics/macgui/macwindowmanager.cpp
+++ b/graphics/macgui/macwindowmanager.cpp
@@ -1058,8 +1058,18 @@ bool MacWindowManager::processEvent(Common::Event &event) {
 	}
 
 	// Menu gets events first for shortcuts and menu bar
-	if (_menu && _menu->processEvent(event))
+	if (_menu && _menu->processEvent(event)) {
+		if (_mode & kWMModalMenuMode) {
+			_menu->draw(_screen);
+			_menu->eventLoop();
+
+			// Do not do full refresh as we took care of restoring
+			// the screen. WM is not even aware we were drawing.
+			setFullRefresh(false);
+		}
+
 		return true;
+	}
 
 	if (_activeWindow != -1) {
 		if ((_windows[_activeWindow]->isEditable() && _windows[_activeWindow]->getType() == kWindowWindow &&

I won't be able to test that until tomorrow at the earliest, though. And I still don't know exactly what effect kWMModalMenuMode is supposed to have.

[eriktorbjorn]

This has been fixed now