| | <<<Back 1 day (to 2017/09/21) | 20170922 |
| tor8 | Robin_Watts: #define IF_OVERPRINT_COMPONENT(k) if (1) | 13:34.04 |
| | could we not just have that define blank, so that we don't get an if-branch in debug code? | 13:34.24 |
| Robin_Watts | That would fail with IF_OVERPRINT_COMPONENT(k) { ... } else { .... } | 13:34.58 |
| tor8 | I did not see any elses | 13:35.26 |
| Robin_Watts | nor do I, but if I edit the file in future... | 13:36.06 |
| | It would be poor style to change it, I feel. | 13:36.27 |
| tor8 | in draw-point.c at line 2531 (in commit 38c85780c7) you include "paint-glyph.h" twice | 13:37.59 |
| | without changing the defines | 13:38.04 |
| Robin_Watts | paint-glyph clears the defines. | 13:38.34 |
| tor8 | would it not be clearer to have the no-defines include before then? it looks like a copy-paste mistake as it is :) | 13:39.05 |
| Robin_Watts | tor8: yes, I could support moving the no defines one to the top. | 13:39.22 |
| tor8 | I wonder how much space we can save with the FZ_ENABLE_SPOT_RENDERING (and other FZ_ENABLE plotters) and if it's worth the ifdef maintenance overhead | 13:41.19 |
| | there are going to be configurations which get no testing, and that concerns me | 13:42.05 |
| | and I don't feel like adding more testing... | 13:42.23 |
| Robin_Watts | tor8: I feel you. | 13:42.49 |
| tor8 | fz_overprint op = { 0 } should have another set of braces around initialization of subobject: op = { { 0 } }. | 13:44.46 |
| | unused function 'template_span_with_mask_N_general_op' | 13:44.54 |
| Robin_Watts | yes, to both of those. | 13:52.16 |
| tor8 | Robin_Watts: modulo those comments, up to and including "Don't apply default decode array to ICC Lab image data." LGTM | 14:32.48 |
| Robin_Watts | Thanks. | 14:32.58 |
| QML_Noob | Does MuPDF have any support for QML? | 14:38.38 |
| Robin_Watts | No. What's QML? :) | 14:42.07 |
| | Qt Modelling Language? | 14:42.39 |
| | tor8: fixes pushed. | 14:57.50 |
| | tor8: So... the moment I push any more of these fixes, we start to see diffs. | 15:08.25 |
| | I think I'd like to push them all at once at the end so we can do a sensible diff to check regressions. | 15:09.22 |
| tor8 | Robin_Watts: right, so where is the cutoff where we start getting diffs? | 15:16.55 |
| Robin_Watts | The overprint commit you just reviewed. next one to be pushed. | 15:17.24 |
| | I've pulled a load of changes back onto robin/master. | 15:17.32 |
| | These are independent fixes. | 15:17.41 |
| tor8 | Robin_Watts: the first two on robin/master LGTM | 15:19.50 |
| | not sure about the 'name too long' one | 15:19.57 |
| | it will return a non-zero-terminated string on overflow in the no throw_if_long case which can crash pdf_token_from_keyword | 15:21.12 |
| Robin_Watts | Ah, ok. | 15:21.37 |
| tor8 | the last two are okay as well | 15:25.26 |
| | but not the "name too long" one | 15:25.30 |
| | have you got a file for that one, I can see if I can tweak it somehow | 15:26.07 |
| Robin_Watts | yeah. | 15:29.13 |
| | 193.pdf in my casper home dir | 15:29.25 |
| tor8 | maybe a better approach is to skip to the next delimiter/whitespace and resume from there? | 15:31.42 |
| Robin_Watts | yeah. | 15:32.40 |
| | This is for repair, so we'd either be looking for "23 0 obj" or "trailer" or "xref" or something, I think. | 15:34.02 |
| | so the next whitespace should be fine, I think. | 15:34.24 |
| tor8 | Robin_Watts: fix for that on tor/master | 15:40.33 |
| Robin_Watts | tor8: Can we push that fix into pdf_lex_no_string? | 15:41.13 |
| | I guess it's OK there, but it would be nicer to avoid throwing the error rather than catching it and coping. | 15:42.24 |
| tor8 | the error is thrown deep down in the common lexer | 15:42.45 |
| | which is shared between the repair and regular parser | 15:42.56 |
| | it's throwing from lex_name | 15:43.26 |
| | and this would catch other syntax errors and try to recover as well | 15:43.41 |
| Robin_Watts | yes, i ws thinking of chnging the call to lex_name on the default clause of pdf_lex_no_string... | 15:48.22 |
| | but what you have is better. | 15:48.30 |
| tor8 | Robin_Watts: okay, I rebased on top of your robin/master (minus the 'too long error' one) on tor/master | 15:50.43 |
| | those are good to go from me | 15:50.51 |
| Robin_Watts | cool. go for it. | 15:50.57 |
| tor8 | and on robin/spots, up to "Don't apply default decode array to ICC Lab image data." is LGTM | 15:51.18 |
| Robin_Watts | yeah, I'll rebase onto yours and then push. | 15:51.34 |
| | oh, wait, no I won't, for the reasons stated earlier :( | 15:51.59 |
| | I pushed your master | 15:52.49 |
| tor8 | "Fix overprint color component detection." should also be good | 15:52.59 |
| | in "Look for changes to Default colorspaces in XObjects." why do you have a try/catch-ignore for the second fz_set_default_colorspaces? | 15:55.52 |
| Robin_Watts | hmm. That was Michaels originally I think. | 15:57.21 |
| tor8 | it looks like his code :) | 15:57.37 |
| | get_default_cs looks like a mvrhel name too. pdf_load_default_colorspaces_imp would be the tor name | 15:58.19 |
| | and I'd also have some fz_warn instead of empty catch statements | 15:58.33 |
| Robin_Watts | I can't see the point of that. | 15:58.45 |
| | (the try/catch/ignore) | 15:58.54 |
| | the only error we can realistically get is oom when we go to the list device. | 15:59.06 |
| | and that should be a failure. | 15:59.11 |
| tor8 | yeah, I'd just nuke it and let the error propagate as a failure as it ought to | 15:59.17 |
| | "Fix separation finding code." LGTM but ought to be squished down to the offending commit if that one is still unpushed | 15:59.33 |
| | "Only allow overprint when color spaces are subtractive." LGTM | 15:59.50 |
| Robin_Watts | will fix. | 15:59.52 |
| tor8 | in "Avoid using overprinting routines unnecessarily." half of the commit removes the if (eop != NULL) with if (fz_overprint_required()) and the second half has if (eop != NULL && fz_overprint_required()) | 16:01.16 |
| | since fz_overprint_required already checks eop for null, the second half of the commit ought to follow the style of the first half | 16:01.34 |
| Robin_Watts | Oh! | 16:01.48 |
| | I see why we have the try/catch. | 16:01.57 |
| | It's in an fz_always block. | 16:02.02 |
| | so we want to ensure that do all the rest of the always block. | 16:02.20 |
| tor8 | a throw from the always block jumps to the catch block | 16:02.21 |
| Robin_Watts | tor8: right, but it skips the rest of the always block, so we'd leak. | 16:02.36 |
| tor8 | move that block to the end so it dosen't matter? | 16:03.13 |
| | hm, there's a lot of stuff going on in that always | 16:03.49 |
| Robin_Watts | The 'postpone' code should be moved into the catch block. | 16:04.02 |
| tor8 | won't we have multiple such bugs, if anything in that always block throws | 16:04.51 |
| | it looks like it has always been fishy :( | 16:04.57 |
| Robin_Watts | tor8: if several things in that always block throws, then we'll only get the last ones error reported. | 16:05.20 |
| | but we WILL tidy up properly, and *an* error will be reported. | 16:05.32 |
| tor8 | not if pdf_grestore can throw | 16:06.04 |
| | ah, there's a comment inside pdf_grestore that mentions it may never throw | 16:06.20 |
| | I think that function is in need of a spring cleaning sometime | 16:06.34 |
| | so change the blank catch for the set_default_colorspaces to do the same postpone the problem thing | 16:07.35 |
| Robin_Watts | have done. see robin/spots | 16:07.44 |
| tor8 | Robin_Watts: fab. | 16:08.37 |
| | and if you could s/get_default_cs/pdf_load_default_colorspaces_imp/ I'll give that one a LGTM | 16:09.15 |
| | and if you add some fz_warn I'll put a cherry on top! | 16:09.27 |
| Robin_Watts | Where did you want the fz_warn ? | 16:14.13 |
| | in get_default_cs (as was?) | 16:14.21 |
| | or in the fz_always? | 16:15.59 |
| tor8 | in get_default_cs in the empty fz_catch a fz_warn("could not load DefaultGray") etc | 16:18.16 |
| | biab, dinner | 16:18.30 |
| Robin_Watts | tor8: The spec says we should ignore broken stuff. | 16:19.13 |
| | so warning there could be seen as contrary to that. | 16:19.25 |
| | Ok, fixed. Updated robin/spots for when you get back (or tomorrow) | 16:24.15 |
| | mvrhel_laptop: Morning. | 16:33.36 |
| | Can I borrow you for a mo? | 16:33.41 |
| | Forward 1 day (to 2017/09/23)>>> | |
Log of #mupdf at irc.freenode.net.