[Scummvm-git-logs] scummvm master -> 182ce944a5be5e286fabb7ad20018ef11f452929

AndywinXp noreply at scummvm.org
Sun Feb 19 19:23:14 UTC 2023


This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
182ce944a5 SCUMM: MI1 (Sega CD): Implement correct text centering routine


Commit: 182ce944a5be5e286fabb7ad20018ef11f452929
    https://github.com/scummvm/scummvm/commit/182ce944a5be5e286fabb7ad20018ef11f452929
Author: AndywinXp (andywinxp at gmail.com)
Date: 2023-02-19T20:23:07+01:00

Commit Message:
SCUMM: MI1 (Sega CD): Implement correct text centering routine

...this was painful to research. Check the comment on string.cpp for a detailed
explaination on how text centering supposedly works on this particular version.
This fixes the biggest issue of #10447, but there are still things left to do about it.

Changed paths:
    engines/scumm/charset.cpp
    engines/scumm/scumm.h
    engines/scumm/string.cpp


diff --git a/engines/scumm/charset.cpp b/engines/scumm/charset.cpp
index d7225d23f5b..0b3f0d000a8 100644
--- a/engines/scumm/charset.cpp
+++ b/engines/scumm/charset.cpp
@@ -434,10 +434,15 @@ void CharsetRendererV3::setCurID(int32 id) {
 }
 
 int CharsetRendererCommon::getFontHeight() const {
-	if (_vm->_useCJKMode)
+	bool isSegaCD = _vm->_game.platform == Common::kPlatformSegaCD;
+
+	if (isSegaCD && _vm->_segaForce2ByteCharHeight) {
+		return MAX(_vm->_2byteHeight, _fontHeight);
+	} else if (_vm->_useCJKMode && !isSegaCD) {
 		return MAX(_vm->_2byteHeight + 1, _fontHeight);
-	else
+	} else {
 		return _fontHeight;
+	}
 }
 
 // do spacing for variable width old-style font
@@ -526,12 +531,23 @@ int CharsetRenderer::getStringWidth(int arg, const byte *text) {
 					// This is the only way to get an accurate text formatting in the MI1 intro.
 					chr = (int8)text[pos++] | (chr << 8);
 			} else if (chr & 0x80) {
-				pos++;
-				width += _vm->_2byteWidth;
-				// Original keeps glyph width and character dimensions separately
-				if (_vm->_language == Common::KO_KOR || _vm->_language == Common::ZH_TWN) {
-					width++;
+				if (_vm->_game.platform == Common::kPlatformSegaCD) {
+					// Special character: this one has to be rendered as a space (0x20)
+					// and also inherits its rendering dimensions.
+					if (chr == 0xFD && text[pos] == 0xFA) {
+						width += getCharWidth(0x20);
+					} else {
+						width += _vm->_2byteWidth;
+					}
+				} else {
+					width += _vm->_2byteWidth;
+					// Original keeps glyph width and character dimensions separately
+					if (_vm->_language == Common::KO_KOR || _vm->_language == Common::ZH_TWN) {
+						width++;
+					}
 				}
+
+				pos++;
 				continue;
 			}
 		}
