| | <<<Back 1 day (to 2020/09/23) | Fwd 1 day (to 2020/09/25)>>> | 20200924 |
| clam | sebras: you got yourself a french alter ego? | 01:03.34 |
| sebras | clam: I'm confused myself, maybe it is me?! | 04:29.14 |
| ator | Robin_Watts_: one fixup on tor/graftpage (updating mupdf_native.h) | 09:59.07 |
| | Robin_Watts_: I think we're going to have to skip the fax commit from the release, we want to work some more on it | 09:59.25 |
| sebras | ator: agreed about the fax commit. | 09:59.38 |
| Robin_Watts_ | ator: OK. I'll grab that in a mo. | 09:59.48 |
| ator | is there anything else we should get in, or are we good to do a RC now? | 09:59.49 |
| sebras | ator: I don't think the fax commit is correct. | 09:59.53 |
| Robin_Watts_ | ator: THe jpx fix! | 10:03.09 |
| | That's for a security fix, so it really ought to go in. | 10:03.26 |
| ator | oh yeah. | 10:04.22 |
| | thought that had gone in already :) | 10:04.36 |
| | tor/master has an updated CHANGES, could you give that a go over and see if I missed anything, or phrased something awkwardly | 10:05.21 |
| Robin_Watts_ | I looked the other day, and it looked good. Have you changed it? | 10:08.31 |
| ator | I have not | 10:08.41 |
| malc_ | what's Tesseract? ... some kind of contra OCR? | 10:16.54 |
| Robin_Watts_ | http://tesseract-ocr.io | 10:22.50 |
| | Bah. https://github.com/tesseract-ocr | 10:23.21 |
| | It's an OCR engine. | 10:23.29 |
| malc_ | thanks | 10:25.34 |
| Robin_Watts_ | sebras: ping | 14:06.21 |
| | ator: OK, I've pulled your fixup into my graftpage commit. | 14:08.14 |
| | Can I push that to master? | 14:08.19 |
| | That just leaves the jpx one that we're waiting on sebras for. | 14:08.33 |
| | sebras: Are you going to get a chance to look at it in the next 24 hours or so? If not, I think we should push it anyway and go for the release | 14:09.08 |
| ator | Robin_Watts_: push away | 14:29.19 |
| Robin_Watts_ | pushed. just waiting for the jpx one, but I have no idea a) where sebras is in his sleep cycle, or b) what pressures he is under from other bits of the company to look at stuff. | 14:30.37 |
| ator | I suspect he's away for dinner | 14:31.17 |
| | and busy with the app stuff as always | 14:31.38 |
| | I'm heading out for a few hours, while the weather is nice | 14:35.56 |
| sebras | I've returned from dinner. | 15:03.48 |
| | previously I have been looking at the fax thing to see if I can see a quick fix for it, but what I have is not yet ready. | 15:04.10 |
| Robin_Watts_ | sebras: Are you going to have a chance to look at the jpx thing within the next 24 hours? | 15:59.49 |
| | ator: 2 more commits on robin/master | 15:59.58 |
| fredross-perry | ator: please have a look at https://bugs.ghostscript.com/show_bug.cgi?id=702936. Somewhat urgent customer issue. | 16:01.03 |
| Robin_Watts_ | fredross-perry: That's my code. I'll look now. | 16:02.34 |
| fredross-perry | thanks. | 16:02.53 |
| Robin_Watts_ | I think actually the correct fix is : if (use == NULL) return NULL; | 16:05.06 |
| | cos otherwise we may attempt to strcmp(whatever, use) later on, and that'll crash. | 16:05.46 |
| | fredross-perry: Fix on my repo. I'll get ator to review it when he comes back. | 16:07.51 |
| | ator: 3 commits on robin/master then. | 16:07.58 |
| fredross-perry | grazi | 16:12.07 |
| | lmk if this makes it into the current release | 16:18.55 |
| ator | Robin_Watts_: I'll need to look over "Bug 702566: Avoid leaking run_obj from pdf_add_cid_font_widths" a bit more to convince myself | 16:39.34 |
| | the others LGTM | 16:39.37 |
| Robin_Watts_ | ator: Please do. | 16:40.25 |
| | fredross-perry: pushed. So, yes, it will be in the release. | 16:43.30 |
| ator | Robin_Watts_: okay, that one LGTM too. | 16:48.14 |
| Robin_Watts_ | ator: thanks. | 16:48.40 |
| sebras | I have been looking at the patch now. | 16:49.55 |
| Robin_Watts_ | cool. Hopefully it makes more sense than the last one. | 16:53.29 |
| | No stupid casting. | 16:53.39 |
| sebras | I need hand holding to get throught this one too. | 16:54.56 |
| | http://git.ghostscript.com/?p=user/robin/mupdf.git;a=blob;f=source/fitz/load-jpx.c;h=bdf50a4e99a25621f6a3becf4efd6ebfce241bef;hb=1becc24e1b64f0accd791f6c9d5648ef17adb1dd#l722 | 16:54.57 |
| | vad happens on lines 722-741 and 766-771 | 16:55.18 |
| Robin_Watts_ | vad? | 16:55.47 |
| sebras | s/vad/what/I-accidentally-used-swedish. :) | 16:56.02 |
| Robin_Watts_ | Each time around the y loop, we copy out 1 source pixel to (possibly) multiple destination pixels. | 16:56.33 |
| | No, let me start again. | 16:56.47 |
| sebras | start by explaining 722-727 | 16:57.01 |
| | I think this has to do with overflow. | 16:57.11 |
| Robin_Watts_ | It's not. | 16:57.18 |
| sebras | 743-748 seem to be similar but horizontally | 16:57.26 |
| Robin_Watts_ | To explain that, I have to explain it inside out. | 16:57.27 |
| sebras | ok. | 16:57.35 |
| Robin_Watts_ | So start by looking at lines 749 onwards. | 16:57.41 |
| | That copies a single pixel to (possibly) multiple pixels. | 16:57.57 |
| | In general we copy 1 source pixel to cdx x cdy pixels. | 16:58.33 |
| sebras | one subsampled source pixel to the destination area of pixels, yes. | 16:58.35 |
| | yes | 16:58.51 |
| Robin_Watts_ | but we have to allow for that block of pixels being partially off the edge of the image. | 16:59.00 |
| sebras | y | 16:59.06 |
| Robin_Watts_ | hence we check for dxmin < 0 and cut down dymin as appropriate. | 16:59.23 |
| | dxwid as appropriate, sorry. | 16:59.40 |
| | and we check for x + dxwid being too wide, and also cut down dxwin as appropriate. | 17:00.05 |
| sebras | what happens on line 767 when you cut down? | 17:00.33 |
| | because you increase dxwid..? | 17:00.42 |
| ator | by a negative number | 17:00.57 |
| Robin_Watts_ | dxmin < 0, so dxwid += dxmin means it gets smaller. | 17:01.02 |
| sebras | oh, yes. | 17:01.04 |
| ator | I love how we've corrupted Robin to use comma expressions :) | 17:01.41 |
| Robin_Watts_ | I got that from gs, I think. | 17:02.01 |
| sebras | he wrote this code in anger. I can see it. ;) | 17:02.07 |
| Robin_Watts_ | It's a nice way to avoid { ... } :) | 17:02.11 |
| ator | Robin_Watts_: exactly! | 17:02.26 |
| Robin_Watts_ | So, the little loop before the x loop is basically saying "Skip over source pixels, until we're at least partly within the clipping area" | 17:02.54 |
| | oox + cdx = the start of the next area to copy. | 17:04.14 |
| | so if that is <= 0, we never need any of the pixels that will be output from the current source pixel. | 17:05.18 |
| | sebras: Yes/No/Retry? | 17:06.01 |
| sebras | is oox + cdx <= 0 then the entire output area for the subsampled source pixel in its entirety to the left of the destination output image. | 17:06.53 |
| | s/is/if/ | 17:06.57 |
| Robin_Watts_ | Yes. | 17:07.15 |
| | Hence we can just skip to the next one. | 17:07.28 |
| sebras | and potentially the entire component area might reside the outside of the image area, hence you start with x = cw. | 17:08.00 |
| | and go to 0 | 17:08.04 |
| Robin_Watts_ | x going from cw to 0 is just the same as x going from 0 to cw - we never use the value of 'x'. | 17:08.45 |
| | but going from cw to 0 is more efficient. | 17:09.01 |
| sebras | yes, but the range of that loop is the width of the component plane. | 17:09.20 |
| Robin_Watts_ | Yes. | 17:09.25 |
| sebras | that is what I was trying to understand. | 17:09.32 |
| Robin_Watts_ | So, the 2 loops for y are the same deal. | 17:09.46 |
| sebras | hm and ox (and by extension dxmin) can be < 0 if afe_mul32(ctx, comp->x0, cdx) - jpx->x0 yields a negative value. | 17:10.57 |
| Robin_Watts_ | Yes. | 17:11.08 |
| | It may be that that never happens in "well formed" JPX files, but the format appears to allow for it. | 17:11.38 |
| | (indeed, we have a fuzzing bug that triggers it) | 17:12.10 |
| sebras | I don't know what parts of this is due to the specified format and what parts are due to openjpegs implementation. | 17:12.13 |
| | and (void)safe_mla32(ctx, ch, cdy, oy); sees to it that if the component area overflows an int32_t we error out (instead of writing wildly) | 17:13.36 |
| Robin_Watts_ | right. | 17:13.47 |
| Robin_Watts_ | steps away to feed/walk dogs. bbs. | 17:17.08 |
| sebras | with these explanations this looks reasonable. don't ask me whether some expression can overflow under some circumstances, e.g. dxmin * comps or something. | 17:19.31 |
| Robin_Watts_ | 0 <= dxmin < cdw | 17:20.40 |
| | and we check that cdw*comps doesn't overflow etc. | 17:20.54 |
| malc_ | why this sudden desire to make a release? | 17:20.54 |
| Robin_Watts_ | malc_: We make 2 releases a year. | 17:21.04 |
| sebras | malc_: and this release has been planned for a while. | 17:21.25 |
| malc_ | ah, so it just seems sudden for uninitiated.. thanks and/och tack | 17:22.03 |
| sebras | Robin_Watts_: that should be cw, right? | 17:24.52 |
| | not cdw. | 17:24.57 |
| | what I tried to say was more that I don't know if this code introduces other overflow/underflow situations. | 17:26.25 |
| | ator: is this good enough of an LGTM to push the commit and go? | 17:27.43 |
| | you did mention you have also looked at this commit, so..? | 17:27.54 |
| Robin_Watts_ | sebras: Yeah, ator has lgtm'd it. | 17:47.12 |
| | sebras: Thanks. | 17:47.14 |
| | ator, sebras: Just looking at bug 702543. | 18:04.29 |
| | It's a file with an annotation in it with /F 4 | 18:04.40 |
| | 4 = Print | 18:05.12 |
| | (PDF 1.2) If set, print the annotation when the page is printed. If clear, never print the annotation, regardless of whether it is displayed on the screen. This can be useful, for example, for annotations representing interactive pushbuttons, which would serve no meaningful purpose on the printed page. (See implementation note 83 in Appendix H.) | 18:05.13 |
| | KK is claiming that when calls with Usage="Print", we shouldn't render that annotation, because it has no appearance stream. | 18:05.53 |
| | BUT we've synthesised an appearance stream for it. | 18:06.58 |
| sebras | looks at the bug. | 18:13.07 |
| Robin_Watts_ | I think it's going to require a rethink of how we handle synthesized appearance streams. | 18:16.31 |
| | which I think is low priority. | 18:16.48 |
| sebras | well. what does printing mean for a viewer? | 18:17.17 |
| | oh, some one has implemented pdf_run_page_with_usage()! either I've never known or I have forgotten about it complete. | 18:20.54 |
| | ly | 18:20.57 |
| | Robin_Watts_: couldn't we decide to bail in pdf_run_annot_with_usage() if the annotation doesn't have the print flag and the usage is print? | 18:22.46 |
| | the spec doesn't say anything about appearance streams. | 18:23.04 |
| Robin_Watts_ | It *does* have the print flag, and the usage is print, and yet acrobat doesn't print it. | 18:23.17 |
| | I implemented pdf_run_page_with_usage :) | 18:23.41 |
| sebras | did I review it? I might have! | 18:23.59 |
| Robin_Watts_ | usage will typically be "View" or "Print". | 18:24.07 |
| | but it's marvellously loosely defined in the spec. | 18:24.27 |
| | Indeed, the spec doesn't say anything about appearance streams. | 18:24.43 |
| sebras | I can't test printing in acroread. | 18:25.25 |
| Robin_Watts_ | print to a pdf printer? | 18:26.34 |
| sebras | I'm looking up how to do that from within wine. | 18:28.27 |
| | aha! printer-driver-cups-pdf did the business. | 18:29.25 |
| | Robin_Watts_: ok, so when I print a.pdf in acroread 9.5.5 via a pdf printer I only see the text "attachment", the annotation is gone. | 18:29.57 |
| | seems to me like acroread respects the flag..? | 18:30.11 |
| Robin_Watts_ | right, that's the OP's claim. | 18:30.15 |
| | ??? | 18:30.17 |
| | The flag for the annot is 4. | 18:30.36 |
| | That's "Include this annotation when printing". | 18:30.44 |
| sebras | maybe I confused set/cleared in the spec. let me read it again. | 18:31.11 |
| Robin_Watts_ | I'd love for me to simply be misreading something... | 18:31.28 |
| sebras | pdfref17 page 613 mentions that the normal appearance stream is the one used for printing. | 18:33.03 |
| | no, you read it right unfortunately. | 18:33.09 |
| | so then, if I have mupdf recreate the appearance stream, save the pdf and try to print _that_ saved pdf to a pdf printer in acroread... | 18:36.13 |
| | then it still doesn't print the annotation. | 18:36.48 |
| Robin_Watts_ | That's surprising. | 18:37.03 |
| | Does the saved PDF have the appearance stream in? | 18:37.15 |
| sebras | yes. and I have mutool showed the apperance stream. it is there. | 18:37.24 |
| Robin_Watts_ | So Acrobat is choosing not to print that annotation... for some random reason other than the presence/absence of the appearance stream. | 18:38.01 |
| sebras | let me manipulate /F 4 to /F 0 and see what acroread does. | 18:38.02 |
| Robin_Watts_ | I wonder if acrobat NEVER prints file annotations? | 18:38.24 |
| sebras | it turns out that /F 0 also caused acroread to not print it. | 18:38.50 |
| | you're probably right, file annotations might be a special case. | 18:39.25 |
| fredross-perry | Robin_Watts_ : thanks | 19:11.01 |
| | Robin_Watts: can you please apply that patch to the branches that Android and ios are using? Or remind me how to do it? | 20:24.37 |
| Robin_Watts_ | fredross-perry: I have no clue what branches android and ios are using. | 23:01.36 |
| fredross-perry | Nor do I. But I can tell you commit numbers. | 23:01.57 |
| | I did not find tags/heads that matched them, however. | 23:02.15 |
| | This is what git submodule says: | 23:03.58 |
| | a38556c13b132c2e3b275fac0d2e2f0f32895036 epage/platform/android/apps/solib/mupdf-android-fitz/libmupdf (1.4-ios-4351-ga38556c13) | 23:03.59 |
| | 97390684675e98db4e89b95c2756b815eeb3d6dc epage/platform/ios/smart-office-nui/sodk/mupdfdk-subtree/mupdfdk/mupdf (1.4-ios-4420-g973906846) | 23:03.59 |
| | I can push the change and make new branches, but that got me into trouble last time. :-) | 23:08.10 |
| | <<<Back 1 day (to 2020/09/23) | Forward 1 day (to 2020/09/25)>>> | |
Log of #mupdf at irc.freenode.net.