The Dreaded Rosemary Bug

The Rosemary Bug, as we call it, is the first bug I’ve run into in ScummVM.

That means – after 10 years I found a bug in an unsupported, unstable, unreleased (in builds) piece of code, which must mean something about the QA at ScummVM.

It caused the main character to appear “jigsawed” when looking east AND you crossed her with the mouse pointer.
Like this:

Not pretty, huh?

This is due to the dirty rect system.
Basically, the idea is that normally you don’t need to redraw ALL of the rectangle containing the sprite, but just the little square where the cursor was and is no more.
This applies not only to the cursor, of course, but to ANY need that may arise to redraw part of a sprite that’s been obscured by something smaller without redrawing the rest, which stays unchanged.

E.g. if your game had a lunar eclipse, when it’s over you would need to redraw the moon, not the whole sky.

Now, there is a problem.

Consider this example:

1) Our background sprite, painted in our viewport:

2) An eclipse! Another sprite (the black disc) is drawn on top of the first one.

3) For the eclipse to be over, we only need redraw the bit “ruined” by the black disc; everything else is *already* on the viewport.
We take the area that corresponds to the pink square from the original sprite and paint it over:

4) All is fine!

Everything is okay, but what if a sprite is mirrored – that is , it has a bit set that tells the engine “always draw this flipped”?
Look what happens if we don’t take special measures:

1) We draw the original texture FLIPPED.

2)

3) Now, as before, we take the area that corresponds to the pink square from the original texture (which we had drawn flipped before)

4)

Uh, oh.

Mike Oldfield is not happy.

We need to “mirror” the offsets too for dirty rects to work with mirrored sprites, of course.

That’s what was happening with Rosemary, in transparent_surface.cpp:

Common::Rect TransparentSurface::blit(Graphics::Surface &target, int posX, int posY, int flipping, Common::Rect *pPartRect, uint color, int width, int height) {
    int ca = (color >> 24) & 0xff;
 
    Common::Rect retSize;
    retSize.top = 0;
    retSize.left = 0;
    retSize.setWidth(0);
    retSize.setHeight(0);
    // Check if we need to draw anything at all
    if (ca == 0)
        return retSize;
 
    int cr = (color >> 16) & 0xff;
    int cg = (color >> 8) & 0xff;
    int cb = (color >> 0) & 0xff;
 
    // Compensate for transparency. Since we're coming
    // down to 255 alpha, we just compensate for the colors here
    if (ca != 255) {
        cr = cr * ca >> 8;
        cg = cg * ca >> 8;
        cb = cb * ca >> 8;
    }
 
    // Create an encapsulating surface for the data
    TransparentSurface srcImage(*this, false);
    // TODO: Is the data really in the screen format?
    if (format.bytesPerPixel != 4) {
        warning("TransparentSurface can only blit 32 bpp images");
        return retSize;
    }
 
    if (pPartRect) {
        srcImage.pixels = getBasePtr(pPartRect->left, pPartRect->top);
        srcImage.w = pPartRect->width();
        srcImage.h = pPartRect->height();
    }
 
    if (width == -1)
        width = srcImage.w;
    if (height == -1)
        height = srcImage.h;
 
    Graphics::Surface *img = nullptr;
    Graphics::Surface *imgScaled = nullptr;
    byte *savedPixels = nullptr;
        /*
         * et cetera
         * ...
         */
    }
}

This is how it’s been fixed:

if (pPartRect) {
 
    int xOffset = pPartRect->left;
    int yOffset = pPartRect->top;
 
    if (flipping & FLIP_V) {
        yOffset = srcImage.h - pPartRect->bottom;
    }
 
    if (flipping & FLIP_H) {
        xOffset = srcImage.w - pPartRect->right;
    }
 
    srcImage.pixels = getBasePtr(xOffset, yOffset);
    srcImage.w = pPartRect->width();
    srcImage.h = pPartRect->height();
}

You can now move the cursor over Rosemary all you want – it’s not gonna break ?