[Scummvm-git-logs] scummvm master -> 82f79ad13cf8356d8f1b0a1871e8bae752ac3a83

bluegr noreply at scummvm.org
Sun Jan 5 07:49:33 UTC 2025


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:
4b87d2c9d5 AGI: Remove unused picture version
6cf095d7dd AGI: PREAGI: Move PictureMgr to game engine classes
3e199217ce AGI: PREAGI: Subclass PictureMgr for MICKEY and WINNIE
f63704eeae AGI: PREAGI: Subclass PictureMgr for TROLL
109bb3dc59 AGI: PREAGI: Implement TROLL flood fill
ce88b3dc81 AGI: Add support for early KQ1 pictures
82f79ad13c AGI: Document PictureMgr


Commit: 4b87d2c9d55713c8b918d76c95a1fe7f848289b1
    https://github.com/scummvm/scummvm/commit/4b87d2c9d55713c8b918d76c95a1fe7f848289b1
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2025-01-05T09:49:27+02:00

Commit Message:
AGI: Remove unused picture version

This code was from TrollVM in 2003, where it was unused and undocumented

Changed paths:
    engines/agi/picture.cpp
    engines/agi/picture.h


diff --git a/engines/agi/picture.cpp b/engines/agi/picture.cpp
index a5d3a85f74d..5ca3c10ec82 100644
--- a/engines/agi/picture.cpp
+++ b/engines/agi/picture.cpp
@@ -406,9 +406,6 @@ void PictureMgr::drawPicture() {
 	case AGIPIC_C64:
 		drawPictureC64();
 		break;
-	case AGIPIC_V1:
-		drawPictureV1();
-		break;
 	case AGIPIC_V15:
 		drawPictureV15();
 		break;
@@ -468,48 +465,6 @@ void PictureMgr::drawPictureC64() {
 	}
 }
 
