From: Thomas Martitz <kugel_at_rockbox.org>
Date: Sun, 17 Feb 2013 21:40:37 +0100

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
> contents.
>
> My motivation is to properly implement [1] (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 [2], 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.
>
> [1]: http://gerrit.rockbox.org/r/#/c/384/
> [2]: 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
already implemented.

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
> anyway?)

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.

Best regards.
Received on 2013-02-17