| <<<Back 1 day (to 2017/10/15) | 20171016 |
Robin_Watts | tor8, sebras: bug 698605 - has anyone tested whether that's fixed in the current code or not? | 10:51.23 |
tor8 | Robin_Watts: it still crashes | 10:52.12 |
| but it's deep in the new xref code so haven't had time to dig into it | 10:52.31 |
Robin_Watts | ok. I'll try to have a look later if no one beats me to it. | 10:52.39 |
tor8 | I'll take poke for a while | 10:52.54 |
| I'll let you know if I run into a wall | 10:52.59 |
| Robin_Watts: it's an integer overflow in pdf_read_new_xref_section, i0+i1 becomes negative | 11:10.19 |
Robin_Watts | yeah, it says that in the bug report. That's as far as I got reading it :) | 11:10.45 |
tor8 | Robin_Watts: why did you comment out the 'xref stream has too many entries' check? I don't see any mention of that in the commit message or a comment around it. | 11:11.05 |
Robin_Watts | Can we fix it by "if (i0 + i1 < 0) fz_throw.... ? | 11:11.10 |
tor8 | commit e767bd783d91ae88cd79da19e79afb2c36bcf32a | 11:11.12 |
Robin_Watts | let me check. | 11:11.24 |
tor8 | there's a i0 < 0 and i1 < 0 but no i0+i1 < 0 check though | 11:11.38 |
Robin_Watts | tor8: That looks accidental :( | 11:14.50 |
| Ah, | 11:15.22 |
tor8 | there's a fix and an extra warning check for %PDF-X.Y version number on tor/master | 11:15.28 |
Robin_Watts | It looks like pdf_xref_find_subsection attempts to cope now. | 11:15.59 |
| tor8: Those 2 lgtm. | 11:18.41 |
| tor8: There are a few small commits on the end of robin/spots still awaiting review before I can push the whole branch. | 11:19.26 |
| And the first commit on robin/master is required too (just adds pdfsign.c to the MSVC project) | 11:21.27 |
tor8 | Robin_Watts: "Add missing pdfsign.c" LGTM | 11:39.59 |
Robin_Watts | tor8: thanks. | 11:40.14 |
tor8 | Robin_Watts: I vaguely recall having LGTM'd up to "Add FZ_ENABLE_SPOT_RENDERING define" before | 11:42.17 |
| will look at the rest now ... it's that or writing docs for android... | 11:42.40 |
Robin_Watts | tor8: sounds plausible. Thanks. | 11:42.47 |
tor8 | Robin_Watts: I do have a few comments and requests written down, but those are cleanups we can do afterwards | 11:43.11 |
Robin_Watts | tor8: sure. | 11:43.18 |
tor8 | most important (to me) would be adding a colorspace-type enum, which I've mentioned before | 11:43.52 |
| to remove both the 'if (n == 3) ...it must be RGB...' type code, and the awkward fz_is_colorspace_foo implementations | 11:44.31 |
Robin_Watts | tor8: yeah. We can look at that as soon as I get all this in. | 11:45.15 |
tor8 | fz_paint_pixmap_op -> fz_paint_pixmap_with_overprint might be less cryptic | 11:45.42 |
| 'Look for changes to Default colorspaces in XObjects.' has a weird camelCase nestedDepth argument in the header file | 11:47.52 |
| those two niggles aside, up to and including "Fix fz_paint_pixmap alpha blending." (commit 64918f366f1119adf2d2cb62372f4e1cb2893498) LGTM | 11:51.07 |
Robin_Watts | Ah, nestedDepth has always been there, but I will fix. | 11:58.53 |
tor8 | I'm cleaning up the next one (mudraw: Add ability to specify icc profile for target colorspace.) and will provide a patch for it | 11:59.38 |
Robin_Watts | ok, those 2 niggles fixed. | 12:02.29 |
tor8 | Robin_Watts: cool. a few commits on tor/spots | 12:20.02 |
Robin_Watts | Will pull those in. | 12:20.17 |
tor8 | one that should be squished into michael's 'mudraw: Add ability...' commit | 12:20.20 |
| hopefully it doesn't conflict down the line | 12:22.00 |
Robin_Watts | tor8: One is labelled 'WIP:'. One is simply called 'x' ? | 12:23.10 |
tor8 | forget the 'x' one | 12:23.25 |
| the 'WIP' one should be squished into the one preceding it | 12:23.33 |
| but I might have a further cleanup of that | 12:23.45 |
| I just checked how you iterated the permitted_cs before | 12:23.54 |
Robin_Watts | I'll wait then. | 12:24.19 |
tor8 | give me a minute and I'll push an updated one | 12:24.21 |
| okay, pushed. | 12:25.44 |
Robin_Watts | tor8: yes, it conflicts with later ones. | 12:38.49 |
tor8 | rats. | 12:38.54 |
| I tried to keep it minimal :( | 12:39.11 |
Robin_Watts | tor8: Let me start again. | 12:39.19 |
| tor8: Did you put that function in a headerfile anywhere? | 12:47.11 |
| My bad. | 12:47.54 |
| ok, robin/spots updated. | 12:53.55 |
tor8 | Robin_Watts: in the comment to fz_new_draw_device_with_proof you mention 'proof_cs: Color space to render to prior to mapping to color space defined by pixmap.' | 12:57.45 |
| shouldn't that be 'to map through before mapping to color space defined by pixmap' rather than render? | 12:58.11 |
Robin_Watts | tor8: Hell if I know. | 13:44.19 |
tor8 | cause if we render to a proof colorspace, and then convert to the final ... why do we even need the 'proof' colorspace in the draw device? | 13:45.05 |
| just render to a pixmap with the proofing cs and then convert using fz_convert_pixmap | 13:45.35 |
Robin_Watts | tor8: We should confer with Michael for final wording, I suspect. | 13:45.35 |
tor8 | but I *think* without understanding all the details that the proofing colorspace is an intermediate step in the conversions | 13:46.11 |
| go from source -> proof -> destination to make sure clipping and gamut compression going from source->proof is visible in the destination pixmap | 13:46.55 |
Robin_Watts | I suspect you're right, but my brain is too frazzled to be sure. | 13:47.21 |
tor8 | but if it is just rendering to an intermediate proof-cs pixmap inside the draw device, I'd advocate simplifying the API and doing that outside of the draw device | 13:48.02 |
Robin_Watts | tor8: We only use 1 extra group. | 13:48.41 |
| be it for overprint, or otherwise. | 13:48.52 |
| and it's far better to do that internally to the device rather than externally. | 13:49.07 |
tor8 | Robin_Watts: I don't immediately see why that would be the case, but I'll defer to your experience having actually worked with this stuff :) | 13:55.42 |
| "Fix overprinting simulation in RGB output with no spots." adds a check for what I assume is CMYK in push_group_for_separation that is not mentioned in the commit message | 13:57.56 |
Robin_Watts | tor8: Is too. | 14:07.01 |
| The last 2 paragraphs of the commit message. | 14:07.12 |
tor8 | Robin_Watts: ah, I misread the if-claus. "Not needed" not "Needed"... | 14:40.11 |
| Forward 1 day (to 2017/10/17)>>> | |