Opened 5 months ago

Closed 5 months ago

Last modified 5 months ago

#15222 closed defect (fixed)

SCI: PEPPER: The carrier pigeon is barely visible when flying to Penn Mansion

Reported by: eriktorbjorn Owned by: sluicebox
Priority: normal Component: Engine: SCI
Version: Keywords:
Cc: Game: Pepper's Adventures in Time

Description

I only have a Windows version of Pepper's Adventures in Time (from Sierra's School House), so this is based on the assumption that the data files are identical to the DOS version, and that https://youtu.be/CpASiXJhrcM?list=PLwprFgobvKXbqEr9DSq0TNSnfT5Tu9YZU&t=144 shows the correct behavior.

When releasing the carrier pigeon in chapter 4, it should be seen flying between the mansions. It's not that visible in the original, since it's just a single pixel that occasionally disappears completely. But it's even less visible in ScummVM, where it's a single pixel that occasionally appears.

Since you can start the game at any chapter, and this happens at the start of chapter 4, it's easy enough to reproduce. But I'll attach my savegame anyway. Open the window and the birdcage. The scene plays out a little different depending on which you do first, but I assume the flight is the same either way.

Attachments (2)

pepper.006 (34.7 KB ) - added by eriktorbjorn 5 months ago.
scummvm-pepper-00020.png (86.3 KB ) - added by eriktorbjorn 5 months ago.

Download all attachments as: .zip

Change History (12)

by eriktorbjorn, 5 months ago

Attachment: pepper.006 added

by eriktorbjorn, 5 months ago

Attachment: scummvm-pepper-00020.png added

comment:1 by eriktorbjorn, 5 months ago

The problem is probably in the way the pigeon graphics are scaled. If I force the graphics to be drawn at their actual size, like this:

diff --git a/engines/sci/graphics/animate.cpp b/engines/sci/graphics/animate.cpp
index b7af374f173..97c1a57a643 100644
--- a/engines/sci/graphics/animate.cpp
+++ b/engines/sci/graphics/animate.cpp
@@ -217,6 +217,7 @@ void GfxAnimate::makeSortedList(List *list) {
                if (getSciVersion() >= SCI_VERSION_1_1) {
                        // Cel scaling
                        listEntry.scaleSignal = readSelectorValue(_s->_segMan, curObject, SELECTOR(scaleSignal));
+                       listEntry.scaleSignal &= ~kScaleSignalDoScaling;
                        if (listEntry.scaleSignal & kScaleSignalDoScaling) {
                                listEntry.scaleX = readSelectorValue(_s->_segMan, curObject, SELECTOR(scaleX));
                                listEntry.scaleY = readSelectorValue(_s->_segMan, curObject, SELECTOR(scaleY));

The bird is drawn the entire flight:


Of course, that does not match the original.

The problem is more likely to be in GfxView::drawScaled(). The bird is often scaled down to 0x2 or even 0x0 pixels.

comment:2 by sluicebox, 5 months ago

It is probably a scaling discrepancy. We had a home-grown scaling algorithm until a few years ago when I replaced it with Sierra's: 4ecfa0c2170d8376fe09bdcdb698fb2ee919430f

It's easy to believe the code has edge cases that haven't been noticed until now.

comment:3 by eriktorbjorn, 5 months ago

Makes sense. Normally I wouldn't quibble about minor pixel differences like this, except in this case it's the only pixel. When I played it, I thought for a moment that the game had hung. (I played it with the sound turned off to not disturb anyone, so if there was a musical cue I missed it.)

I wonder why they didn't just redraw the bird when they decided it was too big. Is that animation used elsewhere?

comment:4 by sluicebox, 5 months ago

I've confirmed that the scene looked right prior to my change in 2020. I don't know if it was pixel perfect or anything, but it drew something instead of nothing. I hope that's good news.

It's a rare scene because it has a one pixel sprite, and the the sprite is produced by scaling, and the color of that one pixel is noticeable. Each of those are independently rare. Great find! It makes sense to me that this scaling edge case hasn't been noticed until now.

I don't think there's anything weird about a programmer adjusting a sprite's scale to taste and shipping that instead of going back and requesting/generating new art, because that's what I did and still got paid.

Version 0, edited 5 months ago by sluicebox (next)

in reply to:  4 comment:5 by eriktorbjorn, 5 months ago

Replying to sluicebox:

I don't think there's anything weird about a programmer adjusting a sprite's scale to taste and shipping that instead of going back and requesting/generating new art, because that's what I did and still got paid.

I guess, but if my taste was what looked to me like a single pixel, I'd probably go "I CAN DRAW THAT!" Though I guess it also depends on how hard Sierra's graphical tools were to use.

comment:6 by eriktorbjorn, 5 months ago

With the old scaling algorithm, the scaled size can be 1x1, 1x2, and sometimes 2x2 pixels. As far as I can tell, the algorithm is basically just:

    scaledWidth = (celInfo->width * scaleX) >> 7;
    scaledHeight = (celInfo->height * scaleY) >> 7;

The smallest celInfo size is 4x4 pixels. The scale factors start at 47, and are gradually decreased to 32. Another sign that originally a larger bird sprite was displayed, perhaps?

So even in the worst case, we have (4 * 32) >> 7 = 1.

With the new scaling algorithm, the scaled size can be 0x0, 0x2, and sometimes 2x2 pixels.

The celInfo size and scale factors are the same, of course. The scaled size is calculated by createScalingTable().

It uses the same formula as the old algorithm:

    const int16 scaledSize = (celSize * scale) >> 7;

But then stepCount is scaledSize - 1, which means it can easily be 0. And then we fall into this case:

	if (stepCount <= 0) {
		table.clear();
		return;
	}

The final scaled size is the size of the scale table, so that's why we get 0 here. But if I just try to initialize table[0] to something, ScummVM crashes. I guess I just haven't managed to wrap my head around how that scale table works...

comment:7 by sluicebox, 5 months ago

I fixed this a while ago but want to test it some more before committing

comment:8 by sluicebox <22204938+sluicebox@…>, 5 months ago

In 3bc4378:

SCI: Fix SCI1.1 scaling inaccuracies

Fixes the pigeon flight in Pepper's Adventures in Time.

The pigeon view is scaled down to one pixel but the scaling code was
incorrectly producing an empty table for certain inputs. Other scaling
inaccuracies were identified and fixed too.

Fixes bug #15222

comment:9 by sluicebox, 5 months ago

Owner: set to sluicebox
Resolution: fixed
Status: newclosed

comment:10 by eriktorbjorn, 5 months ago

Thanks! That's so much better. (Not great, but that's Sierra's own fault. :-)

Note: See TracTickets for help on using tickets.