-void PictureMgr::drawPictureV1() {
-	debugC(kDebugLevelPictures, "Drawing V1 picture");
-
-	while (_dataOffset < _dataSize) {
-		byte curByte = getNextByte();
-
-		switch (curByte) {
-		case 0xf1:
-			draw_SetColor();
-			_scrOn = true;
-			_priOn = false;
-			break;
-		case 0xf3:
-			draw_SetColor();
-			_scrOn = true;
-			draw_SetPriority();
-			_priOn = true;
-			break;
-		case 0xfa:
-			_scrOn = false;
-			_priOn = true;
-			draw_LineAbsolute();
-			_scrOn = true;
-			_priOn = false;
-			break;
-		case 0xfb:
-			draw_LineShort();
-			break;
-		case 0xfc:
-			draw_SetColor();
-			draw_SetPriority();
-			draw_Fill();
-			break;
-		case 0xff: // end of data
-			return;
-		default:
-			warning("Unknown picture opcode (%x) at (%x)", curByte, _dataOffset - 1);
-			break;
-		}
-	}
-}
-
 void PictureMgr::drawPictureV15() {
 	debugC(kDebugLevelPictures, "Drawing V1.5 picture");
 
diff --git a/engines/agi/picture.h b/engines/agi/picture.h
index 035f719d9b5..b0ec83c0bc1 100644
--- a/engines/agi/picture.h
+++ b/engines/agi/picture.h
@@ -44,7 +44,6 @@ struct AgiPicture {
 
 enum AgiPictureVersion {
 	AGIPIC_C64,     // Winnie (Apple II, C64, CoCo)
-	AGIPIC_V1,      // Currently unused
 	AGIPIC_V15,     // Troll (DOS)
 	AGIPIC_PREAGI,  // Winnie (DOS, Amiga), Mickey (DOS)
 	AGIPIC_V2       // AGIv2, AGIv3
@@ -92,7 +91,6 @@ public:
 private:
 	void drawPicture();
 	void drawPictureC64();
-	void drawPictureV1();
 	void drawPictureV15();
 	void drawPicturePreAGI();
 	void drawPictureV2();


Commit: 6cf095d7dddc6eed0d6947b60449f7a1285a4728
    https://github.com/scummvm/scummvm/commit/6cf095d7dddc6eed0d6947b60449f7a1285a4728
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2025-01-05T09:49:27+02:00

Commit Message:
AGI: PREAGI: Move PictureMgr to game engine classes

Changed paths:
    engines/agi/preagi/mickey.cpp
    engines/agi/preagi/mickey.h
    engines/agi/preagi/preagi.cpp
    engines/agi/preagi/preagi.h
    engines/agi/preagi/troll.cpp
    engines/agi/preagi/troll.h
    engines/agi/preagi/winnie.cpp
    engines/agi/preagi/winnie.h


diff --git a/engines/agi/preagi/mickey.cpp b/engines/agi/preagi/mickey.cpp
index 784554d0695..ce33019a6eb 100644
--- a/engines/agi/preagi/mickey.cpp
+++ b/engines/agi/preagi/mickey.cpp
@@ -2215,15 +2215,19 @@ void MickeyEngine::debugGotoRoom(int room) {
 }
 
 MickeyEngine::MickeyEngine(OSystem *syst, const AGIGameDescription *gameDesc) : PreAgiEngine(syst, gameDesc) {
+	_picture = nullptr;
 	_isGameOver = false;
 	setDebugger(new MickeyConsole(this));
 }
 
 MickeyEngine::~MickeyEngine() {
+	delete _picture;
 	//_console deleted by Engine
 }
 
 void MickeyEngine::init() {
+	_picture = new PictureMgr(this, _gfx);
+
 	uint8 buffer[512];
 
 	// clear game struct
diff --git a/engines/agi/preagi/mickey.h b/engines/agi/preagi/mickey.h
index 13ac1c9ef1a..d8839c76193 100644
--- a/engines/agi/preagi/mickey.h
+++ b/engines/agi/preagi/mickey.h
@@ -692,6 +692,8 @@ public:
 	void drawObj(ENUM_MSA_OBJECT, int, int);
 
 protected:
+	PictureMgr *_picture;
+
 	MSA_GAME _gameStateMickey;
 	bool _clickToMove;
 	bool _isGameOver;
diff --git a/engines/agi/preagi/preagi.cpp b/engines/agi/preagi/preagi.cpp
index 19cd455e204..db05873d1a9 100644
--- a/engines/agi/preagi/preagi.cpp
+++ b/engines/agi/preagi/preagi.cpp
@@ -46,7 +46,6 @@ void PreAgiEngine::initialize() {
 
 	_font = new GfxFont(this);
 	_gfx = new GfxMgr(this, _font);
-	_picture = new PictureMgr(this, _gfx);
 
 	_font->init();
 
@@ -78,7 +77,6 @@ PreAgiEngine::~PreAgiEngine() {
 	delete _speakerStream;
 	delete _speakerHandle;
 
-	delete _picture;
 	delete _gfx;
 	delete _font;
 }
diff --git a/engines/agi/preagi/preagi.h b/engines/agi/preagi/preagi.h
index e30ed3ee1d6..3355f999517 100644
--- a/engines/agi/preagi/preagi.h
+++ b/engines/agi/preagi/preagi.h
@@ -71,8 +71,6 @@ protected:
 	~PreAgiEngine() override;
 	int getGameId() const { return _gameId; }
 
-	PictureMgr *_picture;
-
 	void clearImageStack() override {}
 	void recordImageStackCall(uint8 type, int16 p1, int16 p2, int16 p3,
 	                          int16 p4, int16 p5, int16 p6, int16 p7) override {}
diff --git a/engines/agi/preagi/troll.cpp b/engines/agi/preagi/troll.cpp
index d3573f9ee82..685edda8303 100644
--- a/engines/agi/preagi/troll.cpp
+++ b/engines/agi/preagi/troll.cpp
@@ -31,9 +31,11 @@
 namespace Agi {
 
 TrollEngine::TrollEngine(OSystem *syst, const AGIGameDescription *gameDesc) : PreAgiEngine(syst, gameDesc) {
+	_picture = nullptr;
 }
 
 TrollEngine::~TrollEngine() {
+	delete _picture;
 }
 
 // User Interface
@@ -713,6 +715,7 @@ void TrollEngine::fillOffsets() {
 // Init
 
 void TrollEngine::init() {
+	_picture = new PictureMgr(this, _gfx);
 	_picture->setPictureVersion(AGIPIC_V15);
 	//SetScreenPar(320, 200, (char *)ibm_fontdata);
 
diff --git a/engines/agi/preagi/troll.h b/engines/agi/preagi/troll.h
index 8dff12de243..041c4626466 100644
--- a/engines/agi/preagi/troll.h
+++ b/engines/agi/preagi/troll.h
@@ -164,6 +164,8 @@ public:
 	Common::Error go() override;
 
 private:
+	PictureMgr *_picture;
+
 	int _roomPicture;
 	int _treasuresLeft;
 	int _currentRoom;
diff --git a/engines/agi/preagi/winnie.cpp b/engines/agi/preagi/winnie.cpp
index 515fed28c0f..7a6cce55980 100644
--- a/engines/agi/preagi/winnie.cpp
+++ b/engines/agi/preagi/winnie.cpp
@@ -1468,13 +1468,17 @@ void WinnieEngine::debugCurRoom() {
 }
 
 WinnieEngine::WinnieEngine(OSystem *syst, const AGIGameDescription *gameDesc) : PreAgiEngine(syst, gameDesc) {
+	_picture = nullptr;
 	setDebugger(new WinnieConsole(this));
 }
 
 WinnieEngine::~WinnieEngine() {
+	delete _picture;
 }
 
 void WinnieEngine::init() {
+	_picture = new PictureMgr(this, _gfx);
+
 	// Initialize sound
 
 	switch (MidiDriver::getMusicType(MidiDriver::detectDevice(MDT_PCSPK | MDT_PCJR))) {
diff --git a/engines/agi/preagi/winnie.h b/engines/agi/preagi/winnie.h
index c149483385c..4f197b847cc 100644
--- a/engines/agi/preagi/winnie.h
+++ b/engines/agi/preagi/winnie.h
@@ -305,6 +305,8 @@ public:
 	void debugCurRoom();
 
 private:
+	PictureMgr *_picture;
+
 	WTP_SAVE_GAME _gameStateWinnie;
 	int _room;
 	int _mist;


Commit: 3e199217ce0c85a7eb4620b01e591781f8f3feb0
    https://github.com/scummvm/scummvm/commit/3e199217ce0c85a7eb4620b01e591781f8f3feb0
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2025-01-05T09:49:27+02:00

Commit Message:
AGI: PREAGI: Subclass PictureMgr for MICKEY and WINNIE

Changed paths:
  A engines/agi/preagi/picture_mickey_winnie.cpp
  A engines/agi/preagi/picture_mickey_winnie.h
    engines/agi/module.mk
    engines/agi/picture.cpp
    engines/agi/picture.h
    engines/agi/preagi/mickey.cpp
    engines/agi/preagi/mickey.h
    engines/agi/preagi/winnie.cpp
    engines/agi/preagi/winnie.h


diff --git a/engines/agi/module.mk b/engines/agi/module.mk
index de26a8709fa..0dc795d6599 100644
--- a/engines/agi/module.mk
+++ b/engines/agi/module.mk
@@ -41,6 +41,7 @@ MODULE_OBJS := \
 	words.o \
 	preagi/preagi.o \
 	preagi/mickey.o \
+	preagi/picture_mickey_winnie.o \
 	preagi/troll.o \
 	preagi/winnie.o
 
diff --git a/engines/agi/picture.cpp b/engines/agi/picture.cpp
index 5ca3c10ec82..b8f229492e2 100644
--- a/engines/agi/picture.cpp
+++ b/engines/agi/picture.cpp
@@ -48,23 +48,12 @@ PictureMgr::PictureMgr(AgiBase *agi, GfxMgr *gfx) {
 	_pictureVersion = AGIPIC_V2;
 	_width = 0;
 	_height = 0;
-	_xOffset = 0;
-	_yOffset = 0;
 
 	_flags = 0;
-	_maxStep = 0;
 }
 
-void PictureMgr::putVirtPixel(int x, int y) {
-	if (x < 0 || y < 0 || x >= _width || y >= _height)
-		return;
-
-	x += _xOffset;
-	y += _yOffset;
-
-	// validate coordinate after applying preagi offset.
-	// winnie objects go past the bottom of the screen.
-	if (x >= SCRIPT_WIDTH || y >= SCRIPT_HEIGHT) {
+void PictureMgr::putVirtPixel(int16 x, int16 y) {
+	if (!getGraphicsCoordinates(x, y)) {
 		return;
 	}
 
@@ -107,37 +96,21 @@ byte PictureMgr::getNextNibble() {
 }
 
 bool PictureMgr::getNextXCoordinate(byte &x) {
-	if (!(getNextParamByte(x))) {
-		return false;
-	}
-
-	if (_pictureVersion == AGIPIC_PREAGI) {
-		if (x >= _width) {
-			debugC(kDebugLevelPictures, "preagi: clipping x from %d to %d", x, _width - 1);
-			x = _width - 1; // 139
-		}
-	}
-	return true;
+	return getNextParamByte(x);
 }
 
 bool PictureMgr::getNextYCoordinate(byte &y) {
-	if (!(getNextParamByte(y))) {
-		return false;
-	}
-
-	if (_pictureVersion == AGIPIC_PREAGI) {
-		if (y > _height) {
-			debugC(kDebugLevelPictures, "preagi: clipping y from %d to %d", y, _height);
-			y = _height; // 159
-		}
-	}
-	return true;
+	return getNextParamByte(y);
 }
 
 bool PictureMgr::getNextCoordinates(byte &x, byte &y) {
 	return getNextXCoordinate(x) && getNextYCoordinate(y);
 }
 
+bool PictureMgr::getGraphicsCoordinates(int16 &x, int16 &y) {
+	return (0 <= x && x < _width && 0 <= y && y < _height);
+}
+
 /**************************************************************************
 ** xCorner
 **
@@ -215,7 +188,7 @@ void PictureMgr::yCorner(bool skipOtherCoords) {
 ** Draws pixels, circles, squares, or splatter brush patterns depending
 ** on the pattern code.
 **************************************************************************/
-void PictureMgr::plotPattern(int x, int y) {
+void PictureMgr::plotPattern(byte x, byte y) {
 	static const uint16 binary_list[] = {
 		0x8000, 0x4000, 0x2000, 0x1000, 0x800, 0x400, 0x200, 0x100,
 		0x80, 0x40, 0x20, 0x10, 0x8, 0x4, 0x2, 0x1
@@ -333,62 +306,6 @@ void PictureMgr::plotBrush() {
 	}
 }
 
-void PictureMgr::plotBrush_PreAGI() {
-	_patCode = getNextByte();
-	if (_patCode > 12) {
-		_patCode = 12;
-	}
-
-	for (;;) {
-		byte x, y;
-		if (!getNextCoordinates(x, y))
-			break;
-
-		plotPattern_PreAGI(x, y);
-	}
-}
-
-void PictureMgr::plotPattern_PreAGI(byte x, byte y) {
-	// PreAGI patterns are 13 solid circles
-	static const byte circleData[] = {
-		0x00,
-		0x01, 0x01,
-		0x01, 0x02, 0x02,
-		0x01, 0x02, 0x03, 0x03,
-		0x02, 0x03, 0x04, 0x04, 0x04,
-		0x02, 0x03, 0x04, 0x05, 0x05, 0x05,
-		0x02, 0x04, 0x05, 0x05, 0x06, 0x06, 0x06,
-		0x02, 0x04, 0x05, 0x06, 0x06, 0x07, 0x07, 0x07,
-		0x02, 0x04, 0x06, 0x06, 0x07, 0x07, 0x08, 0x08, 0x08,
-		0x03, 0x05, 0x06, 0x07, 0x08, 0x08, 0x08, 0x09, 0x09, 0x09,
-		0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x09, 0x0a, 0x0a, 0x0a, 0x0a,
-		0x03, 0x05, 0x07, 0x08, 0x09, 0x09, 0x0a, 0x0a, 0x0b, 0x0b, 0x0b, 0x0b,
-		0x03, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0a, 0x0b, 0x0b, 0x0c, 0x0c, 0x0c, 0x0c
-	};
-
-	int circleDataIndex = (_patCode * (_patCode + 1)) / 2;
-
-	// draw the circle by drawing its vertical lines two at a time, starting at the
-	// left and right edges and working inwards. circles have odd widths, so the
-	// final iteration draws the middle line twice.
-	for (int i = _patCode; i >= 0; i--) {
-		const byte height = circleData[circleDataIndex++];
-		int16 x1, y1, x2, y2;
-
-		// left vertical line
-		x1 = x - i;
-		x2 = x1;
-		y1 = y - height;
-		y2 = y + height;
-		draw_Line(x1, y1, x2, y2);
-
-		// right vertical line
-		x1 = x + i;
-		x2 = x1;
-		draw_Line(x1, y1, x2, y2);
-	}
-}
-
 /**************************************************************************
 ** Draw AGI picture
 **************************************************************************/
@@ -403,15 +320,9 @@ void PictureMgr::drawPicture() {
 	_priColor = 4;
 
 	switch (_pictureVersion) {
-	case AGIPIC_C64:
-		drawPictureC64();
-		break;
 	case AGIPIC_V15:
 		drawPictureV15();
 		break;
-	case AGIPIC_PREAGI:
-		drawPicturePreAGI();
-		break;
 	case AGIPIC_V2:
 		drawPictureV2();
 		break;
@@ -420,51 +331,6 @@ void PictureMgr::drawPicture() {
 	}
 }
 
-void PictureMgr::drawPictureC64() {
-	debugC(kDebugLevelPictures, "Drawing Apple II / C64 / CoCo picture");
-
-	_scrColor = 0;
-
-	while (_dataOffset < _dataSize) {
-		byte curByte = getNextByte();
-
-		if ((curByte >= 0xF0) && (curByte <= 0xFE)) {
-			_scrColor = curByte & 0x0F;
-			continue;
-		}
-
-		switch (curByte) {
-		case 0xe0:  // x-corner
-			xCorner();
-			break;
-		case 0xe1:  // y-corner
-			yCorner();
-			break;
-		case 0xe2:  // dynamic draw lines
-			draw_LineShort();
-			break;
-		case 0xe3:  // absolute draw lines
-			draw_LineAbsolute();
-			break;
-		case 0xe4:  // fill
-			draw_SetColor();
-			draw_Fill();
-			break;
-		case 0xe5:  // enable screen drawing
-			_scrOn = true;
-			break;
-		case 0xe6:  // plot brush
-			plotBrush_PreAGI();
-			break;
-		case 0xff: // end of data
-			return;
-		default:
-			warning("Unknown picture opcode (%x) at (%x)", curByte, _dataOffset - 1);
-			break;
-		}
-	}
-}
-
 void PictureMgr::drawPictureV15() {
 	debugC(kDebugLevelPictures, "Drawing V1.5 picture");
 
@@ -512,61 +378,6 @@ void PictureMgr::drawPictureV15() {
 	}
 }
 
-void PictureMgr::drawPicturePreAGI() {
-	debugC(kDebugLevelPictures, "Drawing PreAGI picture");
-
-	int step = 0;
-	while (_dataOffset < _dataSize) {
-		byte curByte = getNextByte();
-
-		switch (curByte) {
-		case 0xf0:
-			draw_SetColor();
-			_scrOn = true;
-			break;
-		case 0xf1:
-			_scrOn = false;
-			break;
-		case 0xf4:
-			yCorner();
-			break;
-		case 0xf5:
-			xCorner();
-			break;
-		case 0xf6:
-			draw_LineAbsolute();
-			break;
-		case 0xf7:
-			draw_LineShort();
-			break;
-		case 0xf8: {
-			// The screen-on flag does not prevent PreAGI flood fills.
-			// Winnie picture 7 (Roo) contains F1 before several fills.
-			byte prevScrOn = _scrOn;
-			_scrOn = true;
-			draw_Fill();
-			_scrOn = prevScrOn;
-			break;
-		}
-		case 0xf9:
-			plotBrush_PreAGI();
-			break;
-		case 0xff: // end of data
-			return;
-		default:
-			warning("Unknown picture opcode (%x) at (%x)", curByte, _dataOffset - 1);
-			break;
-		}
-
-		// Limit drawing to the optional maximum number of opcodes.
-		// Used by Mickey for crystal animation.
-		step++;
-		if (step == _maxStep) {
-			return;
-		}
-	}
-}
-
 void PictureMgr::drawPictureV2() {
 	debugC(kDebugLevelPictures, "Drawing V2/V3 picture");
 
@@ -842,19 +653,6 @@ void PictureMgr::draw_Fill() {
 	byte x, y;
 
 	while (getNextCoordinates(x, y)) {
-		// PreAGI: getNextCoordinates clips to (139, 159), and then
-		// flood fill checks if y >= 159 and decrements to 158.
-		// The flood fill check is not in in Apple II/C64/CoCo
-		// versions of Winnie, as can be seen by the table edge
-		// being a different color than Winnie's shirt in the first
-		// room, but the same color in DOS/Amiga (picture 28).
-		if (_pictureVersion == AGIPIC_PREAGI) {
-			if (y >= _height) { // 159
-				debugC(kDebugLevelPictures, "preagi: fill clipping y from %d to %d", y, _height - 1);
-				y = _height - 1; // 158
-			}
-		}
-
 		draw_Fill(x, y);
 	}
 }
@@ -905,15 +703,7 @@ void PictureMgr::draw_Fill(int16 x, int16 y) {
 }
 
 bool PictureMgr::draw_FillCheck(int16 x, int16 y) {
-	if (x < 0 || x >= _width || y < 0 || y >= _height)
-		return false;
-
-	x += _xOffset;
-	y += _yOffset;
-
-	// validate coordinate after applying preagi offset.
-	// winnie objects go past the bottom of the screen.
-	if (x >= SCRIPT_WIDTH || y >= SCRIPT_HEIGHT) {
+	if (!getGraphicsCoordinates(x, y)) {
 		return false;
 	}
 
@@ -1032,11 +822,7 @@ void PictureMgr::showPictureWithTransition() {
 
 void PictureMgr::setPictureVersion(AgiPictureVersion version) {
 	_pictureVersion = version;
-
-	if (version == AGIPIC_C64)
-		_minCommand = 0xe0;
-	else
-		_minCommand = 0xf0;
+	_minCommand = 0xf0;
 }
 
 } // End of namespace Agi
diff --git a/engines/agi/picture.h b/engines/agi/picture.h
index b0ec83c0bc1..1a25491f985 100644
--- a/engines/agi/picture.h
+++ b/engines/agi/picture.h
@@ -43,9 +43,7 @@ struct AgiPicture {
 };
 
 enum AgiPictureVersion {
-	AGIPIC_C64,     // Winnie (Apple II, C64, CoCo)
 	AGIPIC_V15,     // Troll (DOS)
-	AGIPIC_PREAGI,  // Winnie (DOS, Amiga), Mickey (DOS)
 	AGIPIC_V2       // AGIv2, AGIv3
 };
 
@@ -64,35 +62,34 @@ class PictureMgr {
 
 public:
 	PictureMgr(AgiBase *agi, GfxMgr *gfx);
+	virtual ~PictureMgr() { }
 
 	int16 getResourceNr() const { return _resourceNr; };
 
-private:
-	void putVirtPixel(int x, int y);
+protected:
+	void putVirtPixel(int16 x, int16 y);
 	void xCorner(bool skipOtherCoords = false);
 	void yCorner(bool skipOtherCoords = false);
-	void plotPattern(int x, int y);
-	void plotBrush();
-	void plotPattern_PreAGI(byte x, byte y);
-	void plotBrush_PreAGI();
+	virtual void plotPattern(byte x, byte y);
+	virtual void plotBrush();
 
 	byte getNextByte();
 	bool getNextParamByte(byte &b);
 	byte getNextNibble();
 
-	bool getNextXCoordinate(byte &x);
-	bool getNextYCoordinate(byte &y);
+	virtual bool getNextXCoordinate(byte &x);
+	virtual bool getNextYCoordinate(byte &y);
 	bool getNextCoordinates(byte &x, byte &y);
 
+	virtual bool getGraphicsCoordinates(int16 &x, int16 &y);
+
 public:
 	void decodePicture(int16 resourceNr, bool clearScreen, bool agi256 = false, int16 width = _DEFAULT_WIDTH, int16 height = _DEFAULT_HEIGHT);
 	void decodePictureFromBuffer(byte *data, uint32 length, bool clearScreen, int16 width = _DEFAULT_WIDTH, int16 height = _DEFAULT_HEIGHT);
 
-private:
-	void drawPicture();
-	void drawPictureC64();
+protected:
+	virtual void drawPicture();
 	void drawPictureV15();
-	void drawPicturePreAGI();
 	void drawPictureV2();
 	void drawPictureAGI256();
 
@@ -106,7 +103,7 @@ private:
 	void draw_LineAbsolute();
 
 	bool draw_FillCheck(int16 x, int16 y);
-	void draw_Fill(int16 x, int16 y);
+	virtual void draw_Fill(int16 x, int16 y);
 	void draw_Fill();
 
 public:
@@ -117,15 +114,7 @@ public:
 
 	void setPictureFlags(int flags) { _flags = flags; }
 
-	void setOffset(int offX, int offY) {
-		_xOffset = offX;
-		_yOffset = offY;
-	}
-
-	void setMaxStep(int maxStep) { _maxStep = maxStep; }
-	int getMaxStep() const { return _maxStep; }
-
-private:
+protected:
 	int16  _resourceNr;
 	uint8 *_data;
 	uint32 _dataSize;
@@ -144,11 +133,8 @@ private:
 	AgiPictureVersion _pictureVersion;
 	int16 _width;
 	int16 _height;
-	int16 _xOffset;
-	int16 _yOffset;
 
 	int _flags;
-	int _maxStep; // Max opcodes to draw, zero for all. Used by preagi (Mickey)
 };
 
 } // End of namespace Agi
diff --git a/engines/agi/preagi/mickey.cpp b/engines/agi/preagi/mickey.cpp
index ce33019a6eb..93b91fe935f 100644
--- a/engines/agi/preagi/mickey.cpp
+++ b/engines/agi/preagi/mickey.cpp
@@ -26,6 +26,7 @@
 #include "graphics/cursorman.h"
 
 #include "agi/preagi/preagi.h"
+#include "agi/preagi/picture_mickey_winnie.h"
 #include "agi/preagi/mickey.h"
 #include "agi/graphics.h"
 
@@ -2226,7 +2227,7 @@ MickeyEngine::~MickeyEngine() {
 }
 
 void MickeyEngine::init() {
-	_picture = new PictureMgr(this, _gfx);
+	_picture = new PictureMgr_Mickey_Winnie(this, _gfx);
 
 	uint8 buffer[512];
 
@@ -2278,8 +2279,6 @@ void MickeyEngine::init() {
 #endif
 
 	setFlag(VM_FLAG_SOUND_ON, true); // enable sound
-
-	_picture->setPictureVersion(AGIPIC_PREAGI);
 }
 
 Common::Error MickeyEngine::go() {
diff --git a/engines/agi/preagi/mickey.h b/engines/agi/preagi/mickey.h
index d8839c76193..5fa2a2132cb 100644
--- a/engines/agi/preagi/mickey.h
+++ b/engines/agi/preagi/mickey.h
@@ -677,6 +677,7 @@ struct MSA_GAME {
 };
 
 class PreAgiEngine;
+class PictureMgr_Mickey_Winnie;
 
 class MickeyEngine : public PreAgiEngine {
 public:
@@ -692,7 +693,7 @@ public:
 	void drawObj(ENUM_MSA_OBJECT, int, int);
 
 protected:
-	PictureMgr *_picture;
+	PictureMgr_Mickey_Winnie *_picture;
 
 	MSA_GAME _gameStateMickey;
 	bool _clickToMove;
diff --git a/engines/agi/preagi/picture_mickey_winnie.cpp b/engines/agi/preagi/picture_mickey_winnie.cpp
new file mode 100644
index 00000000000..36e1dd32b4b
--- /dev/null
+++ b/engines/agi/preagi/picture_mickey_winnie.cpp
@@ -0,0 +1,293 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+ 
+#include "agi/agi.h"
+#include "agi/graphics.h"
+#include "agi/picture.h"
+
+#include "agi/preagi/picture_mickey_winnie.h"
+
+namespace Agi {
+
+PictureMgr_Mickey_Winnie::PictureMgr_Mickey_Winnie(AgiBase *agi, GfxMgr *gfx) :
+	PictureMgr(agi, gfx) {
+
+	switch (agi->getPlatform()) {
+	case Common::kPlatformAmiga:
+	case Common::kPlatformDOS:
+		_isDosOrAmiga = true;
+		break;
+	default:
+		_isDosOrAmiga = false;
+		_minCommand = 0xe0;
+		break;
+	}
+
+	_xOffset = 0;
+	_yOffset = 0;
+	_maxStep = 0;
+}
+
+void PictureMgr_Mickey_Winnie::drawPicture() {
+	debugC(kDebugLevelPictures, "Drawing picture");
+
+	_dataOffset = 0;
+	_dataOffsetNibble = false;
+	_patCode = 0;
+	_patNum = 0;
+	_priOn = false;
+	_scrOn = false;
+	_priColor = 4;
+
+	if (_isDosOrAmiga) {
+		_scrColor = 15;
+		drawPicture_DOS_Amiga();
+	} else {
+		_scrColor = 0;
+		drawPicture_A2_C64_CoCo();
+	}
+}
+
+void PictureMgr_Mickey_Winnie::drawPicture_DOS_Amiga() {
+	int step = 0;
+	while (_dataOffset < _dataSize) {
+		byte curByte = getNextByte();
+
+		switch (curByte) {
+		case 0xf0:
+			draw_SetColor();
+			_scrOn = true;
+			break;
+		case 0xf1:
+			_scrOn = false;
+			break;
+		case 0xf4:
+			yCorner();
+			break;
+		case 0xf5:
+			xCorner();
+			break;
+		case 0xf6:
+			draw_LineAbsolute();
+			break;
+		case 0xf7:
+			draw_LineShort();
+			break;
+		case 0xf8: {
+			// The screen-on flag does not prevent PreAGI flood fills.
+			// Winnie picture 7 (Roo) contains F1 before several fills.
+			byte prevScrOn = _scrOn;
+			_scrOn = true;
+			PictureMgr::draw_Fill();
+			_scrOn = prevScrOn;
+			break;
+		}
+		case 0xf9:
+			plotBrush();
+			break;
+		case 0xff: // end of data
+			return;
+		default:
+			warning("Unknown picture opcode %02x at %04x", curByte, _dataOffset - 1);
+			break;
+		}
+
+		// Limit drawing to the optional maximum number of opcodes.
+		// Used by Mickey for crystal animation.
+		step++;
+		if (step == _maxStep) {
+			return;
+		}
+	}
+}
+
+void PictureMgr_Mickey_Winnie::drawPicture_A2_C64_CoCo() {
+	while (_dataOffset < _dataSize) {
+		byte curByte = getNextByte();
+
+		if ((curByte >= 0xF0) && (curByte <= 0xFE)) {
+			_scrColor = curByte & 0x0F;
+			continue;
+		}
+
+		switch (curByte) {
+		case 0xe0:  // x-corner
+			xCorner();
+			break;
+		case 0xe1:  // y-corner
+			yCorner();
+			break;
+		case 0xe2:  // dynamic draw lines
+			draw_LineShort();
+			break;
+		case 0xe3:  // absolute draw lines
+			draw_LineAbsolute();
+			break;
+		case 0xe4:  // fill
+			draw_SetColor();
+			PictureMgr::draw_Fill();
+			break;
+		case 0xe5:  // enable screen drawing
+			_scrOn = true;
+			break;
+		case 0xe6:  // plot brush
+			plotBrush();
+			break;
+		case 0xff: // end of data
+			return;
+		default:
+			warning("Unknown picture opcode %02x at %04x", curByte, _dataOffset - 1);
+			break;
+		}
+	}
+}
+
+void PictureMgr_Mickey_Winnie::plotBrush() {
+	_patCode = getNextByte();
+	if (_patCode > 12) {
+		_patCode = 12;
+	}
+
+	for (;;) {
+		byte x, y;
+		if (!getNextCoordinates(x, y))
+			break;
+
+		plotPattern(x, y);
+	}
+}
+
+void PictureMgr_Mickey_Winnie::plotPattern(byte x, byte y) {
+	// PreAGI patterns are 13 solid circles
+	static const byte circleData[] = {
+		0x00,
+		0x01, 0x01,
+		0x01, 0x02, 0x02,
+		0x01, 0x02, 0x03, 0x03,
+		0x02, 0x03, 0x04, 0x04, 0x04,
+		0x02, 0x03, 0x04, 0x05, 0x05, 0x05,
+		0x02, 0x04, 0x05, 0x05, 0x06, 0x06, 0x06,
+		0x02, 0x04, 0x05, 0x06, 0x06, 0x07, 0x07, 0x07,
+		0x02, 0x04, 0x06, 0x06, 0x07, 0x07, 0x08, 0x08, 0x08,
+		0x03, 0x05, 0x06, 0x07, 0x08, 0x08, 0x08, 0x09, 0x09, 0x09,
+		0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x09, 0x0a, 0x0a, 0x0a, 0x0a,
+		0x03, 0x05, 0x07, 0x08, 0x09, 0x09, 0x0a, 0x0a, 0x0b, 0x0b, 0x0b, 0x0b,
+		0x03, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0a, 0x0b, 0x0b, 0x0c, 0x0c, 0x0c, 0x0c
+	};
+
+	int circleDataIndex = (_patCode * (_patCode + 1)) / 2;
+
+	// draw the circle by drawing its vertical lines two at a time, starting at the
+	// left and right edges and working inwards. circles have odd widths, so the
+	// final iteration draws the middle line twice.
+	for (int i = _patCode; i >= 0; i--) {
+		const byte height = circleData[circleDataIndex++];
+		int16 x1, y1, x2, y2;
+
+		// left vertical line
+		x1 = x - i;
+		x2 = x1;
+		y1 = y - height;
+		y2 = y + height;
+		draw_Line(x1, y1, x2, y2);
+
+		// right vertical line
+		x1 = x + i;
+		x2 = x1;
+		draw_Line(x1, y1, x2, y2);
+	}
+}
+
+/**
+ * Flood fills from a start position, with a clipped height.
+ */
+void PictureMgr_Mickey_Winnie::draw_Fill(int16 x, int16 y) {
+	// Flood fill does extra height clipping, and pictures rely on this.
+	// The get-coordinates routine clips to (139, 159) and then the
+	// flood fill routine checks if y >= 159 and decrements to 158.
+	// The flood fill clip is not in in Apple II/C64/CoCo versions
+	// of Winnie, as can be seen by the table edge being a different
+	// color than Winnie's shirt in the first room, but the same
+	// color as the shirt in DOS/Amiga. (Picture 28)
+	if (_isDosOrAmiga) {
+		if (y >= _height) { // 159
+			debugC(kDebugLevelPictures, "clipping %c from %d to %d", 'y', y, _height - 1);
+			y = _height - 1; // 158
+		}
+	}
+
+	PictureMgr::draw_Fill(x, y);
+}
+
+/**
+ * Gets the next x coordinate in the current picture instruction,
+ * and clip it to the picture width. Many Winnie pictures contain
+ * out of bounds coordinates and rely on this clipping.
+ */
+bool PictureMgr_Mickey_Winnie::getNextXCoordinate(byte &x) {
+	if (!getNextParamByte(x)) {
+		return false;
+	}
+
+	if (_isDosOrAmiga) { // TODO: is this check in A2/C64/CoCo?
+		if (x >= _width) { // 140
+			debugC(kDebugLevelPictures, "clipping %c from %d to %d", 'x', x, _width - 1);
+			x = _width - 1; // 139
+		}
+	}
+
+	return true;
+}
+
+/**
+ * Gets the next y coordinate in the current picture instruction,
+ * and clip it to the picture height. Many Winnie pictures contain
+ * out of bounds coordinates and rely on this clipping.
+ */
+bool PictureMgr_Mickey_Winnie::getNextYCoordinate(byte &y) {
+	if (!getNextParamByte(y)) {
+		return false;
+	}
+
+	if (_isDosOrAmiga) { // TODO: is this check in A2/C64/CoCo?
+		// note that this is a different clip than for the x coordinate
+		if (y > _height) { // 159
+			debugC(kDebugLevelPictures, "clipping %c from %d to %d", 'y', y, _height);
+			y = _height; // 159
+		}
+	}
+
+	return true;
+}
+
+bool PictureMgr_Mickey_Winnie::getGraphicsCoordinates(int16 &x, int16 &y) {
+	if (!PictureMgr::getGraphicsCoordinates(x, y)) {
+		return false;
+	}
+	
+	x += _xOffset;
+	y += _yOffset;
+
+	// validate that the offset coordinates are within the screen's boundaries
+	return (x < SCRIPT_WIDTH && y < SCRIPT_HEIGHT);
+}
+
+} // End of namespace Agi
diff --git a/engines/agi/preagi/picture_mickey_winnie.h b/engines/agi/preagi/picture_mickey_winnie.h
new file mode 100644
index 00000000000..169072af36a
--- /dev/null
+++ b/engines/agi/preagi/picture_mickey_winnie.h
@@ -0,0 +1,62 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef AGI_PREAGI_PICTURE_MICKEY_WINNIE_H
+#define AGI_PREAGI_PICTURE_MICKEY_WINNIE_H
+
+namespace Agi {
+
+class PictureMgr_Mickey_Winnie : public PictureMgr {
+public:
+	PictureMgr_Mickey_Winnie(AgiBase *agi, GfxMgr *gfx);
+
+	void drawPicture() override;
+	void drawPicture_DOS_Amiga();
+	void drawPicture_A2_C64_CoCo();
+
+	void plotPattern(byte x, byte y) override;
+	void plotBrush() override;
+
+	void draw_Fill(int16 x, int16 y) override;
+
+	bool getNextXCoordinate(byte &x) override;
+	bool getNextYCoordinate(byte &y) override;
+	
+	bool getGraphicsCoordinates(int16 &x, int16 &y) override;
+	
+	void setOffset(int xOffset, int yOffset) {
+		_xOffset = xOffset;
+		_yOffset = yOffset;
+	}
+
+	void setMaxStep(int maxStep) { _maxStep = maxStep; }
+	int getMaxStep() const { return _maxStep; }
+
+private:
+	bool _isDosOrAmiga;
+	int16 _xOffset;
+	int16 _yOffset;
+	int _maxStep; // Max opcodes to draw, zero for all. Used by Mickey
+};
+
+} // End of namespace Agi
+
+#endif
diff --git a/engines/agi/preagi/winnie.cpp b/engines/agi/preagi/winnie.cpp
index 7a6cce55980..57eb23f9a17 100644
--- a/engines/agi/preagi/winnie.cpp
+++ b/engines/agi/preagi/winnie.cpp
@@ -20,6 +20,7 @@
  */
 
 #include "agi/preagi/preagi.h"
+#include "agi/preagi/picture_mickey_winnie.h"
 #include "agi/preagi/winnie.h"
 #include "agi/graphics.h"
 
@@ -1477,7 +1478,7 @@ WinnieEngine::~WinnieEngine() {
 }
 
 void WinnieEngine::init() {
-	_picture = new PictureMgr(this, _gfx);
+	_picture = new PictureMgr_Mickey_Winnie(this, _gfx);
 
 	// Initialize sound
 
@@ -1523,17 +1524,6 @@ void WinnieEngine::init() {
 		break;
 	}
 
-	switch (getPlatform()) {
-	case  Common::kPlatformApple2:
-	case  Common::kPlatformC64:
-	case  Common::kPlatformCoCo:
-		_picture->setPictureVersion(AGIPIC_C64);
-		break;
-	default:
-		_picture->setPictureVersion(AGIPIC_PREAGI);
-		break;
-	}
-
 	hotspotNorth = Common::Rect(20, 0, (IDI_WTP_PIC_WIDTH + 10) * 2, 10);
 	hotspotSouth = Common::Rect(20, IDI_WTP_PIC_HEIGHT - 10, (IDI_WTP_PIC_WIDTH + 10) * 2, IDI_WTP_PIC_HEIGHT);
 	hotspotEast  = Common::Rect(IDI_WTP_PIC_WIDTH * 2, 0, (IDI_WTP_PIC_WIDTH + 10) * 2, IDI_WTP_PIC_HEIGHT);
diff --git a/engines/agi/preagi/winnie.h b/engines/agi/preagi/winnie.h
index 4f197b847cc..d7244724c89 100644
--- a/engines/agi/preagi/winnie.h
+++ b/engines/agi/preagi/winnie.h
@@ -293,6 +293,7 @@ struct WTP_SAVE_GAME {
 };
 
 class PreAgiEngine;
+class PictureMgr_Mickey_Winnie;
 
 class WinnieEngine : public PreAgiEngine {
 public:
@@ -305,7 +306,7 @@ public:
 	void debugCurRoom();
 
 private:
-	PictureMgr *_picture;
+	PictureMgr_Mickey_Winnie *_picture;
 
 	WTP_SAVE_GAME _gameStateWinnie;
 	int _room;


Commit: f63704eeae8d9cf4d101e69f044b54f839311fe6
    https://github.com/scummvm/scummvm/commit/f63704eeae8d9cf4d101e69f044b54f839311fe6
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2025-01-05T09:49:27+02:00

Commit Message:
AGI: PREAGI: Subclass PictureMgr for TROLL

Changed paths:
  A engines/agi/preagi/picture_troll.cpp
  A engines/agi/preagi/picture_troll.h
    engines/agi/module.mk
    engines/agi/picture.cpp
    engines/agi/picture.h
    engines/agi/preagi/troll.cpp
    engines/agi/preagi/troll.h


diff --git a/engines/agi/module.mk b/engines/agi/module.mk
index 0dc795d6599..fce6dbb921c 100644
--- a/engines/agi/module.mk
+++ b/engines/agi/module.mk
@@ -42,6 +42,7 @@ MODULE_OBJS := \
 	preagi/preagi.o \
 	preagi/mickey.o \
 	preagi/picture_mickey_winnie.o \
+	preagi/picture_troll.o \
 	preagi/troll.o \
 	preagi/winnie.o
 
diff --git a/engines/agi/picture.cpp b/engines/agi/picture.cpp
index b8f229492e2..2525ce726d2 100644
--- a/engines/agi/picture.cpp
+++ b/engines/agi/picture.cpp
@@ -45,7 +45,6 @@ PictureMgr::PictureMgr(AgiBase *agi, GfxMgr *gfx) {
 
 	_minCommand = 0xf0;
 
-	_pictureVersion = AGIPIC_V2;
 	_width = 0;
 	_height = 0;
 
@@ -310,6 +309,8 @@ void PictureMgr::plotBrush() {
 ** Draw AGI picture
 **************************************************************************/
 void PictureMgr::drawPicture() {
+	debugC(kDebugLevelPictures, "Drawing picture %d", _resourceNr);
+
 	_dataOffset = 0;
 	_dataOffsetNibble = false;
 	_patCode = 0;
@@ -319,68 +320,6 @@ void PictureMgr::drawPicture() {
 	_scrColor = 15;
 	_priColor = 4;
 
-	switch (_pictureVersion) {
-	case AGIPIC_V15:
-		drawPictureV15();
-		break;
-	case AGIPIC_V2:
-		drawPictureV2();
-		break;
-	default:
-		break;
-	}
-}
-
-void PictureMgr::drawPictureV15() {
-	debugC(kDebugLevelPictures, "Drawing V1.5 picture");
-
-	while (_dataOffset < _dataSize) {
-		byte curByte = getNextByte();
-
-		switch (curByte) {
-		case 0xf0:
-			// happens in all Troll's Tale pictures
-			// TODO: figure out what it was meant for
-			break;
-		case 0xf1:
-			draw_SetColor();
-			_scrOn = true;
-			break;
-		case 0xf3:
-			if (_flags & kPicFf3Stop)
-				return;
-			break;
-		case 0xf8:
-			yCorner(true);
-			break;
-		case 0xf9:
-			xCorner(true);
-			break;
-		case 0xfa:
-			// TODO: is this really correct?
-			draw_LineAbsolute();
-			break;
-		case 0xfb:
-			// TODO: is this really correct?
-			draw_LineAbsolute();
-			break;
-		case 0xfe:
-			draw_SetColor();
-			_scrOn = true;
-			draw_Fill();
-			break;
-		case 0xff: // end of data
-			return;
-		default:
-			warning("Unknown picture opcode (%x) at (%x)", curByte, _dataOffset - 1);
-			break;
-		}
-	}
-}
-
-void PictureMgr::drawPictureV2() {
-	debugC(kDebugLevelPictures, "Drawing V2/V3 picture");
-
 	// AGIv3 nibble parameters are indicated by a flag in the picture's directory entry
 	bool nibbleMode = (_vm->_game.dirPic[_resourceNr].flags & RES_PICTURE_V3_NIBBLE_PARM) != 0;
 
@@ -434,7 +373,7 @@ void PictureMgr::drawPictureV2() {
 		case 0xff: // end of data
 			return;
 		default:
-			warning("Unknown picture opcode (%x) at (%x)", curByte, _dataOffset - 1);
+			warning("Unknown picture opcode %02x at %04x", curByte, _dataOffset - 1);
 			break;
 		}
 	}
@@ -820,9 +759,4 @@ void PictureMgr::showPictureWithTransition() {
 	_gfx->render_Block(0, 0, SCRIPT_WIDTH, SCRIPT_HEIGHT);
 }
 
-void PictureMgr::setPictureVersion(AgiPictureVersion version) {
-	_pictureVersion = version;
-	_minCommand = 0xf0;
-}
-
 } // End of namespace Agi
diff --git a/engines/agi/picture.h b/engines/agi/picture.h
index 1a25491f985..b49f5627a78 100644
--- a/engines/agi/picture.h
+++ b/engines/agi/picture.h
@@ -42,11 +42,6 @@ struct AgiPicture {
 	AgiPicture() { reset(); }
 };
 
-enum AgiPictureVersion {
-	AGIPIC_V15,     // Troll (DOS)
-	AGIPIC_V2       // AGIv2, AGIv3
-};
-
 enum AgiPictureFlags {
 	kPicFNone      = (1 << 0),
 	kPicFf3Stop    = (1 << 1), // Troll, certain pictures
@@ -89,8 +84,6 @@ public:
 
 protected:
 	virtual void drawPicture();
-	void drawPictureV15();
-	void drawPictureV2();
 	void drawPictureAGI256();
 
 	void draw_SetColor();
@@ -110,8 +103,6 @@ public:
 	void showPicture(int16 x = 0, int16 y = 0, int16 width = _DEFAULT_WIDTH, int16 height = _DEFAULT_HEIGHT);
 	void showPictureWithTransition();
 
-	void setPictureVersion(AgiPictureVersion version);
-
 	void setPictureFlags(int flags) { _flags = flags; }
 
 protected:
@@ -130,7 +121,6 @@ protected:
 
 	uint8 _minCommand;
 
-	AgiPictureVersion _pictureVersion;
 	int16 _width;
 	int16 _height;
 
diff --git a/engines/agi/preagi/picture_troll.cpp b/engines/agi/preagi/picture_troll.cpp
new file mode 100644
index 00000000000..123c46b2b84
--- /dev/null
+++ b/engines/agi/preagi/picture_troll.cpp
@@ -0,0 +1,90 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "agi/agi.h"
+#include "agi/graphics.h"
+#include "agi/picture.h"
+
+#include "agi/preagi/picture_troll.h"
+
+namespace Agi {
+
+PictureMgr_Troll::PictureMgr_Troll(AgiBase *agi, GfxMgr *gfx) :
+	PictureMgr(agi, gfx) {
+}
+
+void PictureMgr_Troll::drawPicture() {
+	debugC(kDebugLevelPictures, "Drawing picture");
+
+	_dataOffset = 0;
+	_dataOffsetNibble = false;
+	_patCode = 0;
+	_patNum = 0;
+	_priOn = false;
+	_scrOn = false;
+	_priColor = 4;
+	_scrColor = 15;
+
+	while (_dataOffset < _dataSize) {
+		byte curByte = getNextByte();
+
+		switch (curByte) {
+		case 0xf0:
+			// happens in all Troll's Tale pictures
+			// TODO: figure out what it was meant for
+			break;
+		case 0xf1:
+			draw_SetColor();
+			_scrOn = true;
+			break;
+		case 0xf3:
+			if (_flags & kPicFf3Stop)
+				return;
+			break;
+		case 0xf8:
+			yCorner(true);
+			break;
+		case 0xf9:
+			xCorner(true);
+			break;
+		case 0xfa:
+			// TODO: is this really correct?
+			draw_LineAbsolute();
+			break;
+		case 0xfb:
+			// TODO: is this really correct?
+			draw_LineAbsolute();
+			break;
+		case 0xfe:
+			draw_SetColor();
+			_scrOn = true;
+			draw_Fill();
+			break;
+		case 0xff: // end of data
+			return;
+		default:
+			warning("Unknown picture opcode %02x at %04x", curByte, _dataOffset - 1);
+			break;
+		}
+	}
+}
+
+} // End of namespace Agi
diff --git a/engines/agi/preagi/picture_troll.h b/engines/agi/preagi/picture_troll.h
new file mode 100644
index 00000000000..e52617ee2b3
--- /dev/null
+++ b/engines/agi/preagi/picture_troll.h
@@ -0,0 +1,36 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef AGI_PREAGI_PICTURE_TROLL_H
+#define AGI_PREAGI_PICTURE_TROLL_H
+
+namespace Agi {
+
+class PictureMgr_Troll : public PictureMgr {
+public:
+	PictureMgr_Troll(AgiBase *agi, GfxMgr *gfx);
+
+	void drawPicture() override;
+};
+
+} // End of namespace Agi
+
+#endif
diff --git a/engines/agi/preagi/troll.cpp b/engines/agi/preagi/troll.cpp
index 685edda8303..8c52fb497fd 100644
--- a/engines/agi/preagi/troll.cpp
+++ b/engines/agi/preagi/troll.cpp
@@ -20,6 +20,7 @@
  */
 
 #include "agi/preagi/preagi.h"
+#include "agi/preagi/picture_troll.h"
 #include "agi/preagi/troll.h"
 #include "agi/graphics.h"
 
@@ -715,8 +716,7 @@ void TrollEngine::fillOffsets() {
 // Init
 
 void TrollEngine::init() {
-	_picture = new PictureMgr(this, _gfx);
-	_picture->setPictureVersion(AGIPIC_V15);
+	_picture = new PictureMgr_Troll(this, _gfx);
 	//SetScreenPar(320, 200, (char *)ibm_fontdata);
 
 	const int gaps[] = { 0x3A40,  0x4600,  0x4800,  0x5800,  0x5a00,  0x6a00,
diff --git a/engines/agi/preagi/troll.h b/engines/agi/preagi/troll.h
index 041c4626466..699b241c2b9 100644
--- a/engines/agi/preagi/troll.h
+++ b/engines/agi/preagi/troll.h
@@ -156,6 +156,8 @@ struct Item {
 	char name[16];
 };
 
+class PictureMgr_Troll;
+
 class TrollEngine : public PreAgiEngine {
 public:
 	TrollEngine(OSystem *syst, const AGIGameDescription *gameDesc);
@@ -164,7 +166,7 @@ public:
 	Common::Error go() override;
 
 private:
-	PictureMgr *_picture;
+	PictureMgr_Troll *_picture;
 
 	int _roomPicture;
 	int _treasuresLeft;


Commit: 109bb3dc597a25223fc3f6536417de08b87d02af
    https://github.com/scummvm/scummvm/commit/109bb3dc597a25223fc3f6536417de08b87d02af
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2025-01-05T09:49:27+02:00

Commit Message:
AGI: PREAGI: Implement TROLL flood fill

Fixes missing or incomplete flood fills when drawing the Troll

Changed paths:
    engines/agi/picture.cpp
    engines/agi/picture.h
    engines/agi/preagi/picture_troll.cpp
    engines/agi/preagi/picture_troll.h
    engines/agi/preagi/troll.cpp


diff --git a/engines/agi/picture.cpp b/engines/agi/picture.cpp
index 2525ce726d2..500f7ce31bd 100644
--- a/engines/agi/picture.cpp
+++ b/engines/agi/picture.cpp
@@ -47,8 +47,6 @@ PictureMgr::PictureMgr(AgiBase *agi, GfxMgr *gfx) {
 
 	_width = 0;
 	_height = 0;
-
-	_flags = 0;
 }
 
 void PictureMgr::putVirtPixel(int16 x, int16 y) {
@@ -608,19 +606,19 @@ void PictureMgr::draw_Fill(int16 x, int16 y) {
 	while (!stack.empty()) {
 		Common::Point p = stack.pop();
 
-		if (!draw_FillCheck(p.x, p.y))
+		if (!draw_FillCheck(p.x, p.y, false))
 			continue;
 
 		// Scan for left border
 		uint c;
-		for (c = p.x - 1; draw_FillCheck(c, p.y); c--)
+		for (c = p.x - 1; draw_FillCheck(c, p.y, true); c--)
 			;
 
 		bool newspanUp = true;
 		bool newspanDown = true;
-		for (c++; draw_FillCheck(c, p.y); c++) {
+		for (c++; draw_FillCheck(c, p.y, true); c++) {
 			putVirtPixel(c, p.y);
-			if (draw_FillCheck(c, p.y - 1)) {
+			if (draw_FillCheck(c, p.y - 1, false)) {
 				if (newspanUp) {
 					stack.push(Common::Point(c, p.y - 1));
 					newspanUp = false;
@@ -629,7 +627,7 @@ void PictureMgr::draw_Fill(int16 x, int16 y) {
 				newspanUp = true;
 			}
 
-			if (draw_FillCheck(c, p.y + 1)) {
+			if (draw_FillCheck(c, p.y + 1, false)) {
 				if (newspanDown) {
 					stack.push(Common::Point(c, p.y + 1));
 					newspanDown = false;
@@ -641,7 +639,7 @@ void PictureMgr::draw_Fill(int16 x, int16 y) {
 	}
 }
 
-bool PictureMgr::draw_FillCheck(int16 x, int16 y) {
+bool PictureMgr::draw_FillCheck(int16 x, int16 y, bool horizontalCheck) {
 	if (!getGraphicsCoordinates(x, y)) {
 		return false;
 	}
@@ -649,9 +647,6 @@ bool PictureMgr::draw_FillCheck(int16 x, int16 y) {
 	byte screenColor = _gfx->getColor(x, y);
 	byte screenPriority = _gfx->getPriority(x, y);
 
-	if (_flags & kPicFTrollMode)
-		return ((screenColor != 11) && (screenColor != _scrColor));
-
 	if (!_priOn && _scrOn && _scrColor != 15)
 		return (screenColor == 15);
 
diff --git a/engines/agi/picture.h b/engines/agi/picture.h
index b49f5627a78..a222e50f6ea 100644
--- a/engines/agi/picture.h
+++ b/engines/agi/picture.h
@@ -42,19 +42,10 @@ struct AgiPicture {
 	AgiPicture() { reset(); }
 };
 
-enum AgiPictureFlags {
-	kPicFNone      = (1 << 0),
-	kPicFf3Stop    = (1 << 1), // Troll, certain pictures
-	kPicFTrollMode = (1 << 2)  // Troll, drawing the Troll
-};
-
 class AgiBase;
 class GfxMgr;
 
 class PictureMgr {
-	AgiBase *_vm;
-	GfxMgr *_gfx;
-
 public:
 	PictureMgr(AgiBase *agi, GfxMgr *gfx);
 	virtual ~PictureMgr() { }
@@ -95,17 +86,18 @@ protected:
 	void draw_LineShort();
 	void draw_LineAbsolute();
 
-	bool draw_FillCheck(int16 x, int16 y);
+	virtual bool draw_FillCheck(int16 x, int16 y, bool horizontalCheck);
 	virtual void draw_Fill(int16 x, int16 y);
-	void draw_Fill();
+	virtual void draw_Fill();
 
 public:
 	void showPicture(int16 x = 0, int16 y = 0, int16 width = _DEFAULT_WIDTH, int16 height = _DEFAULT_HEIGHT);
 	void showPictureWithTransition();
 
-	void setPictureFlags(int flags) { _flags = flags; }
-
 protected:
+	AgiBase *_vm;
+	GfxMgr *_gfx;
+
 	int16  _resourceNr;
 	uint8 *_data;
 	uint32 _dataSize;
@@ -123,8 +115,6 @@ protected:
 
 	int16 _width;
 	int16 _height;
-
-	int _flags;
 };
 
 } // End of namespace Agi
diff --git a/engines/agi/preagi/picture_troll.cpp b/engines/agi/preagi/picture_troll.cpp
index 123c46b2b84..1f2c01a665d 100644
--- a/engines/agi/preagi/picture_troll.cpp
+++ b/engines/agi/preagi/picture_troll.cpp
@@ -56,7 +56,7 @@ void PictureMgr_Troll::drawPicture() {
 			_scrOn = true;
 			break;
 		case 0xf3:
-			if (_flags & kPicFf3Stop)
+			if (_stopOnF3)
 				return;
 			break;
 		case 0xf8:
@@ -74,8 +74,6 @@ void PictureMgr_Troll::drawPicture() {
 			draw_LineAbsolute();
 			break;
 		case 0xfe:
-			draw_SetColor();
-			_scrOn = true;
 			draw_Fill();
 			break;
 		case 0xff: // end of data
@@ -87,4 +85,84 @@ void PictureMgr_Troll::drawPicture() {
 	}
 }
 
+/**
+ * Flood fills from a series of start positions.
+ *
+ * Troll's Tale contains two separate flood fill implementations to handle the
+ * special case of drawing the Troll. The game sets a global before drawing to
+ * to activate Troll mode. We implement this by overriding this method and
+ * the check method.
+ */
+void PictureMgr_Troll::draw_Fill() {
+	draw_SetColor();
+	_scrOn = true;
+
+	byte x, y;
+	if (_scrColor == 15) { // white
+		// White flood fills are only allowed when drawing the Troll, otherwise they
+		// are completely ignored. Several room pictures contain white flood fills.
+		while (getNextCoordinates(x, y)) {
+			if (_trollMode) {
+				PictureMgr::draw_Fill(x, y);
+			}
+		}
+	} else {
+		// When not drawing the Troll, do a regular flood fill.
+		// When drawing the Troll, first fill with white, and then fill normally.
+		byte fillColor = _scrColor;
+		while (getNextCoordinates(x, y)) {
+			if (_trollMode) {
+				_scrColor = 15; // white
+				PictureMgr::draw_Fill(x, y);
+				_scrColor = fillColor;
+			}
+			PictureMgr::draw_Fill(x, y);
+		}
+	}
+}
+
+/**
+ * Checks if flood fill is allowed at a position.
+ *
+ * Troll's Tale contains two separate flood fill implementations to handle the
+ * special case of drawing the Troll. The game sets a global before drawing to
+ * to activate Troll mode.
+ *
+ * The Troll is a large picture with flood fills that is drawn over many busy
+ * room pictures, and always in the same location. This is a problem because the
+ * picture format is only meant to fill white areas. Sierra handled this by
+ * reserving a color for the Troll's lines (11, light blue) and implementing a
+ * second set of routines that fill white and treat the Troll's color as a
+ * boundary, and sometimes white as well. When drawing the Troll, a non-white
+ * fill is preceded by a special white fill to clear the area. This does not
+ * work well if there are existing white pixels, and rooms do have these.
+ * The Troll has incomplete fills in these rooms, but this is original behavior.
+ * In some rooms, such as those with white checkered floors, the results are
+ * so bad that Sierra added them to the list of rooms the Troll never visits.
+ *
+ * We implement Troll mode without a second flood fill algorithm; instead we
+ * override the check method, and our AGI algorithm in the base class provides
+ * the context so we know which of the two color checks to use.
+ */
+bool PictureMgr_Troll::draw_FillCheck(int16 x, int16 y, bool horizontalCheck) {
+	if (_trollMode && _scrColor == 15) {
+		if (!getGraphicsCoordinates(x, y)) {
+			return false;
+		}
+
+		byte screenColor = _gfx->getColor(x, y);
+
+		// when filling white during troll mode...
+		if (horizontalCheck) {
+			// horizontal checks only stop on troll line color
+			return (screenColor != 11);
+		} else {
+			// all other checks stop on troll line color or white
+			return (screenColor != 11) && (screenColor != 15);
+		}
+	}
+	
+	return PictureMgr::draw_FillCheck(x, y, horizontalCheck);
+}
+
 } // End of namespace Agi
diff --git a/engines/agi/preagi/picture_troll.h b/engines/agi/preagi/picture_troll.h
index e52617ee2b3..e389be9c3c8 100644
--- a/engines/agi/preagi/picture_troll.h
+++ b/engines/agi/preagi/picture_troll.h
@@ -29,6 +29,16 @@ public:
 	PictureMgr_Troll(AgiBase *agi, GfxMgr *gfx);
 
 	void drawPicture() override;
+	
+	void draw_Fill() override;
+	bool draw_FillCheck(int16 x, int16 y, bool horizontalCheck) override;
+
+	void setStopOnF3(bool stopOnF3) { _stopOnF3 = stopOnF3; }
+	void setTrollMode(bool trollMode) { _trollMode = trollMode; }
+
+private:
+	bool _stopOnF3;
+	bool _trollMode;
 };
 
 } // End of namespace Agi
diff --git a/engines/agi/preagi/troll.cpp b/engines/agi/preagi/troll.cpp
index 8c52fb497fd..24b15b42e7b 100644
--- a/engines/agi/preagi/troll.cpp
+++ b/engines/agi/preagi/troll.cpp
@@ -137,19 +137,13 @@ void TrollEngine::drawPic(int iPic, bool f3IsCont, bool clr, bool troll) {
 	}
 
 	// draw the frame picture
-	_picture->setPictureFlags(kPicFNone);
+	_picture->setStopOnF3(false);
+	_picture->setTrollMode(false);
 	_picture->decodePictureFromBuffer(_gameData + IDO_TRO_FRAMEPIC, 4096, clr, IDI_TRO_PIC_WIDTH, IDI_TRO_PIC_HEIGHT);
 
 	// draw the picture
-	int flags = 0;
-	if (!f3IsCont) {
-		// stop on opcode F3
-		flags |= kPicFf3Stop;
-	}
-	if (troll) {
-		flags |= kPicFTrollMode;
-	}
-	_picture->setPictureFlags(flags);
+	_picture->setStopOnF3(!f3IsCont);
+	_picture->setTrollMode(troll);
 	_picture->decodePictureFromBuffer(_gameData + _pictureOffsets[iPic], 4096, false, IDI_TRO_PIC_WIDTH, IDI_TRO_PIC_HEIGHT);
 
 	_picture->showPicture(0, 0, IDI_TRO_PIC_WIDTH, IDI_TRO_PIC_HEIGHT);


Commit: ce88b3dc81c8d59ca020ed9b7956229afbda81e5
    https://github.com/scummvm/scummvm/commit/ce88b3dc81c8d59ca020ed9b7956229afbda81e5
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2025-01-05T09:49:27+02:00

Commit Message:
AGI: Add support for early KQ1 pictures

Changed paths:
  A engines/agi/picture_gal.cpp
  A engines/agi/picture_gal.h
    engines/agi/module.mk
    engines/agi/picture.cpp
    engines/agi/picture.h


diff --git a/engines/agi/module.mk b/engines/agi/module.mk
index fce6dbb921c..b79da837f3c 100644
--- a/engines/agi/module.mk
+++ b/engines/agi/module.mk
@@ -26,6 +26,7 @@ MODULE_OBJS := \
 	op_dbg.o \
 	op_test.o \
 	picture.o \
+	picture_gal.o \
 	saveload.o \
 	sound.o \
 	sound_2gs.o \
diff --git a/engines/agi/picture.cpp b/engines/agi/picture.cpp
index 500f7ce31bd..51f44a25225 100644
--- a/engines/agi/picture.cpp
+++ b/engines/agi/picture.cpp
@@ -673,7 +673,7 @@ void PictureMgr::decodePicture(int16 resourceNr, bool clearScreen, bool agi256,
 	_height = height;
 
 	if (clearScreen) {
-		_gfx->clear(15, 4); // white, priority 4
+		_gfx->clear(15, getInitialPriorityColor()); // white, priority 4 or 1
 	}
 
 	if (!agi256) {
@@ -704,7 +704,7 @@ void PictureMgr::decodePictureFromBuffer(byte *data, uint32 length, bool clearSc
 	_height = height;
 
 	if (clearScreen) {
-		_gfx->clear(15, 4); // white, priority 4
+		_gfx->clear(15, getInitialPriorityColor()); // white, priority 4 or 1
 	}
 
 	drawPicture();
diff --git a/engines/agi/picture.h b/engines/agi/picture.h
index a222e50f6ea..cb28e72ff5b 100644
--- a/engines/agi/picture.h
+++ b/engines/agi/picture.h
@@ -53,6 +53,8 @@ public:
 	int16 getResourceNr() const { return _resourceNr; };
 
 protected:
+	virtual byte getInitialPriorityColor() const { return 4; }
+
 	void putVirtPixel(int16 x, int16 y);
 	void xCorner(bool skipOtherCoords = false);
 	void yCorner(bool skipOtherCoords = false);
@@ -82,7 +84,7 @@ protected:
 	void draw_SetNibbleColor();
 	void draw_SetNibblePriority();
 
-	void draw_Line(int16 x1, int16 y1, int16 x2, int16 y2);
+	virtual void draw_Line(int16 x1, int16 y1, int16 x2, int16 y2);
 	void draw_LineShort();
 	void draw_LineAbsolute();
 
diff --git a/engines/agi/picture_gal.cpp b/engines/agi/picture_gal.cpp
new file mode 100644
index 00000000000..e083fc08d5b
--- /dev/null
+++ b/engines/agi/picture_gal.cpp
@@ -0,0 +1,267 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "agi/agi.h"
+#include "agi/graphics.h"
+#include "agi/picture.h"
+
+#include "agi/picture_gal.h"
+
+namespace Agi {
+	
+// PictureMgr_GAL decodes and draws picture resources in early King's Quest 1.
+//
+// This "Game Adaptation Language" format was used in PC Booters and Apple II.
+//
+// This format supports lines and flood fills, and visual and priority screens.
+//
+// As this is the format that evolved into AGI, it is quite similar apart from
+// the specific opcodes. The major difference is the line drawing routine;
+// it produces different results than AGI. Flood fills implicitly rely on this.
+// When KQ1 was ported to AGI, the new lines prevented some fills from working,
+// so they just added more. There are still a few white pixels they missed.
+//
+// As with Troll's Tale, room pictures depend on the game first drawing a frame
+// within the entire picture area. When KQ1 was converted to AGI, this frame
+// was added to each individual picture resource.
+
+PictureMgr_GAL::PictureMgr_GAL(AgiBase *agi, GfxMgr *gfx) :
+	PictureMgr(agi, gfx) {
+}
+
+void PictureMgr_GAL::drawPicture() {
+	debugC(kDebugLevelPictures, "Drawing picture %d", _resourceNr);
+
+	drawBlackFrame();
+
+	_dataOffset = 0;
+	_dataOffsetNibble = false;
+	_patCode = 0;
+	_patNum = 0;
+	_priOn = true; // initially off in AGI
+	_scrOn = false;
+	_scrColor = 15;
+	_priColor = 1;
+
+	// GAL toggles the current screen between visual and priority
+	// with opcode F0. This affects opcodes F4-F7, but the rest of
+	// the opcodes are explicit about which screen(s) they draw to.
+	byte prevScrOn = _scrOn;
+	byte prevPriOn = _priOn;
+
+	while (_dataOffset < _dataSize) {
+		byte curByte = getNextByte();
+
+		switch (curByte) {
+		case 0xf0: // toggle current screen
+			draw_SetScreens(!_scrOn, !_priOn);
+			break;
+		case 0xf1:
+			draw_SetColor();
+			break;
+		case 0xf2:
+			draw_SetPriority();
+			break;
+		case 0xf3:
+			draw_SetColor();
+			draw_SetPriority();
+			break;
+
+		// Line operations drawn to both visual and priority screens
+		case 0xf4:
+			draw_SetScreens(true, true, prevScrOn, prevPriOn);
+			yCorner();
+			draw_SetScreens(prevScrOn, prevPriOn);
+			break;
+		case 0xf5:
+			draw_SetScreens(true, true, prevScrOn, prevPriOn);
+			xCorner();
+			draw_SetScreens(prevScrOn, prevPriOn);
+			break;
+		case 0xf6:
+			draw_SetScreens(true, true, prevScrOn, prevPriOn);
+			draw_LineAbsolute();
+			draw_SetScreens(prevScrOn, prevPriOn);
+			break;
+		case 0xf7:
+			draw_SetScreens(true, true, prevScrOn, prevPriOn);
+			draw_LineShort();
+			draw_SetScreens(prevScrOn, prevPriOn);
+			break;
+
+		// Line operations drawn to the current screen
+		case 0xf8:
+			yCorner();
+			break;
+		case 0xf9:
+			xCorner();
+			break;
+		case 0xfa:
+			draw_LineAbsolute();
+			break;
+		case 0xfb:
+			draw_LineShort();
+			break;
+
+		// Fill operations drawn to one or both screens
+		case 0xfc:
+			draw_SetScreens(true, true, prevScrOn, prevPriOn);
+			draw_SetColor();
+			draw_SetPriority();
+			draw_Fill();
+			draw_SetScreens(prevScrOn, prevPriOn);
+			break;
+		case 0xfd:
+			draw_SetScreens(false, true, prevScrOn, prevPriOn);
+			draw_SetPriority();
+			draw_Fill();
+			draw_SetScreens(prevScrOn, prevPriOn);
+			break;
+		case 0xfe:
+			draw_SetScreens(true, false, prevScrOn, prevPriOn);
+			draw_SetColor();
+			draw_Fill();
+			draw_SetScreens(prevScrOn, prevPriOn);
+			break;
+
+		case 0xff: // end of data
+			return;
+		default:
+			warning("Unknown picture opcode %02x at %04x", curByte, _dataOffset - 1);
+			break;
+		}
+	}
+}
+
+/**
+ * Sets the status of the visual and priority screens.
+ */
+void PictureMgr_GAL::draw_SetScreens(byte scrOn, byte priOn) {
+	_scrOn = scrOn;
+	_priOn = priOn;
+}
+
+/**
+ * Sets the status of the visual and priority screens,
+ * and returns their previous values.
+ */
+void PictureMgr_GAL::draw_SetScreens(byte scrOn, byte priOn, byte &prevScrOn, byte &prevPriOn) {
+	prevScrOn = _scrOn;
+	prevPriOn = _priOn;
+	_scrOn = scrOn;
+	_priOn = priOn;
+}
+
+/**
+ * Draws a hard-coded black frame in both screens.
+ * All room pictures require this to draw correctly.
+ *
+ * Original data: F3 00 00 F5 00 00 9F A7 00 00 FF
+ */
+void PictureMgr_GAL::drawBlackFrame() {
+	_scrOn = true;
+	_scrColor = 0;
+	_priOn = true;
+	_priColor = 0;
+	draw_Line(0, 0, _width - 1, 0);
+	draw_Line(_width - 1, 0, _width - 1, _height - 1);
+	draw_Line(_width - 1, _height - 1, 0, _height - 1);
+	draw_Line(0, _height - 1, 0, 0);
+}
+
+/**
+ * Draws a horizontal, vertical, or diagonal line using the GAL drawing routine.
+ *
+ * This routine produces different diagonal lines than the AGI routine.
+ */
+void PictureMgr_GAL::draw_Line(int16 x1, int16 y1, int16 x2, int16 y2) {
+	x1 = CLIP<int16>(x1, 0, _width - 1);
+	x2 = CLIP<int16>(x2, 0, _width - 1);
+	y1 = CLIP<int16>(y1, 0, _height - 1);
+	y2 = CLIP<int16>(y2, 0, _height - 1);
+
+	const byte width  = (x2 > x1) ? (x2 - x1) : (x1 - x2);
+	const byte height = (y2 > y1) ? (y2 - y1) : (y1 - y2);
+
+	byte x = 0;
+	byte y = 0;
+	if (width > height) {
+		while (x != width) {
+			x++;
+			y = (x * height) / width;
+			if (((x * height) % width) * 2 > width) {
+				y++;
+			}
+
+			byte pixelX = (x2 > x1) ? (x1 + x) : (x1 - x);
+			byte pixelY = (y2 > y1) ? (y1 + y) : (y1 - y);
+			putVirtPixel(pixelX, pixelY);
+		}
+	} else {
+		while (y != height) {
+			y++;
+			x = (y * width) / height;
+			if (((y * width) % height) * 2 > height) {
+				x++;
+			}
+
+			byte pixelX = (x2 > x1) ? (x1 + x) : (x1 - x);
+			byte pixelY = (y2 > y1) ? (y1 + y) : (y1 - y);
+			putVirtPixel(pixelX, pixelY);
+		}
+	}
+}
+
+/**
+ * Gets the next x coordinate in the current picture instruction,
+ * and clip it to the picture width.
+ */
+bool PictureMgr_GAL::getNextXCoordinate(byte &x) {
+	if (!getNextParamByte(x)) {
+		return false;
+	}
+
+	if (x >= _width) { // 160
+		debugC(kDebugLevelPictures, "clipping %c from %d to %d", 'x', x, _width - 1);
+		x = _width - 1; // 159
+	}
+
+	return true;
+}
+
+/**
+ * Gets the next y coordinate in the current picture instruction,
+ * and clip it to the picture height.
+ */
+bool PictureMgr_GAL::getNextYCoordinate(byte &y) {
+	if (!getNextParamByte(y)) {
+		return false;
+	}
+
+	if (y >= _height) { // 168
+		debugC(kDebugLevelPictures, "clipping %c from %d to %d", 'y', y, _height);
+		y = _height - 1; // 167
+	}
+
+	return true;
+}
+
+} // End of namespace Agi
diff --git a/engines/agi/picture_gal.h b/engines/agi/picture_gal.h
new file mode 100644
index 00000000000..de97586280a
--- /dev/null
+++ b/engines/agi/picture_gal.h
@@ -0,0 +1,50 @@
+/* ScummVM - Graphic Adventure Engine
+ *
+ * ScummVM is the legal property of its developers, whose names
+ * are too numerous to list here. Please refer to the COPYRIGHT
+ * file distributed with this source distribution.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef AGI_PICTURE_GAL_H
+#define AGI_PICTURE_GAL_H
+
+namespace Agi {
+
+class PictureMgr_GAL : public PictureMgr {
+public:
+	PictureMgr_GAL(AgiBase *agi, GfxMgr *gfx);
+
+protected:
+	byte getInitialPriorityColor() const override { return 1; }
+
+	void drawPicture() override;
+
+	void draw_Line(int16 x1, int16 y1, int16 x2, int16 y2) override;
+
+	bool getNextXCoordinate(byte &x) override;
+	bool getNextYCoordinate(byte &y) override;
+
+private:
+	void draw_SetScreens(byte scrOn, byte priOn);
+	void draw_SetScreens(byte scrOn, byte priOn, byte &prevScrOn, byte &prevPriOn);
+
+	void drawBlackFrame();
+};
+
+} // End of namespace Agi
+
+#endif


Commit: 82f79ad13cf8356d8f1b0a1871e8bae752ac3a83
    https://github.com/scummvm/scummvm/commit/82f79ad13cf8356d8f1b0a1871e8bae752ac3a83
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2025-01-05T09:49:27+02:00

Commit Message:
AGI: Document PictureMgr

Changed paths:
    engines/agi/picture.cpp
    engines/agi/picture.h
    engines/agi/preagi/picture_mickey_winnie.cpp
    engines/agi/preagi/picture_troll.cpp


diff --git a/engines/agi/picture.cpp b/engines/agi/picture.cpp
index 51f44a25225..70ce126d9bf 100644
--- a/engines/agi/picture.cpp
+++ b/engines/agi/picture.cpp
@@ -25,6 +25,24 @@
 #include "common/textconsole.h"
 
 namespace Agi {
+	
+// PictureMgr decodes and draws AGI picture resources.
+//
+// AGI pictures are vector-based, and contain the visual and priority screens.
+// Drawing instructions begin with an opcode byte within the range F0-FF.
+// Opcode parameters are each one byte, with the exception of AGIv3 nibble
+// compression. Some opcodes take a variable number of parameters. The end of
+// an instruction is detected by the next byte with a value in the opcode range.
+// If an instruction has extra bytes, or a picture contains an unknown opcode
+// byte, then these bytes ignored. Pictures end with opcode FF.
+//
+// AGIv3 introduced a compression scheme where two opcode parameters were
+// each reduced to one nibble; this is indicated by a flag in the picture's
+// resource directory entry.
+//
+// AGI's picture format evolved from variants used in earlier Sierra games.
+// We implement support for these formats as subclasses of PictureMgr.
+// In this way, we treat AGI as the baseline to be overridden as needed.
 
 PictureMgr::PictureMgr(AgiBase *agi, GfxMgr *gfx) {
 	_vm = agi;
@@ -49,6 +67,9 @@ PictureMgr::PictureMgr(AgiBase *agi, GfxMgr *gfx) {
 	_height = 0;
 }
 
+/**
+ * Draws a pixel to the visual and/or control screen.
+ */
 void PictureMgr::putVirtPixel(int16 x, int16 y) {
 	if (!getGraphicsCoordinates(x, y)) {
 		return;
@@ -63,6 +84,9 @@ void PictureMgr::putVirtPixel(int16 x, int16 y) {
 	_gfx->putPixel(x, y, drawMask, _scrColor, _priColor);
 }
 
+/**
+ * Gets the next byte in the picture data.
+ */
 byte PictureMgr::getNextByte() {
 	if (!_dataOffsetNibble) {
 		return _data[_dataOffset++];
@@ -72,6 +96,11 @@ byte PictureMgr::getNextByte() {
 	}
 }
 
+/**
+ * Gets the next byte in the current picture instruction.
+ * If the next byte in the picture data is an opcode, then this
+ * function returns false and the data offset is not advanced.
+ */
 bool PictureMgr::getNextParamByte(byte &b) {
 	byte value = getNextByte();
 	if (value >= _minCommand) {
@@ -82,6 +111,9 @@ bool PictureMgr::getNextParamByte(byte &b) {
 	return true;
 }
 
+/**
+ * Gets the next nibble in the picture data.
+ */
 byte PictureMgr::getNextNibble() {
 	if (!_dataOffsetNibble) {
 		_dataOffsetNibble = true;
@@ -92,27 +124,57 @@ byte PictureMgr::getNextNibble() {
 	}
 }
 
+/**
+ * Gets the next x coordinate in the current picture instruction.
+ *
+ * Subclasses override this to implement coordinate clipping.
+ */
 bool PictureMgr::getNextXCoordinate(byte &x) {
 	return getNextParamByte(x);
 }
 
+/**
+ * Gets the next y coordinate in the current picture instruction.
+ *
+ * Subclasses override this to implement coordinate clipping.
+ */
 bool PictureMgr::getNextYCoordinate(byte &y) {
 	return getNextParamByte(y);
 }
 
+/**
+ * Gets the next x and y coordinates in the current picture instruction.
+ *
+ * Returns false if both coordinates are not present. If only an x coordinate is
+ * present, then the data offset is only advanced by one, and the x coordinate
+ * will be ignored.
+ */
 bool PictureMgr::getNextCoordinates(byte &x, byte &y) {
 	return getNextXCoordinate(x) && getNextYCoordinate(y);
 }
 
+/**
+ * Validates picture coordinates and translates them to GfxMgr coordinates.
+ *
+ * Subclasses can override this to implement the PreAGI offset feature that
+ * allowed a picture to be drawn at an arbitrary point on the screen.
+ * Returns false if picture coordinates are out of bounds, or for subclasses,
+ * if the PreAGI offset would place the coordinate outside of GfxMgr's screen.
+ */
 bool PictureMgr::getGraphicsCoordinates(int16 &x, int16 &y) {
 	return (0 <= x && x < _width && 0 <= y && y < _height);
 }
 
-/**************************************************************************
-** xCorner
-**
-** Draws an xCorner  (drawing action 0xF5)
-**************************************************************************/
+/**
+ * xCorner
+ *
+ * Draws a series of lines with right angles between them.
+ * The first two bytes are the start point, followed by alternating
+ * x and y coordinates for subsequent points.
+ *
+ * Set skipOtherCoords to ignore extra coordinates in Troll's pictures.
+ * Troll includes both the x and y coordinate of each point.
+ */
 void PictureMgr::xCorner(bool skipOtherCoords) {
 	byte x1, x2, y1, y2, dummy;
 
@@ -144,11 +206,16 @@ void PictureMgr::xCorner(bool skipOtherCoords) {
 	}
 }
 
-/**************************************************************************
-** yCorner
-**
-** Draws an yCorner  (drawing action 0xF4)
-**************************************************************************/
+/**
+ * yCorner
+ *
+ * Draws a series of lines with right angles between them.
+ * The first two bytes are the start point, followed by alternating
+ * y and x coordinates for subsequent points.
+ *
+ * Set skipOtherCoords to ignore extra coordinates in Troll's pictures.
+ * Troll includes both the x and y coordinate of each point.
+ */
 void PictureMgr::yCorner(bool skipOtherCoords) {
 	byte x1, x2, y1, y2, dummy;
 
@@ -179,12 +246,14 @@ void PictureMgr::yCorner(bool skipOtherCoords) {
 	}
 }
 
-/**************************************************************************
-** plotPattern
-**
-** Draws pixels, circles, squares, or splatter brush patterns depending
-** on the pattern code.
-**************************************************************************/
+/**
+ * plotPattern
+ *
+ * Draws a circle or square. Size and optional splatter brush pattern
+ * are determined by the current pattern code.
+ *
+ * This routine is originally from NAGI.
+ */
 void PictureMgr::plotPattern(byte x, byte y) {
 	static const uint16 binary_list[] = {
 		0x8000, 0x4000, 0x2000, 0x1000, 0x800, 0x400, 0x200, 0x100,
@@ -283,11 +352,11 @@ void PictureMgr::plotPattern(byte x, byte y) {
 	}
 }
 
-/**************************************************************************
-** plotBrush
-**
-** Plots points and various brush patterns.
-**************************************************************************/
+/**
+ * plotBrush
+ *
+ * Plots the current brush pattern.
+ */
 void PictureMgr::plotBrush() {
 	for (;;) {
 		if (_patCode & 0x20) {
@@ -303,9 +372,9 @@ void PictureMgr::plotBrush() {
 	}
 }
 
-/**************************************************************************
-** Draw AGI picture
-**************************************************************************/
+/**
+ * Draws the current picture to the visual and priority screens.
+ */
 void PictureMgr::drawPicture() {
 	debugC(kDebugLevelPictures, "Drawing picture %d", _resourceNr);
 
@@ -377,7 +446,10 @@ void PictureMgr::drawPicture() {
 	}
 }
 
-void PictureMgr::drawPictureAGI256() {
+/**
+ * Draws the current AGI256 picture to the visual screen.
+ */
+void PictureMgr::drawPicture_AGI256() {
 	const uint32 maxFlen = _width * _height;
 	int16 x = 0;
 	int16 y = 0;
@@ -416,6 +488,9 @@ void PictureMgr::drawPictureAGI256() {
 		warning("Oversized AGI256 picture resource %d, decoding only %ux%u part of it", _resourceNr, _width, _height);
 }
 
+/**
+ * Sets the visual screen color to the next byte in the picture data.
+ */
 void PictureMgr::draw_SetColor() {
 	_scrColor = getNextByte();
 
@@ -425,12 +500,18 @@ void PictureMgr::draw_SetColor() {
 	}
 }
 
+/**
+ * Sets the priority screen color to the next byte in the picture data.
+ */
 void PictureMgr::draw_SetPriority() {
 	_priColor = getNextByte();
 }
 
-// this gets a nibble instead of a full byte
-// used by some V3 games, special resource flag RES_PICTURE_V3_NIBBLE_PARM is set
+/**
+ * Sets the visual screen color to the next nibble in the picture data.
+ * Used in AGIv3 to compress the set-color instructions when the flag
+ * RES_PICTURE_V3_NIBBLE_PARM is set in the picture's directory entry.
+ */
 void PictureMgr::draw_SetNibbleColor() {
 	_scrColor = getNextNibble();
 
@@ -440,18 +521,22 @@ void PictureMgr::draw_SetNibbleColor() {
 	}
 }
 
+/**
+ * Sets the priority screen color to the next nibble in the picture data.
+ * Used in AGIv3 to compress the set-color instructions when the flag
+ * RES_PICTURE_V3_NIBBLE_PARM is set in the picture's directory entry.
+ */
 void PictureMgr::draw_SetNibblePriority() {
 	_priColor = getNextNibble();
 }
 
 /**
- * Draw an AGI line.
- * A line drawing routine sent by Joshua Neal, modified by Stuart George
- * (fixed >>2 to >>1 and some other bugs like x1 instead of y1, etc.)
- * @param x1  x coordinate of start point
- * @param y1  y coordinate of start point
- * @param x2  x coordinate of end point
- * @param y2  y coordinate of end point
+ * Draws a horizontal, vertical, or diagonal line.
+ *
+ * This routine is originally from Sarien. Original comment:
+ *
+ *   A line drawing routine sent by Joshua Neal, modified by Stuart George
+ *   (fixed >>2 to >>1 and some other bugs like x1 instead of y1, etc.)
  */
 void PictureMgr::draw_Line(int16 x1, int16 y1, int16 x2, int16 y2) {
 	x1 = CLIP<int16>(x1, 0, _width - 1);
@@ -533,8 +618,9 @@ void PictureMgr::draw_Line(int16 x1, int16 y1, int16 x2, int16 y2) {
 }
 
 /**
- * Draw a relative AGI line.
- * Draws short lines relative to last position. (drawing action 0xF7)
+ * draw_LineShort
+ *
+ * Draws short lines between positions in relative coordinates.
  */
 void PictureMgr::draw_LineShort() {
 	byte x1, y1, disp;
@@ -562,11 +648,11 @@ void PictureMgr::draw_LineShort() {
 	}
 }
 
-/**************************************************************************
-** absoluteLine
-**
-** Draws long lines to actual locations (cf. relative) (drawing action 0xF6)
-**************************************************************************/
+/**
+ * draw_LineAbsolute
+ *
+ * Draws lines between positions in absolute coordinates.
+ */
 void PictureMgr::draw_LineAbsolute() {
 	byte x1, y1, x2, y2;
 
@@ -585,7 +671,9 @@ void PictureMgr::draw_LineAbsolute() {
 	}
 }
 
-// flood fill
+/**
+ * Flood fills from a series of start positions.
+ */
 void PictureMgr::draw_Fill() {
 	byte x, y;
 
@@ -594,6 +682,9 @@ void PictureMgr::draw_Fill() {
 	}
 }
 
+/**
+ * Flood fills from a start position.
+ */
 void PictureMgr::draw_Fill(int16 x, int16 y) {
 	if (!_scrOn && !_priOn)
 		return;
@@ -639,6 +730,13 @@ void PictureMgr::draw_Fill(int16 x, int16 y) {
 	}
 }
 
+/**
+ * Checks if flood fill is allowed at a position.
+ *
+ * horizontalCheck indicates if the flood fill algorithm is scanning the current
+ * line horizontally for a boundary. This is used by PictureMgr_Troll to handle
+ * Troll's Tale custom flood fill behavior when drawing the Troll over pictures.
+ */
 bool PictureMgr::draw_FillCheck(int16 x, int16 y, bool horizontalCheck) {
 	if (!getGraphicsCoordinates(x, y)) {
 		return false;
@@ -657,13 +755,11 @@ bool PictureMgr::draw_FillCheck(int16 x, int16 y, bool horizontalCheck) {
 }
 
 /**
- * Decode an AGI picture resource. Used by regular AGI games.
- * This function decodes an AGI picture resource into the correct slot
- * and draws it on the AGI screen, optionally clearing the screen before
- * drawing.
- * @param n      AGI picture resource number
- * @param clear  clear AGI screen before drawing
- * @param agi256 load an AGI256 picture resource
+ * Draws a picture by resource number to the visual and control screens.
+ * This interface is used by AGI games and GAL (KQ1 early).
+ *
+ * The picture resource must already be loaded. This function sets the current
+ * picture and optionally clears the screens before drawing.
  */
 void PictureMgr::decodePicture(int16 resourceNr, bool clearScreen, bool agi256, int16 width, int16 height) {
 	_resourceNr = resourceNr;
@@ -679,7 +775,7 @@ void PictureMgr::decodePicture(int16 resourceNr, bool clearScreen, bool agi256,
 	if (!agi256) {
 		drawPicture();
 	} else {
-		drawPictureAGI256();
+		drawPicture_AGI256();
 	}
 
 	if (clearScreen) {
@@ -689,13 +785,11 @@ void PictureMgr::decodePicture(int16 resourceNr, bool clearScreen, bool agi256,
 }
 
 /**
- * Decode an AGI picture resource. Used by preAGI.
- * This function decodes an AGI picture resource into the correct slot
- * and draws it on the AGI screen, optionally clearing the screen before
- * drawing.
- * @param data   the AGI Picture data
- * @param length the size of the picture data buffer
- * @param clear  clear AGI screen before drawing
+ * Draws a picture from a buffer to the visual and control screens.
+ * This interface is used by PreAGI games.
+ *
+ * This function sets the current picture and optionally clears the screens
+ * before drawing.
  */
 void PictureMgr::decodePictureFromBuffer(byte *data, uint32 length, bool clearScreen, int16 width, int16 height) {
 	_data = data;
@@ -710,12 +804,25 @@ void PictureMgr::decodePictureFromBuffer(byte *data, uint32 length, bool clearSc
 	drawPicture();
 }
 
+/**
+ * Renders a drawn picture from the active screen to the display screen.
+ *
+ * The active screen is usually the visual screen, but this can be toggled
+ * to the priority screen in debug modes.
+ */
 void PictureMgr::showPicture(int16 x, int16 y, int16 width, int16 height) {
 	debugC(kDebugLevelPictures, "Show picture");
 
 	_gfx->render_Block(x, y, width, height);
 }
 
+/**
+ * Renders a drawn picture from the active screen to the display screen
+ * with transition effects. The effect is determined by the render mode.
+ *
+ * The active screen is usually the visual screen, but this can be toggled
+ * to the priority screen in debug modes.
+ */
 void PictureMgr::showPictureWithTransition() {
 	_width = SCRIPT_WIDTH;
 	_height = SCRIPT_HEIGHT;
diff --git a/engines/agi/picture.h b/engines/agi/picture.h
index cb28e72ff5b..395955fe2e9 100644
--- a/engines/agi/picture.h
+++ b/engines/agi/picture.h
@@ -77,7 +77,7 @@ public:
 
 protected:
 	virtual void drawPicture();
-	void drawPictureAGI256();
+	void drawPicture_AGI256();
 
 	void draw_SetColor();
 	void draw_SetPriority();
diff --git a/engines/agi/preagi/picture_mickey_winnie.cpp b/engines/agi/preagi/picture_mickey_winnie.cpp
index 36e1dd32b4b..2f994dbe0fb 100644
--- a/engines/agi/preagi/picture_mickey_winnie.cpp
+++ b/engines/agi/preagi/picture_mickey_winnie.cpp
@@ -18,7 +18,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
- 
+
 #include "agi/agi.h"
 #include "agi/graphics.h"
 #include "agi/picture.h"
@@ -27,6 +27,42 @@
 
 namespace Agi {
 
+// PictureMgr_Mickey_Winnie decodes and draws picture resources in Mickey's
+// Space Adventure (DOS) and Winnie the Pooh (DOS/Amiga/A2/C64/CoCo).
+//
+// Mickey and Winnie DOS/Amiga use the same format. The picture code in
+// their executables appears to be the same.
+//
+// The A2/C64/CoCo versions of Winnie use a completely different format, but
+// they do support the same features. These games start in ScummVM but they
+// don't really work yet. TODO: display the right colors, figure out the line
+// differences, support these versions.
+//
+// Both formats support lines, flood fills, and patterns. No priority screen.
+//
+// Unique features to these formats:
+//
+// 1. Pictures can be drawn on top of others at arbitrary locations. Used to
+//    draw items in rooms, and to draw room pictures with a buffer on each side
+//    in DOS/Amiga. The pictures don't fill the screen width because they were
+//    designed for the Apple II.
+//
+// 2. Mickey's crystals animate. Most of the work is done in MickeyEngine;
+//    this class just allows the engine to set a maximum number of picture
+//    instructions to execute. Unclear if this is same effect as the original.
+//
+// 3. The pattern opcode draws solid circles in up to 17 sizes.
+//
+// Mickey features animating spaceship lights, but the engine handles that.
+// The lights are a picture whose instructions are modified before drawing.
+//
+// TODO: There are extremely minor inaccuracies in several Winnie pictures.
+// The F1 opcode's effects are not fully understood, and it creates subtle
+// discrepancies. It may be related to dithering. However, so few pictures
+// contain F3, and even fewer are effected by ignoring it or not, and only
+// by a few pixels, that it doesn't matter except for completeness.
+// See: picture 34 door handles (Rabbit's kitchen)
+
 PictureMgr_Mickey_Winnie::PictureMgr_Mickey_Winnie(AgiBase *agi, GfxMgr *gfx) :
 	PictureMgr(agi, gfx) {
 
@@ -160,6 +196,11 @@ void PictureMgr_Mickey_Winnie::drawPicture_A2_C64_CoCo() {
 	}
 }
 
+/**
+ * plotBrush (PreAGI)
+ *
+ * Plots the given brush pattern. All brushes are solid circles.
+ */
 void PictureMgr_Mickey_Winnie::plotBrush() {
 	_patCode = getNextByte();
 	if (_patCode > 12) {
@@ -175,6 +216,11 @@ void PictureMgr_Mickey_Winnie::plotBrush() {
 	}
 }
 
+/**
+ * plotPattern
+ *
+ * Draws a solid circle. Size is determined by the pattern code.
+ */
 void PictureMgr_Mickey_Winnie::plotPattern(byte x, byte y) {
 	// PreAGI patterns are 13 solid circles
 	static const byte circleData[] = {
@@ -278,11 +324,20 @@ bool PictureMgr_Mickey_Winnie::getNextYCoordinate(byte &y) {
 	return true;
 }
 
+/**
+ * Validates picture coordinates and translates them to GfxMgr coordinates.
+ *
+ * This function applies the current picture object and validates that the
+ * graphics coordinates are within GfxMgr's boundaries. Validation is necessary
+ * because Winnie places tall objects at the bottom of the screen in several
+ * rooms, and GfxMgr does not validate coordinates.
+ */
 bool PictureMgr_Mickey_Winnie::getGraphicsCoordinates(int16 &x, int16 &y) {
+	// validate that the coordinates are within the picture's boundaries
 	if (!PictureMgr::getGraphicsCoordinates(x, y)) {
 		return false;
 	}
-	
+
 	x += _xOffset;
 	y += _yOffset;
 
diff --git a/engines/agi/preagi/picture_troll.cpp b/engines/agi/preagi/picture_troll.cpp
index 1f2c01a665d..d71e93110c3 100644
--- a/engines/agi/preagi/picture_troll.cpp
+++ b/engines/agi/preagi/picture_troll.cpp
@@ -26,6 +26,26 @@
 #include "agi/preagi/picture_troll.h"
 
 namespace Agi {
+	
+// PictureMgr_Troll decodes and draws Troll's Tale picture resources.
+//
+// Troll's Tale supports lines and flood fills. There is no priority screen.
+//
+// There are two unique picture features:
+//
+// 1. The F3 opcode can dynamically act as a no-op or terminator (FF).
+//    This allows pictures to have an optional set of instructions for
+//	  drawing or hiding a room's object or the king's crown.
+//
+// 2. A custom flood fill technique is used for drawing the Troll over
+//    room pictures. Normally, flood fill requires an empty (white) area.
+//
+// One quirk is that the xCorner and yCorner instructions contain a redundant
+// coordinate, even though it is ignored because it is derived from the others.
+//
+// Each room picture depends on the game first drawing a frame within the entire
+// picture area. This is not decorative; the flood fill routines rely on this
+// border because they do not do boundary test, and pictures are drawn for it.
 
 PictureMgr_Troll::PictureMgr_Troll(AgiBase *agi, GfxMgr *gfx) :
 	PictureMgr(agi, gfx) {
@@ -47,30 +67,24 @@ void PictureMgr_Troll::drawPicture() {
 		byte curByte = getNextByte();
 
 		switch (curByte) {
-		case 0xf0:
-			// happens in all Troll's Tale pictures
-			// TODO: figure out what it was meant for
-			break;
+		case 0xf0: // F0 is in all Troll's Tale pictures, but it is a no-op.
+			break; // Confirmed in disassembly of opcode table.
 		case 0xf1:
 			draw_SetColor();
 			_scrOn = true;
 			break;
-		case 0xf3:
+		case 0xf3: // F3 would impersonate opcode F0 (no-op) or FF (terminator)
 			if (_stopOnF3)
 				return;
 			break;
 		case 0xf8:
-			yCorner(true);
+			yCorner(true); // skip extra (redundant) coordinates when parsing
 			break;
 		case 0xf9:
-			xCorner(true);
-			break;
-		case 0xfa:
-			// TODO: is this really correct?
-			draw_LineAbsolute();
+			xCorner(true); // skip extra (redundant) coordinates when parsing
 			break;
-		case 0xfb:
-			// TODO: is this really correct?
+		case 0xfa: // FA and FB are both used, but they are the same.
+		case 0xfb: // Confirmed in disassembly of opcode table.
 			draw_LineAbsolute();
 			break;
 		case 0xfe:




More information about the Scummvm-git-logs mailing list