@@ -994,6 +1010,11 @@ void CharsetRendererClassic::printChar(int chr, bool ignoreCharsetMask) {
 	VirtScreen *vs;
 	bool is2byte = (chr >= 256 && _vm->_useCJKMode);
 
+	if (_vm->_game.platform == Common::kPlatformSegaCD && chr == 0xFAFD) {
+		is2byte = false;
+		chr = 0x20;
+	}
+
 	assertRange(1, _curId, _vm->_numCharsets - 1, "charset");
 
 	if ((vs = _vm->findVirtScreen(_top)) == nullptr && (vs = _vm->findVirtScreen(_top + getFontHeight())) == nullptr)
diff --git a/engines/scumm/scumm.h b/engines/scumm/scumm.h
index 7e5d79a9b9d..2ec3af78f0c 100644
--- a/engines/scumm/scumm.h
+++ b/engines/scumm/scumm.h
@@ -1596,6 +1596,7 @@ public:
 	bool _useMultiFont = false;
 	int _numLoadedFont = 0;
 	int _2byteShadow = 0;
+	bool _segaForce2ByteCharHeight = false;
 
 	int _2byteHeight = 0;
 	int _2byteWidth = 0;
diff --git a/engines/scumm/string.cpp b/engines/scumm/string.cpp
index 9d28411bb31..2f909b540a2 100644
--- a/engines/scumm/string.cpp
+++ b/engines/scumm/string.cpp
@@ -551,16 +551,17 @@ bool ScummEngine::newLine() {
 			// the original code it seems that setting _nextLeft to 0 is the right thing to do here.
 			_nextLeft = /*_game.version >= 6 ? _string[0].xpos :*/ 0;
 
-		// The Sega CD version of Monkey Island 1 performs
-		// additional clipping on the X position of the string
+		// See CHARSET_1() for more context about the following Sega CD code.
 		if (_game.platform == Common::kPlatformSegaCD) {
 			// Clip 16 pixels away from the right
-			if (_nextLeft + stringWidth > (_screenWidth - 16))
+			if (_nextLeft + stringWidth > (_screenWidth - 16)) {
 				_nextLeft -= (_nextLeft + stringWidth) - (_screenWidth - 16);
+			}
 
 			// Clip 16 pixels away from the left
-			if (_nextLeft < 16)
+			if (_nextLeft < 16) {
 				_nextLeft = 16;
+			}
 		}
 	} else if (_isRTL) {
 		if (_game.id == GID_MANIAC || _game.heversion >= 72 || ((_game.id == GID_MONKEY || _game.id == GID_MONKEY2) && _charset->getCurID() == 4)) {
@@ -578,19 +579,21 @@ bool ScummEngine::newLine() {
 			// The JAP Sega CD version of Monkey Island 1 doesn't just calculate
 			// the font height, but instead relies on the actual string height.
 			// If the string contains at least a 2 byte character, then we signal it with
-			// the opposite of the hack used below for FM-Towns, so that getFontHeight()
-			// can yield the correct result.
-			_useCJKMode = false;
-
+			// a flag, so that getFontHeight() can yield the correct result.
 			for (int i = 0; _charsetBuffer[i]; i++) {
+				// Handle the special 0xFAFD character, which actually is a 0x20 space char.
+				if (_charsetBuffer[i] == 0xFD && _charsetBuffer[i + 1] == 0xFA) {
+					i++;
+					continue;
+				}
+
 				if (is2ByteCharacter(_language, _charsetBuffer[i])) {
-					_useCJKMode = true;
+					_segaForce2ByteCharHeight = true;
 					break;
 				}
 			}
 
-			_nextTop += _charset->getFontHeight() - 1;
-			_useCJKMode = true;
+			_nextTop += _charset->getFontHeight();
 		} else {
 			bool useCJK = _useCJKMode;
 			// SCUMM5 FM-Towns doesn't use the height of the ROM font here.
@@ -604,6 +607,9 @@ bool ScummEngine::newLine() {
 		// FIXME: is this really needed?
 		_charset->_disableOffsX = true;
 	}
+
+	_segaForce2ByteCharHeight = false;
+
 	return true;
 }
 
@@ -953,39 +959,157 @@ void ScummEngine::CHARSET_1() {
 	}
 
 	if (_game.version > 3) {
-		int maxwidth = _charset->_right - _string[0].xpos;
+		int maxWidth = _charset->_right - _string[0].xpos;
 
-		// The Sega CD text routines are more sensible to text wrapping
-		maxwidth -= _game.platform == Common::kPlatformSegaCD ? 16 : 1;
+		maxWidth -= _game.platform == Common::kPlatformSegaCD ? 16 : 1;
 
 		if (_charset->_center) {
-			if (maxwidth > _nextLeft)
-				maxwidth = _nextLeft;
-			maxwidth *= 2;
+			if (maxWidth > _nextLeft)
+				maxWidth = _nextLeft;
+			maxWidth *= 2;
 		}
 
-		_charset->addLinebreaks(0, _charsetBuffer + _charsetBufPos, 0, maxwidth);
+		// If the string is centered and this is MI1 Sega CD, don't add linebreaks right away;
+		// we will take care of it in a different way just below ... :-)
+		if (_game.platform != Common::kPlatformSegaCD && !_charset->_center) {
+			_charset->addLinebreaks(0, _charsetBuffer + _charsetBufPos, 0, maxWidth);
+		}
 	}
 
 	if (_charset->_center) {
-		int stringWidth = _charset->getStringWidth(0, _charsetBuffer + _charsetBufPos);
-		_nextLeft -= stringWidth / 2;
-
-		if (_nextLeft < 0)
-			_nextLeft = _game.version >= 6 ? _string[0].xpos : 0;
+		// MI1 Sega CD appears to be doing its own thing in here when
+		// the string x coordinate is on the right side of the screen:
+		// - Applies some tentative line breaks;
+		// - Go line by line and check if the string still overflows
+		//   on the last 16 pixels of the right side of the screen;
+		// - If so, take the original string and apply a stricter final wrapping;
+		// - Finally, clip the string final position to 16 pixels from the right
+		//   and from the left sides of the screen.
+		//
+		// I agree that this is very convoluted :-) , but it's the only way
+		// to display pixel accurate text on both ENG and JAP editions of this
+		// version.
 
-		// The Sega CD version of Monkey Island 1 performs
-		// additional clipping on the X position of the string
 		if (_game.platform == Common::kPlatformSegaCD) {
-			// Clip 16 pixels away from the right
-			if (_nextLeft + stringWidth > (_screenWidth - 16))
-				_nextLeft -= (_nextLeft + stringWidth) - (_screenWidth - 16);
+			int predictionMaxWidth = _charset->_right - _string[0].xpos;
+			int predictionNextLeft = _nextLeft;
+
+			bool useStricterWrapping = (_string[0].xpos > _screenWidth / 2);
+
+			// Predict if a stricter wrapping is going to be necessary
+			if (!useStricterWrapping) {
+				if (predictionMaxWidth > predictionNextLeft)
+					predictionMaxWidth = predictionNextLeft;
+				predictionMaxWidth *= 2;
+
+				byte predictionString[512];
+
+				memcpy(predictionString, _charsetBuffer, sizeof(predictionString));
+
+				// Impose a tentative max string width for the wrapping
+				_charset->addLinebreaks(0, predictionString + _charsetBufPos, 0, predictionMaxWidth);
+
+				int predictionStringWidth = _charset->getStringWidth(0, predictionString + _charsetBufPos);
+				predictionNextLeft -= predictionStringWidth / 2;
+
+				if (predictionNextLeft < 16)
+					predictionNextLeft = 16;
+
+				byte *ptrToCurLine = predictionString + _charsetBufPos;
+				byte curChar = *ptrToCurLine;
+
+				// Go line by line and check if the string overflows
+				// on the last 16 pixels on the right side of the screen...
+				while (curChar) {
+					predictionStringWidth = _charset->getStringWidth(0, ptrToCurLine);
+					predictionNextLeft -= predictionStringWidth / 2;
+
+					if (predictionNextLeft < 16)
+						predictionNextLeft = 16;
 
+					useStricterWrapping |= (predictionNextLeft + predictionStringWidth > (_screenWidth - 16));
+
+					if (useStricterWrapping)
+						break;
+
+					// Advance to next line, if any...
+					do {
+						// Control code handling...
+						if (curChar == 0xFE || curChar == 0xFF) {
+							// Advance to the control code and
+							// check if it's a new line instruction...
+							ptrToCurLine++;
+							curChar = *ptrToCurLine;
+
+							// Gotcha!
+							if (curChar == 1 || (_newLineCharacter && curChar == _newLineCharacter)) {
+								ptrToCurLine++;
+								curChar = *ptrToCurLine;
+								break;
+							}
+
+							// If we're here, we don't need this control code,
+							// let's just skip it...
+							ptrToCurLine++;
+						} else if (_useCJKMode && curChar & 0x80) { // CJK char
+							ptrToCurLine++;
+						}
+
+						// Line breaks and string termination
+						if (curChar == '\r' || curChar == '\n') {
+							ptrToCurLine++;
+							curChar = *ptrToCurLine;
+							break;
+						} else if (curChar == '\0') {
+							curChar = *ptrToCurLine;
+							break;
+						}
+
+						ptrToCurLine++;
+						curChar = *ptrToCurLine;
+					} while (true);
+				}
+			}
+
+
+			// Impose the final line breaks with the correct max string width;
+			// this part is practically the default v5 text centering code...
+			int finalMaxWidth = _charset->_right - _string[0].xpos;
+			finalMaxWidth -= useStricterWrapping ? 16 : 0;
+
+			if (finalMaxWidth > _nextLeft)
+				finalMaxWidth = _nextLeft;
+			finalMaxWidth *= 2;
+
+			_charset->addLinebreaks(0, _charsetBuffer + _charsetBufPos, 0, finalMaxWidth);
+
+			int finalStringWidth = _charset->getStringWidth(0, _charsetBuffer + _charsetBufPos);
+			_nextLeft -= finalStringWidth / 2;
+
+			// Final additional clippings (these will also be repeated on newLine()):
+
+			// Clip 16 pixels away from the right
+			if (_nextLeft + finalStringWidth > (_screenWidth - 16)) {
+				_nextLeft -= (_nextLeft + finalStringWidth) - (_screenWidth - 16);
+			}
 
 			// Clip 16 pixels away from the left
-			if (_nextLeft < 16)
+			if (_nextLeft < 16) {
 				_nextLeft = 16;
+			}
+		} else {
+			int stringWidth = _charset->getStringWidth(0, _charsetBuffer + _charsetBufPos);
+			_nextLeft -= stringWidth / 2;
+
+			if (_nextLeft < 0)
+				// The commented out part of the next line was meant as a fix for Kanji text glitches in DIG.
+				// But these glitches couldn't be reproduced in recent tests. So the underlying issue might
+				// have been taken care of in a different manner. And the fix actually caused other text glitches
+				// (FT/German, if you look at the sign on the container at game start). After counterchecking
+				// the original code it seems that setting _nextLeft to 0 is the right thing to do here.
+				_nextLeft = /*_game.version >= 6 ? _string[0].xpos :*/ 0;
 		}
+
 	} else if (_isRTL) {
 		if (_game.id == GID_MANIAC || _game.heversion >= 72 || ((_game.id == GID_MONKEY || _game.id == GID_MONKEY2) && _charset->getCurID() == 4)) {
 			_nextLeft = _screenWidth - _charset->getStringWidth(0, _charsetBuffer + _charsetBufPos) - _nextLeft;




More information about the Scummvm-git-logs mailing list