Am 20.02.2013 13:33, schrieb Jonathan Gordon:
On 20 February 2013 22:34, Thomas Martitz <ku...@rockbox.org <mailto:ku...@rockbox.org>> wrote:I start with list.c because that's the most extensive user, but other screens can follow easily, e.g. various plugins. The API that I suggest can print the whole line (indent, spacing, icon and text) in a single call so it's generally attractive.Ignoring plugins, 99% of the UI is drawn by list.c, so yes adding API's for its use would seem attractive, but that will almost certainly end up being wasted effort. I'm really aiming to have the lists entirely drawn by the skin engine by the next release which would make any effort here redundant.
Why should plugins be ignored. We should make things easier for them, not harder, shouldn't we? Also I think the skin engine can use this API as well, even if not to the full extend, as I want to use it to simplify scrolling at a higher level. Also, the skin engine still needs a method to draw styles and multi-line stuff which is not yet in place. I don't see this effort as wasted.
So, in that spirit I'd advocate you just doing the semi-nasty hack and adding a bit to the style flags so the scroller can redraw the seperater line (or just piggy-back off the gradient num_lines bits). Once the skin engine is doing the list drawing the scroller can be returned to a dumb text drawer (all line decorations would be drawn onto the backdrop buffer so the scroller clearing the line would leave them intact.)If all you want is a line separator then why is it not a simple 1 line lcd_hline() call in the draw_list() loop? This is what the current version on gerrit does. The problem is that scrolling breaks it (the scroll engine redraws the line selector which clears the separator). And it sucks from a software-architecture POV: The line selector is currently drawn in firmware, the separator would be drawn in apps (in this scenario). So two completely different places although they are essentially very similar (style attached to each line). I really want to clean up this mess for good before moving forward with the separator.As above, in the interest of not wasting developer time I'd recommend doing the sucky way knowing it is temporary.
It's not wasted time for me; I have already done most of the work (prior to starting this thread) and it's fun for me. Also I don't think it's /that/ temporary. I expect it'll take a while until the skin engine can usefully replace the built-in lists, the progress in this area is rather slow to put it mildly. I know I could help with it, but I'm sorry it doesn't catch my interest currently.
Thinking about it now, it would be pretty cool to register a callback (per viewport) for the scroll engine to call which would do the line drawing instead of the current code. (It would have to be per viewport because it is very likley to have a skin scrolling line and the list scrolling line both scrolling at the same time, though a generic one would be needed for default which should "just work"). I have the beginnings of a scroll engine rework which does a callback (per line, and each line has a viewport associated). But even the current scroll engine already stores a viewport pointer for each line, that is set when the line is redrawn.This I'm interested in seeing. I'm starting to wonder if the scroller really needs the viewport pointer per line? If this was changed to store just the pixel rectangle to draw in (so the line number is also removed, and relative to the screen, not viewport) then it wouldn't necessarily overdraw the decorations. The callbacks becomes an issue, but I agree that using a callback is the way to go to remove this decoration drawing from firmware.
It stores only a viewport pointer currently, so a pixel rectangle is going to be more expensive from a ram usage POV. The viewport also provides line selector and font information which is currently needed (with no callback mechanism).
Best regards.
