Am 16.02.2013 10:46, schrieb Jonathan Gordon:
> On 16 February 2013 08:30, Thomas Martitz <kugel_at_rockbox.org
> <mailto:kugel_at_rockbox.org>> wrote:
> Hello guys,
> I'm working on a new line print API in apps that's supposed to
> replaces most of lcd_puts_* and lcd_putsxy_*.
> The lcd_puts* became really messy and it still doesn't support
> scrolling properly (not at all for pixel based functions). The
> rework I'm working on will hopefully be simplified and fix
> scrolling, while allowing more control over the line style and
> My motivation is to properly implement  (line separator in
> lists). This line separator cannot be properly implemented just in
> apps because scrolling draws over it, and scrolling is entirely in
> firmware and calls only firmware function. To not further
> complicate lcd_puts* a rework is needed.
> My idea is a having a single function:
> put_line(int x, int y, struct line_desc *desc, const char *fmt, ...).
> - x, y are the position of the line (in pixels!).
> - struct line_desc defines other properties the line: style (for
> line selectors, and the line separators mentioned above),
> scrolling, line height and multiline information.
> - fmt is a format string that controls the contents of the line.
> Similar to printf() tags can be used to put icons, text and
> margins (perhaps other stuff too in the future), the variable
> paramter list is then used to build the contents
> There are some complications, most notably scrolling, multiline
> and RTL, which I can hopefully work out soon (i think I will solve
> scrolling by setting up a callback so apps code can do the actual
> drawing). As if now there is a preview of my work in , see
> apps/gui/bitmap/list.c for an example of how to use the new api.
> So, what do you think? Is going forward worthwhile and welcome? Do
> you have comments/remarks/suggestions as to the proposed API function?
> Best regards.
> : http://gerrit.rockbox.org/r/#/c/384/
> : https://github.com/kugel-/rockbox/tree/newline-api
> Please don't add any non-text drawing to the lcd_put* API. That should
> do nothing but position text. I think you're going about this the
> wrong way (or at least doing too much at the firmware level).
Sorry, I wasn't clear enough. My intent is to put put_line() into apps/.
I totally agree that non-text stuff shouldn't be added to the firmware
level, which is why I even make this work (to avoid putting the line
separator to firmware which I *really* don't want). The current patch
has it in apps/.
Being in apps is also a requirement for extending the line selector to
the icons and indent level (right now the line selector is only drawn
below the text portion of a line), which is something I really want and
Eventually the line selector drawing (i.e. line styles) could be removed
from firmware once a sufficient apps API is in place.
> I like the idea of registering a callback for scrolling lines, this
> would potentially fix existing issues with the list indenting, however
> I don't think this is the way to fix this (and your line seperator).
> My suggestion is:
> 1) fix scrolling so it works on a pixel level and not a line level. We
> are well past the time when scrolling on a line level (and using the
> entire line) is outdated. If scrolling can be told the pixel rectangle
> (or sub viewport would be better/simpler) to scroll in then you are
> 90% of the way to what you want.
This is on my TODO list, but
> 2) modify list.c to know the difference between the text height and
> line height. I'm guessing this is the actual issue you're aving with
> line seperater (though your line height patch hsold have done this
list.c already knows the difference, since the line height patch. What
do you mean by "this is the actual issue"? list separator and line
height are barely related.
> 3) Draw the line separator in the list UI which is the only correct
> place to put it. Sure, you could piggy-back off the gradient code and
> draw a line manually but please remember that everything in the UI
> that is drawn should be customizable (maybe not from the start, but
> certainly not modified to make it impossible later). (tangent: the
> gradient code is really in the wrong place too, the list should be
> drawing the gradient and then drawing the text over the top, not the
> lcd_* code).
This is already implemented by the current version.
> So yes, the lcd_put* API is long outdated and crud, but changing it
> is orthogonal to your motivation.
I don't want to change it. Not now at least. And I particularly dont
want to expand it. This work makes some of lcd_put* obsolete so the API
can be cleaned up in a follow-up change.
Received on 2013-02-17