Opened 14 years ago

Closed 14 years ago

Last modified 11 months ago

#2052 closed defect (fixed)

ALL: Failed assertion in AdvMame scaler

Reported by: eriktorbjorn Owned by: sev-
Priority: normal Component: Graphics: Scalers
Keywords: Cc:
Game:

Description

Latest ScummVM CVS snapshot

Twice in the last two days, ScummVM has crashed on the
following assertion:

Failed assertion in common/scaler/scalebit.cpp

scummvm: common/scaler/scalebit.cpp:118: void
scale2x(void*, unsigned int, const void*, unsigned int,
unsigned int, unsigned int, unsigned int): Assertion
'height >= 2' failed.

The first time it happened was with Sam & Max, I think.
The second time was with Maniac Mansion.

As far as I could tell, it happened during a normal
screen update. The simplest explanation, then, would be
that it was trying to scale a rectangle that was either
0 or 1 pixels tall.

The addDirtyRect() function is called from:

* addDirtyRgnAuto()
* copyRectToOverlay()
* copyRectToScreen()
* drawMouse()
* undrawMouse()

It can't have been addDirtyRgnAuto(). As far as I know
only the Simon games use it, and the smallest region it
ever dirties is 8x8.

It can't have been copyRectToOverlay(). It was during
normal gameplay. And if it was, that function behaves
pretty much like copyRectToScreen() when it comes to
screen dirtying.

I don't think it could have been copyRectToScreen().
The smallest rectangle it ever dirties is 1x1, and
since addDirtyRect() extends the dirty region by one
pixel in each direction (unless mouseRect is true),
even after re-clipping the rectangle would be at least
2x2 pixels.

It's tempting to blame drawMouse() or undrawMouse(),
but I just don't know.

Ticket imported from: #1210836. Ticket imported from: bugs/2052.

Attachments (1)

sdl_1210836.diff (1.4 KB ) - added by cyxx 14 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by fingolfin, 14 years ago

That would be quite possible, yeah...

comment:2 by fingolfin, 14 years ago

Owner: set to sev-

comment:3 by sev-, 14 years ago

Any more xact steps on how to reproduce it?

comment:4 by eriktorbjorn, 14 years ago

Unfortunately not. The steps I made can only be described as
"play Sam & Max for a few hours" and "play Maniac Mansion
for a few hours". Which I realize isn't very helpful.

comment:5 by eriktorbjorn, 14 years ago

I finally found a way to reproduce this bug. (Thought I had
forgotten all about it, did you? ;-)

* Run a 320x200 SCUMM game with AdvMame2x and no aspect
ratio correction.
* Open the Load Game dialog.
* Move the cursor so that _mouseBackup.y = 200. This step
is, of course, almost impossible without adding debugging
messages to the code.
* Keeping that Y coordinate, load a savegame.

When the game is restored, the mouse is "warped" to the
saved mouse position (to ensure that the cursor image is
still correct, as I recall it). This will cause
undrawMouse() to be called, which causes addDirtyRect() to
be called with y = 200. Since 'height' is 200, h will be
clipped to 1. And this is what eventually causes AdvMame to
crash.

To be honest, I don't remember if this could be what I was
doing when I first encountered the crash. My mind always
goes wonderfully blank when ScummVM crashes on me out of the
blue. I usually don't recover until I've managed to
reproduce the crash two or three times.

by cyxx, 14 years ago

Attachment: sdl_1210836.diff added

comment:6 by cyxx, 14 years ago

I must admit that some parts of the code in the SDL backend
are not easy to follow, that's why I haven't committed this
directly.

What my patch does is basically to test if the y cursor
coordinate is more or *equal* to the screenHeight. There
shouldn't be any need to add a dirty rect with y equals to
200 when the screen height is 200 too, right ? At least,
drawMouse() in that case, just returns...

comment:7 by sev-, 14 years ago

Resolution: fixed
Status: newclosed

comment:8 by sev-, 14 years ago

Commited. Thanks for the fixing.

comment:9 by digitall, 11 months ago

Component: --Unset--Graphics: Scalers
Note: See TracTickets for help on using tickets.