Opened 16 years ago

Closed 14 years ago

Last modified 21 months ago

#1790 closed defect (fixed)

SCUMM/SMUSH: Ugly palette change when video finishes

Reported by: fingolfin Owned by: cyxx
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: The Dig

Description

Issue occurs in latest CVS and with 0.6.x, but not with 0.5.x.

When a SMUSH video finishes, the palette is changed, from the video palette to the regular in-game palette. The problem is that a redraw occurs at the wrong time (i.e. when fadeIn is called) so some graphics are displayed using the wrong palette, resulting in a very ugly "flash".

A good way to reproduce this is the end of the intro video in The Dig. Or also in the Dig, the video you get when using the "elevator" going down.

Ticket imported from: #1035739. Ticket imported from: bugs/1790.

Attachments (7)

dig.s34 (244.5 KB ) - added by fingolfin 16 years ago.
In front of alien elevator
indy3.png (50.8 KB ) - added by Kirben 16 years ago.
indy3 example
loom.png (12.2 KB ) - added by Kirben 16 years ago.
Loom example
1035739.diff (2.8 KB ) - added by cyxx 14 years ago.
Possible fix for TheDig (SVN Jun 2nd, 2006)
1035739-2.diff (4.1 KB ) - added by cyxx 14 years ago.
Possible fix for TheDig and FT (v2)
tentacle.s03 (26.9 KB ) - added by Kirben 14 years ago.
Wash the carriage to trigger storm
var_fade_delay.diff (1.8 KB ) - added by cyxx 14 years ago.
VAR_FADE_DELAY patch

Download all attachments as: .zip

Change History (36)

comment:1 by fingolfin, 16 years ago

The attached savegame nicely demonstrates the problem at the elevator: just click on the stone slab to trigger the cutscene, and wait until its end. In 0.5.0, we get a smooth (palette wise, at least) transition from movie to game graphics, while with latest CVS (and 0.6.x) you get an ugly palette change effect.

by fingolfin, 16 years ago

Attachment: dig.s34 added

In front of alien elevator

by Kirben, 16 years ago

Attachment: indy3.png added

indy3 example

comment:2 by Kirben, 16 years ago

Could this be the same palette issue causing glitches during some fades in indy3/loom too ? see attached screenshots.

by Kirben, 16 years ago

Attachment: loom.png added

Loom example

comment:3 by eriktorbjorn, 16 years ago

Palette issues during this kind of transition effects are pretty much unavoidable, aren't they? I mean, we're displaying parts of two different rooms at once. If they use different palettes one of them will look wrong. I remember playing through EGA Loom with DOSbox to compare another issue with the Rusty/dragon scene, and even with the original interpreter Rusty the Grey briefly turns into Rusty of Many Colours.

In The Dig case, though, my impression was that there wasn't any transition effect at all. It just decided to change the palette at the wrong moment.

comment:4 by Kirben, 16 years ago

Your right, I double checked and those glitches in indy3 and loom occur in the original games too. I'm surprised they left glitches like that in the games.

The palette glitches after smush playback are ScummVM specific though, I compared palette glitches at start of Full Throttle when skipping smush scenes and they don't occur in the original game.

comment:5 by fingolfin, 16 years ago

It would be intersting to investigate.. * whether The Dig / COMI ever use "fade effects" anywhere * if they do so, which ones do they use, and where? * maybe we can analyze / RE their counterparts to fadeIn and/or the SO_ROOM_FADE code

comment:6 by fingolfin, 16 years ago

I put a workaround for this into CVS (simply suppress fade effects directly after a video played).

It would still be very nice to learn how the original engine behaved in the elevator scene. It would also be nice to learn about the camera position at the end of the cutscene video; the camera "jumps" in ScummVM, maybe the original didn't show this ugliness, but i dunno what it did instead...

comment:7 by fingolfin, 16 years ago

Another thing which is apparent here is that the original engine spaced characterd in this scene wider than we do. I have no idea what might control that, though.

comment:8 by fingolfin, 16 years ago

(ignore my last comment from 2004-10-09, it was meant for another tracker item)

comment:9 by eriktorbjorn, 15 years ago

There are also some ugly palette glitches in Full Throttle when two SMUSH movies are played after each other. E.g. at the end of a "getting a new weapon" movie at the old mine road.

At least, that happened to me several times when using a boot param to test the scene while trying to understand the "invalid seek" bug.

comment:10 by sev-, 15 years ago

FT INSANE sequences behave exactly like in original. Same palette glitches appear in original engine. It just loads palette before SMUSH playback begins. Possible solution would be to postpone palette change and do it at the same time as first frame gets displayed.

comment:11 by fingolfin, 15 years ago

I haven't tested this in FT at all, actually, I am mostly talking about The Dig (and as I stated, this used to work fine; I tried to track down what changed by stepping through the code, but failed to find anything).

