Rockbox Development > Starting Development and Compiling
Annouce: patch to speed up SOME list scrolling by a factor of ~12
TP Diffenbach:
--- Quote from: TiMiD on June 27, 2006, 10:12:36 AM ---I didn't applied the patch, but just looking at the source code makes me think this doesn't work for 2 screens devices because of the way you test if a list has to be completely redrawn or just partially (in the case of 2 screens devices, the list will be redrawn each time).
--- End quote ---
Yes, you're quite right. Thanks for catching that.
Just as screens is a global array of displays, we need to save last_gui_list into an array as long as the screens array (that is, either one or two elements), and index the array using the screen value passed to the gui_synclinst functions.
Or! Better, we could make just last_gui_list a member of struct screen. Since a gui_list hold a pointer to its screen we could have
if( list == list->screen.last_list ) // this list is still displayed on this screen
This gets rid of the file scope last_gui_list, so the default case in gui_synclist_do_button would the be:
default: { // brackets because we're going to declare an automatic variable
int I ;
FOR_NB_SCREENS(I) // same code as in all gui_synclist functions
screens[ i ].last_gui_list = 0 ;
}
And it's always nice to get rid of globals and file scope variables precisely because they don't accommodate having two of anything.
TiMiD:
I think an additionnal parameter to the draw function is the way to go.
a program could call the draw function to redraw completely the screen without creating a new list
oh and btw with all my respect, I think it's a really bad idea to use the screen struct to put things like this without any relationship; ideally this one should hold only informations about the low level access to the screen.
TP Diffenbach:
--- Quote from: TiMiD on June 27, 2006, 12:39:25 PM ---I think an additional parameter to the draw function is the way to go.
a program could call the draw function to redraw completely the screen without creating a new list
--- End quote ---
Ok, but how does the caller know when to redraw or not? The only way to know that is to monitor changes to the struct gui_list, which we can do in the list_draw function.
It might seem that the caller could know this without looking at the current list item, but that's not the case. Consider:
#ifdef LIST_PGUP
case LIST_PGUP:
gui_synclist_limit_scroll(lists, false);
case LIST_PGUP | BUTTON_REPEAT:
gui_synclist_select_previous_page(lists, SCREEN_MAIN);
gui_synclist_draw(lists);
yield();
return LIST_NEXT;
#endif
A previous page should mean that every line has changed, so it's pointless to optimize. And that'll be true if the key isn't repeating, because gui_synclist_limit_scroll won't be called. But if the key is repeating, and we reach the end of the list, we might have some lines we can reuse. The only way to tell would be to find if gui_list->start_item - last_displayed_start_item < screen->nb_lines.
And it's duplicative to do that in several callers when we can do it in one called function.
The bottom line is, regardless of the caller, if we can save time by not reprinting a line, we want to do it.
--- Quote ---oh and btw with all my respect, I think it's a really bad idea to use the screen struct to put things like this without any relationship; ideally this one should hold only informations about the low level access to the screen.
--- End quote ---
I see your point, but knowing what last used the screen (which list or no list at all) seems like a good candidate for low level screen information. No, it's not as "purely low level" as width or height, but neither are char_width or char_height, which are members of struct screen even though their values depend on the current font.
We /should/ strive to be purists, but we shouldn't contort ourselves doing so.
Zoide:
Witch of the two patches do we use for the 5G iPod? The iPodLinux Generations page (http://ipodlinux.org/Generations) lists its processor as "PP5021C-TDF (like PP5020)", so I'm not sure whether that counts as PP5020 or not.
TP Diffenbach:
--- Quote from: Zoide on June 27, 2006, 03:06:36 PM ---Witch of the two patches do we use for the 5G iPod? The iPodLinux Generations page (http://ipodlinux.org/Generations) lists its processor as "PP5021C-TDF (like PP5020)", so I'm not sure whether that counts as PP5020 or not.
--- End quote ---
If you're asking about versions of this patch: Patch one, if your build includes logf, patch two if it doesn't. If you're not sure, patch two.
If you're asking about this patch and the /other/ list optimization patch: this one. The other one will give you a speed increase but make your background images look terrible. (Of course, if you don't use a background image, or use one like brushed metal that doesn't change much from top to bottom, then you can use both patches.)
If you want this list optimization and my scrolling accel (which is still half-baked), then use this patch: http://www.rockbox.org/tracker/task/5594
If you have an ipod 5G, and ONLY an ipod 5G, definitely get the lcd patch too -- it breaks nothing and it increases your framerate by 5%, so there's no reason not to get it:
http://www.rockbox.org/tracker/task/5432
(It won't break other platforms, but it won't do anything for them either.)
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version