Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10099 - lib, which display formatted text on every target

Attached to Project: Rockbox
Opened by Johannes Schwarz (Ubuntuxer) - Sunday, 05 April 2009, 17:14 GMT+2
Last edited by Teruaki Kawashima (teru) - Tuesday, 14 July 2009, 15:36 GMT+2
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

The text is displayed on the full screen size and can also be formatted. The lib should work on every target.
The code is based on the function do_help() from Will Robertson.
Please help me to improve the lib.
   display_text.diff (3.9 KiB) 
 apps/plugins/lib/SOURCES |    1 
 apps/plugins/lib/text.c  |   83 +++++++++++++++++++++++++++++++++++++++++++++++
 apps/plugins/lib/text.h  |   33 ++++++++++++++++++
 3 files changed, 117 insertions(+)

This task depends upon

Closed by  Teruaki Kawashima (teru)
Tuesday, 14 July 2009, 15:36 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  Committed in r21861.
Comment by Johannes Schwarz (Ubuntuxer) - Sunday, 05 April 2009, 17:51 GMT+2
The patch aroses from the problem, that if you use rb->lcd_putsxy(x, y, "text") the text doesn't always fits on the screen, because every target has an different screen size.
Some other plugins(e.g. star) uses a function called display_text(), which displays the text in a right way on the screen, but the function doesn't support styles, so the text is hard to read and looks a little bit boring.
The lib give you the ability to display formatted text in a nice way.
The main disadvantage of the lib is that it uses a lot of memory, but I can't think of an other solution to combine style and adaptation to the screen size.
The remaining text will be displayed, if you press any button.
Comment by hitesh mantrala (hittudiv) - Monday, 06 April 2009, 09:53 GMT+2
can we include different fonts on the same page?
like bold, italics or actual different fonts?
Comment by hitesh mantrala (hittudiv) - Monday, 06 April 2009, 09:55 GMT+2
we can scale the x and y in if you use rb->lcd_putsxy(x, y, "text") ....
we can get the actual screen size of the player and convert it to the specific screen size using a simple % calculation.
Comment by Johannes Schwarz (Ubuntuxer) - Wednesday, 08 April 2009, 12:58 GMT+2
@hittudiv It's not available to include different fonts easily in display_font(), because a bigger font needs another line distance, so the whole line have to be displayed again. It would be great to display bold and italics, but unfornunately most fonts doesn't support it.
I have included the lib in brickmania, but I have to work on the viewport integration. The problem is, that I need the x, y, width and height from the viewport struct, but I don't know how to get it from the current viewport.
Comment by Johannes Schwarz (Ubuntuxer) - Wednesday, 08 April 2009, 12:59 GMT+2
brickmania example usage of the lib
   text_display.diff (13.7 KiB) 
 apps/plugins/lib/SOURCES  |    1 
 apps/plugins/lib/text.c   |  116 ++++++++++++++++++++++++++
 apps/plugins/lib/text.h   |   34 +++++++
 apps/plugins/brickmania.c |  204 +++++++++++++---------------------------------
 4 files changed, 212 insertions(+), 143 deletions(-)

Comment by Johannes Schwarz (Ubuntuxer) - Thursday, 09 April 2009, 16:53 GMT+2
The lib supports now also viewport, but it's important, that you pass the viewport to the funtion. I hope the lib is useful and doesn't use to much memory.
Please test the lib and if necessary rename it.
   lib_display_text.diff (7.1 KiB) 
 plugins/lib/SOURCES |    1 
 plugins/lib/text.c  |  131 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 plugins/lib/text.h  |   41 ++++++++++++++++
 3 files changed, 173 insertions(+)

   brickmania_display_text.diff (8.1 KiB) 
 plugins/brickmania.c |  218 +++++++++++++++++----------------------------------
 1 file changed, 75 insertions(+), 143 deletions(-)

Comment by Johannes Schwarz (Ubuntuxer) - Friday, 10 April 2009, 20:02 GMT+2
tiny fix
   lib_display_text.diff (7.1 KiB) 
 plugins/lib/SOURCES |    1 
 plugins/lib/text.c  |  131 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 plugins/lib/text.h  |   41 ++++++++++++++++
 3 files changed, 173 insertions(+)

Comment by Johannes Schwarz (Ubuntuxer) - Saturday, 09 May 2009, 16:14 GMT+2
set drawmode to solid
   lib_display_text.diff (7.3 KiB) 
 apps/plugins/lib/text.c |  151 ++++++++++++++++++++++++++++++++++++++++++++++++
 apps/plugins/lib/text.h |   41 +++++++++++++
 2 files changed, 192 insertions(+)

Comment by Teruaki Kawashima (teru) - Sunday, 17 May 2009, 14:49 GMT+2
your patch seems to do some strange things for me.

> unsigned short xmargin = 5, ymargin = 5;
maybe this margin is too large for the lcd with charcells.
When I tested on the simulator for player before, nothing was displayed.

> xmargin = xmargin + (*vp_text).x;
> ymargin = ymargin + (*vp_text).y;
This will cause problem especially when someone wants to set vp_text->x bigger than vp_text->width/2,
because the xmargin is subtracted from text_width(==vp_text->width) at line 70.
It would be better to use "rb->screens[SCREEN_MAIN]->set_viewport(vp_text);",
and no need to set foreground/background and branch when setting drawmode.

Declaration of standard_fcolor also shoud be surrounded by "#if LCD_DEPTH > 1".
"standard_fcolor=(*vp_text).fg_pattern;" at line 53 is duplicated.

