Warning: Undefined array key "useheading" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 1099 Warning: Undefined array key "target" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 557 Warning: Trying to access array offset on value of type null in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 557 FS#11095 : Fixing the joining algorithm after adding diacritics support

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 - 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

   bidi.patch (2.3 KiB)
amr 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

   before_diacritics1.bmp (75.7 KiB)
   before_diacritics2.bmp (75.7 KiB)
   after_diacritics1.bmp (75.7 KiB)
   after_diacritics2.bmp (75.7 KiB)
   after_diacritics3.bmp (75.7 KiB)
   after_patch1.bmp (75.7 KiB)
   after_patch2.bmp (75.7 KiB)
   after_patch3.bmp (75.7 KiB)
amr commented on 2010-03-21 02:22

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

   arab_diac (2.9 KiB)
amr commented on 2010-03-23 17:35

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

   arab_diac.patch (3.4 KiB)
   fixed1.bmp (75.7 KiB)
   fixed2.bmp (75.7 KiB)
amr commented on 2010-03-27 12:48

applying amiconn's suggestion

   arab_diac.patch (5.1 KiB)
tomers 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 commented on 2010-03-27 22:51

comments considered

   arab_diac.patch (6.1 KiB)
tomers 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

   arab_diac_v5.diff (6 KiB)
amr commented on 2010-03-29 06:23
Warning: Undefined array key "useheading" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 1099 Warning: Undefined array key "target" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 557 Warning: Trying to access array offset on value of type null in /home/rockbox/flyspray/plugins/dokuwiki/inc/parser/xhtml.php on line 557

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

   arab_diac.patch_v6.diff (6.1 KiB)
amr 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)

   arab_diac_v7.patch (5.9 KiB)
amr commented on 2010-04-03 03:43

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

   arab_diac_v8.patch (6.2 KiB)

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing