Opened 15 years ago

Closed 8 months ago

#4535 closed defect (fixed)

SCUMM: MI (Amiga) - Voodoo lady palette glitches

Reported by: SF/quietust Owned by: athrxx
Priority: normal Component: Engine: SCUMM
Version: Keywords:
Cc: Game: Monkey Island 1

Description

Several spots in the game exhibit significant palette glitches, possibly related.

1. When entering the forest maze, the yellow flowers aren't yellow, but are being colored red instead. 2. When talking to the voodoo lady, the entire screen flashes blue in a rather glitchy way when the boiling pot rises out of the floor or retracts back in.

Ticket imported from: #2838205. Ticket imported from: bugs/4535.

Change History (20)

comment:1 by SF/quietust, 15 years ago

Also, this is happening with latest code from SVN.

comment:2 by sev-, 15 years ago

could you, please, try GSoC Amiga modules branch? There were palette-related fixes. The code is in branches/gsoc2009-mods

comment:3 by SF/quietust, 15 years ago

That's actually where I first noticed the problem (so nice to finally have music and sound effects in the game), and I double-checked with the base version to verify that it also happened there - a quick recheck indicates that the latest svn for gsoc2009-mods also exhibits the problem.

comment:4 by SF/quietust, 15 years ago

Further inspection reveals that this apparently stopped working between the 0.7.1 release and 0.8.0. Best I can tell, o5_roomOps() case 2 (SO_ROOM_COLOR) seems to not be working as it should (I suspect _paletteMod is confusing things).

comment:5 by lordhoto, 15 years ago

I guess it must be some regression introduced in one of those functions using "_roomPalette" in gfx.cpp.

It seems 0.7.1 still had this code in the Gdi constructor:

_roomPalette = vm->_roomPalette; if ((vm->_features & GF_AMIGA) && (vm->_version >= 4)) _roomPalette += 16;

While 0.8.0 started to use just:

_roomPalette = vm->_roomPalette;

To compensate that it seems there was "_paletteMod" introduced, which is set in "Gdi::decompressBitmap". It seems some of the functions called from Gdi::decompressBitmap were changed to use "_paletteMod" in their "_roomPalette" access.

Here's an example of drawStripEGA:

0.7.1:

*(dst + y * dstPitch + x) = (z & 1) ? _roomPalette[color & 0xf] : _roomPalette[color >> 4];

0.8.0:

*(dst + y * dstPitch + x) = (z & 1) ? _roomPalette[color & 0xf] + _paletteMod : _roomPalette[color >> 4] + _paletteMod;

I have to say that looks strange. For example why isn't the "+ _paletteMod" after the indexed access to _roomPalette and not added to the index, which looks like what the 0.7.1 code did?

Maybe there is a good explanation for this, I don't know whether the palette handling itself changed during 0.7.1 to 0.8.0 changes. But the above mentioned looks odd.

Also maybe some other functions called from "Gdi::decompressBitmap" need adaption to take "_paletteMod" into calculation, it looks to me like not every function was changed. Those functions are:

unkDecode8 unkDecode9 unkDecode10 unkDecode11 drawStrip3DO drawStripHE

Judging by the names the latter 3 should not be used in MI1 Amiga though. The former four seem to be only used by FM-Towns games, according to the comments. So probably they do not need to be changed.

There's another function accessing _roomPalette and not taking _paletteMod into calculation:

drawStripC64Background

Maybe that's just fine though, since it seems for C64 "_paletteMod" is always 0 :-).

Anyway I guess the problem lies in the above mentioned change in "drawStripEGA" (and maybe the other functions changed in a similar way). I do not have MI 1 Amiga to test that though, so it would be nice if someone with that game version could look into it. Personally I would for example just change the above mentioned line in drawStripEGA to:

*(dst + y * dstPitch + x) = (z & 1) ? _roomPalette[(color & 0xf) + _paletteMod] : _roomPalette[(color >> 4) + _paletteMod];

And of course all other places doing the same addition of _paletteMod after _roomPalette index look up :-). Maybe that fixes the problem or it will just look even more incorrect and thus the problem is caused by another change...

comment:6 by fingolfin, 15 years ago

Your analysis is 100% correct. In particular, in the old code base, the _palette_mod was indeed added inside the array access, which is the only logical way, too ;). See also <http://scummvm.svn.sourceforge.net/viewvc/scummvm/scummvm/trunk/engines/scumm/gfx.cpp?r1=10776&r2=10778>

So please go ahead and fix it ;).

comment:7 by fingolfin, 15 years ago

Owner: set to lordhoto

comment:8 by lordhoto, 15 years ago

I did commit it to trunk now, would be still nice if someone could verify it and when it's working just backport it to branch maybe.

comment:9 by lordhoto, 15 years ago

Talking with Kirben showed that my simple patch did cause a regression in FOA/Amiga. I also fixed that and backported it to 1.0.0.

Kirben also pointed out that the voodoo lady issue is still present (as it was in 0.7.1 according to him).

I'll leave this bug report open till that is fixed. I'm also reassigning the report to "None", since I do not have any way to fix that issue :-).

comment:10 by lordhoto, 15 years ago

Owner: lordhoto removed

comment:11 by Kirben, 15 years ago

There were no effects when the Voodoo lady rises and retracts from the floor, in the original Amiga version. So it looks like some effects were disabled in the Amiga version, and need to be disabled under ScummVM too.

I noticed that roomOps case 4 is been used at these points, so maybe_shadowPalette was not used in Amiga version?

comment:12 by fingolfin, 15 years ago

Summary: MONKEY: Palette glitches in Amiga versionMONKEY: Voodoo lady palette glitches in Amiga version

comment:13 by fingolfin, 15 years ago

Possible. Only way to know for sure is to look at the disassembly, I guess. But in the meantime, you could try modifying / disabling roomOps case 4, resp. shadowPalette handling, to see if that resolve the issues. If it does, then we'd also have to test the rest of the game for regressions, though :)

comment:14 by fingolfin, 14 years ago

What is the status of this item?

comment:15 by Kirben, 14 years ago

Unchanged, the problem still occurs with ScummVM SVN.

comment:16 by sev-, 13 years ago

This bug is nice to get fixed before the release. Raising priority for keeping the track.

comment:17 by sev-, 13 years ago

Priority: normalhigh

comment:18 by sev-, 12 years ago

Priority: highnormal

comment:19 by raziel-, 4 years ago

Summary: MONKEY: Voodoo lady palette glitches in Amiga versionSCUMM: MI (Amiga) - Voodoo lady palette glitches

ScummVM 2.2.0git (Jul 15 2020 10:24:49)
Features compiled in: Vorbis FLAC MP3 RGB zLib MPEG2 Theora AAC A/52 FreeType2 FriBiDi JPEG PNG cloud (servers, local)

Confirmed still an issue with above ScummVM version.

The Secret of Monkey Island (Amiga/English)

AmigaOS4 - PPC - BE - SDL

comment:20 by athrxx, 8 months ago

Owner: set to athrxx
Resolution: fixed
Status: newclosed

I have made a fix. The original interpreter ignores the shadow palette.

Note: See TracTickets for help on using tickets.