Help with graphics

[PowerWyrm]

When I try to enable graphics, I get the following message: "Cannot read bitmap file '16x16.bmp'".

Error occurs in ReadDIB() when trying to read the bitmap:

if (lpbi->biSizeImage == lread(fh, (LPSTR)lpbi + offBits, lpbi->biSizeImage))... (this condition is false)

Any idea why? It seems that the bitmap file has not the right format... Even using the vanilla graf files doesn't work...

[Flambard]

I take it's from WIN client? Haven't tried that one in a while, but worked last time just fine. Maybe some revision broke it (altho ReadDIB is isolated) .. ?

[PowerWyrm]

WIN client yup. Graphics are broken atm because MAngband has more objects than Vanilla (so the bitmap should be reworked), but that would impact only some of the tiles, not all. The graphics should be loaded just fine.
My idea is that the ReadDIB function doesn't initialize the LPBITMAPINFOHEADER properly: the header (non-bitmap info) is probably not read properly. It's hard to debug with binary files and all those offsets...

[PowerWyrm]

Tracking values for BITMAPFILEHEADER and BITMAPINFOHEADER shows that garbage values are read. I'll need to investigate more...

[PowerWyrm]

A quick example of the problem:

The BITMAPFILEHEADER structure is defined as follows:

Code: Select all

typedef struct tagBITMAPFILEHEADER {
  WORD  bfType;
  DWORD bfSize;
  WORD  bfReserved1;
  WORD  bfReserved2;
  DWORD bfOffBits;
}BITMAPFILEHEADER, *PBITMAPFILEHEADER;

I open the 16x16.bmp file with a HEX editor to see the values for the BITMAPFILEHEADER fields:
HEX values: 424D368407000000000036040000
bfType=0x4D42 ("BM")
bfSize=0x00078436 (492598 bytes)
bfReserved1=0x0000
bfReserved2=0x0000
bfOffBits=0x00000436 (1078 bytes)

All theses values are perfectly correct.

Now I launch the client with Graphics=2 (to activate the 16x16 bitmap file) and track the values in ReadDIB:
bfType=19778 (0x4D42)
bfSize=7 (0x00000007)
bfReserved1=0 (0x0000)
bfReserved2=1078 (0x0436)
bfOffBits=2621440 (0x00280000)

It seems that ReadDIB is reading DWORD value for bfType instead of WORD! All following fields are shifted by 4 bytes...

So this seems to be a problem with structure alignment...

[PowerWyrm]

*** drumroll ***
IT WOOOOOOORKS!!!!

As I said, the BITMAPFILEHEADER contains 3 WORD fields and 2 DWORD fields.
When I print the size of WORD, DWORD and BITMAPFILEHEADER, I obtain the following:
sizeof(WORD) = 2
sizeof(DWORD) = 4
sizeof(BITMAPFILEHEADER) = 16
But 3 WORDS + 2 DWORDS should be size = 14!
There's a problem with structure alignment: all fields seem to be aligned with DWORD. So the the BITMAPFILEHEADER really looks like something like:
WORD bfType;
WORD dummy; (to align the WORD to a DWORD address)
DWORD bfSize;
WORD bfReserved1;
WORD bfReserved2; (WORD + WORD is ok...)
DWORD bfOffBits;

When reading from a binary file, each dummy WORD added for alignment will shift data read by 4 bytes...

So I rewrote ReadDIB to read each field from the structure separately... and to my surprise the graphics were loaded properly this time!!!

[Flambard]

Fantastic! Great investigation :) Patch please?

[PowerWyrm]

Code: Select all

/*
 * Reads a DIB from a file, obtains a handle to its BITMAPINFO struct, and
 * loads the DIB.  Once the DIB is loaded, the function also creates a bitmap
 * and palette out of the DIB for a device-dependent form.
 *
 * Returns TRUE if the DIB is loaded and the bitmap/palette created, in which
 * case, the DIBINIT structure pointed to by pInfo is filled with the appropriate
 * handles, and FALSE if something went wrong.
 */
