ibus-zinnia - Japanese handwriting recognition engine

ibus-zinnia - Japanese handwriting recognition engine.

ibus-zinnia is a very thin wrapper of libzinnia, a open source handwriting recognition engine written in C++.

The code of ibus-zinnia is based on suzhe's work at https://github.com/suzhe/ibus-xkb-layouts (thus Apache-2.0 license).

[Peng, 2011-04-21 15:03:29]

http://codereview.appspot.com/4428051/diff/5001/src/engine.c
File src/engine.c (right):

http://codereview.appspot.com/4428051/diff/5001/src/engine.c#newcode117
src/engine.c:117: for (i = 2; i < coordinates_len; i += 2) {
maybe i = 1 or i <= coordinates_len?

http://codereview.appspot.com/4428051/diff/5001/src/engine.c#newcode126
src/engine.c:126: ++(zinnia->stroke_count);
I think all pointers in coordinates is belong to one stroke. Should we
++(zinnia->stroke_count)?

I think it might be: 

for (i = 1; i < coordinates_len; i+= 2) {
    zinnia_character_add (zinnia->character,
                          zinnia->stroke_count,
                          normalize(coordinates[i - 1]),
                          normalize(coordinates[i]));
}
zinnia->stroke_count++;

Please double check zinnia API.

http://codereview.appspot.com/4428051/diff/5001/src/main.c
File src/main.c (left):

http://codereview.appspot.com/4428051/diff/5001/src/main.c#oldcode18
src/main.c:18: { "xml", 'x', 0, G_OPTION_ARG_NONE, &xml, "generate xml for
engines", NULL },
Maybe it is useful for supporting multiple language in future

http://codereview.appspot.com/4428051/diff/5001/src/zinnia_component.c
File src/zinnia_component.c (right):

http://codereview.appspot.com/4428051/diff/5001/src/zinnia_component.c#newcode9
src/zinnia_component.c:9: engine = ibus_engine_desc_new_varargs ("name",
"ibus-zinnia-japanese",
Suggest renaming it to ibus-zinnia-japanese-debug, because I think it is not for
debug proposal. it will not conflict with installed zinnia engine system. So you
might have two zinnia engine in system. One is installed in system. another is
for debug.

http://codereview.appspot.com/4428051/diff/5001/src/zinnia_component.c#newcode10
src/zinnia_component.c:10: "longname", "Japanese hand-writing engine",
Suggest adding suffix (debug)

[Yusuke Sato, 2011-04-22 03:39:46]

Please take another look.

http://codereview.appspot.com/4428051/diff/5001/src/engine.c
File src/engine.c (right):

http://codereview.appspot.com/4428051/diff/5001/src/engine.c#newcode117
src/engine.c:117: for (i = 2; i < coordinates_len; i += 2) {
On 2011/04/21 15:03:29, Peng wrote:
> maybe i = 1 or i <= coordinates_len?

Done.

http://codereview.appspot.com/4428051/diff/5001/src/engine.c#newcode126
src/engine.c:126: ++(zinnia->stroke_count);
On 2011/04/21 15:03:29, Peng wrote:
> I think all pointers in coordinates is belong to one stroke. Should we
> ++(zinnia->stroke_count)?
> 
> I think it might be: 
> 
> for (i = 1; i < coordinates_len; i+= 2) {
>     zinnia_character_add (zinnia->character,
>                           zinnia->stroke_count,
>                           normalize(coordinates[i - 1]),
>                           normalize(coordinates[i]));
> }
> zinnia->stroke_count++;
> 
> Please double check zinnia API.

Done.

http://codereview.appspot.com/4428051/diff/5001/src/main.c
File src/main.c (left):

http://codereview.appspot.com/4428051/diff/5001/src/main.c#oldcode18
src/main.c:18: { "xml", 'x', 0, G_OPTION_ARG_NONE, &xml, "generate xml for
engines", NULL },
agreed. added a FIXME in engine.c.

On 2011/04/21 15:03:29, Peng wrote:
> Maybe it is useful for supporting multiple language in future

http://codereview.appspot.com/4428051/diff/5001/src/zinnia_component.c
File src/zinnia_component.c (right):

http://codereview.appspot.com/4428051/diff/5001/src/zinnia_component.c#newcode9
src/zinnia_component.c:9: engine = ibus_engine_desc_new_varargs ("name",
"ibus-zinnia-japanese",
On 2011/04/21 15:03:29, Peng wrote:
> Suggest renaming it to ibus-zinnia-japanese-debug, because I think it is not
for
> debug proposal. it will not conflict with installed zinnia engine system. So
you
> might have two zinnia engine in system. One is installed in system. another is
> for debug.

Done.

http://codereview.appspot.com/4428051/diff/5001/src/zinnia_component.c#newcode10
src/zinnia_component.c:10: "longname", "Japanese hand-writing engine",
On 2011/04/21 15:03:29, Peng wrote:
> Suggest adding suffix (debug)

Done.

[Peng, 2011-04-22 11:05:26]

On 2011/04/22 03:39:46, Yusuke Sato wrote:
> Please take another look.
> 
> http://codereview.appspot.com/4428051/diff/5001/src/engine.c
> File src/engine.c (right):
> 
> http://codereview.appspot.com/4428051/diff/5001/src/engine.c#newcode117
> src/engine.c:117: for (i = 2; i < coordinates_len; i += 2) {
> On 2011/04/21 15:03:29, Peng wrote:
> > maybe i = 1 or i <= coordinates_len?
> 
> Done.
> 
> http://codereview.appspot.com/4428051/diff/5001/src/engine.c#newcode126
> src/engine.c:126: ++(zinnia->stroke_count);
> On 2011/04/21 15:03:29, Peng wrote:
> > I think all pointers in coordinates is belong to one stroke. Should we
> > ++(zinnia->stroke_count)?
> > 
> > I think it might be: 
> > 
> > for (i = 1; i < coordinates_len; i+= 2) {
> >     zinnia_character_add (zinnia->character,
> >                           zinnia->stroke_count,
> >                           normalize(coordinates[i - 1]),
> >                           normalize(coordinates[i]));
> > }
> > zinnia->stroke_count++;
> > 
> > Please double check zinnia API.
> 
> Done.
> 
> http://codereview.appspot.com/4428051/diff/5001/src/main.c
> File src/main.c (left):
> 
> http://codereview.appspot.com/4428051/diff/5001/src/main.c#oldcode18
> src/main.c:18: { "xml", 'x', 0, G_OPTION_ARG_NONE, &xml, "generate xml for
> engines", NULL },
> agreed. added a FIXME in engine.c.
> 
> On 2011/04/21 15:03:29, Peng wrote:
> > Maybe it is useful for supporting multiple language in future
> 
> http://codereview.appspot.com/4428051/diff/5001/src/zinnia_component.c
> File src/zinnia_component.c (right):
> 
>
http://codereview.appspot.com/4428051/diff/5001/src/zinnia_component.c#newcode9
> src/zinnia_component.c:9: engine = ibus_engine_desc_new_varargs ("name",
> "ibus-zinnia-japanese",
> On 2011/04/21 15:03:29, Peng wrote:
> > Suggest renaming it to ibus-zinnia-japanese-debug, because I think it is not
> for
> > debug proposal. it will not conflict with installed zinnia engine system. So
> you
> > might have two zinnia engine in system. One is installed in system. another
is
> > for debug.
> 
> Done.
> 
>
http://codereview.appspot.com/4428051/diff/5001/src/zinnia_component.c#newcode10
> src/zinnia_component.c:10: "longname", "Japanese hand-writing engine",
> On 2011/04/21 15:03:29, Peng wrote:
> > Suggest adding suffix (debug)
> 
> Done.

LGTM