[320x240] Tile based main menu

[Frankenpod]

@millim

Hey, I found a glitch with the theme - but to be honest, it's the exact same glitch I've been having with my own icon-using themes.  If you go into the files menu, and there are long file names that need to scroll, it causes a problem when you come back out again - see the screenshots.

Either there's something we both get wrong about the logic of viewports, or there's a bug to do with scrolling text that only comes up when one does things with custom %vi statements to use icons.  The fact that there's already a bug with scrolling text, relating to the backgrounds of the SBS and WPS, makes me think it's the latter.

[Frankenpod]

I do suspect there's a bug in rockbox with how scrolling is implemented.  As well as this, I get other strange things happening with scrolling lines if I define a text-based %vi viewport.  And at least at one point there was a known existing bug where scrolling text on the WPS gets the background from the SBS.

[millim]

Hello Frankenpod,

oh, I was not aware of this either. But, ginkgo should have run into the same issue? He did not mention this. Maybe there is a missing link how to manage this? I think %Vi( ) uses the display buffers in a different way. After booting the ipod, the SBS backdrop gets loaded nicely, when the interpreter gets to the point %Vi(-,32,20,256,192,1), this portion of the screen defaults back to the rockbox bootscreen for 1 second, then the tiles get visible. This might connect the the scrolling? Maybe the line gets registered somewhere to scroll, when going back to the tile screen, it does not get automatically de-registered or obsolete to update, respectively.   

Maybe ginkgo or the development team can comment on this?

millim

[millim]

Some update...

I was checking a bit the code and I think it is indeed a bug in rockbox. Maybe it is connected to the fact that the skinlist concept is not completed, it still needs the classical skin list engine:

in list.c

Code: [Select]

    FOR_NB_SCREENS(i)
    {
#ifdef HAVE_LCD_BITMAP
        if (!skinlist_draw(&screens[i], gui_list))
#endif
        list_draw(&screens[i], gui_list);
    }

it looks that the gui_list structure is not correctly liked when stepping out from the list_draw to the skinlist_draw viewports, by pressing MENU actually when a list item is highlighted and scrolling so:

Code: [Select]

struct viewport *parent = (list->parent[screen]);
display->scroll_stop_viewport(parent);

has no effect.

millim

[millim]

Some update...

found a simple fix. Unfortunately, need to modify the code. You may test if this also works for you. In list-skinned.c: need to add:

Code: [Select]

    if (listcfg[screen]->tile == true)
      display->scroll_stop();

Showing the skinlist_draw() function and how to add the fix:

Code: [Select]

bool skinlist_draw(struct screen *display, struct gui_synclist *list)
{
    int cur_line, display_lines;
    const int screen = display->screen_type;
    struct viewport *parent = (list->parent[screen]);
    char* label = NULL;
    const int list_start_item = list->start_item[screen];
    struct gui_wps wps;
    if (!skinlist_is_configured(screen, list))
        return false;

    current_list = list;
    wps.display = display;
    wps.data = listcfg[screen]->data;
    display_lines = skinlist_get_line_count(screen, list);
    label = (char *)SKINOFFSETTOPTR(get_skin_buffer(wps.data), listcfg[screen]->label);
    display->set_viewport(parent);
    display->clear_viewport();

    // fix the scrolling glitch
    if (listcfg[screen]->tile == true)
      display->scroll_stop();

millim
 

[Bilgus]

I think last time I ran into this bug the order of displaying viewports made a difference but this was like 2016

[Bilgus]

BTW @millim nice sleuthing

if you remove the if statement and make it
unconditionally call scroll_stop does it
work properly for you?

[millim]

.. well had to investigate a bit how things play together  . Yes, making it un-conditional works also for me. Do you plan to add this fix for the next release?

BTW, I have also added pictureflow to the main menu. It was already partially there, only three lines of code are necessary to make it work. Would also be nice if you can consider this extension.

I have attached the code with these modifications.

millim

[Frankenpod]

@millim

Sounds as if you have honed-in on the problem.  Dealing with the Rockbox code itself is, as the phrase has it "above my pay-grade", so I'd rather leave it to any devs who care to take an interest to sort out.  I wonder to what extent it's the same problem I've found with custom %vi statements that aren't tile-based?  Difficult to provide a screenshot as they don't seem to happen the same way in the simulator as on the ipod.  Either the text and background come out the same colour (regardless of what colours you tell it to use) for selected lines that are long enough to trigger scrolling, or, even more weirdly, the scrolling fails to work correctly and the text instead of scrolling just sort of smears itself like wet ink being smudged across a page.

[dconrad]

I finally got a chance to try this theme, it's looking great! I love the vu meters on the wps 

This might have been covered in the thread already (I confess not to have read it all), but is there any chance the main menu icons could support reordering and arbitrary hiding/showing? (this is done in the main menu config plugin). That's probably a fairly hard problem but figured I would ask.

I have the Recent Bookmarks enabled, my device doesn't have recording, and I've reordered the menu a bit (put "Now Playing" at the top), so half of the icons no longer match.