BOOL ReadDIB(HWND hWnd, LPSTR lpFileName, DIBINIT *pInfo)
{
    HFILE fh;
    LPBITMAPINFOHEADER lpbi;
    OFSTRUCT of;
    BITMAPFILEHEADER bf;
    WORD nNumColors;
    BOOL result = FALSE;
    DWORD offBits, bSize;
    HDC hDC;
    BOOL bCoreHead = FALSE;
    BITMAPCOREHEADER bc;

    /* Open the file and get a handle to its BITMAPINFO */
    fh = OpenFile(lpFileName, &of, OF_READ);
    if (fh == -1) return (FALSE);

    pInfo->hDIB = GlobalAlloc(GHND, (DWORD)(sizeof(BITMAPINFOHEADER) +
        256 * sizeof(RGBQUAD)));

    if (!pInfo->hDIB) return (FALSE);

    lpbi = (LPBITMAPINFOHEADER)GlobalLock(pInfo->hDIB);

    /*
     * Read the BITMAPFILEHEADER (field by field to avoid structure alignment problems)
     *
     * typedef struct tagBITMAPFILEHEADER {
     *   WORD  bfType;
     *   DWORD bfSize;
     *   WORD  bfReserved1;
     *   WORD  bfReserved2;
     *   DWORD bfOffBits;
     * } BITMAPFILEHEADER;
     */
    _LREAD(bf.bfType);
    _LREAD(bf.bfSize);
    _LREAD(bf.bfReserved1);
    _LREAD(bf.bfReserved2);
    _LREAD(bf.bfOffBits);

    /* 'BM' */
    if (bf.bfType != 0x4d42) goto ErrExit;

    /* Read the size of the bitmap */
    _LREAD(bSize);

    /* Set BITMAPINFOHEADER size */
    lpbi->biSize = sizeof(BITMAPINFOHEADER);

    /*
     * Read the BITMAPCOREHEADER (field by field to avoid structure alignment problems)
     *
     * typedef struct tagBITMAPCOREHEADER {
     *   DWORD bcSize;
     *   WORD  bcWidth;
     *   WORD  bcHeight;
     *   WORD  bcPlanes;
     *   WORD  bcBitCount;
     * } BITMAPCOREHEADER;
     */
    if (bSize == sizeof(BITMAPCOREHEADER))
    {
        _LREAD(bc.bcWidth);
        _LREAD(bc.bcHeight);
        _LREAD(bc.bcPlanes);
        _LREAD(bc.bcBitCount);

        /* Set BITMAPINFOHEADER values */
        lpbi->biWidth = bc.bcWidth;
        lpbi->biHeight = bc.bcHeight;
        lpbi->biPlanes = bc.bcPlanes;
        lpbi->biBitCount = bc.bcBitCount;
        bCoreHead = TRUE;
    }

    /*
     * Read the BITMAPINFOHEADER (field by field to avoid structure alignment problems)
     *
     * typedef struct tagBITMAPINFOHEADER {
     *   DWORD biSize;
     *   LONG  biWidth;
     *   LONG  biHeight;
     *   WORD  biPlanes;
     *   WORD  biBitCount;
     *   DWORD biCompression;
     *   DWORD biSizeImage;
     *   LONG  biXPelsPerMeter;
     *   LONG  biYPelsPerMeter;
     *   DWORD biClrUsed;
     *   DWORD biClrImportant;
     * } BITMAPINFOHEADER;
     */
    else if (bSize == sizeof(BITMAPINFOHEADER))
    {
        _LREAD(lpbi->biWidth);
        _LREAD(lpbi->biHeight);
        _LREAD(lpbi->biPlanes);
        _LREAD(lpbi->biBitCount);
        _LREAD(lpbi->biCompression);
        _LREAD(lpbi->biSizeImage);
        _LREAD(lpbi->biXPelsPerMeter);
        _LREAD(lpbi->biYPelsPerMeter);
        _LREAD(lpbi->biClrUsed);
        _LREAD(lpbi->biClrImportant);
    }

    /* Error */
    else goto ErrExit;

    if (!(nNumColors = (WORD)lpbi->biClrUsed))
    {
        /* No color table for 24-bit, default size otherwise */
        if (lpbi->biBitCount != 24) nNumColors = 1 << lpbi->biBitCount;
    }

    /* Fill in some default values if they are zero */
    if (lpbi->biClrUsed == 0) lpbi->biClrUsed = nNumColors;

    if (lpbi->biSizeImage == 0)
        lpbi->biSizeImage = (((((lpbi->biWidth * (DWORD)lpbi->biBitCount) + 31)
            & ~31) >> 3) * lpbi->biHeight);

    /* Otherwise wouldn't work with 16 color bitmaps */
    else if ((nNumColors == 16) && (lpbi->biSizeImage > bf.bfSize))
        lpbi->biSizeImage /= 2;

    /* Get a proper-sized buffer for header, color table and bits */
    GlobalUnlock(pInfo->hDIB);
    pInfo->hDIB = GlobalReAlloc(pInfo->hDIB, lpbi->biSize +
        nNumColors * sizeof(RGBQUAD) + lpbi->biSizeImage, 0);

    /* Can't resize buffer for loading */
    if (!pInfo->hDIB) goto ErrExit2;

    lpbi = (LPBITMAPINFOHEADER)GlobalLock(pInfo->hDIB);

    /* Read the color table */
    if (!bCoreHead)
        _lread(fh, (LPSTR)(lpbi) + lpbi->biSize, nNumColors * sizeof(RGBQUAD));
    else
    {
        signed int i;
        RGBQUAD FAR *pQuad;
        RGBTRIPLE FAR *pTriple;

        _lread(fh, (LPSTR)(lpbi) + lpbi->biSize, nNumColors * sizeof(RGBTRIPLE));

        pQuad = (RGBQUAD FAR *)((LPSTR)lpbi + lpbi->biSize);
        pTriple = (RGBTRIPLE FAR *) pQuad;
        for (i = nNumColors - 1; i >= 0; i--)
        {
            pQuad[i].rgbRed = pTriple[i].rgbtRed;
            pQuad[i].rgbBlue = pTriple[i].rgbtBlue;
            pQuad[i].rgbGreen = pTriple[i].rgbtGreen;
            pQuad[i].rgbReserved = 0;
        }
    }

    /* Offset to the bits from start of DIB header */
    offBits = lpbi->biSize + nNumColors * sizeof(RGBQUAD);

    if (bf.bfOffBits != 0L) _llseek(fh,bf.bfOffBits, SEEK_SET);

    /* Use local version of '_lread()' above */
    if (lpbi->biSizeImage == lread(fh, (LPSTR)lpbi + offBits, lpbi->biSizeImage))
    {
        GlobalUnlock(pInfo->hDIB);

        hDC = GetDC(hWnd);
        if (!MakeBitmapAndPalette(hDC, pInfo->hDIB, &pInfo->hPalette, &pInfo->hBitmap))
        {
            ReleaseDC(hWnd, hDC);
            goto ErrExit2;
        }
        else
        {
            ReleaseDC(hWnd, hDC);
            result = TRUE;
        }
    }
    else
    {
ErrExit:
        GlobalUnlock(pInfo->hDIB);
ErrExit2:
        GlobalFree(pInfo->hDIB);
    }

    _lclose(fh);
    return (result);
}

