Opened 15 years ago

Closed 15 years ago

Last modified 21 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 15 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by fingolfin, 15 years ago

That would be quite possible, yeah...

comment:2 by fingolfin, 15 years ago

Owner: set to sev-

comment:3 by sev-, 15 years ago

Any more xact steps on how to reproduce it?

comment:4 by eriktorbjorn, 15 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, 15 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, 15 years ago

Attachment: sdl_1210836.diff added

comment:6 by cyxx, 15 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-, 15 years ago

Resolution: fixed
Status: newclosed

comment:8 by sev-, 15 years ago

Commited. Thanks for the fixing.

comment:9 by digitall, 21 months ago

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