comment:12 by Kirben, 15 years ago

scumm/gfx.cpp revision 2.257, was when the palette glitch when skipping the introduction of the Dig starts to occur. But it just looks like the palette glitch was hidden in the past, since the fade effect was not shown at all before that change.

comment:13 by fingolfin, 14 years ago

Summary: SMUSH: Ugly palette change when video finishesSCUMM/SMUSH: Ugly palette change when video finishes

comment:14 by sev-, 14 years ago

This doesn't happen for me in SVN, at least in the crypt. Can anyone second that?

comment:15 by sev-, 14 years ago

Status: newpending

comment:16 by Kirben, 14 years ago

Status: pendingnew

comment:17 by Kirben, 14 years ago

The palette glitches when skipping the introduction of Full Throttle or The Dig still occur.

comment:18 by fingolfin, 14 years ago

Status: newpending

comment:19 by fingolfin, 14 years ago

I can still trivially reproduce the issue: Just start The Dig in ScummVM, and skip over the intro video by pressing ESC. The fade triggered by this shows the problem quite nicely.

comment:20 by Kirben, 14 years ago

Status: pendingnew

comment:21 by cyxx, 14 years ago

A possible patch to fix the glitches in TheDig.

The original interpreter has a special flag for that, fadeIn effects are simply disabled after a SMUSH movie (good catch, Max). As the doEffect flag can be reinitialised elsewhere in the code, I think it's better to add a special flag for that (and remove the 'hack' in SmushPlayer::release()).

Adding the extra flag doesn't fix all the problems (in the first cutscene at least). We should also disable/ignore the _screenEffectFlag. I suppose this was added to "guarantee" the fadeOut and fadeIn calls are sequential (in case of scripts errors), but if we check this flag, then the screen doesn't always get cleared and this lead to the glitch we observe in the first cutscene.

This looks like a big hack, but considering how the original scripts are written, I think it's the easiest solution.

FT seems to handle things yet differently.

by cyxx, 14 years ago

Attachment: 1035739.diff added

Possible fix for TheDig (SVN Jun 2nd, 2006)

comment:22 by cyxx, 14 years ago

Updated patch which should fix some FT glitches.

Original v7/v8 interpreters do update the palette before drawing the dirty blocks of the screen. This fixes the remaining glitches of FT and allow me to cleanup a bit my previous patch.

Also introduced VAR_FADE_DELAY in transitionEffect() (to match the original interpreters, again).

I haven't tested this extensively, so I wouldn't be surprised it introduces regressions.

by cyxx, 14 years ago

Attachment: 1035739-2.diff added

Possible fix for TheDig and FT (v2)

comment:23 by Kirben, 14 years ago

A few notes about VAR_FADE_DELAY: VAR_FADE_DELAY is used as a timer for several other screen effects in earlier games too.

As far as I know VAR_FADE_DELAY is only changed in script 10 of DOTT, which is used when storm is trigger in game. I have attached a saved game.

by Kirben, 14 years ago

Attachment: tentacle.s03 added

Wash the carriage to trigger storm

comment:24 by sev-, 14 years ago

the patch looks ok to me and several tests show no problem. However I thinks i's too late for 0.9.0. May be commit it to trunk?

comment:25 by Kirben, 14 years ago

I added patch to main branch of ScummVM SVN.

VAR_FADE_DELAY should also be used as a timer in scrollEffect() too. But I'm not sure on the timer calculation, compared to ScummVM.

comment:26 by Kirben, 14 years ago

Owner: set to cyxx
Resolution: fixed
Status: newclosed

comment:27 by cyxx, 14 years ago

Looking at the disassembly of monkey2, the timing is based on the _scummTimer variable which is incremented in the dos interrupt 8 handler. All of the screen effects functions just busy wait like this :

_scummTimerOld = _scummTimer // process the effect on the current frame while (_scummTimer < _scummTimerOld + VAR(VAR_FADE_DELAY))

In the sound driver code, the counter 0 of the 8253 chip is initialised with a value of 5041. Its internal clock rate being equal to 1193180 Hz, _scummTimer is incremented 236 times a second. Which is almost equivalent to 1/4th of a scumm engine jiffie.

So, in order to match the original interpreters, I think we simply need to multiply the VAR_FADE_DELAY value by 4 (1000/240) to get the waiting delay.

Patch attached.

by cyxx, 14 years ago

Attachment: var_fade_delay.diff added

VAR_FADE_DELAY patch

comment:28 by Kirben, 14 years ago

Good work, I added the patch to ScummVM SVN.

comment:29 by digitall, 21 months ago

Component: --Unset--Engine: SCUMM
Game: The Dig
Note: See TracTickets for help on using tickets.