Opened 17 years ago

Closed 19 months ago

#3208 closed defect (fixed)

INDY3: Minor Graphics Glitch (script/data bug)

Reported by: SF/mthreepwood Owned by: dwatteau
Priority: low Component: Engine: SCUMM
Version: Keywords: original
Cc: Game: Indiana Jones 3

Description

While in Castle Brunwald, in the room with the tapestry on the second floor, there is a pink line out of place. Saved game attached.

Win XP 0.10.0svn May 25

Indy3 VGA English

Ticket imported from: #1726606. Ticket imported from: bugs/3208.

Attachments (4)

indy3-vga.s08 (13.5 KB ) - added by SF/mthreepwood 17 years ago.
Saved Game
room23object324x2.png (4.1 KB ) - added by dwatteau 2 years ago.
Restored room23object324x2.png screenshot from an old backup of mine; original imageshack resource is gone
scummvm-indy3-vga-pink-original.png (14.0 KB ) - added by dwatteau 2 years ago.
original pink line in the middle of room 135
scummvm-indy3-vga-grey-hack.png (14.0 KB ) - added by dwatteau 2 years ago.
grey hack hiding the line a bit

Download all attachments as: .zip

Change History (17)

by SF/mthreepwood, 17 years ago

Attachment: indy3-vga.s08 added

Saved Game

comment:1 by fingolfin, 17 years ago

Is this a regression, or did it already appear with 0.9.x / 0.8.x ?

comment:2 by SF/mthreepwood, 17 years ago

Yes, it existed in 0.8.x/0.9.x as well. I guess I just never noticed it before

comment:3 by fingolfin, 17 years ago

I wonder if this occurs in dosbox, too, hrm.

comment:4 by SF/thunderpeel2001, 17 years ago

Just tested this, it's in the original VGA game. It's not a bug with ScummVM.

comment:5 by fingolfin, 16 years ago

Priority: normallow
Summary: INDY3: Minor Graphics GlitchINDY3: Minor Graphics Glitch (script/data bug)

comment:6 by sev-, 15 years ago

This bugreport has been moved to Wiki to relevant Engine/TODO page. When the bug will be resolved, an appropriate message will be posted here and the bugreport link removed from Wiki

comment:7 by sev-, 15 years ago

Keywords: original added
Status: newclosed

comment:8 by SF/counting_pine, 15 years ago

Hi, I thought it might be an interesting exercise to take a look into this... The problem is in the resource file itself - Object 324 in room 23 (visible in room 135). The tapestry has a magenta line 8 pixels high in the bottom left corner.
http://img29.imageshack.us/img29/1920/room23object324x2.png

