From: Daniel Stenberg <daniel_at_rockbox.org>
Date: Thu, 9 Jun 2005 20:07:59 +0200 (CEST)

On Thu, 9 Jun 2005, Tomas wrote:

> Please comment... and test... suggestions are welcome.

Nice work Tomas. I've only read the code, not tested it. Five things in random
order:

1) I suggest you group the data for images within a single struct instead of
using multiple arrays.

Currently you use:

   int img_x[MAX_IMAGES];
   int img_y[MAX_IMAGES];
   ...

While I instead prefer:

   struct image {
     int x;
     int y;
   };

   struct image images[MAX_IMAGES];

2) strncpy(imgtmp, fmt+1, 1);
    n = atoi(imgtmp); /* get image number */

    ... could possibly be replaced to not use the copy and do:

    n=isdigit((int)fmt[1])?fmt[1]-'0':0;

3) I don't particularly like the fixed image names, but I can live with them.

    I would however prefer to have them loaded from a subdir in .rockbox, not
    from the plain .rockbox dir.

4) ret = read_bmp_file(imgtmp, &img_w[n], &img_h[n], img_buf, img_buf_free);

    ... this code seems to load every image to the same address 'img_buf' so
    I don't see how this can work for multiple images.

5) I have an idea regarding how to deal with multiple images and buffer use.

    Read the image into a temporary buffer, then OR the image into a full
    LCD-size buffer (on the given coordinates). You can then do _any_ amount of
    pictures and they are all just added into the same memory buffer.

    Then you have one single background-image to deal with in the WPS code.

-- 
  Daniel Stenberg -- http://www.rockbox.org/ -- http://daniel.haxx.se/
_______________________________________________
http://cool.haxx.se/mailman/listinfo/rockbox

Received on 2005-06-09