| | <<<Back 1 day (to 2016/03/22) | 20160323 |
| Robin_Watts | tor8: Various smallish commits on robin/master | 12:12.46 |
| tor8 | Robin_Watts: I put some comments up on mvrhel's graft branch on tor/graft. you might spot some things too if you give it a look through? | 12:13.19 |
| Robin_Watts | tor8: will do. | 12:13.31 |
| tor8 | Robin_Watts: all LGTM, but don't ask me to understand the asm :) | 12:14.52 |
| | Robin_Watts: sebras bmp commit and a handful of others on tor/master | 12:17.45 |
| | I've reviewed the bmp code and it looks good to me, but another pair of eyes wouldn't hurt | 12:18.06 |
| malc_ | Robin_Watts: it looks like you are quite fond of predicated execution | 12:20.27 |
| Robin_Watts | malc_: you mean conditional execution? Are you looking at my ARM code ? | 12:21.00 |
| malc_ | Robin_Watts: aye | 12:21.06 |
| Robin_Watts | Well, that's the way you avoid pipeline stalls. | 12:21.16 |
| malc_ | Robin_Watts: that's the way you make code that much hard to port to v8 :) | 12:21.30 |
| | s;hard;harder | 12:21.38 |
| Robin_Watts | yes, but v8 has NEON and SIMD etc, and it's generally got faster clockspeeds. | 12:22.09 |
| malc_ | lcm = lcm / gcd(lcm, w) * w; | 12:22.18 |
| | uhm.. in a loop | 12:22.22 |
| | okay | 12:22.25 |
| Robin_Watts | has been coding ARM since... 1997 ish? | 12:22.42 |
| | sorry, 1987 ish. | 12:22.49 |
| malc_ | true brit | 12:22.58 |
| Robin_Watts | tor8, sebras: info->xres = 2835; ? | 12:23.50 |
| | bmp_read_bitmap_core_header has some slightly weird logic in it. | 12:25.12 |
| tor8 | Robin_Watts: bmp resolutions and in pixels per meter | 12:25.16 |
| | Robin_Watts: yeah, I missed those slightly weird size checks... | 12:26.31 |
| Robin_Watts | ooh, hats of to sebras for maskinfo. That's nice. | 12:28.48 |
| | s/of/off/ | 12:28.53 |
| | I understand the logic in bmp_read_bitmap_core_header now. It parallels the constructs in the other functions that cope with different sized blocks. | 12:30.38 |
| | The nested if's inside a per pixel loop in bmp_read_bitmap are going to be slow. | 12:32.27 |
| | We should use a switch, and hoist it outside (at least one of) the for loops. | 12:33.08 |
| tor8 | Robin_Watts: the per-pixel switches for <8bpp will also be slow | 12:33.20 |
| | Robin_Watts: we could unroll the x-loop without too much effort, I think | 12:33.44 |
| Robin_Watts | yeah. But otherwise looks good. | 12:34.10 |
| | Ok, all the other commits on your repo look good to me. | 12:44.18 |
| | looking at michaels now. | 12:45.04 |
| | fz_calloc(ctx, 1, sizeof(pdf_graft_map)); should be fz_malloc_struct(ctx, pdf_graft_map); | 12:49.34 |
| tor8 | Robin_Watts: look at tor/graft for my existing comments | 12:49.50 |
| Robin_Watts | tor8: oh, right. | 12:50.00 |
| | I disagree about your paranoia comment. | 12:51.29 |
| | pdf_new_graft_map should copy with NULL source. | 12:51.46 |
| tor8 | copying with null source makes no sense... all pdf_obj come from one document or another | 12:52.28 |
| Robin_Watts | tor8: It makes code easier. | 12:53.43 |
| | For the same reason that we have free things take NULLs. | 12:54.00 |
| tor8 | what would be the expected behaviour though? silently failing to copy the object is not good behaviour. | 12:54.32 |
| Robin_Watts | a copy of a NULL object is a NULL object. | 12:54.45 |
| tor8 | src is the pdf_document, not the object | 12:54.56 |
| Robin_Watts | D'Oh. | 12:55.11 |
| | Well, a graft map from or too a NULL object could reasonably be NULL, as long as everywhere that takes a graft object knows to do nothing on a NULL graft map. | 12:56.18 |
| | tor8: So I should add my comments onto your comments? | 12:57.42 |
| | Or as a second commit ? | 12:57.57 |
| tor8 | we can pile them on, and squish when michael comes online | 12:58.20 |
| Robin_Watts | actually, swapping back and forth is gonna annoy me. I might just write an email :) | 12:59.09 |
| Robin_Watts | squashes branch and reviews squashed branch :) | 13:17.26 |
| sebras | tor8: I think there is a possibility to add things to make coverity understand that fz_throw/rethrow actually return from the function. | 15:16.59 |
| Robin_Watts | tor8: You pushed sebras bmp fixes earlier. | 15:18.05 |
| | s/fixes/implementation/ | 15:18.13 |
| sebras | Robin_Watts: did something break?! :-/ | 15:18.17 |
| Robin_Watts | sebras: No, I had some comments on it, was all. | 15:18.31 |
| tor8 | oh, I forgot to add it the vcproj... | 15:18.37 |
| sebras | Robin_Watts: ok, let me know and I'll fix it tonight. | 15:18.52 |
| Robin_Watts | tor8: I will add it to the vcproj. | 15:19.34 |
| | sebras: The nested if's inside a per pixel loop in bmp_read_bitmap are going to be slow. We should use a switch, and hoist it outside (at least one of) the for loops. | 15:19.57 |
| | So it's a performane thing, not a correctness thing. | 15:20.13 |
| sebras | Robin_Watts: ok, so switch (bitcount) { case 32: for (x...) {}; case 16: for (x...) {} } etc..? | 15:22.00 |
| | Robin_Watts: sure. I'll fix that later tonight. and try to fix so that gif decoding doesnt use unpack_tile(). | 15:23.09 |
| Robin_Watts | tor8: I have build problems with the font changes on MSVC. | 15:26.26 |
| tor8 | Robin_Watts: bah. :( | 15:26.39 |
| Robin_Watts | I can fix, I think. | 15:26.48 |
| tor8 | and you cleared out the generated directory? cause I've found it doesn't cope well with changes. | 15:26.57 |
| Robin_Watts | tor8: Yes. Neither you nor sebras built your stuff on compilers that accept declarations in the middle of blocks. | 15:29.06 |
| tor8 | oh... I forgot the braces in the BASE14 macro? | 15:30.02 |
| Robin_Watts | Now I'm just finding fz_font_Dingbats_cff etc undefined on link. | 15:30.13 |
| | yeah. | 15:30.15 |
| | do { ... } while (0) | 15:30.29 |
| tor8 | change it to if (!strcpm() { DEC_FONT(); RET_FONT(); } | 15:30.42 |
| sebras | Robin_Watts: did we manage to sneak in a declaration in a middle of a block? | 15:30.50 |
| tor8 | sebras: mea culpa. | 15:31.02 |
| sebras | tor8: oh... startlow is the culprit. | 15:31.12 |
| Robin_Watts | sebras: Yeah, no worries, easy fix. | 15:31.23 |
| tor8 | I made sure the Noto macros coped, but I slipped up on the other ones | 15:31.29 |
| sebras | tor8: is there some way of telling gcc to trigger on things like that? hm... | 15:31.31 |
| Robin_Watts | tor8: That doesn't play well with the trailing ;. | 15:31.36 |
| | --std=C89 or something ? | 15:31.50 |
| sebras | Robin_Watts: yeah, but that also drags other limitations along with it. | 15:32.13 |
| tor8 | unknown type name 'sigjmp_buf' etc | 15:32.47 |
| jogux | finally Robin is annoying someone other than me with his hideously outdated compiler ;-) | 15:33.24 |
| Robin_Watts | jogux: It's not *my compiler*. It's just a belief that we shouldn't stop using standard C without a very good reason. | 15:34.13 |
| sebras | Robin_Watts: tor8: -Wdeclaration-after-statement would catch silly stuff like this. maybe something to enable for debug builds? | 15:34.41 |
| jogux | Robin_Watts: C99 is only 17 years old :-) | 15:34.58 |
| Robin_Watts | Yes. And it's not legal in C99 either :) | 15:35.26 |
| | It's not like it'll suddenly work if we use a later windows compiler either. | 15:36.03 |
| tor8 | sebras: does not work (not with clang anyway) | 15:36.05 |
| | Robin_Watts: actually, it is legal c99 ... I think it's just for (int i=0; ... type stuff that's still illegal in c99 | 15:36.29 |
| Robin_Watts | Tor, so presumably we need to include a load more font .o's in the project? | 15:36.44 |
| tor8 | Robin_Watts: but it's bad form | 15:36.58 |
| jogux | Robin_Watts: My understanding was it was legal in C99, and that as of a 5 or so years ago MS had caught up | 15:36.59 |
| tor8 | Robin_Watts: oh yeah... didn't think about that. | 15:37.05 |
| Robin_Watts | Might require a new target... libfonts or something... | 15:38.14 |
| | At the moment the generate.bat file is producing Dingbats.cff. | 15:41.06 |
| | Shouldn't that be Dingbats_cff.h ? | 15:41.15 |
| | I'll make the change here, if you're happy with that naming scheme. | 15:42.08 |
| jogux | Robin_Watts: Yeah, C99 supports mixed declaration and code, and VS supports that part of C99 as of VS2013. | 15:42.18 |
| Robin_Watts | jogux: Which would preclude anything we build working on anything older than XP SP3 (probably later) | 15:42.57 |
| tor8 | Robin_Watts: d'oh it should be Dingbats.c | 15:46.30 |
| Robin_Watts | ok. | 15:46.40 |
| tor8 | that matches what the other platforms do | 15:46.42 |
| | Robin_Watts: libfonts sounds like a good new target | 15:47.14 |
| | mixed declarations and code fix on tor/master | 15:47.29 |
| | as well as a few js tweaks | 15:47.33 |
| jogux | Robin_Watts: My knowledge gets hazier here, but I /believe/ 2013 can target XP (modulo usual annoyances about installing relevent restributable onto target machine). (You can't run vs2013 itself on XP though.) | 15:48.19 |
| Robin_Watts | jogu: Nothing after VS2005 can target prior to XP SP3 | 15:48.40 |
| tor8 | jogux: that would be a bummer ... I do all my windows development in an XP virtual machine :) | 15:48.48 |
| Robin_Watts | Hence we use VS2005 as a our default compiler. | 15:49.08 |
| jogux | tor8: impressive ancient :) | 15:50.01 |
| | impressively, even | 15:50.05 |
| tor8 | jogux: I do run windows 7 on my gaming machine though... | 15:50.27 |
| | but for development, I like being able to very stuff still works on ancient machines and there's no better way to do that | 15:51.17 |
| | verify* | 15:51.25 |
| rayjj | Robin_Watts: actually, I am pretty sure that vs 2008 is OK. I'll check later (I have an old laptop that I use for low end x86 benchmarking that has ancient XP) | 16:11.48 |
| | I may still have a machine with win 95 or 98 on it | 16:12.59 |
| Robin_Watts | rayjj: No, 2008 breaks XP SP2. | 16:14.59 |
| rayjj | Robin_Watts: OK, I won't bother to try it then :-) | 16:15.16 |
| Robin_Watts | It wouldn't be the end of the world if we *had* to switch for a good reason, but in the absence of having to switch, why do so? | 16:16.05 |
| chrisl | For gs, people would complain if we dropped XP support...... grrrrr! | 16:16.51 |
| rayjj | Robin_Watts: I have three VS versions -- 2005, 2008, and 2015. 2005 in case I need it, 2008 because it has profiling that sort of works, 2015 for profiling that works and to build gsview | 16:17.55 |
| Robin_Watts | chrisl: Most people should be ok XP SP3, but there will always be someone who isn't :( | 16:18.02 |
| rayjj | and I have MSVC 6 on my XP machine | 16:18.15 |
| chrisl | Robin_Watts: I *thought* even XP SP3 needed VS2005 - but I haven't looked into it, TBH | 16:18.46 |
| Robin_Watts | I have VS2005, which I use day to day. VS2008 for no good reason, VS2010 for SOT, VS2015 because I thought it would be useful at some point. | 16:18.51 |
| rayjj | I do have install disks for VS 2003 and 2003.NET if anyone needs it | 16:19.02 |
| Robin_Watts | I think VS2008 will do XP SP3, and maybe VS2010 won't target anything before vista. but I forget the details. The only thing of which I'm sure is that VS2005 works for old things. | 16:19.51 |
| chrisl | We had complaints when the gs binaries stopped supporting Win95.... FFS! | 16:20.57 |
| | Ah, VS2005 still supports Win98, WinMe, Win2k and NT4.0..... again, there would probably be some sort of insurrection if we dropped those for gs | 16:25.45 |
| jogux | there has to be a good case for anyone still running winme being hung, drawn and quartered :( | 16:32.31 |
| chrisl | Well, as I said, Win95, also OS/2 comes up on occasion | 16:33.36 |
| | I keep expecting someone to bitch that we don't support Mac Classic anymore...... | 16:35.03 |
| jogux | hehe | 16:35.35 |
| chrisl | Within my tenure looking after the gs builds, at least one person has complained we don't support DOS any more! | 16:37.09 |
| rayjj | mvrhel: (for the logs) I've updated the twiki performance numbers for mupdf. Not surprisingly, gs is better on 1-bit mono text (PLRM) since we've beat on that so much | 21:36.26 |
| | mvrhel: but otherwise mupdf is better (for the most part). One thing I am questioning is the mupdf gray mode -- the gray to 1-bit difference is a *LOT* and mupdf beats gs | 21:39.57 |
| | Robin_Watts: tor: mupdf "gray" performance stepped up a notch in my latest test. I'll continue to update timings as they come in and will email when the twiki is complete with all of the stats (including the SHA for what I used) | 22:22.26 |
| | Forward 1 day (to 2016/03/24)>>> | |
IRC Logs