| | <<<Back 1 day (to 2017/09/24) | 20170925 |
| tor8 | LRN: that (can't write the data back because the file is still open) is a windows specific problem -- windows can't delete a file that is open so you can't remove the old file to replace it with the new one | 10:25.38 |
| | LRN: the suggested workaround is to save to a different file name, and rename the new file to overwrite the old file afterwards | 10:26.08 |
| | it's also a very dangerous operation -- if something goes wrong when you save the new file, you will have lost the original file | 10:26.41 |
| LRN | tor8, that seems to be less of a windows-specific problem and more of a mupdf-windows-port-specific problem. I could try writing a separate input stream backend backed by CreateFile() with FILE_SHARE_DELETE flag | 10:47.10 |
| tor8 | we use the standard C library for writing files | 10:47.54 |
| | but adding a windows specific backend shouldn't be too much work | 10:48.22 |
| LRN | noted | 10:49.13 |
| tor8 | if you want to try using windows CreateFile for input streams, the file you're looking for is source/fitz/stream-open.c | 10:49.56 |
| | fz_open_file and the functions it uses | 10:50.24 |
| | fz_open_file_ptr is semi-private. nothing other than fz_open_file calls it so it is safe to ignore it. | 10:50.43 |
| sebras | tor8: the three commits on sebras/master cluster fabulously, the top one fixed bug 698592 (the ASAN security thingy). | 11:32.15 |
| tor8 | sebras: why the 'remove -g to ld' commit? | 11:45.18 |
| | I vaguely recall for some crosscompilers it would strip | 11:46.34 |
| | or not include debug symbols unless you added -g to the linker as well | 11:46.46 |
| Robin_Watts | If you don't use the -g flag on debug builds, you don't get debug information. | 11:47.33 |
| | So that commit seems wrong to me. | 11:47.39 |
| tor8 | Robin_Watts: on linux (gnu ld) the -g option is ignored | 11:47.59 |
| | but I suspect it could go wrong on other tool chains, as robin said | 11:48.15 |
| sebras | tor8: Robin_Watts: ignored by ld. | 11:48.17 |
| Robin_Watts | Well, that's certainly not the case for all toolchains. | 11:48.18 |
| | ignored by one particular ld on one particular platform :) | 11:48.30 |
| sebras | Robin_Watts: do you know what toolchain makes use of it? | 11:48.55 |
| Robin_Watts | In the ASAN commit, could we have the arguments to CFLAGS in the same order? | 11:48.59 |
| | sebras: Not offhand. | 11:49.03 |
| sebras | Robin_Watts: ok, now that is something I don't really like. we have code we think might be useful be we don't know by what or where. | 11:49.50 |
| Robin_Watts | sebras: I apologise for my dodgy memory, but then removing stuff on the grounds that "it works fine on this one platform" seems equally bogus. | 11:50.38 |
| tor8 | sebras: Bug 698592 LGTM | 11:50.41 |
| | and I second robin's comment on the CFLAGS argument order in the "sanitize-release" commit | 11:50.57 |
| sebras | Robin_Watts: I'm doing it more to start the discussion. ;) | 11:51.00 |
| Robin_Watts | The -g flag to the linker exists, which implies it is useful somewhere. | 11:51.00 |
| tor8 | commit 497c1196373ed40d9ed9cf045405be7df6ac55c4 introduced the '-g' flag to the linker | 11:51.26 |
| | something for memento and libbacktrace. do you remember why it was added to the non-memento build as well? | 11:53.27 |
| sebras | re: the argument order: I took the release arguments (modified the frame pointer one) and tacked the sanitize arguments at the end. | 11:53.36 |
| Robin_Watts | I do not. | 11:53.41 |
| | sebras: -fsanitize=address -fno-omit-frame-pointer vs -fno-omit-frame-pointer -fsanitize=address | 11:54.14 |
| sebras | I'll clone binutils and see if I can figure out why they have added a compatibility flag to ld. | 11:54.15 |
| tor8 | sebras: I'm comparing it with the order in the 'sanitize' build | 11:54.30 |
| sebras | tor8: so s/sanitize-release/release-sanitize/ and we're all good? ;) | 11:54.53 |
| tor8 | sebras: :) | 11:55.04 |
| sebras | I'll fix it. | 11:55.05 |
| tor8 | might be better to change the "sanitize" build to match the "sanitize-release" | 11:55.28 |
| | easier to tack on other -fsanitize= options then | 11:55.37 |
| Robin_Watts | sanitize would suggest debug to me. | 11:56.06 |
| | in the same way that memento suggests debug. | 11:56.18 |
| sebras | Robin_Watts: I think tor8 meant just the argument order. | 11:56.30 |
| | Robin_Watts: not enabling optimization. | 11:56.36 |
| Robin_Watts | oh, I see :() | 11:56.41 |
| tor8 | yes. I'm being my usual monday ambiguous. | 11:56.54 |
| sebras | tor8: Robin_Watts: new set of patches up, perhaps less appalling. ;) | 13:12.05 |
| | also -g to ld was introduced somewhere between 1.9 and 1.94 beta, but their CVS from back then doesn't seem to have survived, the ChangeLog and their code doesn't really state why -g was included, binutils new linked gold just silently added the ignore, and none of linkers in irix, solaris and aix seem to support -g according to the manpages I have found. | 13:14.23 |
| | probably just enough people that got confused over whether -g need to go to cc or ld and so they eventually added the same flag to both. | 13:14.56 |
| tor8 | sebras: digging through manpages of various unix-ish releases I have only found one instance that references a -g flag to ld without saying "for compatibility with other tools" | 13:19.06 |
| | sebras: https://man.openbsd.org/UNIX-7/ld | 13:19.15 |
| | openbsd pre-gnu binutils era did not list a -g flag to ld | 13:19.41 |
| sebras | tor8: yeah, I know that it comes from binutils' ld. | 13:20.03 |
| tor8 | sebras: sorry, wrong link: https://man.openbsd.org/4.4BSD-Lite2/ld | 13:20.05 |
| | "When linking debugging or profiling objects, include the -g or -pg option (see cc(1V)), as appropriate, in the ld command." | 13:20.20 |
| | I know for profiling with gprof, you need to add the -pg option to ld as well as cc | 13:20.38 |
| sebras | tor8: that's surprising since binutils ld itself doesn't seem to parse it: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=ld/lexsup.c | 13:22.58 |
| tor8 | sebras: sebras/master LGTM | 13:23.00 |
| sebras | I'm probably missing something though. | 13:23.05 |
| | re: -pg | 13:23.12 |
| tor8 | I think it needs a support library or some support functions, which -pg provide | 13:23.35 |
| sebras | tor8: seems plausible, but why doesn't ld parse -pg in that case? | 13:25.09 |
| tor8 | maybe passing -pg is wrong | 13:27.02 |
| | or there's a second layer of option parsing | 13:27.24 |
| sebras | tor8: hm.. now that I think of it, our command for linking is $(CC) $(LDFLAGS)... | 13:29.13 |
| tor8 | oh yeah... to pass options to 'ld' we need the awkward -Wl, option syntax | 13:29.46 |
| sebras | tor8: could it be that other projects have done the same and hence flags for the compiler and for the linker are intermingled in people's Makefiles? | 13:29.57 |
| tor8 | bah. we can't even compile with 'make build=profile' | 13:30.55 |
| sebras | tor8: so the tool that ld needs to be compatible with might not be some other unix's linked, but the cc binary right next to ld..? | 13:30.59 |
| tor8 | the 'namedump' tool segfaults when built with profiling | 13:31.05 |
| | could be that 'cc -g' when used as a linker tacks on some other options to ld | 13:31.31 |
| sebras | tor8: it works here. | 13:31.34 |
| tor8 | ==13317== Invalid read of size 8 | 13:31.49 |
| | ==13317== at 0x561D1AB: mcount (_mcount.S:45) | 13:31.49 |
| | ==13317== by 0x40098F: ??? (in /home/tor/src/mupdf/build/profile/scripts/namedump.exe) | 13:31.49 |
| | ==13317== Address 0x8 is not stack'd, malloc'd or (recently) free'd | 13:31.49 |
| sebras | rebuilds using clang. | 13:32.40 |
| | works for me. | 13:34.57 |
| tor8 | sebras: with build=profile? I get nothing but segfaults running the generate tools :( | 13:36.23 |
| | and the final binaries crash too | 13:36.59 |
| | okay, with CC=gcc it behaves better | 13:37.37 |
| sebras | tor8: yup, make -j10 nuke && make -j10 CC=clang build=profile && valgrind ./build/profile/scripts/namedump.exe prints the help for namedump for me (though it ends with SIGPROF, but I think that's to be expected). | 13:37.39 |
| | I'm even able to run the mupdf-x11 and use gprof ./build/profile/mupdf-x11 gmon.out to get reasonable output. | 13:39.58 |
| tor8 | sebras: yeah. something with my version of clang is effing up. | 13:40.23 |
| sebras | tor8: what version do you have? | 13:40.35 |
| tor8 | Debian clang version 3.5.0-10 | 13:42.29 |
| | I'm not overly bothered. it works with GCC and figuring this out is going to be a waste of time. I just thought for a second there our makefile was broken somehow. | 13:43.43 |
| sebras | tor8: using clang 3.5 and 3.6 I also see segfaults. starting with 3.8 it works though. | 13:43.58 |
| | tor8: so you'll get the update in a decade or so. ;) | 13:44.22 |
| tor8 | good enough for me :) | 13:44.30 |
| | Robin_Watts: in "Make sure shades use proper "default" color space." it feels a bit awkward to pass an override_cs rather than the fz_default_colorspaces struct | 14:10.42 |
| | but that's a minor annoyance, and one I can live with for the simplicity of not depedning on the default_colorspaces API in the shade painting | 14:11.23 |
| | the commit message to "Clear isolated flag within 'inner' knockout groups." misspells "propagate" | 14:13.36 |
| Robin_Watts | tor8: yeah, wasn't convinced that passing the default_colorspaces thing was nice. I should maybe look again. | 14:14.30 |
| | will fix typo. | 14:14.35 |
| tor8 | I suspect I'll dislike passing the default_colorspaces more than what you have :) | 14:14.51 |
| | maybe if you just change where in the argument list it is it'll appear more appealing. | 14:15.09 |
| | in "Add ability to specify icc profile", read_icc_profile has a pointless if (buffer) test | 14:17.51 |
| | and the case CS_ICC switch in apply_layer_config also tests the impossible | 14:18.48 |
| | (sorry, in mudraw_main, not apply_layer_config) | 14:20.54 |
| | and the check for ICC colorspace components matching the output format's supported colorspaces only looks at the default_cs not the full list | 14:22.09 |
| | the number of arguments to paintfn_t is starting to get ridiculous... | 14:23.07 |
| | Robin_Watts: in verify_premultiply why not use assert or abort() instead of a NULL-function-pointer crash()? | 14:29.14 |
| | the message in "Fix overprinting simulation in RGB output with no spots." sounds like it needs more work? | 14:31.12 |
| Robin_Watts | assert won't fire in the cluster. | 14:31.56 |
| tor8 | abort() then? | 14:32.04 |
| Robin_Watts | abort, maybe. Does it give a backtrace in the logs? | 14:32.17 |
| tor8 | it should, it just causes a SIGABRT rather than a SIGSEGV | 14:32.48 |
| Robin_Watts | tor8: a later commit adds stuff in mudraw to cope with RGB stuff. | 14:33.32 |
| | I'm still battling overprint here :( | 14:33.45 |
| tor8 | the "Improved overprint (simulation) control." looks like it works toward the same goal | 14:33.54 |
| | Robin_Watts: so, most things except the "mudraw: Add ability to specify icc profile for target colorspace." LGTM | 14:34.17 |
| | that commit needs a big overhaul | 14:34.21 |
| Robin_Watts | Possibly easiest for you to overhaul that to your satisfaction. | 14:34.57 |
| | tor8: Possibly msvc doesn't stop in the debugger on an abort()... will have to test. | 14:35.44 |
| tor8 | Robin_Watts: yeah. I also need to do something similar for muconvert and the high level document converter API | 14:35.54 |
| | so if you leave that commit out and I'll tackle tweaking it | 14:36.17 |
| Robin_Watts | tor8: There will probably be a couple of new commits for you to review, then I'll squash a load together (all the overprint ones for example) and then we can commit. | 14:36.54 |
| | I want to get it so the whole branch gives a good bmpcmp before I commit it, otherwise it becomes much harder to spot regressions. | 14:37.28 |
| tor8 | Robin_Watts: sure. | 14:37.36 |
| | the group_alpha thing changes so many things, but I assume you have plenty of test cases to spot errors there? | 14:42.36 |
| Robin_Watts | tor8: The altona tests, the cluster tests etc. | 14:43.31 |
| LRN | when i render a page with a matrix initialized by fz_rotate() the page comes out rotated around its top-left corner. Is that the intended behaviour? | 16:33.45 |
| | should i translate the matrix before rotation? | 16:36.35 |
| Robin_Watts | fz_rotate rotates around 0.0 | 16:57.32 |
| | You either need to rotate then translate, or translate then rotate as is appropriate. | 16:58.08 |
| LRN | will page bbox change if i only translate it? | 17:08.22 |
| | no, wrong question | 17:08.45 |
| | actually, no, that's exactly the right question | 17:18.40 |
| | specifically, i rotate a page to find out its bbox in its rotated state. If i translate *then* rotate, would the bbox change? | 17:20.30 |
| | my guess is "no" | 17:33.55 |
| Robin_Watts | LRN: yes. | 18:15.38 |
| | Forward 1 day (to 2017/09/26)>>> | |
Log of #mupdf at irc.freenode.net.