| | <<<Back 1 day (to 2012/11/25) | 2012/11/26 |
| kens | bitHipy, what you remember reading is incorrect in detail. While it is true that objects which are completely clipped out wioll not be preserved in a new PDF file, objects which are only partially clipped will be preserved in their entirety. So if you partially clip an image then yes, the whole image is still preserved in the PDF file. | 08:37.47 |
| tor8 | Robin_Watts: paulgardiner: sebras: several commits on tor/master for review. | 12:32.49 |
| paulgardiner | tor8: That remind me - likewise, I have too. | 12:34.15 |
| | I can review yours if Robin isn't about. | 12:34.41 |
| Robin_Watts | hi. | 12:34.52 |
| | Let me look at tor8's now, unless paulgardiner has beaten me to it. | 12:35.33 |
| paulgardiner | No. You got in just in time | 12:35.50 |
| tor8 | paulgardiner: rename() on posix should ideally be done without unlink() first | 12:36.03 |
| | that way if it gets interrupted, the original file will still be around | 12:36.18 |
| paulgardiner | tor8: I didn't know that. So, rename has the sideeffect of removing the target? | 12:37.19 |
| Robin_Watts | tor8: So we load A.pdf, we save B.pdf, and you want to rename B.pdf to A.pdf ? | 12:37.23 |
| paulgardiner | ... if it succeeds | 12:37.30 |
| tor8 | paulgardiner: yes. | 12:37.34 |
| Robin_Watts | but on windows rename doesn't replace, does it ? | 12:37.44 |
| tor8 | also, mkstemp is a good function. not sure if it exists on windows. | 12:37.53 |
| paulgardiner | tor8: I wondered if there was a function like mkstemp, but then I didn't look because of thinking I would not then be able to use rename | 12:39.31 |
| tor8 | it probably doesn't, but we could add both rename() and mkstemp() to a utility file we compile like we do for gettimeofday | 12:39.37 |
| | also, DeleteFileA and MoveFileA won't work on unicode file names since they use the currently selected code page | 12:40.09 |
| paulgardiner | Robin_Watts: yes, I think rename fails under windows if the target already exists | 12:40.12 |
| tor8 | for demo, fine, but ideally we shouldn't be doing that :) | 12:40.19 |
| | stackoverflow indicates that there is a ReplaceFile call in win32 that does the same as rename() | 12:40.39 |
| paulgardiner | tor8: I thought we were using ascii in unicode builds, so I had to do that. | 12:40.50 |
| Robin_Watts | tor8: Do we need both t3procs and t3lists ? | 12:40.54 |
| | paulgardiner: No, we use utf8 internally, I believe. | 12:41.16 |
| tor8 | Robin_Watts: yes, we do. need the procs for render_direct | 12:41.19 |
| Robin_Watts | Ok. | 12:41.23 |
| | Wondered if that was the case. | 12:41.27 |
| tor8 | and the lists for normal operation | 12:41.28 |
| paulgardiner | tor8: so I had to use the A variants of DeleteFile and MOveFile that is | 12:41.35 |
| Robin_Watts | paulgardiner: If we're using UTF8, then we should convert from UTF8 to Unicode, and then use the unicode versions. | 12:42.14 |
| | I suspect you're getting away with it, because for the files you've tested Utf8 == ascii. | 12:42.30 |
| paulgardiner | Robin_Watts: I guess, but I thought none of the existing code did | 12:42.37 |
| tor8 | paulgardiner: in fz_open_file (in stm_open.c) for example, we convert utf-8 to UTF16 and call _wopen | 12:43.07 |
| paulgardiner | Oh sorry. I didn't notice that. I thought I'd looked and elsewhere char * was being used unconverted. | 12:43.43 |
| tor8 | paulgardiner: yeah, we may be doing that in places in the app. it's a bit shoddy in places :) | 12:44.09 |
| paulgardiner | :-) | 12:44.25 |
| tor8 | but the core should be doing the right thing (utf-8 everywhere, conversions at the call sites if necessary) | 12:44.29 |
| | so the core mupdf api always uses utf-8 | 12:44.41 |
| Robin_Watts | oops. cluster appears ill. | 12:44.44 |
| | I hope that wasn't my commit that did that. | 12:45.06 |
| | s/commit/test/ | 12:45.10 |
| paulgardiner | tor8: Yeah... but I haven't added anything naughty to the core, have I? It was just in the app. | 12:45.13 |
| tor8 | paulgardiner: yeah, just saying :) | 12:45.24 |
| | maybe we should add fz_mkstemp and fz_rename_file wrappers? | 12:45.58 |
| | to the core | 12:46.02 |
| | it sort of doesn't belong though | 12:46.20 |
| paulgardiner | tor8: I don't see a problem with different calls in the different platform apps, although of course I should get it right. :-) | 12:46.52 |
| tor8 | paulgardiner: yeah. so let's not put them into the core just yet. | 12:47.14 |
| Robin_Watts | If it's in the platform apps, sure. We should try to avoid nests of ifdefs though. | 12:47.42 |
| paulgardiner | Definitely. Nests of ifdefs are foul | 12:48.18 |
| Robin_Watts | tor8: Your commits look fine. I'll pull and push them. | 12:48.23 |
| paulgardiner | Yeah, dlogpassproc uses GetDlgItemTextA. I don't think I added that. I then copied the style in dlogtextproc and other places, so there are loads of xxxxxxA calls in win_main.c | 12:49.26 |
| | Not saying we shouldn't fix it, but it might be better as one big separate fix | 12:49.53 |
| Robin_Watts | tor8: pushed | 12:52.48 |
| tor8 | Robin_Watts: thanks. | 12:56.35 |
| paulgardiner | Ah, mkstemp allows me to control where the new file appears. Ok, I should use that under linux. | 12:56.54 |
| tor8 | paulgardiner: well, I'll need to copy lots of that stuff when I get around to porting the gtk+ app to win32 | 12:56.55 |
| paulgardiner | tor8: that's a point | 12:57.15 |
| tor8 | paulgardiner: but it'll be a while until the new viewer is all forms capable :) | 12:57.30 |
| paulgardiner | I can imagine :-) | 12:58.27 |
| Robin_Watts | Hmm. None of the cluster machines appear to be successfully getting through to casper. | 12:59.01 |
| paulgardiner | tor8: Shall I fix the other things you pointed out for now, and maybe deal with all the uses of xxxxxA calls in a separate commit later | 13:00.08 |
| | ? | 13:00.09 |
| tor8 | paulgardiner: sure thing. the android patch also uses the unlink/rename thing so don't forget that place too. | 13:00.40 |
| paulgardiner | tor8: yeah sure. I'll use mkstemp on android too. | 13:01.08 |
| tor8 | fab. | 13:01.37 |
| | Robin_Watts: if you got a spare moment, do try the gamma branch as well and shriek in horror! | 13:01.52 |
| Robin_Watts | tor8: will do. | 13:02.10 |
| paulgardiner | That's handy to know. I thought there was a function like mkstemp, but I'd assumed it created something in /tmp (which I now realise is daft). | 13:02.29 |
| tor8 | Robin_Watts: try the tiger and some inverted (white on black) text documents for comparison as well | 13:02.31 |
| Robin_Watts | oops. I think the cluster thing may have been my fault. Looks like I have an infinite loopy thing. | 13:02.33 |
| tor8 | paulgardiner: yeah, that /tmp won't help if you want to do the create/write/rename dance | 13:02.54 |
| Robin_Watts | VS projects are broken - libmupdf-v8 still has xps/xps_xml.c in it. | 13:08.50 |
| | so, I've broken the cluster by putting every client machine into an infinite loop. oops. | 13:21.33 |
| | And it | 13:21.36 |
| | And it's thanksgiving weekend, so god knows when Marcosw will be around :( | 13:21.55 |
| kens | Well I managed to trash my own network this morning..... | 13:22.02 |
| | All but one laptop back working again now | 13:22.14 |
| | Hmm, this is quite amusing: | 13:23.56 |
| | http://www.theregister.co.uk/2012/11/26/spoof_james_bond_ad_silliness/ | 13:23.57 |
| Robin_Watts | tor8: Updated scan conversion patch on my repo. | 13:27.02 |
| | Can't test it yet though, so feel free to ignore it for a while. | 13:27.16 |
| | cluster machines are coming back to life. | 14:30.17 |
| | I wonder if that's marcosw at work, or whether they have all counted to 2 billion or something :) | 14:30.33 |
| paulgardiner | tor8: seems we need to declare the windows app as targetting XP or later if we wish to use ReplaceFile. Do you think that's worth it? | 14:30.52 |
| Robin_Watts | paulgardiner: Really? | 14:31.10 |
| paulgardiner | http://social.msdn.microsoft.com/forums/en-US/Vsexpressvc/thread/0062a0a1-0f24-48a1-9061-e6106808d0d9/ | 14:31.34 |
| tor8 | too new feature? | 14:32.42 |
| paulgardiner | Looks like it. | 14:32.53 |
| Robin_Watts | ReplaceFile was in windows 2K apparently. | 14:33.04 |
| paulgardiner | Docs say minimum supported client XP | 14:33.11 |
| tor8 | I think sebras changed workplace so he doesn't need support for win2k anymore! | 14:33.31 |
| Robin_Watts | paulgardiner: Yeah, but I don't trust MS docs w.r.t historical things. | 14:34.08 |
| paulgardiner | Also my build fails with ReplaceFile undefined | 14:34.28 |
| | I'll stick with Delete;Move for now | 14:41.05 |
| tor8 | okay. fair enough. | 14:42.03 |
| Robin_Watts | Yeah, sounds fair to me. | 14:42.19 |
| | Maybe it would be worth doing: | 14:42.28 |
| paulgardiner | I've left the ReplaceFile call with a comment | 14:43.00 |
| Robin_Watts | load A.pdf then when we come to save, write B.pdf, rename A.pdf to A.pdf.backup, rename B.pdf to A.pdf, remove A.pdf.backup | 14:43.12 |
| paulgardiner | Oh. I could make it use RplaceFile conditionally on the version of window that we are targetting. That would be better | 14:43.56 |
| Robin_Watts | paulgardiner: Could do. | 14:44.10 |
| | Gah. For some files, the ndk profiler appears to not count ticks. | 14:44.37 |
| sebras | tor8: and my old employer has finally upgraded to xp! | 14:52.10 |
| tor8 | sebras: oooh! | 14:53.21 |
| Robin_Watts | 35 million calls to hash and fz_hash_find | 14:58.25 |
| tor8 | paulgardiner: any idea why adding back the highlight button causes a nullpointerexception when opening the outline view? | 15:14.18 |
| | crash at at com.artifex.mupdf.OutlineAdapter.getView(OutlineAdapter.java:41) | 15:14.39 |
| Robin_Watts | tor8: Guessing, but add: if (v !=null) { ... } around the 2 lines before the return v? | 15:25.34 |
| | or if (R.id != null) { ... } | 15:25.57 |
| tor8 | Robin_Watts: I'm just having a difficult time understanding how my change could cause this... | 15:28.15 |
| | it seems like there must be some assumption I've missed | 15:28.37 |
| Robin_Watts | Maybe the highlight button adds another view that doesn't have an id or something? I can't claim to understand this bit of the code. paulgardiner ? | 15:29.17 |
| tor8 | Robin_Watts: bah. clean rebuild and it works. | 15:37.27 |
| Robin_Watts | gotta love the ndk. | 15:37.41 |
| tor8 | have I mentioned how much I hate build systems that can't keep track of dependencies properly? | 15:37.44 |
| | I'm sure it's cached the ID's in some binary database somewhere that isn't updated when the XML changes | 15:38.14 |
| Robin_Watts | tor8: r8c makes all the files depend on the obj/fitz directories etc. | 15:38.54 |
| | And then updates the timestamps on those during the build. | 15:39.04 |
| | So every build is a full one :( | 15:39.09 |
| | I've dropped back to r8b. | 15:39.32 |
| tor8 | Robin_Watts: so that's why ndk-build always rebuilds the lot? | 15:39.42 |
| | argh! | 15:39.44 |
| Robin_Watts | tor8: indeed. aargh | 15:39.58 |
| sebras | tor8: ? | 15:50.04 |
| | don't worry, google will fix it in 6-12 months... | 15:50.23 |
| tor8 | sebras: ? | 15:50.23 |
| Robin_Watts | with the next release of the ndk. | 15:51.58 |
| sebras | Robin_Watts: indeed. | 16:06.00 |
| Robin_Watts | Morning marcosw | 16:09.56 |
| marcosw | morning. the cluster seems to be working okay now. | 16:10.10 |
| Robin_Watts | Dunno if it was you that fixed the cluster, or whether... | 16:10.16 |
| | indeed. | 16:10.17 |
| | Thanks anyway! | 16:10.21 |
| | Are there timeouts on the running of the mupdf tasks on the clients? | 16:10.34 |
| paulgardiner | tor8: I've made those changes you requested, including using Unicode because it turns out that all the places where we weren't doing so for the filename were down to me. | 16:11.17 |
| marcosw | Robin_Watts: yes, there are timeouts on each job. I think it's five minutes, but would have to look to be sure. | 16:12.07 |
| Robin_Watts | Hmm. This was more like 2 hours. | 16:12.21 |
| marcosw | yes, there is a two hour timeout on the jobs as a whole. | 16:12.56 |
| Robin_Watts | paulgardiner: XXXXXX ? | 16:16.50 |
| | Is that a prefix for the name that mkstemp makes ? | 16:17.04 |
| sebras | Robin_Watts: its a template for the unique string it creates. | 16:18.19 |
| Robin_Watts | so, why not: mupdf_XXXXXX ? | 16:18.40 |
| paulgardiner | I already have the file name in there, so it's oldfile345367 | 16:19.31 |
| Robin_Watts | AH, ok. | 16:19.49 |
| | I can't see the unicode changes in there... | 16:20.05 |
| | oh, I see it. | 16:21.04 |
| | all 3 look good to me, but I'll give tor8 a chance to comment... | 16:21.32 |
| paulgardiner | oh good. I thought I'd messed up my gitery for a minute | 16:21.35 |
| tor8 | paulgardiner: MultiByteToWideChar(CP_ACP ... shouldn't that be CP_UTF8? | 16:25.29 |
| | paulgardiner: we have strlcpy/strlcat with fz_ prefixes. could you please use those so we don't get a lot of security researchers griping about potential overflows? :) | 16:27.36 |
| | (I see a handful of strcpy and strcat in the second patch) | 16:27.59 |
| paulgardiner | yeah, I think they are safe in this case | 16:29.11 |
| | I've malloced the buffer to be sufficient... or is that not the point. | 16:29.38 |
| | ? | 16:29.40 |
| | The CP_ACP is a slip. I copied that from somewhere else in the code without checking what it meant. Oops! | 16:30.25 |
| tor8 | paulgardiner: right, in that case. still, I know some compilers like to gripe about strcpy and family regardless. anyway, it's fine where you malloced, but in the first patch you have a strcpy(filename, buf) that might go wrong? | 16:32.27 |
| kens | finally resuscitates network | 16:32.48 |
| paulgardiner | tor8: Is that the one after the wcscpy? | 16:34.16 |
| tor8 | paulgardiner: yes | 16:37.18 |
| | paulgardiner: more great news! I just got a random android crash :D | 16:37.41 |
| | E/AndroidRuntime( 2980): Caused by: java.lang.IllegalArgumentException: width and height must be > 0 | 16:37.45 |
| | E/AndroidRuntime( 2980): at android.graphics.Bitmap.nativeCreate(Native Method) | 16:37.46 |
| | E/AndroidRuntime( 2980): at android.graphics.Bitmap.createBitmap(Bitmap.java:695) | 16:37.47 |
| | E/AndroidRuntime( 2980): at com.artifex.mupdf.MuPDFCore.drawPage(MuPDFCore.java:117) | 16:37.49 |
| | E/AndroidRuntime( 2980): at com.artifex.mupdf.MuPDFPageView.drawPage(MuPDFPageView.java:134) | 16:37.50 |
| | E/AndroidRuntime( 2980): at com.artifex.mupdf.PageView$4.doInBackground(PageView.java:387) | 16:37.52 |
| | E/AndroidRuntime( 2980): at com.artifex.mupdf.PageView$4.doInBackground(PageView.java:384) | 16:37.53 |
| | E/AndroidRuntime( 2980): at com.artifex.mupdf.AsyncTask$2.call(AsyncTask.java:291) | 16:37.54 |
| | E/AndroidRuntime( 2980): at java.util.concurrent.FutureTask$Sync.innerRun(FutureTask.java:306) | 16:37.55 |
| | E/AndroidRuntime( 2980): ... 5 more | 16:37.56 |
| paulgardiner | Ah no. That ones ok too. That's why I used MIN(PATH_MAX, len) in the conversion... although a comment might have been helpful I now see | 16:38.24 |
| | tor8: Android crash - is it repeatable? Lastest source? | 16:40.41 |
| tor8 | don't know. yes. | 16:41.33 |
| | heading for dinner, back in a bit | 16:41.40 |
| paulgardiner | tor8: I've pushed the ACP to UTF8 change | 16:41.58 |
| tor8 | then it LGTM | 16:42.14 |
| kens | Robin_Watts : you have a Windows 8 Vm don't you ? | 17:05.00 |
| Robin_Watts | I do, but I suspect it won't work now. | 17:05.14 |
| kens | Oh, NVM then | 17:05.22 |
| Robin_Watts | I think they are set to self destruct when the real version ships. | 17:05.25 |
| kens | I wonder if I can splurge the £15 for the update and instead build a VM. | 17:05.47 |
| | reboot | 17:05.57 |
| Robin_Watts | Supposedly if you install Windows 8, but don't authenticate it, then download the media player download for it (free), that turns your installation into a fully authenticated version. | 17:17.18 |
| chrisl | I'm told the upgrade disks are capable of clean installs, too. I haven't tried mine yet | 17:18.18 |
| Robin_Watts | chrisl: ooh. | 17:18.30 |
| chrisl | I think it's in response to how rarely upgrading Windows actually works! | 17:19.02 |
| Robin_Watts | http://www.theregister.co.uk/2012/11/21/win8_media_center_piracy_snafu/ | 17:19.34 |
| chrisl | Interesting. Almost makes me wish I didn't have a fully licensed copy ;-) | 17:20.29 |
| Robin_Watts | Hmm. Sounds like anyone even considering getting windows 8 at some point should get the media centre download now. | 17:20.44 |
| | cos it'll disappear on 31st January (i.e. if you haven't downloaded it by then, you never will) | 17:21.04 |
| chrisl | It looks like you just get a license key. I wonder if it will still work after the end of the offer.... | 17:22.50 |
| Robin_Watts | You get a license key and a download, right? | 17:23.09 |
| | It's the download that ceases to be available. | 17:23.30 |
| chrisl | Reading the instructions on the MS page, it *looks* like the download is initiated thru "Add features to Windows 8" | 17:23.51 |
| Robin_Watts | ah, you may be right. | 17:24.09 |
| chrisl | Well, I have a Windows 8 VM, but it turns out MS gave me the 32 bit version :-( | 18:09.58 |
| Robin_Watts | what did you update to get it ? | 18:12.07 |
| mvrhel_laptop | it is pretty lame that they will start charging for the media center | 18:15.50 |
| chrisl | I went through the MS update for cheap, and I happened to be in the 32 bit Win7 VM, and their download tool must have chosen 32 bit based on the current machine. | 18:16.24 |
| | It never occured to me that the media wouldn't have both on it | 18:16.50 |
| Robin_Watts | tor8: OK, new scanline converter optimisation patch online. It checks out with no diffs now. | 19:58.46 |
| | http://git.ghostscript.com/?p=user/robin/mupdf.git;a=commitdiff;h=4166b88a50e90e3103f4c693adfdb6c17ffaf46c | 19:59.15 |
| mvrhel_laptop | bbiaw | 21:43.17 |
| | Forward 1 day (to 2012/11/27)>>> | |
IRC Logs