| | <<<Back 1 day (to 2018/05/10) | 20180511 |
| ray_laptop | Robin_Watts: sorry I missed the comment about reviewing your patch. I have some questions/concerns... | 01:35.43 |
| | but I'll wait until you reply and keep slogging through. | 01:35.46 |
| | Robin_Watts: OK | 13:28.13 |
| | in commit aa870105: | 13:28.52 |
| | state->line should be allocated with state->mem (not dev->memory) so that it is also in non_gc_memory | 13:28.54 |
| | You need to free 'line' in region_end. | 13:28.55 |
| | and, of course, use state->mem to free them | 13:29.39 |
| Robin_Watts | Yes, will fix. thanks. | 13:30.19 |
| ray_laptop | MINOR nitpick -- several places don't have en empty line separating the local vars from the executable code. | 13:30.35 |
| | another minor nitpick. there is still "pp" everywhere. Can I suggest "tpr" instead (in the future, "pixel patch" will be forgotten, and "pp" won't mean anything) | 13:30.36 |
| | btw, I like the name better :-) | 13:31.11 |
| | in commit 9c8de5c3: | 13:32.20 |
| | mem_transform_pixel_region uses dev->memory to allocate the state rather than non_gc_memory | 13:32.21 |
| | In the case where render == NULL (using the default), after calling the default, if the reason is "end", you also need to call mem_transform_pixel_region_end(dev, state); and set data->state to NULL | 13:32.23 |
| Robin_Watts | I can't see "pp". | 13:33.09 |
| ray_laptop | and in commit cd166ba8, a minor issue. | 13:33.19 |
| | use fixed_1 instead of 256 in: if (pixels->x.step.dQ == 256 && pixels->x.step.dR == 0) | 13:33.21 |
| | Robin_Watts: pp_state and image_render_color_icc_pp and a few other places ? | 13:34.00 |
| Robin_Watts | Ah! | 13:34.08 |
| | gotcha. | 13:34.10 |
| | Some nice catches there. Thanks! | 13:37.45 |
| ray_laptop | Robin_Watts: np. note, I am not the best a reading code, so I may have missed things, but I figured memory issues are always a difficulty in GS :-/ | 13:39.52 |
| Robin_Watts | ray_laptop: The main thing is that you don't appear to be offended by the whole approach. | 13:41.23 |
| ray_laptop | I don't know if it would make a measurable difference for the mem device implementation, but for depth == 4, that case could write out 32-bit words (we know the buffers are 32-bit aligned) | 13:42.04 |
| | which would affect the CMYK case | 13:42.18 |
| Robin_Watts | ray_laptop: Yeah, could do. | 13:42.43 |
| ray_laptop | Robin_Watts: no, I'm happy enough with the general approacn | 13:42.44 |
| | OK, I'm going to start my morning routine getting my son to school. I'll check in a couple of times | 13:45.38 |
| Robin_Watts | ray_laptop: Updated commits online. | 13:50.24 |
| | +chrisl: And I've written a first version of the docs too. | 13:50.58 |
| chrisl | Robin_Watts: When do you need this "properly" reviewed by? I'm kind of the in the middle of something right now..... | 13:56.58 |
| ray_laptop | Robin_Watts: is there an easy way to modify a commit in a chain of commits ? | 14:04.39 |
| kens | If you haven't pushed them to master yet then git rebase --interactive | 14:05.10 |
| Robin_Watts | ray_laptop: Normally, I make a small commit with the change at the end, and then use git rebase -i HEAD~10 to move it back, | 14:05.13 |
| | and "f" to mark it as a fix. | 14:05.26 |
| chrisl | In an interactive rebase, you can select a commit to edit | 14:05.39 |
| Robin_Watts | If that can't work (because of conflicts later on), then I git rebase -i HEAD~10 and mark the ones to edit as 'e'. | 14:05.52 |
| | chrisl: I'm in no massive hurry at all. | 14:06.12 |
| ray_laptop | Robin_Watts: OK, that's what I do, too -- it's just a bit cumbersome to keep it straight in the git rebase -i step | 14:06.13 |
| chrisl | Robin_Watts: Okay, I'll look through it properly on Monday (hopefully) | 14:06.50 |
| Robin_Watts | chrisl: Fab. | 14:06.57 |
| ray_laptop | Robin_Watts: I'll have another look as well, then (just to see if I can find any more nitpicks ;-) ) | 14:07.51 |
| | Forward 1 day (to 2018/05/12)>>> | |
Log of #ghostscript at irc.freenode.net.