#2729 closed defect (fixed)
DOTT: Graphic glitches while kite comes down
| Reported by: | lordhoto | Owned by: | eriktorbjorn |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SCUMM |
| Version: | Keywords: | ||
| Cc: | Game: | Day of the Tentacle |
Description
I'm using a svn trunk version from today (08072006) on Linux/amd64. When the kite got hit by the thunderbolt and it comes down the tail (the red thing) leaves some red pixels which shouldn't be there I suppose (at least it looks wrong). I attached a savegame (from the german CD talkie version). Just push the kite in the right moment as said. And you'll se the graphics glitch.
Ticket imported from: #1519667. Ticket imported from: bugs/2729.
Attachments (3)
Change History (12)
by , 18 years ago
comment:1 by , 18 years ago
I don't have the German version, so I've been trying on an old savegame of mine. A couple of observations:
The glitch happens for me in ScummVM 0.8.2 as well, so it's apparently not a recent regression.
The kite is actor 7. The glitch happens while drawing a scaled sprite for it.
From what I understand, ClassicCostumeRenderer::mainRoutine() calculates the height of the sprite, and these values are later used by ScummEngine::resetActorBgs(). However, in some cases it appears that proc3() draws a sprite that is slightly taller than the calculated height.
comment:2 by , 18 years ago
I see what's happening, though I'm not entirely certain what the correct behaviour is.
When calculating the height of a scaled sprite, we do this:
for (i = 0; i < _height; i++) { if (v1.scaletable[_scaleIndexY++] < _scaleY) rect.bottom++; }
When drawing it, we do... well, we do quite a lot, but if we simplify it a lot, it would look somewhat like this:
scaleytab = &v1.scaletable[_scaleIndexY]; for (i = 0; i < _height; i++) { if (*scaleytab++ < _scaleY) y++; }
In both cases, _scaleIndexY start out as the same value. The two code fragments look deceptively simple. But in this case, _scaleIndexY overflows and wraps around to zero in some cases when calculating the height. That does not, of course, happen when drawing.
Question is, should we prevent wrap-around when calculating the height, or should we allow wrap-around when drawing? Either would fix the problem, I guess.
I also guess that we have the same potential problem in the AKOS renderer. At least in code1_genericDecode().
comment:3 by , 18 years ago
Bah, I meant of course that the two code fragments look deceptively *similar*, not simple.
comment:4 by , 18 years ago
Looking a bit closer at it, it seems the overflow won't happen in AKOS since startScaleIndex is an int, not a byte.
I've attached a patch that introduces the same overflow/wrap-around behaviour in proc3() as in mainRoutine(), and it does fix the glitch. But is it correct to use the scale table that way?
Assigning to Fingolfin, since he claims to understand the scale table.
comment:5 by , 18 years ago
| Owner: | set to |
|---|
comment:6 by , 18 years ago
The costume scale table is accessed via byte sized variables in the code of original game, so would overflow in this situation too.
So the patch to allow overflow/wrap-around, by using a byte sized variable sounds fine.
comment:7 by , 18 years ago
Thanks. I've applied my patch, after adding a comment about the possibility of the scale index wrapping around.
comment:8 by , 18 years ago
| Owner: | changed from to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:9 by , 18 years ago
Only noticed this report today. But yeah, the patch is perfectly valid, even from a logical point of view. The problem more or less stems from the fact that AKOS uses two different kinds of scale tables: An old one, identical to the table used in costume.cpp, and a newer one which is bigger (768 bytes IIRC), and for which we must *not* do wrapping. I am not sure if this was ever implemented quite correctly, or if maybe somebody (most likely I :-) broke it during on of the costume code cleanups years ago...
save game to reproduce the glitch