On 16 February 2013 08:30, Thomas Martitz <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  (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
> 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/<http://gerrit.rockbox.org/r/#/c/384/>
> : https://github.com/kugel-/**rockbox/tree/newline-api<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).
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.
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?)
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).
So yes, the lcd_put* API is long outdated and crud, but changing it
is orthogonal to your motivation.
Received on 2013-02-16