| | <<<Back 1 day (to 2016/02/11) | 20160212 |
| tor8 | Robin_Watts: using putc for 1-byte writes instead of fwrite improves performance by 100% for writing large pgm files | 11:03.03 |
| jo0nas | chrisl: thanks for the fix (to the fix to the fix) - that was fast :-D | 11:06.34 |
| chrisl | jo0nas: no problem. I started down the route of doing it with that change in openjpeg.mak and realised it wasn't going to work, and clearly forgot to revisit that file | 11:07.46 |
| jo0nas | nods | 11:09.21 |
| | chrisl: you may want to consider the three 100* patches here: https://anonscm.debian.org/cgit/printing/ghostscript.git/tree/debian/patches | 11:27.44 |
| | ...or do you want me to formally file a bugreport for each of them? | 11:28.19 |
| chrisl | jo0nas: well, the zlib one is wrong..... | 11:29.41 |
| jo0nas | oh | 11:30.07 |
| chrisl | We're basically going to drop the man pages, and replace them with something that points to the html documents | 11:30.21 |
| jo0nas | ahh - perhaps what I tried to solve has since been fixed by your makefile corrections on 4th | 11:30.46 |
| jo0nas | will try simply drop the zlib patch and see if it compiles fine now | 11:31.28 |
| | chrisl: ...and patch 1001 (which patches not man page but html)? | 11:43.32 |
| chrisl | jo0nas: I just applied that locally - I'll push it in a bit | 11:44.01 |
| jo0nas | ah, sorry - I got used to your being fast :-D | 11:44.21 |
| chrisl | jo0nas: would you just check over the top two commits here: http://git.ghostscript.com/?p=user/chrisl/ghostpdl.git;a=summary and make sure I haven't messed anything up, please? (note that I did make a small tweak to the inkcov one) | 11:47.15 |
| tor8 | Robin_Watts: so, testing with 'mutool draw -r1200 -o out.pgm pdfref17.pdf 1' | 11:49.55 |
| | as origin/master, it takes 2.2s | 11:50.03 |
| | changing to use putc, it takes 1.1s | 11:50.14 |
| | changing to use our own buffered io with file descriptors, it takes 0.7s | 11:50.35 |
| Robin_Watts | With my patch I went from 5s to 0.5s. | 11:50.49 |
| tor8 | this is on linux with gnu libc | 11:51.07 |
| Robin_Watts | Did you do a timing with my patch ? | 11:51.22 |
| | (I am in favour of fixing the general case, but I think I'm also in favour of optimising the png write as a I did if it gives a further benefit) | 11:52.02 |
| | s/png/pgm/ | 11:52.14 |
| tor8 | Robin_Watts: of course, your patch speeds things up a lot more since we won't call the whole chained mess of out->write function pointers per byte | 11:53.12 |
| | ah, actually, it speeds it up because it segfaults... | 11:53.29 |
| Robin_Watts | tor8: really? | 11:53.57 |
| tor8 | Program received signal SIGSEGV, Segmentation fault. | 11:54.14 |
| | 0x000000000043a389 in fz_write_pnm_band (ctx=0x18d3010, out=0x1922980, w=8850, h=11100, n=2, band=0, bandheight=11100, | 11:54.14 |
| | p=0x7ffff78d2000 "") at source/fitz/pixmap.c:619 | 11:54.14 |
| | 619 *o++ = *p; | 11:54.14 |
| Robin_Watts | Crap. What did I break now. | 11:54.41 |
| tor8 | you missed the divide when you assign l2 | 11:55.23 |
| | no, that's not only it | 11:55.50 |
| Robin_Watts | l = len * (n-1) | 11:57.42 |
| | no, crap, even that's wrong. | 11:57.53 |
| jo0nas | chrisl: looks good to me! | 11:58.22 |
| Robin_Watts | tor8: will fix. | 11:58.37 |
| chrisl | jo0nas: thanks - and pushed | 11:58.51 |
| jo0nas | excellent! | 11:58.59 |
| tor8 | Robin_Watts: if we add an unsigned char *p, *end and flush call to fz_output we can do fz_write and fz_write_byte as static inlines like fz_read_byte which will be very fast | 11:59.12 |
| | for the fz_buffer based outputs we can then poke directly into the fz_buffer, flush and get new pointers when full, etc | 12:00.04 |
| Robin_Watts | tor8: so we do our own buffering ? | 12:00.09 |
| | Yes, we could do that. | 12:00.36 |
| tor8 | I added a file descriptor based variant of output with our own buffering (the WIP commit is on tor/master) | 12:00.42 |
| | but I think we could go further and expose the buffering to the fz_output struct and have static inlines like fz_read_byte for max performance | 12:01.04 |
| | and then we only need to invoke the function pointer call overhead for flush | 12:01.30 |
| | though buffering for FILE* is going to make things even slower | 12:02.16 |
| | since FILE* already does a terribly slow and complicated buffering scheme of its own | 12:02.31 |
| | which is what slows down fwrite so much | 12:02.38 |
| | I tried reading the code in glibc and threw up my hands in disgust | 12:02.52 |
| Robin_Watts | tor8: We moved from open to fopen a while ago. We should be working FILE's not ints. | 12:03.51 |
| tor8 | Robin_Watts: the problem is, FILE's are SLOW as molasses | 12:04.05 |
| Robin_Watts | tor8: Only if called 'badly'. | 12:04.14 |
| | and we can avoid calling them badly. | 12:04.22 |
| tor8 | even with putc which is the fastest way to write a single byte at a time to a supposedly buffered FILE* using a macro, is much much much slower | 12:04.50 |
| chrisl | jo0nas: I'm baffled by the ramfs patch you have there...... | 12:05.33 |
| tor8 | fz_write_byte should be fast, all it should be doing is poke a byte into a buffer and test if it needs to flush | 12:05.47 |
| | but stdio is broken if putc and fwrite are that much slower than a simple buffer and write() | 12:06.19 |
| Robin_Watts | tor8: Using your char *p, *end, and flush solves that. | 12:06.27 |
| | That sounds like a good solution. | 12:06.31 |
| tor8 | Robin_Watts: I guess a second layer of buffering on top of FILE will work | 12:06.51 |
| | could possibly use setvbuf to NULL and disable FILE* buffering to see if that helps | 12:07.10 |
| | I did the fd based one just to see how bad it was | 12:07.26 |
| jo0nas | chrisl: intent is to rip out the code that lacked proper copyright - am I doing that wrong, or do you simply disagree that the licensing is vague? | 12:08.13 |
| Robin_Watts | setvbuf sounds like it's something that'll go wrong on some system or other. | 12:08.20 |
| Runner | Hello | 12:08.35 |
| ghostbot | Welcome to #ghostscript, the channel for Ghostscript and MuPDF. If you have a question, please ask it, don't ask to ask it. Do be prepared to wait for a reply as devs will check the logs and reply when they come on line. | 12:08.35 |
| jo0nas | s/rip out/repair linkage after having ripped out/ | 12:08.57 |
| Runner | Is it possible to uninstall GS silently? i am trying all kind of switches like /silent or /s for "uninstgs.exe", but it always shows a confirmation dialog | 12:09.24 |
| chrisl | jo0nas: I disagree that the copyright is vague - the C file has a copyright statement in it | 12:09.26 |
| jo0nas | without consent of the author, right? | 12:10.09 |
| chrisl | jo0nas: the author contributed the code to Ghostscript | 12:10.32 |
| | Runner: probably not, if /S doesn't work | 12:10.44 |
| | jo0nas: what we were unable to do was to arrange for a copyright transfer to Artifex | 12:11.23 |
| jo0nas | which means you did not get consent to relicense later | 12:11.50 |
| | ...as I recall | 12:11.57 |
| chrisl | Relicense? | 12:12.08 |
| | We've only moved *more* restrictive instances of the GPL | 12:12.45 |
| Runner | *sigh* Capital S does work though. Thanks | 12:12.54 |
| chrisl | Runner: but I do wonder *why* you want to do that | 12:13.11 |
| Runner | Software deployment | 12:13.30 |
| chrisl | Runner: well, as long as you are adhering to the license conditions..... | 12:14.16 |
| jo0nas | chrisl: http://bugs.ghostscript.com/show_bug.cgi?id=226943 predates the general relicensing to AGPL - even if the licensing back then was compatible with AGPL which I believe is up in the air and it seems Ken agrees with: http://www.ghostscript.com/irclogs/2014/05/05.html | 12:15.51 |
| Runner | chrisl: Sure | 12:16.14 |
| chrisl | jo0nas: Ghostscript has been distributed under some variant of the GPL since practically the very beginning | 12:17.44 |
| jo0nas | I would love if that licensing issue of ramfs code could be cleaned up, as it is the only part left from Ghostscript being compatible with Debian Free Software Guidelines, as I see it | 12:17.49 |
| chrisl | jo0nas: the license isn't the problem, the problem (for us) is the copyright. The is distributable under the conditions of the GPL | 12:18.36 |
| jo0nas | not all forms of GPL is upwards compatible with AGPL | 12:18.46 |
| | do you still permit redistribution under GPL?!? | 12:19.15 |
| | I believe you've switched from GPL to AGPL | 12:19.42 |
| chrisl | We distribute GS under the AGPL | 12:20.02 |
| Robin_Watts | jo0nas: Versions of the code that were released under the GNU GPL are still usable under the GNU GPL. | 12:20.03 |
| | Versions of the code that postdate our change to the GNU AGPL are only usable under the GNU AGPL. | 12:20.24 |
| jo0nas | so what is the license of ramfs code? | 12:20.45 |
| Robin_Watts | We own the copyright in all the code (except possibly the ramfs code), so we can license it any way we like. | 12:21.06 |
| jo0nas | I believe it predates the shift to AGPL and you are not copyright holder so cannot change license | 12:21.14 |
| chrisl | It was contributed as GPL, we redistribute it as AGPL | 12:21.16 |
| Robin_Watts | And certainly anyone can take GPL code and re-release it as AGPL code. | 12:21.26 |
| jo0nas | if upwards compatible with AGPL, yes | 12:21.49 |
| chrisl | We can legally ship under a *more* restrictive, but compatible license | 12:21.58 |
| Robin_Watts | They can't take away the rights to people that had the GPL code already, but certainly there is no problem in ever shipping GPL code as part of an AGPL package. | 12:22.11 |
| | Cos AGPL is, as chrisl says, more restrictive. | 12:22.22 |
| jo0nas | GPL-2 is more restrictive than GPL-2+ | 12:22.33 |
| | GPL-2+ is upwards compatible with AGPL, but not GPL-2 - I believe | 12:22.58 |
| Robin_Watts | GPL-3 is more restrictive than GPL-2. | 12:23.09 |
| jo0nas | ...because AGPL is sort-of a sidestepping of GPL-3, not GPL-2 | 12:23.18 |
| Robin_Watts | AGPL is more restrictive than GPL-3 | 12:23.23 |
| | I have no idea what "GPL-2+" is. | 12:23.34 |
| jo0nas | which exact license is ramfs code? | 12:23.38 |
| | GPL-2+ is short form of saying GNU General Public License version 2 or newer | 12:24.06 |
| | "+" = "or newer" | 12:24.17 |
| Robin_Watts | The implication of it being submitted to us is that it's "GPLv2 or later". | 12:25.21 |
| | hence we can certainly relicense it as GPL v3. | 12:25.58 |
| | and we can certainly include it within a package with AGPL code. | 12:26.18 |
| jo0nas | how can anyone know that when they file something to your issue tracker then it is automagically licensed as "GPLv2 or later"? | 12:26.27 |
| Robin_Watts | jo0nas: There is a note on our bugzilla explaining that :) | 12:26.52 |
| kens | Filing something ont he bug tracker doesn't make it GPL at all, submitting a patch does, and as Robin says we do explain thast | 12:27.07 |
| | And will go back for explicit permission on anything significant | 12:27.21 |
| chrisl | And why would anyone contribute unusable code | 12:27.29 |
| kens | But I've tried (adn just tried again) to contact the author without success | 12:27.37 |
| chrisl | Is there some shorthand to get to the earliest commit in git? | 12:27.56 |
| jo0nas | How could I provide you a patch yesterday without needing to confirm licensing? | 12:27.59 |
| kens | If you submitted a patch then we assume you read the note Robin mentions | 12:28.27 |
| jo0nas | chrisl: perhaps the author wanted his bounty before licensing the code :-P | 12:28.33 |
| chrisl | not sure there was a bug bounty back then..... | 12:29.00 |
| kens | AQnd I'd love to pauy it to him, if he'd get in touch | 12:29.04 |
| chrisl | jo0nas: in your case, the patches are too small to be a problem | 12:30.52 |
| jo0nas | http://bugs.ghostscript.com/show_bug.cgi?id=226943 say "Keywords: bountiable" | 12:31.11 |
| jo0nas | is just giving a plausable reason for not licensing up front | 12:31.55 |
| | I don't mean to say that my contribution is big enough to need licensing, but that I did file a bugreport which included a patch, and (because I recalled your requiring ownership handover in the past) looked explicitly if I was requred to confirm anything related to licensing or authorship when filing that bug | 12:34.20 |
| chrisl | jo0nas: Well, you possibly didn't look hard enough: on the page for adding attachments, there is: "patch A signed Artifex Contributor License Agreement is prerequesite for patch acceptance." | 12:35.20 |
| jo0nas | so where is the "signed Artifex Contributor License Agreement" from Michael Slade? | 12:36.08 |
| | you evidently did accept the patch, right? :-P | 12:36.40 |
| chrisl | jo0nas: as I said, small patches we have more leeway - you cannot assert copyright over patches that small and that are obviously corrections | 12:38.52 |
| jo0nas | I know you want to solve this issue - and I agree that ideal would be to get in touch with the author and get a consent to hand over ownership - but I disagree that that is not needed | 12:38.58 |
| | chrisl: as I already wrote, I agree that my patch is too small to need licensing - you are missing the point, it seems | 12:40.04 |
| chrisl | We didn't have a "stock" contributor agreement back in 2006, that was a recent innovation | 12:40.56 |
| jo0nas | ...or do you imply that your issue tracker is clever enough to detect threshold of needing licensing, and show requirement only for large enough patches? | 12:41.18 |
| | chrisl: right - I suspected that, but played along with your claim here that explicit consent _was_ in place - so we are back to consent lacking (back then!) | 12:42.02 |
| | so what I did yesterday is irrelevant - not because the patch was small but because I used a different interface than was in place in 2007 | 12:43.02 |
| chrisl | No, I didn't claim that: I claimed *implicit* consent was given. | 12:43.23 |
| jo0nas | my question remains: What is licensing of code contributed in 2007? | 12:43.29 |
| chrisl | GPL | 12:44.33 |
| jo0nas | what was the exact license of ramfs code when contributed in 2007? | 12:44.40 |
| | just "GPL" implies "GNU Public License version 1" with no upgrade path to GPL 3 and therefore no sidegrade path to AGPL 3 | 12:45.15 |
| | you cannot assume that "GPL" implies "or later"! | 12:45.56 |
| | sorry for being too confident: How can you assume that "GPL" imply an upgrade/sidegrade path to your current AGPL licensing? | 12:47.34 |
| chrisl | The problem is, the GPL distro from back then is not in our git repo | 12:47.53 |
| jo0nas | problem is you don't have explicit permission from the author of the ramfs code to relicense it, you just assume it fits your needs | 12:48.44 |
| | I am baffled that you are "baffled" that Debian interprets the status as such | 12:49.51 |
| Robin_Watts | tor8: A fixed version of my commit is on robin/master. Clean cluster results. | 12:50.11 |
| chrisl | I think it is reasonable to assume that code contributed to an open source project is intended to be licensed under the same open source license | 12:50.12 |
| jo0nas | which open source license is that? Which license *exactly* - not just the kind of license? | 12:50.40 |
| chrisl | The patch was against Ghostscript 8.54 was was licensed under the GPL version 2 | 12:50.42 |
| jo0nas | "...or newer"? I seem to recall several chunks of code back then that lacked the "or newer" part, but I might be mistaken | 12:51.15 |
| | if without the "or newer", then that is not relicensable as AGPL | 12:51.45 |
| | ...I believe | 12:52.17 |
| | ...bu I am not a lawyer | 12:52.25 |
| | ...but I am also not out to harrass you - I really would love to help here | 12:52.55 |
| chrisl | My understanding differs from yours, then. | 12:54.36 |
| jo0nas | chrisl: by your understanding, which exact license was the ramfs code in 2007? | 12:56.11 |
| chrisl | GPLv2 | 12:56.23 |
| jo0nas | without "...or later"? | 12:56.35 |
| chrisl | Without | 12:56.45 |
| jo0nas | ok | 12:56.48 |
| | thanks | 12:56.50 |
| chrisl | But, my understanding is that we can legally redistribute GPLv2 licensed code as GPLv3 (although we could not assert the extra restrictions of v3 on that specific code). | 12:57.55 |
| jo0nas | that code cannot - by your understanding of the licensing of it in 2007, and the public record or its evolution since, be redistributed under the terms of the Debian Free Software Guidelines | 12:58.03 |
| chrisl | So, even if it is *legal*, Debian says "no"? | 12:58.40 |
| jo0nas | how can you redistribute GPLv2 code as GPLv3? | 12:58.40 |
| | I believe only "GPLv2 or later" can legally be redistributed as GPLv3 | 12:59.15 |
| kens | thinks its time for ths conversation to end | 12:59.27 |
| chrisl | Because GPLv3 only reduces the rights available, it does not extend any rights | 12:59.37 |
| jo0nas | ...but we disgree on interpretation of the relevancy of "or later" fundamentally, it seems | 12:59.39 |
| kens | jo0nas : we differ in our itnerpretation, you don;t intend to distribute the source, so that's OK, right ? | 12:59.46 |
| jo0nas | it is fine, yes | 13:00.06 |
| kens | Then I thnk we don't need to discuss it any further | 13:00.16 |
| jo0nas | thanks for clarification of your interpretation | 13:00.20 |
| | I agree | 13:00.25 |
| kens | We may choose to discuss it more internally | 13:00.29 |
| | And we have tried (again) to contact MIchael | 13:00.43 |
| | Maybe if he gets deluged enough he'll contact us just to shut us up | 13:00.58 |
| jo0nas | chrisl: you might want to read http://www.gnu.org/licenses/gpl-faq.html#VersionThreeOrLater - but you are free to disagree also with the authors of the license that you (and according to you also Michael Slade) uses | 13:05.31 |
| kens | Lawyers regularly disagree over the interpretation of laws, and often come up with interpretations which do not match those of the original creator. | 13:06.47 |
| | Now can we please end ths ? Please ? | 13:07.01 |
| jo0nas | says no more | 13:10.06 |
| tor8 | Robin_Watts: some more timings: | 14:54.26 |
| | origin/master 2.1s, | 14:54.31 |
| | putc for 1-byte 0.9s, | 14:54.31 |
| | file descriptors with own buffering 0.7s, | 14:54.31 |
| | buffering in fz_output and static inline fz_write_byte 0.3s, | 14:54.31 |
| | your branch 0.15s | 14:54.31 |
| | so at least we should move to putc | 14:55.04 |
| | file descriptors are not worth bothering with | 14:55.21 |
| | a 3x speed up from static inlining and buffering in fz_output | 14:55.41 |
| | might be worth going for as well | 14:55.47 |
| | but the code gets a bit messy :/ | 14:55.51 |
| | I'd want to write to fz_buffer's without double buffering but then you run into a mess of when to grow etc | 14:56.26 |
| Robin_Watts | OK, so my commit seems sensible anyway. | 14:58.18 |
| | The question is how far do we want to go in the search for speed otherwise... | 14:58.34 |
| tor8 | if we go with buffering in fz_output, micro-optimizing the pgm output as in your patch is less necessary | 14:59.27 |
| Robin_Watts | Were it not for the fact that the simple putc fix is so good, I'd be tempted to go for the complex solution with fz_output having buffering etc. | 15:00.13 |
| tor8 | yeah. I'm inclined to just stick with the putc fix... | 15:00.37 |
| Robin_Watts | This stuff is slap bang in the middle of the road when it comes to people like ray doing timings. | 15:00.53 |
| | and it makes the difference between us beating or being beaten by gs etc. | 15:01.13 |
| | How about we commit the putc thing, and my one now, and we can ponder on the larger solution later ? | 15:02.14 |
| tor8 | Robin_Watts: sure. | 15:02.32 |
| Robin_Watts | cool. | 15:02.38 |
| tor8 | we can probably inline the out->write function and save one function call overhead | 15:02.46 |
| | so the fz_write is inlined into directly doing out->write | 15:03.06 |
| | or just a macro to do it | 15:03.14 |
| Robin_Watts | tor8: with care, possibly, yes. | 15:03.37 |
| tor8 | doing so shaves another 50ms off the time over just the putc call | 15:10.47 |
| | and it's a pretty minor change | 15:10.57 |
| | commits on tor/master | 15:11.59 |
| Robin_Watts | Shouldn't fz_write still return an int ? | 15:16.51 |
| | and why manually inline that in the fz_write_int32be ? | 15:17.05 |
| | are compilers too crap to use inlines within inlines? | 15:17.19 |
| tor8 | Robin_Watts: I figure for the debug builds mainly. | 15:17.50 |
| | we never use the return value; and our write either writes fully or throws an exception unlike system write() | 15:18.06 |
| | I'd be okay with taking out the manually inlined ones | 15:18.49 |
| | updated version on tor/master | 15:19.58 |
| Robin_Watts | I have no strong preference. | 15:20.12 |
| | Leave it as is then. lgtm. | 15:20.18 |
| sebras | Robin_Watts: ping? | 17:25.51 |
| Robin_Watts | pong | 17:25.58 |
| sebras | Robin_Watts: can the harfbuzz-stuff split word nodes in two? | 17:26.03 |
| | Robin_Watts: where does this take place? | 17:26.11 |
| Robin_Watts | sebras: bidirectional processing can split word nodes in two. | 17:26.31 |
| | Suppose we get a word node that's ABCDedfg | 17:26.46 |
| | where ABCD is latin, and edfg is arabic. | 17:26.59 |
| | the first fragment needs to be left 2 right, the second right 2 left. | 17:27.19 |
| | We split nodes so that each fragment has just one direction. | 17:27.41 |
| | You want to know where that happens? | 17:27.54 |
| sebras | Robin_Watts: right, so this happens in detect_flow_directionality()? | 17:28.01 |
| Robin_Watts | Yes. | 17:28.24 |
| | detect_flow_directionality gathers a paragraphs worth of stuff up into a unicode buffer, and presents it to the UAX#9 code. | 17:28.59 |
| | That calls us back into newFragCb | 17:29.13 |
| sebras | Robin_Watts: how does that callback work? | 17:29.43 |
| | Robin_Watts: e.g. what is fragment_len? | 17:30.05 |
| Robin_Watts | We get called for each 'fragment' in the passed in buffer at a time. | 17:30.09 |
| | strictly in order. | 17:30.19 |
| | so if we had ABCDefg we'd get called with the pointer to A, len 4, l2r, then we'd get called with the pointer to e, len 4, r2l | 17:31.02 |
| sebras | len = 1; | 17:31.02 |
| | text = " "; | 17:31.02 |
| | oops. | 17:31.05 |
| | why is there a len> fragment_len check in there? | 17:31.28 |
| | Robin_Watts: can the callback be called with a smaller fragment than the node contains? | 17:31.49 |
| Robin_Watts | because len might be > fragment_len ? :) | 17:31.50 |
| | oh yes. | 17:31.54 |
| sebras | why? | 17:32.05 |
| Robin_Watts | Suppose our source text is "Mary had a little lamb" | 17:32.48 |
| | that has nodes: "Mary" " " "had" " " "a" " " "little" " " "lamb" | 17:33.05 |
| | we collate that into "Mary had a little lamb" and pass that to the UAX#9 code. | 17:33.24 |
| | That calls us back saying that the first fragment is "Mary had a little lamb" | 17:33.37 |
| | There, fragment_len > len. | 17:33.49 |
| | That's the opposite case to the one you were worried about? | 17:34.07 |
| sebras | I think so, yes. | 17:34.25 |
| Robin_Watts | For the opposite example take "ABCDefg" | 17:34.30 |
| | there we get called back with "ABCD" and then "efg" | 17:34.52 |
| sebras | ok, because I'm seeing a polish text "zaj" being split into "za" and "j", which is baffling. | 17:35.05 |
| Robin_Watts | so node->len > fragment_len | 17:35.10 |
| | sebras: by this code? | 17:35.23 |
| sebras | it could be because I'm always setting len == 1 for soft hyphens regardless of whether these are going to be displayed or not. | 17:35.43 |
| | Robin_Watts: len should only represent the length of the characters that will actually be displayed then. | 17:36.10 |
| Robin_Watts | sebras: It's possible that my code in newFragCb doesn't understand soft hyphen nodes. | 17:36.10 |
| sebras | Robin_Watts: it doesn't, and I'm trying to fix that, but I'm not sure how newfragcb works. | 17:36.30 |
| sebras | is slowly getting it though. | 17:36.42 |
| Robin_Watts | sebras: Yeah, in newFragCb, I assume that all glue nodes hold " ", hence len 1. | 17:36.47 |
| sebras | Robin_Watts: and what if I were to claim that too, but some of those glue nodes are not actually shown on screen? | 17:37.15 |
| Robin_Watts | You need to alter the if (data->flow->type == FLOW_GLUE) side to set len to the number of chars within the fragment that should be skipped. | 17:37.30 |
| sebras | Robin_Watts: would that mean that fragment_len is off by one somewhere that might trigger a node split? | 17:37.34 |
| Robin_Watts | i.e. if you're not putting anything into the collated Unicode buffer that gets fed into the UAX#9 code, then you should set len to 0 there. | 17:38.04 |
| | Does that make sense? | 17:38.20 |
| | (In detect_flow_directionality, we put stuff into the buffer, I put " " in for glue nodes - have you changed that?) | 17:39.31 |
| | for softhypen nodes presumably you want to not put anything in. | 17:39.53 |
| sebras | ah, ok. | 17:39.56 |
| Robin_Watts | You need to do the matching thing in newFragCb. | 17:40.08 |
| sebras | yes I have, but I probably haven't changed the callback accordingly. now I'm beginning to understan how this works. | 17:40.15 |
| | ok. | 17:40.18 |
| | thanks for the explanation. | 17:40.21 |
| | Robin_Watts: I think I _do_ want to put something in, but only if the soft hyphen has already been deemed to break the line. | 17:40.50 |
| Robin_Watts | sebras: detect_flow_directionality happens BEFORE any lines are broken. | 17:41.40 |
| | The directionality of fragments is the same regardless of layout. | 17:41.53 |
| | hence we load the nodes, find the directionality, then layout. | 17:42.09 |
| | and the breaking of lines happens in layout, right? | 17:42.20 |
| sebras | Robin_Watts: yes, it happens in the function below the call to fz_layout_html(). | 17:43.42 |
| Robin_Watts | sebras: So I think you can get away with not putting anything into the buffer. You just need to arrange to set len=0 when you meet such a node in newFragCb. | 17:44.42 |
| sebras | Robin_Watts: mmm, and handle the node in the same way both during directionality detection and the callback. | 17:45.14 |
| | Robin_Watts: fixed it. | 17:45.57 |
| Robin_Watts | Fab. | 17:46.02 |
| sebras | Robin_Watts: thanks. :) | 17:46.03 |
| Robin_Watts | no worries. | 17:46.06 |
| sebras | Robin_Watts: I was introducing characters in detect directionality but setting len == 0 in the callback. | 17:46.39 |
| | not a smart move. | 17:46.51 |
| | tor8: Robin_Watts: I have something that appears to work now over at sebras/master. feel free to complain about variable names, indentation, spaces between function identifiers and the succeeding parenthesis, wording in comments. but most importantly, please review the algorithm itself. | 18:13.09 |
| Robin_Watts | sebras: Why does add_flow_sbreak need to set node->w = 0; ? | 19:09.41 |
| | In layout_flow, shouldn't "start by skipping whitespace (and newline) at beginning of tags" skip sbreaks too ? | 19:12.47 |
| | likewise find_list_mark_anchor ? | 19:14.48 |
| | In detect_flow_directionality, I would have expected the FLOW_SBREAK: case to be len = 0; text=""; break; rather than broken =1; | 19:16.41 |
| | but I guess it doesn't matter particularly. | 19:16.52 |
| | In the second commit: Now you've got add_flow_break setting node->w = 0 too. | 19:17.49 |
| | If those nodes are ever being set to non-zero width in the layout phase, then you can't rely on them being set to 0 at creation time. | 19:18.29 |
| | because you might layout the same flow several times. | 19:18.43 |
| | I don't follow the need for the new algorithm, in particular the nested loop. | 19:22.28 |
| | Forward 1 day (to 2016/02/13)>>> | |
IRC Logs