[Scummvm-git-logs] scummvm master -> 2dbf32353d2714fca97ed3f3084d5769a4b6fa39

eriktorbjorn eriktorbjorn at telia.com
Tue Oct 19 17:24:38 UTC 2021


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

Summary:
4cf42c6dac SCUMM: Center text better in Mac Loom (bug #12984)
c3ce996275 SCUMM: Add braces to empty loop
e81a016699 SCUMM: Move COMI getStringWidth() to CharsetRendererNut
a67958a71f SCUMM: Implement the incorrect character spacing used in Indy 3 Mac
11bf8833df SCUMM: Add setting for correct font spacing in Indy 3 Mac
63a83d7eaa SCUMM: Warn on unhandled escape sequences in Mac font renderer
2dbf32353d SCUMM: Hack Indy 3 Mac credits to use correct text color


Commit: 4cf42c6dac924f49ff2521ca80fd0d50a5c2c0d3
    https://github.com/scummvm/scummvm/commit/4cf42c6dac924f49ff2521ca80fd0d50a5c2c0d3
Author: Torbjörn Andersson (eriktorbjorn at users.sourceforge.net)
Date: 2021-10-19T19:24:31+02:00

Commit Message:
SCUMM: Center text better in Mac Loom (bug #12984)

Measure the whole string before dividing the length by 2 to get the
low-res width of the string. Before, each character width was divided
which led to rounding errors. The result is better, but not pixel
perfect. I'm not sure what I'm still getting wrong.

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


diff --git a/engines/scumm/charset.cpp b/engines/scumm/charset.cpp
index 21816ed932..e9e2698f09 100644
--- a/engines/scumm/charset.cpp
+++ b/engines/scumm/charset.cpp
@@ -538,7 +538,7 @@ int CharsetRenderer::getStringWidth(int arg, const byte *text, uint strLenMax) {
 					if (arg == 1)
 						break;
 					while (text[pos++] == ' ')
-					;
+						;
 					continue;
 				}
 				if (chr == 10 || chr == 21 || chr == 12 || chr == 13) {
@@ -1666,6 +1666,25 @@ void CharsetRendererMac::setCurID(int32 id) {
 	_curId = id;
 }
 
+int CharsetRendererMac::getStringWidth(int arg, const byte *text, uint strLenMax) {
+	int pos = 0;
+	int width = 0;
+	int chr;
+
+	while ((chr = text[pos++]) != 0) {
+		// The only control codes I've seen in use are line breaks in
+		// Loom. In Indy 3, I haven't seen anything at all like it.
+		if (chr == 255) {
+			chr = text[pos++];
+			if (chr == 1) // 'Newline'
+				break;
+		}
+		width += _macFonts[_curId].getCharWidth(chr);
+	}
+
+	return width / 2;
+}
+
 // HACK: Usually, we want the approximate width and height in the unscaled
 //       graphics resolution. But for font 1 in Indiana Jones and the Last
 //       crusade we want the actual dimensions for drawing the text boxes.
diff --git a/engines/scumm/charset.h b/engines/scumm/charset.h
index e4c781d5e8..e5e6aab97c 100644
--- a/engines/scumm/charset.h
+++ b/engines/scumm/charset.h
@@ -97,7 +97,7 @@ public:
 	virtual void printChar(int chr, bool ignoreCharsetMask) = 0;
 	virtual void drawChar(int chr, Graphics::Surface &s, int x, int y) {}
 
-	int getStringWidth(int a, const byte *str, uint strLenMax = 100000);
+	virtual int getStringWidth(int arg, const byte *text, uint strLenMax = 100000);
 	void addLinebreaks(int a, byte *str, int pos, int maxwidth);
 	void translateColor();
 
@@ -294,6 +294,7 @@ public:
 	~CharsetRendererMac() override;
 
 	void setCurID(int32 id) override;
+	int getStringWidth(int arg, const byte *text, uint strLenMax = 100000);
 	int getFontHeight() override;
 	int getCharWidth(uint16 chr) override;
 	void printChar(int chr, bool ignoreCharsetMask) override;


Commit: c3ce99627557ad3f11e647b530a07efc568ff00f
    https://github.com/scummvm/scummvm/commit/c3ce99627557ad3f11e647b530a07efc568ff00f
Author: Torbjörn Andersson (eriktorbjorn at users.sourceforge.net)
Date: 2021-10-19T19:24:31+02:00

Commit Message:
SCUMM: Add braces to empty loop

It's mandatory in our code formatting conventions.

Changed paths:
    engines/scumm/charset.cpp


diff --git a/engines/scumm/charset.cpp b/engines/scumm/charset.cpp
index e9e2698f09..68719a5811 100644
--- a/engines/scumm/charset.cpp
+++ b/engines/scumm/charset.cpp
@@ -537,8 +537,7 @@ int CharsetRenderer::getStringWidth(int arg, const byte *text, uint strLenMax) {
 				if (chr == 8) { // 'Verb on next line'
 					if (arg == 1)
 						break;
-					while (text[pos++] == ' ')
-						;
+					while (text[pos++] == ' ') {}
 					continue;
 				}
 				if (chr == 10 || chr == 21 || chr == 12 || chr == 13) {


Commit: e81a01669975c7241775fe2491c68231893fb3c7
    https://github.com/scummvm/scummvm/commit/e81a01669975c7241775fe2491c68231893fb3c7
Author: Torbjörn Andersson (eriktorbjorn at users.sourceforge.net)
Date: 2021-10-19T19:24:31+02:00

Commit Message:
SCUMM: Move COMI getStringWidth() to CharsetRendererNut

As far as I know, that's the only game that uses the Nut charset
renderer.

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


diff --git a/engines/scumm/charset.cpp b/engines/scumm/charset.cpp
index 68719a5811..4780832f55 100644
--- a/engines/scumm/charset.cpp
+++ b/engines/scumm/charset.cpp
@@ -456,52 +456,6 @@ int CharsetRendererClassic::getCharWidth(uint16 chr) {
 }
 
 int CharsetRenderer::getStringWidth(int arg, const byte *text, uint strLenMax) {
-	if (_vm->_game.id == GID_CMI) {
-		// SCUMM7 games actually use the same implemention (minus the strLenMax parameter). If
-		// any text placement bugs in one of these games come up it might be worth to look at
-		// that. Or simply for the fact that we could get rid of SmushFont::getStringWidth()...
-		if (!strLenMax)
-			return 0;
-
-		int maxWidth = 0;
-		int width = 0;
-
-		while (*text && strLenMax) {
-			while (text[0] == '^') {
-				switch (text[1]) {
-				case 'f':
-					// We should change the font on the fly at this point
-					// which would result in a different width result.
-					// This has never been observed in the game though, and
-					// as such, we don't handle it.
-					text += 4;
-					break;
-				case 'c':
-					text += 5;
-					break;
-				default:
-					error("CharsetRenderer::getStringWidth(): Invalid escape code in text string");
-				}
-			}
-
-			if (is2ByteCharacter(_vm->_language, *text)) {
-				width += _vm->_2byteWidth + (_vm->_language != Common::JA_JPN ? 1 : 0);
-				++text;
-				--strLenMax;
-			} else if (*text == '\n') {
-				maxWidth = MAX<int>(width, maxWidth);
-				width = 0;
-			} else if (*text != '\r' && *text != _vm->_newLineCharacter) {
-				width += getCharWidth(*text);
-			}
-
-			++text;
-			--strLenMax;
-		}
-
-		return MAX<int>(width, maxWidth);
-	}
-
 	int pos = 0;
 	int width = 1;
 	int chr;
@@ -2033,6 +1987,52 @@ int CharsetRendererNut::getFontHeight() {
 	return _current->getCharHeight('|');
 }
 
+int CharsetRendererNut::getStringWidth(int arg, const byte *text, uint strLenMax) {
+	// SCUMM7 games actually use the same implemention (minus the strLenMax parameter). If
+	// any text placement bugs in one of these games come up it might be worth to look at
+	// that. Or simply for the fact that we could get rid of SmushFont::getStringWidth()...
+	if (!strLenMax)
+		return 0;
+
+	int maxWidth = 0;
+	int width = 0;
+
+	while (*text && strLenMax) {
+		while (text[0] == '^') {
+			switch (text[1]) {
+			case 'f':
+				// We should change the font on the fly at this point
+				// which would result in a different width result.
+				// This has never been observed in the game though, and
+				// as such, we don't handle it.
+				text += 4;
+				break;
+			case 'c':
+				text += 5;
+				break;
+			default:
+				error("CharsetRenderer::getStringWidth(): Invalid escape code in text string");
+			}
+		}
+
+		if (is2ByteCharacter(_vm->_language, *text)) {
+			width += _vm->_2byteWidth + (_vm->_language != Common::JA_JPN ? 1 : 0);
+			++text;
+			--strLenMax;
+		} else if (*text == '\n') {
+			maxWidth = MAX<int>(width, maxWidth);
+			width = 0;
+		} else if (*text != '\r' && *text != _vm->_newLineCharacter) {
+			width += getCharWidth(*text);
+		}
+
+		++text;
+		--strLenMax;
+	}
+
+	return MAX<int>(width, maxWidth);
+}
+
 void CharsetRendererNut::printChar(int chr, bool ignoreCharsetMask) {
 	Common::Rect shadow;
 
diff --git a/engines/scumm/charset.h b/engines/scumm/charset.h
index e5e6aab97c..0301a977c9 100644
--- a/engines/scumm/charset.h
+++ b/engines/scumm/charset.h
@@ -294,6 +294,7 @@ public:
 	~CharsetRendererMac() override;
 
 	void setCurID(int32 id) override;
+
 	int getStringWidth(int arg, const byte *text, uint strLenMax = 100000);
 	int getFontHeight() override;
 	int getCharWidth(uint16 chr) override;
@@ -316,6 +317,7 @@ public:
 
 	void setCurID(int32 id) override;
 
+	int getStringWidth(int arg, const byte *text, uint strLenMax = 1000000);
 	int getFontHeight() override;
 	int getCharHeight(byte chr) override;
 	int getCharWidth(uint16 chr) override;


Commit: a67958a71ff8dfec528b06a4e63261ce51642b8a
    https://github.com/scummvm/scummvm/commit/a67958a71ff8dfec528b06a4e63261ce51642b8a
Author: Torbjörn Andersson (eriktorbjorn at users.sourceforge.net)
Date: 2021-10-19T19:24:31+02:00

Commit Message:
SCUMM: Implement the incorrect character spacing used in Indy 3 Mac

This is more guesswork, but seems to match the original for where I've
tried it. Next step is to implement an option to use the correct spacing
instead since it arguably looks better.

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


diff --git a/engines/scumm/charset.cpp b/engines/scumm/charset.cpp
index 4780832f55..03c2bd46ed 100644
--- a/engines/scumm/charset.cpp
+++ b/engines/scumm/charset.cpp
@@ -1632,12 +1632,16 @@ int CharsetRendererMac::getStringWidth(int arg, const byte *text, uint strLenMax
 			if (chr == 1) // 'Newline'
 				break;
 		}
-		width += _macFonts[_curId].getCharWidth(chr);
+		width += getDrawWidthIntern(chr);
 	}
 
 	return width / 2;
 }
 
+int CharsetRendererMac::getDrawWidthIntern(uint16 chr) {
+	return _macFonts[_curId].getCharWidth(chr);
+}
+
 // HACK: Usually, we want the approximate width and height in the unscaled
 //       graphics resolution. But for font 1 in Indiana Jones and the Last
 //       crusade we want the actual dimensions for drawing the text boxes.
@@ -1654,7 +1658,7 @@ int CharsetRendererMac::getFontHeight() {
 }
 
 int CharsetRendererMac::getCharWidth(uint16 chr) {
-	int width = _macFonts[_curId].getCharWidth(chr);
+	int width = getDrawWidthIntern(chr);
 
 	// For font 1 in Last Crusade, we want the real width. It is used for
 	// text box titles, which are drawn outside the normal font rendering.
@@ -1759,16 +1763,27 @@ void CharsetRendererMac::printChar(int chr, bool ignoreCharsetMask) {
 
 	// Mark the virtual screen as dirty, using downscaled coordinates.
 
-	int left, right, top, bottom;
+	int left, right, top, bottom, width;
+
+	width = getDrawWidthIntern(chr);
+
+	// HACK: Indiana Jones and the Last Crusade uses incorrect spacing
+	// betweeen letters. Note that this incorrect spacing does not extend
+	// to the text boxes, nor does it seem to be used when figuring out
+	// the width of a string (e.g. to center text on screen). It is,
+	// however, used for things like the Grail Diary.
+
+	if (_vm->_game.id == GID_INDY3 && !drawToTextBox && (width & 1))
+		width++;
 
 	if (enableShadow) {
 		left = macLeft / 2;
-		right = (macLeft + _macFonts[_curId].getCharWidth(chr) + 3) / 2;
+		right = (macLeft + width + 3) / 2;
 		top = macTop / 2;
 		bottom = (macTop + _macFonts[_curId].getFontHeight() + 3) / 2;
 	} else {
 		left = (macLeft + 1) / 2;
-		right = (macLeft + _macFonts[_curId].getCharWidth(chr) + 1) / 2;
+		right = (macLeft + width + 1) / 2;
 		top = (macTop + 1) / 2;
 		bottom = (macTop + _macFonts[_curId].getFontHeight() + 1) / 2;
 	}
@@ -1799,7 +1814,7 @@ void CharsetRendererMac::printChar(int chr, bool ignoreCharsetMask) {
 	// The next character may have to be adjusted to compensate for
 	// rounding errors.
 
-	macLeft += _macFonts[_curId].getCharWidth(chr);
+	macLeft += width;
 	if (macLeft & 1)
 		_pad = true;
 
diff --git a/engines/scumm/charset.h b/engines/scumm/charset.h
index 0301a977c9..500af1924f 100644
--- a/engines/scumm/charset.h
+++ b/engines/scumm/charset.h
@@ -281,6 +281,8 @@ protected:
 	bool _pad;
 	int _lastTop;
 
+	int getDrawWidthIntern(uint16 chr);
+
 	void printCharInternal(int chr, int color, bool shadow, int x, int y);
 	void printCharToTextBox(int chr, int color, int x, int y);
 


Commit: 11bf8833dfcf24b05ae54d8173838b3c6b593d21
    https://github.com/scummvm/scummvm/commit/11bf8833dfcf24b05ae54d8173838b3c6b593d21
Author: Torbjörn Andersson (eriktorbjorn at users.sourceforge.net)
Date: 2021-10-19T19:24:31+02:00

Commit Message:
SCUMM: Add setting for correct font spacing in Indy 3 Mac

For all the purists out there (I know there is at least one), the
default behavior is to try and emulate the original's slightly broken
font spacing. Even though I prefer correct spacing myself.

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


diff --git a/engines/scumm/charset.cpp b/engines/scumm/charset.cpp
index 03c2bd46ed..df74b730e5 100644
--- a/engines/scumm/charset.cpp
+++ b/engines/scumm/charset.cpp
@@ -1502,9 +1502,10 @@ void CharsetRendererPCE::setDrawCharIntern(uint16 chr) {
 }
 #endif
 
-CharsetRendererMac::CharsetRendererMac(ScummEngine *vm, const Common::String &fontFile)
+CharsetRendererMac::CharsetRendererMac(ScummEngine *vm, const Common::String &fontFile, bool correctFontSpacing)
 	 : CharsetRendererCommon(vm) {
 
+	_correctFontSpacing = correctFontSpacing;
 	_pad = false;
 	_glyphSurface = NULL;
 
@@ -1773,7 +1774,7 @@ void CharsetRendererMac::printChar(int chr, bool ignoreCharsetMask) {
 	// the width of a string (e.g. to center text on screen). It is,
 	// however, used for things like the Grail Diary.
 
-	if (_vm->_game.id == GID_INDY3 && !drawToTextBox && (width & 1))
+	if (!_correctFontSpacing && !drawToTextBox && (width & 1))
 		width++;
 
 	if (enableShadow) {
diff --git a/engines/scumm/charset.h b/engines/scumm/charset.h
index 500af1924f..20573cf1e2 100644
--- a/engines/scumm/charset.h
+++ b/engines/scumm/charset.h
@@ -278,9 +278,11 @@ public:
 class CharsetRendererMac : public CharsetRendererCommon {
 protected:
 	Graphics::MacFONTFont _macFonts[2];
+	bool _correctFontSpacing;
 	bool _pad;
 	int _lastTop;
 
+
 	int getDrawWidthIntern(uint16 chr);
 
 	void printCharInternal(int chr, int color, bool shadow, int x, int y);
@@ -292,7 +294,7 @@ protected:
 	Graphics::Surface *_glyphSurface;
 
 public:
-	CharsetRendererMac(ScummEngine *vm, const Common::String &fontFile);
+	CharsetRendererMac(ScummEngine *vm, const Common::String &fontFile, bool correctFontSpacing);
 	~CharsetRendererMac() override;
 
 	void setCurID(int32 id) override;
diff --git a/engines/scumm/detection.cpp b/engines/scumm/detection.cpp
index c57ad166b0..d60be3d963 100644
--- a/engines/scumm/detection.cpp
+++ b/engines/scumm/detection.cpp
@@ -200,6 +200,13 @@ static const ExtraGuiOption macV3LowQualityMusic = {
 	false
 };
 
+static const ExtraGuiOption macV3CorrectFontSpacing = {
+	_s("Use correct font spacing"),
+	_s("Draw text with correct font spacing. This arguably looks better, but doesn't match the original behavior."),
+	"mac_v3_correct_font_spacing",
+	false
+};
+
 static const ExtraGuiOption smoothScrolling = {
 	_s("Enable smooth scrolling"),
 	_s("(instead of the normal 8-pixels steps scrolling)"),
@@ -227,13 +234,30 @@ const ExtraGuiOptions ScummMetaEngineDetection::getExtraGuiOptions(const Common:
 		if (guiOptions.contains(GUIO_TRIM_FMTOWNS_TO_200_PIXELS))
 			options.push_back(fmtownsTrimTo200);
 	}
-	// The Steam Mac version of Loom is more akin to the VGA DOS version,
-	// and that's how ScummVM usually sees it. But that rebranding does not
-	// happen until later.
+
+	// The Steam Mac versions of Loom and Indy 3 are more akin to the VGA
+	// DOS versions, and that's how ScummVM usually sees them. But that
+	// rebranding does not happen until later.
+
+	// The low quality music in Loom was probably intended for low-end
+	// Macs. It plays only one channel, instead of three.
+
 	if (target.empty() || (gameid == "loom" && platform == Common::kPlatformMacintosh && extra != "Steam")) {
 		options.push_back(macV3LowQualityMusic);
 	}
 
+	// The original Macintosh interpreter didn't use the correct spacing
+	// between characters for some of the text, e.g. the Grail Diary. This
+	// appears to have been because of rounding errors, and was apparently
+	// fixed in Loom. Enabling this setting allows ScummVM to draw the
+	// text more correctly, at the cost of not matching the original quite
+	// as well. (At the time of writing, there are still cases, at least in
+	// Loom, where text isn't correctly positioned.)
+
+	if (target.empty() || (gameid == "indy3" && platform == Common::kPlatformMacintosh && extra != "Steam")) {
+		options.push_back(macV3CorrectFontSpacing);
+	}
+
 	return options;
 }
 
diff --git a/engines/scumm/scumm.cpp b/engines/scumm/scumm.cpp
index 520f9e0c70..b8ef76d6ec 100644
--- a/engines/scumm/scumm.cpp
+++ b/engines/scumm/scumm.cpp
@@ -1661,9 +1661,10 @@ void ScummEngine::setupCharsetRenderer(const Common::String &macFontFile) {
 #endif
 		if (_game.platform == Common::kPlatformFMTowns)
 			_charset = new CharsetRendererTownsV3(this);
-		else if (_game.platform == Common::kPlatformMacintosh && !macFontFile.empty())
-			_charset = new CharsetRendererMac(this, macFontFile);
-		else
+		else if (_game.platform == Common::kPlatformMacintosh && !macFontFile.empty()) {
+			bool correctFontSpacing = _game.id == GID_LOOM || ConfMan.getBool("mac_v3_correct_font_spacing");
+			_charset = new CharsetRendererMac(this, macFontFile, correctFontSpacing);
+		} else
 			_charset = new CharsetRendererV3(this);
 #ifdef ENABLE_SCUMM_7_8
 	} else if (_game.version == 8) {


Commit: 63a83d7eaaffde230ebdaed903fb7910ce7f4ac1
    https://github.com/scummvm/scummvm/commit/63a83d7eaaffde230ebdaed903fb7910ce7f4ac1
Author: Torbjörn Andersson (eriktorbjorn at users.sourceforge.net)
Date: 2021-10-19T19:24:31+02:00

Commit Message:
SCUMM: Warn on unhandled escape sequences in Mac font renderer

This should, of course, never happen.

Changed paths:
    engines/scumm/charset.cpp


diff --git a/engines/scumm/charset.cpp b/engines/scumm/charset.cpp
index df74b730e5..1a2fed6e03 100644
--- a/engines/scumm/charset.cpp
+++ b/engines/scumm/charset.cpp
@@ -1632,8 +1632,10 @@ int CharsetRendererMac::getStringWidth(int arg, const byte *text, uint strLenMax
 			chr = text[pos++];
 			if (chr == 1) // 'Newline'
 				break;
+			warning("getStringWidth: Unexpected escape sequence %d", chr);
+		} else {
+			width += getDrawWidthIntern(chr);
 		}
-		width += getDrawWidthIntern(chr);
 	}
 
 	return width / 2;


Commit: 2dbf32353d2714fca97ed3f3084d5769a4b6fa39
    https://github.com/scummvm/scummvm/commit/2dbf32353d2714fca97ed3f3084d5769a4b6fa39
Author: Torbjörn Andersson (eriktorbjorn at users.sourceforge.net)
Date: 2021-10-19T19:24:31+02:00

Commit Message:
SCUMM: Hack Indy 3 Mac credits to use correct text color

The credits script asks for white shadowed text, but the original (at
least when running in Basilisk II) uses light gray shadowed text. I have
no idea why, and the only workaround I can think of is to force the text
color this way.

It's possible for scripts to remap colors, of course, but that's not
what seems to be going on here.

Changed paths:
    engines/scumm/script_v5.cpp


diff --git a/engines/scumm/script_v5.cpp b/engines/scumm/script_v5.cpp
index e86b3a62c2..ed3fc7e913 100644
--- a/engines/scumm/script_v5.cpp
+++ b/engines/scumm/script_v5.cpp
@@ -2766,6 +2766,7 @@ int ScummEngine_v5::getWordVararg(int *ptr) {
 
 void ScummEngine_v5::decodeParseString() {
 	int textSlot;
+	int color;
 
 	switch (_actorToPrintStrFor) {
 	case 252:
@@ -2791,7 +2792,16 @@ void ScummEngine_v5::decodeParseString() {
 			_string[textSlot].overhead = false;
 			break;
 		case 1:		// SO_COLOR
-			_string[textSlot].color = getVarOrDirectByte(PARAM_1);
+			color = getVarOrDirectByte(PARAM_1);
+
+			// HACK: The Indy 3 credits script asks for white text
+			// with a shadow, but in a Mac emulator the text is
+			// drawn in light gray with a shadow instead. Very
+			// strange.
+			if (_game.id == GID_INDY3 && _game.platform == Common::kPlatformMacintosh && vm.slot[_currentScript].number == 134 && color == 0x8F)
+				color = 0x87;
+
+			_string[textSlot].color = color;
 			break;
 		case 2:		// SO_CLIPPED
 			_string[textSlot].right = getVarOrDirectWord(PARAM_1);




More information about the Scummvm-git-logs mailing list