> x=(LCD_WIDTH/2)-(width/2);
maybe should use text_width instead of LCD_WIDTH?

In my opinion, it is not necessary to restore drawmode.

Is there any reason not waiting key input aftrer displaying last word?

It would be nice if you implement scrolling up/down the text.

sorry for my bad english and description.
Comment by Johannes Schwarz (Ubuntuxer) - Monday, 18 May 2009, 16:20 GMT+2
Thank you for your bugreports and help.
I fixed the bugs you have detected and avoid unused parameter warnings on targets without color or bitmaps.

>In my opinion, it is not necessary to restore drawmode.
I don't think so, it's very important, because the plugin writer, who use the lib can't know, that the drawmode is changed. So I add a reset for the foreground color, too.

>Is there any reason not waiting key input aftrer displaying last word?
The lib should be so flexible as possible, so I wouldn't change it.

>It would be nice if you implement scrolling up/down the text.
At the moment the text which fits on the screen is displayed and after you select a button the rest is displayed. It's a little bit like a book. In my view the main disadvantage is, that you can't go back.
A scolling text would be completely different concept, but we should discuss about it.
   lib_display_text.diff (7.5 KiB) 
 apps/plugins/lib/text.c  |  151 +++++++++++++++++++++++++++++++++++++++++++++++
 apps/plugins/lib/text.h  |   41 ++++++++++++
 apps/plugins/lib/SOURCES |    1 
 3 files changed, 193 insertions(+)

   brickmania_display_text.diff (7.7 KiB) 
 apps/plugins/brickmania.c |  208 ++++++++++++++--------------------------------
 1 file changed, 65 insertions(+), 143 deletions(-)

Comment by Johannes Schwarz (Ubuntuxer) - Monday, 18 May 2009, 16:39 GMT+2
Upps, silly bug in brickmania_display_text.diff
   brickmania_display_text.diff (8.1 KiB) 
 apps/plugins/brickmania.c |  217 +++++++++++++++-------------------------------
 1 file changed, 74 insertions(+), 143 deletions(-)

Comment by Johannes Schwarz (Ubuntuxer) - Sunday, 24 May 2009, 17:24 GMT+2
Fixed a bug, which occurs on Archos Player/Studio, because of its small display size.
   lib_display_text.diff (5.5 KiB) 
 apps/plugins/lib/text.c |  155 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 155 insertions(+)

Comment by Teruaki Kawashima (teru) - Thursday, 25 June 2009, 17:28 GMT+2
*cleanup a bit.
*rename text.* to display_text.* to fit name of function.
*modify some plugins to use display_text.
   lib_display_text.patch (7.3 KiB) 
 apps/plugins/lib/SOURCES        |    1 
 apps/plugins/lib/display_text.c |  139 ++++++++++++++++++++++++++++++++++++++++
 apps/plugins/lib/display_text.h |   48 +++++++++++++
 3 files changed, 188 insertions(+)

   use_lib_display_text.patch (57.2 KiB) 
 apps/plugins/superdom.c         |   51 +-------
 apps/plugins/brickmania.c       |  213 ++++++++++++---------------------
 apps/plugins/robotfindskitten.c |   39 +-----
 apps/plugins/jewels.c           |  255 +++++++++++++++-------------------------
 apps/plugins/pegbox.c           |  172 +++++++-------------------
 apps/plugins/solitaire.c        |  169 +++++---------------------
 apps/plugins/star.c             |  212 +++++++++++++++++++--------------
 7 files changed, 402 insertions(+), 709 deletions(-)

Comment by Johannes Schwarz (Ubuntuxer) - Sunday, 28 June 2009, 11:54 GMT+2
I committed the lib display_text; thanks Teruaki Kawashima. Actually I don't like bitwise operation, but it's a nice solution.
The patch, which modify the plugins to use display_text needs a review. I will do this within the next few days.
Comment by Maurus Cuelenaere (mcuelenaere) - Sunday, 28 June 2009, 11:59 GMT+2
Why do you use struct style_text (and pass a struct pointer as argument for display_text) when you can simply pass an unsigned short? (I would prefer an (unsigned) int though)
Comment by Teruaki Kawashima (teru) - Sunday, 28 June 2009, 15:50 GMT+2
*fix crash in brickmania
repeating entering and quiting help in brickmania crash the player.
*make "formation" static.
*try to reduce size of "formation".
   use_lib_display_text.2.patch (63.5 KiB) 
 apps/plugins/lib/display_text.c |   19 +-
 apps/plugins/lib/display_text.h |   10 -
 apps/plugins/superdom.c         |   51 +------
 apps/plugins/brickmania.c       |  284 +++++++++++++---------------------------
 apps/plugins/robotfindskitten.c |   39 +----
 apps/plugins/jewels.c           |  256 +++++++++++++-----------------------
 apps/plugins/pegbox.c           |  173 ++++++------------------
 apps/plugins/solitaire.c        |  169 ++++-------------------
 apps/plugins/star.c             |  213 +++++++++++++++++-------------
 9 files changed, 435 insertions(+), 779 deletions(-)

Comment by Johannes Schwarz (Ubuntuxer) - Thursday, 02 July 2009, 19:02 GMT+2
As far I just look at the lib; the idea to make a struct, which consist of index and the formation is great.
But it could be a nerve limiatation, that the last text item musn't be a formatted word. Of course you could use just "" but it isn't elegant and can lead to problems!
I fix some minor typos and remove the limitation that the last item must be a unformatted word.

Loading...