From: Tomas <tomas_at_salfischberger.nl>
Date: Thu, 09 Jun 2005 20:31:24 +0200

Hi,

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

Thanks

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

Great, I'll do

> 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;

Can you explain this one?

> 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.

Ok, I have to stop using the fixed rockbox dir anyway, so I'll change
both at once.

> 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.

Thanks for finding bug 2 on my list :D
that img_buf should have been img_buf_ptr

> 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.

Hm... this can be a way of doing it... but I was still in doubt for
setting the clear option on painting or not... and using this way will
make that impossible..
And I think I'll have to duplicate some lcd_bitmap code then... wich is
not really what we want? Because that has to be re-done for grayscale then.
I'll add this to the end of my todo list, as it sounds nice... but has
some unfinished things to look at.

Tomas
_______________________________________________
http://cool.haxx.se/mailman/listinfo/rockbox
Received on 2005-06-09