| <<<Back 1 day (to 2016/04/11) | 20160412 |
tor8 | Robin_Watts: wouldn't 'policy' or 'intercept' be a more apt name for your proposed 'heuristics' function pointer table? | 08:40.20 |
| though, I can't help but think a few more device hints would be more helpful and easier to use for integrators | 08:41.21 |
Robin_Watts | tor8: "policy", or "tuning" might work, yes. | 08:55.48 |
| I'm not sure that device hints give the required control. | 08:55.59 |
| (And we don't have a device in the place where I'd need some of them, I think) | 08:56.44 |
kens | Hmm, I can't build MuPDF from the current golden on WIndows. | 08:57.46 |
tor8 | Robin_Watts: ah, yes. those functions are helpers called by the device itself. | 08:58.03 |
kens | Its failing to open "../../thrdparty/....." | 08:58.11 |
tor8 | Robin_Watts: or just a 'configuration' struct with various global settings | 08:58.20 |
Robin_Watts | kens: What target ? | 08:58.31 |
kens | Umm win64 I htink, let me check | 08:58.48 |
| No win32 | 08:58.59 |
| debug | 08:59.06 |
tor8 | kens: did you 'git submodule update --init'? | 08:59.14 |
kens | Building entire solution | 08:59.15 |
tor8 | Depends on whether you've built since we added 'harfbuzz' to the project or not. | 08:59.35 |
kens | No, I forgot, I'll do that now | 08:59.37 |
| I defintely haven't no | 08:59.45 |
| Looks like this will take a minute or two | 09:00.30 |
| Hmm, "Error your local changes to the following files would be overwritten by checkout (all kinds of jbig files) | 09:03.45 |
| "unable to checkout <hash> in submodule path 'thrdparty/jbig2dec' | 09:04.10 |
| Any idea how I cna fix that ? I certainly haven't changed jbig2dec myself | 09:04.55 |
Robin_Watts | kens: cd thirdparty/jbig2dec | 09:05.08 |
| git stash && git stash drop | 09:05.16 |
kens | OK | 09:05.20 |
| I did that already for my conversion of the solution :) | 09:05.44 |
| submodule init again ? | 09:06.00 |
Robin_Watts | yeah. | 09:13.03 |
kens | No luck :( | 09:15.25 |
| Now its complaining that changes to autogen.sh, jbig2_halftone.h, memento.c and memento.h woudl be overwritten | 09:15.51 |
Robin_Watts | tor8, paulgardiner: So, there is a problem with mupdf 1.9rc1 for android. | 09:16.25 |
| Sebras spotted it last night. | 09:16.32 |
| If you open a 3+ page pdf, and add an ink highlight on page 1, the highlight appears until you click the tick to commit it to the document. | 09:17.12 |
| Then it vanishes. | 09:17.16 |
| No amount of zooming or panning that page can make it appear. | 09:17.35 |
| If you flip forward 2 pages, then back again, it appears. | 09:17.45 |
fredross-perry | Robin_Watts: Do we know if this happens on iOS as well? | 09:18.03 |
Robin_Watts | I do not. | 09:18.09 |
| I've traced the code a bit and discovered that: | 09:18.34 |
| addInkAnnotationInternal is being called. | 09:19.12 |
| The last thing that does is to call dump_annotation_display_lists(); | 09:19.24 |
| That sets page->annot_list to NULL. | 09:19.33 |
| We then get asked for an updatePageInternal. | 09:20.14 |
| That spots that annot_list == NULL, and tries to regenerate the list. | 09:20.29 |
| That runs through the annots on the page, and does indeed find that there is one. | 09:20.52 |
| Nothing actually appears in the displaylist though. (The displaylist length remains 0). | 09:21.26 |
| That makes me wonder if the matrix or bbox for the annotation are off the page. | 09:22.10 |
| tor8: Does this sound like something related to the annotation problems you were seeing when you tweaked the device interface recently (beginpage/endpage) | 09:24.15 |
| ? | 09:24.17 |
| It looks to me that although we call fz_run_annot, we never actually call pdf_run_annot in the case where nothing appears. | 09:25.45 |
fredross-perry | I see that iOS does not build currently. Working on that. | 09:26.55 |
tor8 | Robin_Watts: the beginpage/endpage errors in the stuff I tweaked I think were due to the pdfwrite device coordinate space flipping matrices gone wrong | 09:28.57 |
jogux | fredross-perry: I got as far as removing errant semi-colons and changing pdf_write_document -> pdf_save_document. but then hit something else that I forget I think. | 09:29.03 |
fredross-perry | yes, itâs pointing to some png files at the old location of the android viewer. | 09:29.40 |
kens | Robin_Watts : deleted thirdparty, did a submodule update, and it will no attempt to build, but throws *many* fatal error 'cannot update program database...\libmupdf\vc90.pdb', which is weird as the file existed. Deleted that and built just 'mupdf' and it got further | 09:30.52 |
| pdf-clean-file.c is throwing a error 'PDF_NAME_Last' undeclared identifier | 09:31.19 |
jogux | fredross-perry ah, yes.That's in. I think you'd decided to copy those into the iOS folder? I remember a conversation about it but I guess it never happened. | 09:31.21 |
| s/in/it/ | 09:31.27 |
fredross-perry | yes that was the plan. I think I did that but it got lost in the shuffle. | 09:31.55 |
tor8 | kens: delete the 'generated' folder... that one doesn't seem to trigger regenerates properly with the windows build system | 09:31.56 |
kens | More errors of the form 'PDF_Name_****' undeclared identifier | 09:31.59 |
| tor8 right,m thanks | 09:32.05 |
| sigh... 2 suceeded, 7 failed | 09:33.11 |
| same error PDF_NAME_Last undeclared identifier | 09:33.52 |
| I'll delete the object and generated and start from clen | 09:34.03 |
tor8 | kens: when in doubt, I delete everything but the .git and 'git reset --hard' :) | 09:34.19 |
kens | THat gets cross about the untracked files because I'm using a newer versionof VS and had to convert the solution | 09:34.55 |
chrisl | Given you have nothing in the tree to worry about, I'd close VS, and do "git clean -x -f -d". Then update everything, and try again | 09:36.02 |
kens | Yeah I could do that, got 3 success and 2 failed ths time :-( | 09:36.21 |
fredross-perry | jogux: in the morning | 09:36.28 |
kens | Its refusing to build libmupdf basically | 09:36.34 |
jogux | fredross-perry: :) | 09:37.04 |
kens | OK finally I thnk it all built..... | 09:43.19 |
Robin_Watts | paulgardiner: See my rumblings from 10:16 onwards. | 09:56.54 |
paulgardiner | will look | 09:57.22 |
Robin_Watts | Yeah, we're calling fz_run_annot, but annot->run_annot is null. | 10:00.18 |
paulgardiner | I don't know if it's practical to do so, but it would be good to perform the same test with a checkout from the point the ink annotation was first added. | 10:00.58 |
Robin_Watts | I see the problem. | 10:02.24 |
| pdf_create_annot and pdf_load_annot use different code to actually make the annot, and one doesn't set the method pointers. | 10:02.49 |
| One was changed in december, and one wasn't. | 10:04.56 |
| tor8: Would you object to a pdf_new_annot function, so I can use the same code in both placs? | 10:05.33 |
| places | 10:05.35 |
tor8 | Robin_Watts: go ahead. | 10:08.50 |
Robin_Watts | Ta. | 10:08.59 |
sebras | tor8: how do we handle issues where system libraries and thirdparty library snapshots differ in their interfaces? | 10:09.49 |
tor8 | sebras: you better include the correct headers and link against the correct ones! | 10:10.24 |
| where they actually differ in API, we have some #ifdefs | 10:10.45 |
sebras | I've found a bug where someone is pointing to a bugzilla patch which is needed if you are using your system's openjpeg 2.1.0 but which is not needed for the bundled openjpeg 1.5.0 (or whatever we are using) | 10:10.49 |
tor8 | see the freetype ifdefs | 10:10.50 |
| sebras: if our thirdparty is out of date, we should upgrade it | 10:11.15 |
Robin_Watts | sebras: I believe we are using 2.x now. | 10:11.21 |
sebras | clones again to make sure. | 10:11.50 |
chrisl | Looks like 2.0.0 in mupdf - strange | 10:12.26 |
sebras | but it sure looks to me like chrisl's update to openjpeg 2.1.0 was never really checked in. | 10:12.27 |
| well, it was for the thirdparty git of course, but not into mupdf proper. | 10:13.28 |
| chrisl: where do you see that 2.0.0 is used? | 10:14.15 |
tor8 | sebras: we do the same subtree extraction from ghostpdl to create our openjpeg.git submodule like we do with jbig2dec | 10:14.54 |
chrisl | sebras: opj_config_private.h:#define OPJ_PACKAGE_VERSION "2.0.0" | 10:15.02 |
tor8 | sebras: so the mupdf submodule is one commit behind ghostpdl's version | 10:16.46 |
sebras | chrisl: in a completely clean master tree I get 4c7d23d for openjpeg where I see opj_config.h.in.user:#define PACKAGE_VERSION "1.5.0" | 10:17.00 |
| tor8: yes indeed. oh! and the version number in that commit is 2.0.0 and not 2.1.0 as stated in the commit message. | 10:17.41 |
tor8 | sebras: oh, don't forget we have a hacked opj_config_private.h in mupdf/scripts/openjpeg/*.h | 10:18.11 |
| which needs to be updated in sync with our thirdparty module | 10:18.40 |
sebras | tor8: right. | 10:18.54 |
tor8 | unlike gs, we don't use autoconf to build anything :) | 10:19.16 |
chrisl | Hmm, odd about the version number..... | 10:19.35 |
sebras | tor8: why is the subtree extraction thingy (why is is needed?) not done on a branch from master on a mirror of the upstream tree? | 10:21.27 |
| maybe they were not using git before, but they appear to be doing that now. | 10:22.00 |
chrisl | It mirrors the gs openjpeg, not the opj git | 10:22.25 |
tor8 | sebras: their git includes all the openjpeg thirdparty dependencies, but inline. | 10:22.28 |
| we only care about the actual library, not all the extra tools and tool dependencies | 10:22.45 |
sebras | tor8: that makes sense to me, but why is it difficult to address that on a branch? | 10:23.31 |
Robin_Watts | sebras: We don't use submodules in gs. | 10:25.33 |
sebras | anyway, I'll try to update to the latest master and see if I can make it build. | 10:25.34 |
chrisl | Hmm, I bet I cherry picked the opj_config_private.h from an earlier commit.... | 10:25.50 |
kens | tor8 Robin_Watts FWIW that guy with the jp2k file (for whch he has just uploaded a new file, and of course its a different problem) I canreproduce the error on the current master code debug and release 32-bit. But all I get is 'error son page' dialog, no idea how to find out what was actually wrong. | 10:26.11 |
Robin_Watts | kens: Let me fight my way out of the current tar pit, and then I'll have a look :) | 10:26.45 |
kens | THere's no rush I'm sure, just thought one of you might lie to take a quick look before release | 10:27.05 |
| Oh and although he *still* hasn't said what platform he's using, its clearly windows 10 from the screenshot | 10:29.00 |
tor8 | it's "only" a 5k x 7k image | 10:30.13 |
kens | Well, MuPDF pealk worming set reaches 1.1Gb | 10:30.29 |
tor8 | I still suspect that may cause out of memory problems with openjpeg | 10:30.35 |
kens | GS is 27k | 10:30.43 |
tor8 | since openjpeg uses a full 32-bit integer for each color component | 10:30.44 |
kens | But both GS and MuPDF are using OpenJPEG (I@m using the GPL release) | 10:31.09 |
tor8 | but that's still only 500Mb | 10:31.31 |
kens | Well, the peak working set may not be relevant | 10:31.46 |
Robin_Watts | 27k ? | 10:31.53 |
kens | Working is at 402Kb and Memory at 383Kb for MuPDF | 10:32.04 |
| Robin_Watts : yeah I'm suspicious of that too, but its what task manager says | 10:32.26 |
Robin_Watts | Possibly that's the exe rather than the DLL. | 10:32.54 |
kens | It usually reports the whole lot | 10:33.06 |
| ah no I have *2* gswin32 open | 10:34.12 |
| was looking at the wrong one | 10:34.18 |
| currently its at 278Kb and peaked at 1.4Gb | 10:34.34 |
| So pretty comparable | 10:34.46 |
| Gs renders both pages without increasing the peak workign set | 10:35.27 |
| The size difffgerence may be because GS was a debug build and MuPDF was release | 10:36.00 |
Robin_Watts | MuPDF caches, so the peak values may be higher than required. | 10:46.05 |
| Try using -L to get low memory mode. | 10:46.15 |
kens | Hmm, ok | 10:46.25 |
tor8 | kens: -L only works with mudraw, IIRC | 10:46.36 |
kens | Oh well, can't do that then.... | 10:46.53 |
Robin_Watts | Sorry, I obviously missed something key here. What is kens doing ? | 10:47.13 |
kens | Nothing really | 10:47.20 |
| I just tested the bug report | 10:47.25 |
| THe original report was bogus | 10:47.31 |
Robin_Watts | What does the bug report do if not mudraw ? | 10:47.38 |
kens | Mupdf | 10:47.54 |
Robin_Watts | oh, right. | 10:48.03 |
kens | The original report claimed tha t'master' on 'all' hardware couldn't open the file. I was able to open it on 32-bit windows wioth everything from 0.6 onwards | 10:48.40 |
tor8 | kens: I suspect the error is openjpeg malloc failing, and us catching and sort of recovering from that | 10:49.11 |
kens | The user is (from the screenshot) on WIndows 8 or 10 I can't recall exactly what a windows 8 screen looks like | 10:49.19 |
| tor8 I wouldn't be surprised, the size of the image, and peak memory usage made me suspect that towld be the likely culprit | 10:49.51 |
Robin_Watts | Running as mudraw rather than mupdf would enable you to see a warning about such on the console I suspect. | 10:50.11 |
kens | I can tryit, but I've no idea what would be sensible parameters | 10:50.31 |
Robin_Watts | Don't sweat it. Will look in a mo. | 10:51.15 |
kens | Like I said, no rush, just thought it was worth someone knowledgeable looking at it before a release, just in case | 10:51.45 |
Robin_Watts | tor8: Some commits on robin/master then. | 10:51.56 |
tor8 | Robin_Watts: hm. assert and warning patch-over... if (existing) fz_throw() instead perhaps? | 10:54.03 |
| the first two LGTM and should go into the next release candidate; which it looks very likely we'll have to redo | 10:54.43 |
Robin_Watts | tor8: It should never happen, hence a runtime check for it in release builds is wasted time. | 10:54.44 |
kens | Using mudraw -o out%d.png -smt completed with no problems, but the peak memory use was 5Gb | 10:55.31 |
Robin_Watts | kens: For a 32bit binary?!?! | 10:55.46 |
kens | SOrry no 500 Mb | 10:55.51 |
Robin_Watts | Phew! | 10:55.54 |
tor8 | #ifdef DEBUG? | 10:56.01 |
kens | Dropped a 10 | 10:56.03 |
tor8 | kens: that looks in line with what I'd expect from that file | 10:56.17 |
| 12 bytes per pixel for openjpeg + 4 bytes per pixel for mupdf | 10:56.29 |
kens | total memory usage was just over 1Gb | 10:56.33 |
tor8 | twice over, for both pages | 10:56.48 |
kens | That's from the mudraw reported figures | 10:56.51 |
Robin_Watts | tor8: The assert is effectively #ifdef DEBUG. The code would be messier with that than with the void line, IMAO. | 10:57.07 |
tor8 | Robin_Watts: fine :) | 10:57.25 |
Robin_Watts | tor8: Ok, so we need to talk about the partial image decode stuff I've done. | 10:59.30 |
| I believe that what I have on my repo now works. | 11:01.44 |
| Possible drawbacks are: | 11:01.52 |
| 1) We redecode the entire image every time to extract just the sections we need. | 11:02.21 |
| 2) If we request a section A from an image, the caching is not smart enough to return a ready decoded version of the same image for section B, where B is a superset of A. | 11:03.40 |
tor8 | So, this partial decode is only used in banded mode right? | 11:04.33 |
| in which case, we care primarily about memory not performance. I don't see an easy way to extract just the section we need without messing with the decode filters *a lot* | 11:05.36 |
| maybe for jpeg, but none of the other filters will be able to start decoding in the middle | 11:05.58 |
Robin_Watts | tor8: Currently it's always used. | 11:06.02 |
tor8 | but the partial decode is of the full size image in the non-banded case? | 11:06.26 |
Robin_Watts | Yes. | 11:06.33 |
| For normal page by page rendering, it's only ever a saving. | 11:06.35 |
| For interactive pan/zooming, then it's potentially a cost. | 11:06.50 |
tor8 | with potential to save memory if the image is clipped away | 11:06.53 |
| ah, right, because when we render a subview we won't cache the decoded pixmap | 11:07.20 |
Robin_Watts | (we can expect lower peak memory use, but we expect more decoding). | 11:07.37 |
| For banded operation, we'll get much less memory use, but more decoding. | 11:09.23 |
tor8 | I think that's a reasonable compromise. | 11:10.20 |
Robin_Watts | So the question is, what do I need to do to get this to the stage where we're happy to commit it? | 11:10.24 |
| (<chrisl>after the release</chrisl> :) ) | 11:10.47 |
chrisl | thinks: "Not my problem....." | 11:11.15 |
tor8 | Robin_Watts: http://twiki.ghostscript.com/do/view/MuPDF/Tasks I updated the task list for mupdf | 11:11.27 |
| Robin_Watts: there's a section there with some ramblings on "Non-AA rendering" | 11:11.40 |
| our non-AA rendering is a bit lacking in terms of quality | 11:12.10 |
Robin_Watts | It is. | 11:12.16 |
tor8 | and with all this talk about printers, I think it's time we did something about it | 11:12.19 |
| I think it would be helpful to have a 'pixel perfect' rendering mode | 11:12.23 |
| where we never touch the same pixel twice for abutting objects, so we don't get strange anti-aliasing artifacts | 11:12.41 |
Robin_Watts | but that's "next job" rather than "what is left to finishing this job", right? | 11:12.49 |
| s/finishing/finish/ | 11:13.09 |
tor8 | and I think a big part of that is our image rendering | 11:13.10 |
| especially now where we might render images in parts | 11:13.25 |
| this might tie into your changes, but if not, I think I'm happy to have it go in as it stands | 11:14.22 |
Robin_Watts | tor8: I think it's probable that doing 'pixel perfect' will require changes in the draw device that overlap with the changes I've made here. | 11:14.40 |
tor8 | though there are some quirks with the order of arguments. standard_image_get_pixmap has the w,h arguments in a different order than everything else | 11:14.46 |
| Robin_Watts: I'm fine with committing this, and then starting on the 'pixel perfect' work after that | 11:15.12 |
Robin_Watts | But I don't see that putting this in first, will be a retrograde step. | 11:15.16 |
| yeah, cool. | 11:15.19 |
tor8 | I suspect we'll have to drop image grid fitting and adjust how image scaling works | 11:15.33 |
| and that might necessitate us doing type3 font anti-aliasing using supersampling | 11:15.45 |
| which would probably be a step up in quality anyway | 11:15.51 |
| currently when we set the aa level to 0, we lose anti-aliasing on paths *and* text. it would be helpful to keep text anti-aliased while having pixel perfect rendering for all non-text | 11:16.38 |
Robin_Watts | tor8: Given that both gs and Acrobat do grid fitting in at least some circumstances, I'm not hopeful that we can get rid of it completely. | 11:16.47 |
tor8 | Robin_Watts: it could be a device hint | 11:17.05 |
Robin_Watts | I think a device hint is too blunt a tool in this case. | 11:17.30 |
| but we can experiment. | 11:17.42 |
tor8 | I think grid fitting should only be done for when images are less than 1 device pixel wide/tall | 11:17.55 |
Robin_Watts | We *could* make the fz_aa_context have separate text and line art values. | 11:18.08 |
tor8 | so we don't drop stupid latex table borders which are done as 1-pixel images | 11:18.19 |
Robin_Watts | Cos the 2 parts of the code are completely separate. | 11:18.25 |
| tor8: That would still not cope with some of the cases that gs and acrobat use it for. | 11:18.45 |
| (if memory serves). | 11:18.58 |
tor8 | our image rendering does not AA edges any more, does it? | 11:19.14 |
Robin_Watts | I *did* use to use grid fitting for just 1 pixel high things, I think. | 11:19.15 |
tor8 | it might be possible that's the case, yes | 11:19.24 |
Robin_Watts | tor8: I don't believe so. | 11:19.28 |
tor8 | I'm very fuzzy on the details of this code by now | 11:19.37 |
Robin_Watts | We have: fz_set_aa_level to set the number of bits. | 11:20.09 |
| We could have fz_set_gfx_aa_level and fz_set_txt_aa_level | 11:20.24 |
tor8 | Robin_Watts: we should also probably enable freetypes autohint module for non-aa text | 11:20.25 |
| fz_set_aa_level(gfx, txt)? | 11:20.46 |
| and -A4 would set both, but -A0/8 would set gfx to 0 and text to 8 | 11:21.15 |
Robin_Watts | tor8: I was hoping to keep fz_set_aa_level as was to avoid breaking callers code. | 11:21.32 |
tor8 | or what you suggested, I'm really not bothered | 11:21.38 |
Robin_Watts | This is a trivial change. | 11:21.55 |
| The most difficult bit would be handling -A0/8 :) | 11:22.10 |
tor8 | indeed :) | 11:22.16 |
Robin_Watts | I'll do that now. | 11:22.24 |
| But, you're happy for the partial image decode code to go in as is, even though it potentially slows down viewers (like the android/ios viewers and gsiew) ? | 11:23.12 |
| The alternative would be to add either a device hint, or a "tuning" thing to disable partial decode. | 11:24.01 |
tor8 | Robin_Watts: random thought .. is it possible to detect if we have already cached the full pixmap of the image we want a subarea of? | 11:24.21 |
| in that case, we could extract the subarea from that (or just return the full thing) | 11:24.36 |
Robin_Watts | tor8: Not the way it's currently written. | 11:24.46 |
| That was issue 2 in what I had above. | 11:24.56 |
| It's to do with the way stuff goes in the store, and we search for it. | 11:25.11 |
tor8 | isn't image->pixmap the cached decoded thing? | 11:25.12 |
Robin_Watts | No. | 11:25.17 |
tor8 | or is that just used for non-compressed images? | 11:25.20 |
Robin_Watts | Yes, that. | 11:25.25 |
tor8 | Ah. Nvm my idea then. | 11:25.29 |
Robin_Watts | We *could* drop the subrectangle from the hash calculation. | 11:25.59 |
tor8 | Robin_Watts: merge the aa and style contexts into a 'config' context, and add the subarea flag there? | 11:26.20 |
Robin_Watts | tor8: I can imagine that callers might want to allow decoding of 'small' images in toto, but for 'large' ones to band. | 11:27.13 |
tor8 | gah, I don't really like global statefulness but I can't see a cleaner solution | 11:27.16 |
Robin_Watts | And I can imagine that they might want to try it one way, and if it fails, to rerun in a 'low memory mode'. | 11:27.43 |
tor8 | Robin_Watts: disable subarea of anything smaller than a hard coded size? | 11:27.50 |
| that we can tune with a #define at the top of the file | 11:27.58 |
Robin_Watts | That doesn't allow runtime choices from the caller. | 11:28.13 |
tor8 | true, and it doesn't take into account general memory pressure | 11:28.33 |
Robin_Watts | The nice thing about the "tuning" functions is that that caller has runtime control (and we can have sensible defaults to keep things simple for 'dumb' callers) | 11:28.49 |
| At the moment, for instance, we choose to always smooth downscale images. | 11:29.28 |
| but never to smooth upscale. | 11:29.33 |
| A caller might want to change that decision, so that's another "tuning" function. | 11:29.52 |
tor8 | yeah, upscaling only uses bilinear filtering | 11:29.53 |
| Robin_Watts: adding too many choices is going to mean we have untested and bitrotting code though, which is worth keeping in mind | 11:32.49 |
| picking good defaults is better, IMAO. | 11:33.01 |
Robin_Watts | tor8: Well, our defaults would be implemented using the same mechanism. | 11:33.31 |
| (i.e. fz_new_context would fill the tuning section with our default functions, and the caller can override them later). | 11:34.16 |
| Thus no possibility of bitrot. | 11:34.23 |
tor8 | Robin_Watts: yes, in this one case everything would get exercised by the banded mode. | 11:34.44 |
Robin_Watts | Let me throw a quick version together for you to look at. | 11:35.00 |
| tor8: First version on robin/master. Cluster testing/bmpcmping now. Going to grab some lunch. | 12:18.01 |
sebras | tor8: btw, two patches to review over at sebras/master | 13:21.50 |
| i.e. premultiplying worked fine. | 13:22.14 |
Robin_Watts | sebras: Presumably we are holding off on those until after the final 1.9 release? | 13:25.24 |
| (not that that prevents them being reviewed) | 13:25.49 |
sebras | that's up to you guys. they are neither invasive nor important for the release. :) | 13:31.09 |
Robin_Watts | Are gifs always rgb? | 13:35.08 |
sebras | Robin_Watts: yes | 13:35.59 |
Robin_Watts | In gif_read_image... | 13:36.14 |
| What's the point of the fz_try/fz_catch around the fz_new_pixmap ? | 13:36.26 |
| We don't do any cleanup there, so no need for it ? | 13:36.48 |
sebras | Robin_Watts: correct. | 13:37.31 |
Robin_Watts | Other than that, both commits look good to me. | 13:38.43 |
sebras | Robin_Watts: oki. I'll think a bit more about the error handling in there. | 13:39.02 |
Robin_Watts | sebras: I think just removing the try/catch should be fine, shouldn't it? | 13:39.22 |
sebras | Robin_Watts: I'm afraid that those pesky gif_read_*() functions may throw. | 13:39.45 |
Robin_Watts | oh, wait... the info->mask allocation needs to clean up the pixmap if it throws. | 13:40.12 |
sebras | exactly. | 13:40.24 |
Robin_Watts | I'd extend the later fz_try backwards a bit. | 13:40.34 |
sebras | mmm, that would likely help. | 13:40.43 |
Robin_Watts | And you don't need an fz_var(info->lct) because 'info' has already been passed out of scope. | 13:41.08 |
| So I'd have the fz_try just after the pix = | 13:41.24 |
sebras | Robin_Watts: are there any stylistic reasons for not having the pixmap creation inside the try as well? | 13:44.23 |
tor8 | sebras: yes. if you assign 'pix' inside the try, you cannot use it in the always/catch without using fz_var | 13:49.42 |
Robin_Watts | sebras: What tor8 said. | 13:53.46 |
| essentially if you do your first malloc outside the try, you can avoid some hassle. | 13:54.14 |
| (cos if the first malloc fails, there is generally no cleanup to do) | 13:54.27 |
tor8 | Robin_Watts: s/fz_default_image_decode/fz_default_image_decode_tuner/ perhaps, other than that looks fine | 13:56.27 |
| Robin_Watts: commit on tor/master for adding a high level document writer interface | 13:56.37 |
Robin_Watts | tor8: Will rename, but won't commit yet, obviously. | 13:57.00 |
| tor8: What other stuff do we need to fix before doing an rc2? | 13:57.17 |
| (do we need an rc2? or shall we just release?) | 13:57.27 |
tor8 | Robin_Watts: will need to fix the ios and android stuff | 13:58.54 |
Robin_Watts | tor8: Android stuff is fixed, right? | 13:59.10 |
tor8 | ios doesn't build, and the annotation crash on android is fixed with your current commits I believe | 13:59.18 |
Robin_Watts | tor8: Yes, have pushed those. | 13:59.53 |
| so I can't do ios. I guess we need fred for that? | 14:00.36 |
tor8 | fred or jogux, I reckon | 14:01.57 |
jogux | fred is fixing iOS to build, and doing the release. I think I'm then updating the cocoapod. | 14:02.28 |
Robin_Watts | tor8: looking over your commits now. | 14:16.26 |
| I note that we have a load of enums in html.h that don't have the FZ_ prefix. | 14:16.48 |
tor8 | html.h is not included by default; and should really go in source/html/ | 14:17.22 |
| much like xps.h | 14:17.31 |
| but it's conceivable that someone might want to play with the html structures directly | 14:18.03 |
Robin_Watts | "Rename int32be to int32_be to be consistent". To be consistent with what? | 14:18.43 |
tor8 | fz_read_int32_be | 14:18.51 |
Robin_Watts | Ah. fz_read_int32_le :) | 14:19.37 |
| OK. | 14:19.39 |
| We could do with some more comments in the .h files at some stage. We used to be nicely documented and we're slipping. | 14:21.21 |
| muconvert claims to support both pdf and cz, but fz_new_document_writer only appears to do cbz ? | 14:22.26 |
| We have an svg output device, right? | 14:22.58 |
tor8 | oh, right. should update that (or add pdf support) | 14:23.00 |
Robin_Watts | Can svg do multiple pages? | 14:23.05 |
tor8 | it cannot :( | 14:23.13 |
Robin_Watts | Is there a cbs format? :) | 14:23.27 |
tor8 | that would have been useful... | 14:23.47 |
Robin_Watts | Other than that, those look good to me. | 14:23.53 |
tor8 | will update the comments and usage line | 14:24.05 |
Robin_Watts | SVG 1.2 can do multiple pages. | 14:24.20 |
tor8 | Robin_Watts: so, about the output options | 14:24.22 |
| Robin_Watts: oh, it can!? | 14:24.28 |
Robin_Watts | https://www.w3.org/TR/2004/WD-SVG12-20041027/multipage.html | 14:24.38 |
tor8 | should fix the svg parser then :) | 14:24.40 |
| there are two issues that I need to resolve | 14:25.10 |
| one: multi-page or multiple single-page outputs, I guess the presence of a %[0-9]*d in the filename should trigger that | 14:25.54 |
Robin_Watts | I wonder if SVG 1.2 has been withdrawn... | 14:26.22 |
tor8 | but for formats that are multi-page by nature, would it make sense to split to one document per page if a %d is present, or should I just make that up to each document output type? | 14:26.22 |
| I suspect it has | 14:26.28 |
Robin_Watts | cos it dates from 2004 and they still say 1.1 is the latest. | 14:26.36 |
tor8 | or never adopted | 14:26.39 |
Robin_Watts | You mentioned some args being in odd orders in my commit earlier. Can you be more specific ? | 14:29.44 |
tor8 | Robin_Watts: standard_get_image_pixmap has w,h before subarea; all the others have the subarea before w,h | 14:30.11 |
Robin_Watts | which would you prefer ? | 14:31.10 |
| cos I'll move them all in line. | 14:31.22 |
tor8 | Robin_Watts: put the ones that are most optional at the end | 14:42.19 |
| if they're both equally optional, I have no preference | 14:42.29 |
| Robin_Watts: so, the other issue with options is how to communicate the various pdf write options | 14:42.53 |
| and image resolution and sizing | 14:42.58 |
| etc | 14:42.59 |
fredross-perry | Robin_Watts, tor8 - same annotation bug in iOS btw. | 14:43.17 |
tor8 | I was thinking a string with 'option=value,opt2=value2' type thing | 14:43.19 |
| it would be nice to have the same option language for mutool clean and mudraw and the JS bindings, etc | 14:44.25 |
fredross-perry | Robin_Watts, tor8 - I see you fixed it while I was sleeping. ;-) | 14:49.18 |
Robin_Watts | fredross-perry: Yeah, turned out to be easy enough. | 14:49.56 |
| You're fixing the ios app, right? Cos once we've got that, we can do an rc2. | 14:50.16 |
fredross-perry | sweet. Iâve got changes on my master for the iOS app. Who wants to look? | 14:50.33 |
jogux | I can | 14:51.04 |
fredross-perry | thanks | 14:51.08 |
jogux | stuff like | 14:54.16 |
| if (!pdf_name_eq(ctx, PDF_NAME_Standard, obj) != 0) is deliberately written like that, right? it makes xcode warn 'warning: logical not is only applied to the left hand side of this comparison ' | 14:54.36 |
| fredross-perry: your changes LGTM. | 14:55.44 |
fredross-perry | ok, thanks. | 14:55.56 |
Robin_Watts | Urgh. | 14:56.16 |
| My poor head can't parse that. | 14:56.30 |
fredross-perry | OK the iOS fix is pushed. | 14:56.31 |
jogux | Robin_Watts: there's loads of them | 14:57.04 |
Robin_Watts | Can't we have if (pdf_name_eq(ctx, PDF_NAME_Standard, obj)) ? | 14:57.13 |
| pdf_name_eq returns 0 or 1, right? | 14:57.42 |
| So using pdf_name_eq or !pdf_name_eq seems fine. | 14:57.53 |
| That seems like something that should be globally fixed unless there is a good reason that I'm too myopic to see ? | 14:58.14 |
tor8 | jogux: Robin_Watts: these are the consequences of search and replace | 14:58.21 |
jogux | Robin_Watts: git blame fingers you for one of them ;-) | 14:58.24 |
tor8 | it used to be we used strcmp instead of pdf_name_eq | 14:58.31 |
| and I write if (!strcmp()) whereas robin writes if (strcmp() == 0) | 14:58.53 |
Robin_Watts | jogux: I'll take the blame if we get to fix 'em. | 15:00.15 |
jogux | :-) | 15:00.23 |
| it was f533104d6e66b3fc6d3b63b98ec7fe4fb175b366 probably | 15:00.31 |
| I'd certainly be happy to see them fixed | 15:01.05 |
Robin_Watts | OK, so I'm going to look at a fix. | 15:01.11 |
jogux | do you want a list of htem | 15:01.22 |
Robin_Watts | jogux: If that would be easy, yes please. | 15:01.36 |
jogux | robin: actually there's not as many as I thought (or some have mysteriously vanished) | 15:02.52 |
| ah, there's a few, but everyone is printed out 3 times during the build which makes it look like a lot. | 15:04.38 |
| Robin_Watts: there's only 3 then. just emailed your the file/line numbers. | 15:05.33 |
Robin_Watts | Ta. | 15:05.58 |
| tor8: Fix on robin/master. I think we can do an rc2 now? | 15:55.11 |
| tor8: separate text/graphics AA code on robin/master too. | 16:28.18 |
| oh, actually, better do the mudraw change too. | 16:29.06 |
| done. | 16:53.09 |
mvrhel_laptop | lunch time | 18:50.27 |
| I may be off for a bit too. A usb connector came off on my laptop and is rattling around inside. I am going to have to open this thing up and see if I can fix it | 18:51.02 |
| Forward 1 day (to 2016/04/13)>>> | |