[PowerWyrm]

Forgot this...

Code: Select all

#define _LREAD(field) \
    if (sizeof(field) != _lread(fh, &field, sizeof(field))) goto ErrExit

[Billsey]

This patch has been implemented in trunk (Ticket #931), we'll do some testing before deciding to implement it in 1.1.3.

[PowerWyrm]

I'm struggling with the graphics atm. I've done massive changes to make stuff work, and still more to come.

Done:
- ReadDIB problem
- reactivated the WinClient menus for graphic options (graphic mode, bigtile mode, tile size increase/decrease...)
- changed process_menus() to send visual preferences to server (without that, nothing happens when you change graphic mode, and you need to relaunch the client)
- added a packet to actually send the visual preferences and update the player on the server with the new attr/char pairs (the update on the server was only done in Enter_player, and it had to be also done when changing graphic mode)

Todo:
- screen doesn't refresh when changing graphic mode (need to do a manual ctrl-R... even if the request for redraw is already done)
- some tiles don't refresh when changing graphic mode (some are probably reset when they should not)
- bigtile mode doesn't work
- remap all graphic attr/char pairs (grafxxx.prf files)
- redraw the missing tiles (or remap them to existing attr/char pairs)

[Billsey]

Keep the patches coming. :-)

It looked to me, at first glance, that the ordering of the graphics in the 32x32 bitmap was completely different than that in the 16x16 bitmap. If so it makes things even more complex, since we have to change the graphic attr/char pairs when a different sized graphic is chosen.

[Emulord]

or you could move the tiles around in the bitmap? Shouldnt the code be as indifferent to bitmap size as possible?

Idk it seems like better style and less likely to have bugs.

[Billsey]

I lean toward taking the existing bitmap (and .png) files from Angband, adding just the minimum to cover our items (mostly vegetables). They already support many items in the files that aren't in Angband but are in variants (for instance, volcano and cactus tiles). I like the idea of using the .png files, since they have transparent backgrounds, so you can place an item on the floor and still see floor around it. It looks like it'd be a lot of work though to full spin up to speed with what they have going.

[PowerWyrm]

It looked to me, at first glance, that the ordering of the graphics in the 32x32 bitmap was completely different than that in the 16x16 bitmap. If so it makes things even more complex, since we have to change the graphic attr/char pairs when a different sized graphic is chosen.

This is done in the grafxxx.prf files. Only these files are impacted. As I said, I'm gonna look into that...