From: Fredrik Öhrn <ohrn_at_chl.chalmers.se>
Date: Sat, 11 Mar 2006 18:40:09 +0100

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
ohrn_at_chl.chalmers.se                                                  Sweden

Received on 2006-03-11