[Scummvm-git-logs] scummvm master -> c99265cf9725b8ec32698b83088fe2c8f304749f
AndywinXp
noreply at scummvm.org
Thu Sep 28 09:02:34 UTC 2023
This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
d222359fde SWORD1: More thread-safeness measures as part of #14638
c99265cf97 SWORD1: Optimize menu handling during palette fades
Commit: d222359fde912255e23aca6f61b182bf9c42b3ff
https://github.com/scummvm/scummvm/commit/d222359fde912255e23aca6f61b182bf9c42b3ff
Author: AndywinXp (andywinxp at gmail.com)
Date: 2023-09-28T11:00:20+02:00
Commit Message:
SWORD1: More thread-safeness measures as part of #14638
This commit deals with all the copyRectToScreen calls which, during
a palette fade, can collide with screen updates. These changes are
verified with a Thread Sanitizer enabled build.
I know that the latest commits have introduced lots of locks, and
I'm going to optimize the optimizable, but first I have to reach a
no-crash point :-)
Changed paths:
engines/sword1/screen.cpp
diff --git a/engines/sword1/screen.cpp b/engines/sword1/screen.cpp
index a40eed1431c..276731e075a 100644
--- a/engines/sword1/screen.cpp
+++ b/engines/sword1/screen.cpp
@@ -327,9 +327,8 @@ bool Screen::showScrollFrame() {
uint16 avgScrlX = (uint16)(_oldScrollX + Logic::_scriptVars[SCROLL_OFFSET_X]) / 2;
uint16 avgScrlY = (uint16)(_oldScrollY + Logic::_scriptVars[SCROLL_OFFSET_Y]) / 2;
- _system->copyRectToScreen(_screenBuf + avgScrlY * _scrnSizeX + avgScrlX, _scrnSizeX, 0, 40, SCREEN_WIDTH, SCREEN_DEPTH);
-
_screenAccessMutex.lock();
+ _system->copyRectToScreen(_screenBuf + avgScrlY * _scrnSizeX + avgScrlX, _scrnSizeX, 0, 40, SCREEN_WIDTH, SCREEN_DEPTH);
_system->updateScreen();
_screenAccessMutex.unlock();
@@ -364,7 +363,10 @@ void Screen::updateScreen() {
copyWidth = _scrnSizeX - scrlX;
if (scrlY + copyHeight > _scrnSizeY)
copyHeight = _scrnSizeY - scrlY;
+
+ _screenAccessMutex.lock();
_system->copyRectToScreen(_screenBuf + scrlY * _scrnSizeX + scrlX, _scrnSizeX, 0, 40, copyWidth, copyHeight);
+ _screenAccessMutex.unlock();
} else {
// partial screen update only. The screen coordinates probably won't fit to the
// grid holding the informations on which blocks have to be updated.
@@ -387,14 +389,21 @@ void Screen::updateScreen() {
int16 xPos = (cntx - cpWidth) * SCRNGRID_X - diffX;
if (xPos < 0)
xPos = 0;
+
+ _screenAccessMutex.lock();
_system->copyRectToScreen(scrnBuf + xPos, _scrnSizeX, xPos, 40, cpWidth * SCRNGRID_X, diffY);
+ _screenAccessMutex.unlock();
+
cpWidth = 0;
}
if (cpWidth) {
int16 xPos = (gridW - cpWidth) * SCRNGRID_X - diffX;
if (xPos < 0)
xPos = 0;
+
+ _screenAccessMutex.lock();
_system->copyRectToScreen(scrnBuf + xPos, _scrnSizeX, xPos, 40, SCREEN_WIDTH - xPos, diffY);
+ _screenAccessMutex.unlock();
}
scrlY += diffY;
}
@@ -410,14 +419,21 @@ void Screen::updateScreen() {
cpHeight++;
} else if (cpHeight) {
uint16 yPos = (cnty - cpHeight) * SCRNGRID_Y;
+
+ _screenAccessMutex.lock();
_system->copyRectToScreen(scrnBuf + yPos * _scrnSizeX, _scrnSizeX, 0, yPos + diffY + 40, diffX, cpHeight * SCRNGRID_Y);
+ _screenAccessMutex.unlock();
+
cpHeight = 0;
}
gridPos += _gridSizeX;
}
if (cpHeight) {
uint16 yPos = (gridH - cpHeight) * SCRNGRID_Y;
+
+ _screenAccessMutex.lock();
_system->copyRectToScreen(scrnBuf + yPos * _scrnSizeX, _scrnSizeX, 0, yPos + diffY + 40, diffX, SCREEN_DEPTH - (yPos + diffY));
+ _screenAccessMutex.unlock();
}
scrlX += diffX;
}
@@ -434,12 +450,17 @@ void Screen::updateScreen() {
gridPos[cntx] >>= 1;
cpWidth++;
} else if (cpWidth) {
+ _screenAccessMutex.lock();
_system->copyRectToScreen(scrnBuf + (cntx - cpWidth) * SCRNGRID_X, _scrnSizeX, (cntx - cpWidth) * SCRNGRID_X + diffX, cnty * SCRNGRID_Y + diffY + 40, cpWidth * SCRNGRID_X, cpHeight);
+ _screenAccessMutex.unlock();
+
cpWidth = 0;
}
if (cpWidth) {
uint16 xPos = (gridW - cpWidth) * SCRNGRID_X;
+ _screenAccessMutex.lock();
_system->copyRectToScreen(scrnBuf + xPos, _scrnSizeX, xPos + diffX, cnty * SCRNGRID_Y + diffY + 40, SCREEN_WIDTH - (xPos + diffX), cpHeight);
+ _screenAccessMutex.unlock();
}
gridPos += _gridSizeX;
scrnBuf += _scrnSizeX * SCRNGRID_Y;
@@ -1461,7 +1482,9 @@ void Screen::showFrame(uint16 x, uint16 y, uint32 resId, uint32 frameNo, const b
}
}
+ _screenAccessMutex.lock();
_system->copyRectToScreen(frame, 40, x, y, 40, 40);
+ _screenAccessMutex.unlock();
}
// ------------------- Router debugging code --------------------------------
Commit: c99265cf9725b8ec32698b83088fe2c8f304749f
https://github.com/scummvm/scummvm/commit/c99265cf9725b8ec32698b83088fe2c8f304749f
Author: AndywinXp (andywinxp at gmail.com)
Date: 2023-09-28T11:02:04+02:00
Commit Message:
SWORD1: Optimize menu handling during palette fades
We are only updating the menu bars when the screen is not fading.
This optimization restores the same code speed as previous
codebase iterations without thread-safeness whenever a menu
is interacted with during a palette fade.
Changed paths:
engines/sword1/menu.cpp
engines/sword1/menu.h
engines/sword1/sword1.cpp
engines/sword1/sword1.h
diff --git a/engines/sword1/menu.cpp b/engines/sword1/menu.cpp
index 8be31375431..fc42657c0a3 100644
--- a/engines/sword1/menu.cpp
+++ b/engines/sword1/menu.cpp
@@ -367,6 +367,24 @@ void Menu::checkTopMenu() {
checkMenuClick(MENU_TOP);
}
+void Menu::setToTargetState() {
+ // This is an optimization for all the locks introduced
+ // with the fade palette changes: we disable the menu
+ // updates whenever the palette is fading, and we bring
+ // the menu to its target state.
+ // Note that we are only doing this for the top menu:
+ // I haven't seen any instance of a bottom menu (dialog)
+ // being able to immediately open after a palette fade.
+ if (_objectBarStatus == MENU_CLOSING)
+ _objectBarStatus = MENU_CLOSED;
+
+ if (_objectBarStatus == MENU_OPENING) {
+ _objectBarStatus = MENU_OPEN;
+ _fadeObject = 8;
+ showMenu(MENU_TOP);
+ }
+}
+
int Menu::logicChooser(Object *compact) {
uint8 objSelected = 0;
if (_objectBarStatus == MENU_OPEN)
diff --git a/engines/sword1/menu.h b/engines/sword1/menu.h
index b5c1a2585d6..eb8ff50a7ad 100644
--- a/engines/sword1/menu.h
+++ b/engines/sword1/menu.h
@@ -75,6 +75,7 @@ public:
void fnStartMenu();
void fnEndMenu();
void checkTopMenu();
+ void setToTargetState();
static const MenuObject _objectDefs[TOTAL_pockets + 1];
private:
diff --git a/engines/sword1/sword1.cpp b/engines/sword1/sword1.cpp
index 1c0ec83a4b1..2807ad6f4dc 100644
--- a/engines/sword1/sword1.cpp
+++ b/engines/sword1/sword1.cpp
@@ -919,6 +919,10 @@ void SwordEngine::showDebugInfo() {
}
}
+void SwordEngine::setMenuToTargetState() {
+ _menu->setToTargetState();
+}
+
void SwordEngine::checkCd() {
uint8 needCd = _cdList[Logic::_scriptVars[NEW_SCREEN]];
if (_systemVars.runningFromCd) { // are we running from cd?
@@ -1212,12 +1216,20 @@ static void vblCallback(void *refCon) {
vm->_vblCount++;
vm->_vbl60HzUSecElapsed += TIMER_USEC;
- if ((vm->_vblCount == 1) || (vm->_vblCount == 5)) {
- vm->updateTopMenu();
- }
+ if (!vm->screenIsFading()) {
+ if ((vm->_vblCount == 1) || (vm->_vblCount == 5)) {
+ vm->updateTopMenu();
+ }
- if ((vm->_vblCount == 3) || (vm->_vblCount == 7)) {
- vm->updateBottomMenu();
+ if ((vm->_vblCount == 3) || (vm->_vblCount == 7)) {
+ vm->updateBottomMenu();
+ }
+ } else {
+ // This is an optimization for all the locks introduced
+ // with the fade palette changes: we disable the menu
+ // updates whenever the palette is fading, and we bring
+ // the menu to its target state.
+ vm->setMenuToTargetState();
}
if (vm->_vbl60HzUSecElapsed >= PALETTE_FADE_USEC) {
@@ -1232,6 +1244,10 @@ static void vblCallback(void *refCon) {
vm->_inTimer--;
}
+bool SwordEngine::screenIsFading() {
+ return _screen->stillFading() != 0;
+}
+
void SwordEngine::installTimerRoutines() {
debug(2, "SwordEngine::installTimerRoutines(): Installing timers...");
_ticker = 0;
diff --git a/engines/sword1/sword1.h b/engines/sword1/sword1.h
index 8b1b448e2e6..e270baca835 100644
--- a/engines/sword1/sword1.h
+++ b/engines/sword1/sword1.h
@@ -123,6 +123,8 @@ public:
void startFadePaletteDown(int speed);
void startFadePaletteUp(int speed);
void waitForFade();
+ bool screenIsFading();
+ void setMenuToTargetState();
void showDebugInfo();
More information about the Scummvm-git-logs
mailing list