(I've checked the English and German versions, and the resources are identical.)

The rest of that first column is identical to the wall, so if the first column can be skipped, that would effectively fix the problem. But I don't know whether SCUMM provides that capability, and if it does, whether SCUMMVM can access it from a high enough level.

There might be another possiblity as well: I think that the line's color might be the transparent color, so maybe it's possible this could be fixed by changing the way that strip is rendered. Unfortunately, it doesn't look like the strip drawing code gets to see the object ID, so that's probably not really going to be feasible. But maybe a simple resource patch can be done, I don't know ScummVM well enough to know whether resource patches are simple/acceptable, and I don't know the object format well enough to know if a simple patch is possible.

But in any case I guess the fix would have to be really trivial to be worth considering. Maybe on this info you can decide whether to fix or reject this bug report.

by dwatteau, 2 years ago

Attachment: room23object324x2.png added

Restored room23object324x2.png screenshot from an old backup of mine; original imageshack resource is gone

comment:9 by dwatteau, 2 years ago

Status: closednew

Reopening, since this glitch is still there. An easy way to see it is just to room 135. (Looking at some online videos, it looks like the FM-Towns version has this too, but I don't own that one.)

It looks like the bogus data is in this part of OI 324 (in its first 'strip').

0xe2 [0x05] 0x80 0x23 0xfc 0xe2 0x15 0xda

and Gdi::unkDecode11() is what handles this format.

The problem I see is that this is a compressed format, so patching this resource is a bit tricky. As far as I can say, there isn't any "invisible" pixel in the palette for objects in v3 games, so replacing that 0x05 color with something else is always going to give suboptimal values. Trimming the leftmost pixel column of that object could work too, but I don't know how to do this.

Good old BMRP.exe lets me rewrite this object (for a similar effect, change E2 05 80 23 FC E2 15 to E2 08 AF E0 C6 47 B8 F1 11 FC E2 15 in this OI, and then recompute the various offsets at the start, the object size, the room size…), but live-patching this inside ScummVM looks a bit complex, since the size changes and since it looks like ScummVM just deals with the object resources from their original offsets. So, I couldn't make a simple patch for this, myself.

A simpler hack consists in replacing that pink color, in that room (it looks unused anyway) with the grey color from the wall. The glitch is still there, but it's much less noticeable:

diff --git a/engines/scumm/room.cpp b/engines/scumm/room.cpp
index 406b270644d..c50d584087c 100644
--- a/engines/scumm/room.cpp
+++ b/engines/scumm/room.cpp
@@ -107,6 +107,11 @@ void ScummEngine::startScene(int room, Actor *a, int objectNr) {
 			if (_shadowPalette)
 				_shadowPalette[i] = i;
 		}
+
+		// XXX
+		if (_game.id == GID_INDY3 && _game.platform == Common::kPlatformDOS && (_game.features & GF_OLD256) && room == 135)
+			_roomPalette[5] = 8;
+
 		if (_game.features & GF_SMALL_HEADER)
 			setDirtyColors(0, 255);
 	}

but I don't think that it's a good-enough hack, unless someone really wants that for now.

by dwatteau, 2 years ago

original pink line in the middle of room 135

by dwatteau, 2 years ago

grey hack hiding the line a bit

comment:10 by dwatteau, 2 years ago

If I apply the following diff to Gdi::drawStrip()

diff --git a/engines/scumm/gfx.cpp b/engines/scumm/gfx.cpp
index d033d1f3d5f..501bc7eb1f2 100644
--- a/engines/scumm/gfx.cpp
+++ b/engines/scumm/gfx.cpp
@@ -1979,4 +1979,16 @@ bool Gdi::drawStrip(byte *dstPtr, VirtScreen *vs, int x, int y, const int width,
 	}
 
+	// WORKAROUND bug #3208: in all 256-color versions of Indy3, the tapestry in
+	// one of the first rooms of Castle Brunwald has a strange vertical purple line
+	// at the bottom of its first 'strip'. We work around this by not printing the
+	// last pixels of that strip, since fortunately the wall behind this tapestry
+	// is already fully drawn behind this object.
+	if (_vm->_game.id == GID_INDY3 && (_vm->_game.features & GF_OLD256) && _vm->_currentRoom == 135
+		&& vs->number == kMainVirtScreen && smapLen == 6146 && width == 104 && height == 104 && stripnr == 0
+		&& x == 24 && y == 0 && offset == 0x38 /*&& _vm->_enableEnhancements*/) {
+		// XXX: is `height - 10` really safe for unkDecode11()??
+		return decompressBitmap(dstPtr, vs->pitch, smap_ptr + offset, height - 10);
+	}
+
 	return decompressBitmap(dstPtr, vs->pitch, smap_ptr + offset, height);
 }

then I can hide the bottom pixels of this strip, but I'm really unsure about cheating on the height value given to unkDecode11(). For example, if I use height - 8 instead, then the room sometimes glitch a bit, or a lot, or then I trigger some variable (writing) assertions when repeatedly leaving/entering the room. I'm probably corrupting something, so this approach looks too risky.

comment:11 by dwatteau, 19 months ago

Well, the reward of patience is probably more SCUMM enhancements.

I've finally found something that looks reliable-enough to be submitted as an official PR:
https://github.com/scummvm/scummvm/pull/4293

Basically, I'm just patching the bytes of the faulty pixels with fixed ones made from my BMRP.EXE output. That's quite a low-level trick, but I think it should be safe.

This issue can finally be closed if this PR is accepted!

comment:12 by dwatteau, 19 months ago

We now have a workaround for this original glitch :) It will be part of the next ScummVM 2.7.0 release.

comment:13 by dwatteau, 19 months ago

Owner: set to dwatteau
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.