| | <<<Back 1 day (to 2015/12/30) | 20151231 |
| rayjj | Robin_Watts: were you looking for me yesterday ? | 16:28.17 |
| Robin_Watts | Morning rayjj. | 16:28.28 |
| | I was. And I still am. | 16:28.38 |
| rayjj | Robin_Watts: good morning | 16:28.39 |
| Robin_Watts | I have a fix for the clist stuff. | 16:28.43 |
| | Morning :) | 16:28.45 |
| rayjj | well, I'm here | 16:28.54 |
| Robin_Watts | There are 3 or 4 commits on robin/master for the clist stuff. | 16:29.13 |
| | of varying degrees of triviality. | 16:29.25 |
| rayjj | Robin_Watts: I'll have a look. Why 3 or 4 (just curious) ? | 16:29.32 |
| Robin_Watts | Could I trouble you for a review please? | 16:29.37 |
| | There are 4 in fact (I couldn't remember) | 16:30.01 |
| | cos I separated out the issues into different commits. | 16:30.09 |
| | I suspect that some of the commits may no longer be strictly necessary because other commits might stop them occurring. But the code is more correct with the fixes applied. | 16:31.20 |
| rayjj | Robin_Watts: first, I wouldn't bother with a commit (or comment) on a whitespace fix (I think that was your first one). The next two are fairly trivial and could be squashed with their comments if were up to me (or squashed into the one meaningful commit) | 16:35.06 |
| | Robin_Watts: so, the 4th one is the important one... looking it over ... | 16:35.51 |
| Robin_Watts | rayjj: Multiple commits cost nothing, and when reviewing the changes in future, it's better to have the complex commits uncluttered, IMAO. | 16:36.06 |
| rayjj | Robin_Watts: OK, so in do_save_page, you save the {cb}file before nulling them in the pcldev->page_info, but you don't save the names and restore them after list_print | 16:45.42 |
| Robin_Watts | do_save_page does save the names. | 16:46.07 |
| | and do_load_page restores them. | 16:46.18 |
| | The existing code did that already, so it may not show up in the diffs. | 16:46.43 |
| | /* Save the page information. */ | 16:47.13 |
| | strncpy(page->cfname, pcldev->page_info.cfname, sizeof(page->cfname)); | 16:47.15 |
| | strncpy(page->bfname, pcldev->page_info.bfname, sizeof(page->bfname)); | 16:47.16 |
| | (That's from do_save_page, just before my changes) | 16:47.24 |
| | /* We probably don't need to copy in the filenames, but do it in case something expects it */ | 16:47.44 |
| rayjj | Robin_Watts: oh, right -- it saves them in the saved_page struct | 16:47.45 |
| Robin_Watts | strncpy(crdev->page_info.cfname, page->cfname, sizeof(crdev->page_info.cfname)); | 16:47.45 |
| | strncpy(crdev->page_info.bfname, page->bfname, sizeof(crdev->page_info.bfname)); | 16:47.47 |
| | (That's from do_page_load) | 16:48.03 |
| rayjj | and restores them from there | 16:48.06 |
| | that | 16:48.12 |
| Robin_Watts | ew, bad robin. mismatching names. | 16:48.13 |
| | do_page_save/do_page_load would be better. Will fix. | 16:48.30 |
| rayjj | that's the saved_page struct on the heap during list_print | 16:48.32 |
| Robin_Watts | yeah. | 16:48.37 |
| | An 8k+ structure on the heap... | 16:48.58 |
| | on the stack, sorry. | 16:50.21 |
| rayjj | yeah, that's what I meant. gp_file_name_sizeof didn't used to be so large, so there a few places where it was kept on the stack. | 16:51.31 |
| Robin_Watts | It's something we could fix at some point, but the current commit doesn't make it any worse. | 16:53.39 |
| rayjj | I agree with changing the function names, but don't know how hard it would be for you since you have other commits after. Other than that, the (1 to 4) commits look fine | 16:53.45 |
| Robin_Watts | rayjj: I've changed them locally. | 16:53.59 |
| | Thanks. | 16:54.00 |
| | I'll push 'em, and we'll see whether that sorts marcosw's issues. | 16:54.27 |
| rayjj | Robin_Watts: yeah, the clist reader is pretty bad with stack use and needs some other dynamic allocations, IMHO | 16:54.32 |
| | Robin_Watts: you can cluster test it with extras=--saved-pages-test | 16:56.23 |
| Robin_Watts | tries, ta. | 16:57.10 |
| | HenryStiles: You about today? | 16:59.21 |
| HenryStiles | yes I am | 17:01.30 |
| Robin_Watts | In base/gscrdp.c, and line 565 we set code_rt | 17:05.03 |
| | and then we never use it. | 17:05.07 |
| | Is that for debugging purposes, or just an oversight? | 17:05.24 |
| | (cleaning some warnings) | 17:05.28 |
| | Never mind, I can tweak the code to avoid the warnings. | 17:07.25 |
| | rayjj: Should I expect diffs when I run like that? | 17:27.25 |
| | I get 2224 diffs, and 2 files SEGVing. | 17:27.42 |
| rayjj | Robin_Watts: well, the SEGV | 17:29.50 |
| Robin_Watts | I should expect the SEGV, but not the diffs? | 17:31.12 |
| rayjj | probably needs investigating. On diffs, it is not supposed to differ (AFAIK) but saved-pages-test forces clist mode | 17:31.14 |
| Robin_Watts | rayjj: Right. | 17:31.29 |
| | I'll cluster push a saved-pages-test before my changes. | 17:31.47 |
| rayjj | Robin_Watts: so if you run with extras=-dMaxBitmap=0 you should see the same diffs if they are due to banding | 17:32.04 |
| Robin_Watts | -dMaxBitmap=10000, right? 0 doesn't work ? | 17:32.26 |
| rayjj | Robin_Watts: 0 didn't used to work (there was a lower limit for some reason), but I changed/fixed that about 5+ years ago. | 17:33.16 |
| | but for git bisecting, you may still need 10000 | 17:33.30 |
| Robin_Watts | All of the diffs are for files that were unbanded, so that seems reasonable. | 17:33.57 |
| rayjj | Robin_Watts: commit 5b50a46 March 2012 removed the 10000 lower limit | 17:35.58 |
| | so almost 4 years ago (seemed like a while ago, anyway) :-) | 17:37.03 |
| HenryStiles | bbiam rebooting new linux distro woes | 17:44.07 |
| Robin_Watts | rayjj: So, pushing with --saved-pages-test just BEFORE my changes gives 8086 diffs, and 1 more SEGV. | 17:54.40 |
| | So, I'm going to claim that as a win. | 17:55.26 |
| | mvrhel_laptop: You here? | 17:58.13 |
| | mvrhel_laptop: For the logs then: in base/gxi12bit.c in image_render_icc16, we are at pains to calculate pcs. (Line 636ish). | 18:01.04 |
| | We then never use it. | 18:01.07 |
| mvrhel_laptop | I am here Robin_Watts | 18:01.11 |
| Robin_Watts | Should we be using it instead of penum->pcs a bit later. | 18:01.22 |
| | mvrhel_laptop: Ah, hi. | 18:01.25 |
| mvrhel_laptop | hold on let me look | 18:01.26 |
| | looks like that can go away. not sure why I had that there | 18:02.28 |
| Robin_Watts | ok, ta. | 18:02.34 |
| | Next question: | 18:02.41 |
| | need_decode = !((penum->device_color || penum->icc_setup.is_lab) && | 18:02.50 |
| | (penum->icc_setup.need_decode == 0) || | 18:02.52 |
| | gs_color_space_is_CIE(pcs)); | 18:02.53 |
| | That's need_decode = !((A && B || C) | 18:03.07 |
| | which I believe C brackets as !((A && B) || C) | 18:03.21 |
| | wow. I thought C just bound && and || left to right, but apparently && is higher precedence than || | 18:07.45 |
| HenryStiles | Robin_Watts, mvrhel_laptop: && is the analog of * and || is + so... But can't we simplify that terrible expression? | 19:18.40 |
| mvrhel_laptop | Robin_Watts: sorry for the drop off. Something is up today with my internet service | 19:25.23 |
| | Robin_Watts: let me clean that mess up. I need to think back why in the world I wrote such a thing | 19:27.33 |
| rayjj | mvrhel_laptop: it probably seemed like a good idea at the time ;-) | 20:29.31 |
| | mvrhel_laptop: did you find out about the compositor vs. copy_alpha sequencing, or do you want me to take a look (sorry about the background noise yesterday) | 20:30.59 |
| | mvrhel_laptop: it sounds like you have a simple file and a localized "trigger" | 20:31.26 |
| mvrhel_laptop | rayjj: I should be able to figure it out. I did not get much time to work on it today | 22:38.23 |
| | Forward 1 day (to 2016/01/01)>>> | |
IRC Logs