| | <<<Back 1 day (to 2016/08/05) | 20160806 |
| sebras | fredross-perry: when computing the span's bbox I think you could start out with fz_empty_rect and then call fz_union_rect() to add each char. I have seen this done elsewhere. | 05:26.46 |
| fredross-perry | o good I thought there might be something like that. thanks | 05:27.18 |
| sebras | fredross-perry: quite possibly robin will note that you walk each char array twice. he usually complains at me when I do similar things. :) | 05:29.24 |
| fredross-perry | so noted | 05:29.39 |
| sebras | fredross-perry: other than that this code looks reasonable to me. | 05:29.48 |
| fredross-perry | sweet | 05:29.52 |
| | thanks for looking | 05:29.57 |
| sebras | fredross-perry: also I think none of those fz_-functions can throw so no need for fz_try(). so in general LGTM. :) | 05:34.29 |
| Robin_Watts | fredross-perry: For the logs: Please can we reformat the commit message? First line should be no more than 72 chars, ideally less. | 08:36.01 |
| | jobject barr=NULL, larr=NULL, sarr=NULL, carr=NULL; | 08:36.35 |
| | I think I'm right in saying we don't do that elsewhere in MuPDF. | 08:36.54 |
| | If you declare and init a variable, one per line. | 08:37.09 |
| | +fz_stext_block *block = NULL; | 08:37.59 |
| | +fz_stext_line *line = NULL; | 08:38.00 |
| | +fz_stext_span *span = NULL; | 08:38.02 |
| | I like the fact they line up, but tor8 will have a fit and will reformat your code. | 08:38.03 |
| | +jobject jrect = to_Rect(ctx, env, &(block->bbox)); | 08:39.28 |
| | We don't declare variables in the middle of blocks in MuPDF. | 08:39.29 |
| | larr = (*env)->NewObjectArray(env, block->len, cls_TextLine, NULL); | 08:40.31 |
| | You don't check for larr being NULL (ah, but you mention you are missing error checking above, fair enough) | 08:40.32 |
| | +if (c==0) { | 08:41.08 |
| | Wrong brace style. | 08:41.10 |
| | And as sebastian correctly notes, why walk the spans twice? | 08:42.30 |
| | Other than that (and they are mostly fiddly stylistic things) it looks good. | 08:42.46 |
| fredross-perry | sebras, robin - I incorporated your review items and re-pushed. Please take a look thanks. | 17:19.51 |
| | Forward 1 day (to 2016/08/07)>>> | |
IRC Logs