Hello!

I was investigating the scrolling bug in bk's multifont patch and realized that there are 5 copies of identical horizontal scrolling code in the LCD drivers.

So instead of looking for the bug I started out with a patch that removes the duplication: http://www.rockbox.org/bugs/task/4817

Left to fix is the H100 remote driver which can benefit from the common code as well. The problem is that since there are 2 displays, the scrolling code needs access to the appropriate screen struct.

There are 3 obvious solutions:

1. Ignore it and let the duplicate code stay.

2. Add a bunch of ugly wrapper functions to screen_access.c that pass on the
   correct struct, something like:

   puts_scroll_lcd(int x, int y, const unsigned char *string)
   {
       lcd_puts_scroll(x, y, string, &(screen [0]))
   }

   puts_scroll_remote(int x, int y, const unsigned char *string)
   {
       lcd_puts_scroll(x, y, string, &(screen [1]))
   }

   And let the function pointers in the struct point to the wrapper instead of
   the driver function.

3. Change the API and require all callers to pass in the screen struct.
   A big piece of surgery but probably the best option in the long run.
   This also allows the scrolling code to move into apps/gui where I think
   it better belongs.


What you guys think? Should I pursue this further?

Btw, bk's scrolling bug happens because scroll_thread() doesn't have a clue about which font to use, and sooner or later some other thread will change the current font.


Regards,
Fredrik

--
   "40 million, son. You have any idea how many patients
   I had to ignore to get that high score? People died."     --Dr. Kelso

Fredrik Öhrn                               Chalmers University of Technology
[EMAIL PROTECTED]                                                  Sweden

Reply via email to