Rockbox

  • Status Unconfirmed
  • Percent Complete
    0%
  • Task Type Patches
  • Category User Interface
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.4
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Amr Medhat - 2010-03-12

FS#11095 - Fixing the joining algorithm after adding diacritics support

This patch which is a small change in bidi.c contributes with the diacritics patch (http://www.rockbox.org/tracker/task/10720) to fix the diacritized Arabic text joining problem

Amr Medhat commented on 2010-03-12 16:47

screen dumps for the a case before adding diacritics support, after adding diacritics support and after the bidi patch

Amr Medhat commented on 2010-03-21 02:22

A little change in lcd-bitmap-common.c that makes diacritics appear but shifted one character

Amr Medhat commented on 2010-03-23 17:35

finally worked by just generalizing drawmode change.
attached the patch and screenshots of the expected results

Amr Medhat commented on 2010-03-27 12:48

applying amiconn's suggestion

Tomer Shalev commented on 2010-03-27 16:45
+ unsigned char bits_tmp[400];
Amr, can you please use a constant instead of using the hard-coded value several times in the code?
Amr Medhat commented on 2010-03-27 22:51

comments considered

Tomer Shalev commented on 2010-03-28 17:22

The attached patch contains the following changes:
- Rename is_diac to diacs, since this variable is used as a counter
- Split long lines
- Simplify loop that writes skipped diacritics

I've tested with Hebrew text that contains diacritics, and it works for me. I would like to commit this soon - Amr, is that ok? I must have your full name for the credits, and to fulfill Rockbox requirements for contributing code (see docs/CONTRIBUTING).

One last thing: I would like you to explain (better have it written as a comment in the code itself) how did you get into the number 32?
Does this number valid for *all targets*, and for *all fonts* ?
I suggest you put a macro instead, with a formula that contains the defines this number is derived from, e.g.
#define GLYPH_BITS_LENGTH (BLAH_BLAH * MAX_FOO / 8)

Thanks,
Tomer

Amr Medhat commented on 2010-03-29 06:23

Thanks Tomer,

I've returned to the non-simple loop of writing skipped diacritics to avoid segmentation fault that happens in special cases when diacs = -1 from a previous decrement causing entering the loop where it shouldn't

Also a comment of the formula has been added : font.height * font.maxwidth / 8 , but I don't know how to make it a macro calculated by the preprocessor while height and maxwidth can be obtained only in runtime, as they are font-dependent…,

I calculated it manually with debugf() by trying out different fonts where height,maxwidth values range from the lowest (8,6) to max (16,16) for GNU_Unifont…, hence I based the hardcoded value on 16 * 16 / 8 = 32 … I hope I could find some generic value , but there is no stated restriction on font size http://www.rockbox.org/wiki/FontFormat

My full name: Amr Medhat

Thanks

Amr Medhat commented on 2010-04-02 12:31

Changed to runtime buffer allocation (but I get a warning: implicit declaration of buffer_alloc)
(p.s. update of viewer.c since revision 25233 causes problems with diacritized scripts)

Amr Medhat commented on 2010-04-03 03:43

no warnings now
(but just asking, which is more suitable for our case buffer_alloc() or bufalloc() ?)

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing