| <<<Back 1 day (to 2020/09/21) | Fwd 1 day (to 2020/09/23)>>> | 20200922 |
Robin_Watts_ | sebras: https://git.ghostscript.com/?p=user/robin/mupdf.git;a=commitdiff;h=b1037fff88caf6bb54e804b9d3c728e9af91c915 (ator seems happy with it, but it'd be good to get your opinion too) | 10:46.09 |
ator | Robin_Watts_: I've got two commits on tor/master | 10:47.45 |
| I might've missed something in the CHANGES update | 10:48.05 |
Robin_Watts_ | looks | 10:59.26 |
| ator: CHANGES looks good to me, modulo my crap memory. | 11:01.36 |
| the fax one... it's a change in behaviour in the encoded_byte_align && end_of_line case, I believe. | 11:02.05 |
ator | yes. there's an analysis in the bug report. | 11:02.35 |
Robin_Watts_ | I was expecting a change in behaviour in the encoded_byte_align && !end_of_line case. | 11:03.05 |
ator | end_of_line already aligns, IIRC | 11:03.58 |
Robin_Watts_ | In the old code, in that case, we did eat_bits(fax, (12-fax->bids) &7); | 11:04.31 |
| now we do nothing. | 11:04.38 |
ator | I believe the EOL eating code will eat the padding bits | 11:05.11 |
Robin_Watts_ | but if you're confident, then fine. | 11:05.17 |
| Ok. | 11:05.19 |
ator | if there's alignment, it will see the '0' bits and eat them until it finds a real EOL code | 11:07.01 |
Robin_Watts_ | ator: I'm looking at the customer request for merging pages from java. | 11:07.13 |
ator | with the patch, if encoded_byte_align is set but the fax stream doesn't align, it will now work | 11:07.23 |
| Robin_Watts_: mutool merge equivalent? | 11:07.38 |
Robin_Watts_ | If we expose pdf_flatten_inheritable_page_items, then in theory it can all be written in java. | 11:07.44 |
| (well, I guess we could reimplement that in java too...) | 11:08.08 |
ator | we have PDFGraftMap, so yeah, I think it should be possible | 11:08.22 |
Robin_Watts_ | but I'm thinking that actually just having a pdf_graft_mapped_page would be useful. | 11:08.44 |
| which we'd expose as PDFGraftMap_graftPage | 11:09.08 |
ator | flattenInheritablePageItems would have to be applied to the source document before copying the page object to the new document, right? | 11:09.45 |
Robin_Watts_ | yes. | 11:10.04 |
ator | might be better to have a graftPage method then | 11:10.07 |
Robin_Watts_ | Yeah, crap. I was hoping to avoid changing the source doc. | 11:10.25 |
ator | graftPage could copy and flatten non-destructively | 11:10.50 |
Robin_Watts_ | And I was planning to use flatteninheritablewossits inside graftPage. | 11:10.57 |
ator | maybe we should add that to the C api too :D | 11:10.57 |
Robin_Watts_ | ator: Oh, yes. I was thinking add to the C api, and then expose it. | 11:11.12 |
| And it'd simplify pdfmerge.c | 11:11.19 |
ator | well, grafting a page leaf would copy all the parents too | 11:11.24 |
| that would then get garbage collected | 11:11.40 |
| all rather messily | 11:11.42 |
| better I think to indeed add a pdf_graft_page that copies and flattens the page object's inheritable properties | 11:12.30 |
Robin_Watts_ | yes, that would be nicest. but hard, I fear. | 11:12.54 |
| Maybe not so bad. I'll have a crack. | 11:13.32 |
| Also... | 11:14.40 |
| graft maps point from a src doc to a dest doc, right? | 11:15.03 |
| Why then does the graft map only know about the dest doc? | 11:15.17 |
| Ignore me. It knows about both. | 11:15.41 |
ator | yes. the graft maps are used to not copy the same object more than once. | 11:15.42 |
| so it remembers copied object numbers | 11:15.54 |
| both in the source and destination document, obviously :) | 11:16.13 |
| it should be easy, I think | 11:16.32 |
| I can have a stab if you run into issues | 11:16.44 |
Robin_Watts_ | ator, sebras: What version of java are you running? | 13:23.03 |
| I have jdk1.7.0_79 here. I've been avoiding moving to java 8 for some reason. | 13:23.27 |
sebras | 11 something i think, let me check | 13:30.20 |
| Robin_Watts_: 11.0.8 | 13:31.21 |
ator | 11.0.8 here as well | 13:32.40 |
| OpenJDK Runtime Environment (build 11.0.8+10-post-Debian-1deb10u1) | 13:32.51 |
Robin_Watts_ | Is that an oracle thing? or an openjdk thing? | 13:32.51 |
| Right. | 13:32.55 |
sebras | openjdk. | 13:32.57 |
Robin_Watts_ | any reason why you stop at 11? | 13:33.33 |
ator | it's what ships with debian | 13:34.04 |
| haven't given it much thought | 13:34.17 |
sebras | yean, openjdk-12 is not available. | 13:34.34 |
| didn't know one existed. | 13:34.43 |
Robin_Watts_ | https://jdk.java.net/ | 13:35.06 |
| That has 15 as latest stable and 16 as 'early access'. | 13:35.32 |
sebras | well, then we won't accidentally use new library functions introduced after 11. ;) | 13:38.59 |
ator | openjdk.java.net for the open source versions | 13:40.06 |
sebras | ator: oh, there is no openjdk-12, but there is openjdk-13 and openjdk-14. | 13:40.24 |
ator | looks like they're emulating firefox and chrome's versioning number | 13:40.48 |
| bump the major number every two weeks | 13:40.52 |
| http://openjdk.java.net/projects/jdk/12/ | 13:41.25 |
sebras | ator: default-jre depends on openjdk-11 | 13:41.30 |
| ator: might not have been packaged into debian though. | 13:41.45 |
ator | they do two releases a year, looks like | 13:42.19 |
| march and september | 13:42.22 |
sebras | btw, does it matter which one we run? | 13:42.41 |
ator | I don't want to faff about with updates and bleeding edge versions that break everything. java's a stable enough language, I see no urgent need for newer features. especially not given how annoying java is to install outside of the linux distro package manager. | 13:43.45 |
| remember to uncheck the checkbox that lets oracle install spyware on your computer, etc. | 13:44.04 |
sebras | do they do that on linux too nowadays? | 13:44.25 |
Robin_Watts_ | openjdk.java.net doesn't include windows versions, AFAICT. | 13:44.35 |
sebras | mcafee isn't available on linux, I hope. | 13:44.44 |
ator | dunno, it was a nightmare scrubbing that shit off my windows machine last time I wasn't paying attention... | 13:44.45 |
malc_ | ator: pardon my ignorance but wtf is faff? | 13:44.49 |
| nm urban has it | 13:45.10 |
ator | malc_: you got it :) | 13:45.20 |
sebras | there is an openjdk-15 in debian/unstable | 13:45.55 |
malc_ | ator: yeah! i waste time doing nothing as we speak | 13:45.59 |
| however no... i learn completely pointless newspeak | 13:46.14 |
| ho well | 13:46.15 |
Robin_Watts_ | sebras, ator: ReleaseJava builds won't build for me... | 13:59.33 |
| valid &= com_artifex_mupdf_fitz_PDFAnnotation_TYPE_RICH_MEDIA == PDF_ANNOT_RICH_MEDIA; | 13:59.42 |
| and | 13:59.58 |
| valid &= com_artifex_mupdf_fitz_PDFAnnotation_TYPE_PROJECTION == PDF_ANNOT_PROJECTION; | 13:59.58 |
| Also, I get lots of warnings about: return jni_rethrow(env, ctx); | 14:00.32 |
sebras | PDFAnnotation_TYPE_PROJECTION seems to be missing. | 14:01.40 |
ator | Robin_Watts_: yeah, broken for me too | 14:06.35 |
Robin_Watts_ | am fixing now. | 14:06.49 |
sebras | http://git.ghostscript.com/?p=user/sebras/mupdf.git;a=commitdiff;h=caa8772dca0fc9a131b275540450235f72452cf4 | 14:08.54 |
| Robin_Watts_: ^? | 14:08.58 |
Robin_Watts_ | sebras: Yup. Though you caught more cases than I had so far. Will copy yours. | 14:09.42 |
sebras | Robin_Watts_: I see not warnings about jni_rethrow(). | 14:10.06 |
| s/not/no/ | 14:10.13 |
Robin_Watts_ | In C, you can't do return void_function(); | 14:10.29 |
| MSVC spits many hundreds of dummies about it. | 14:10.46 |
ator | Robin_Watts_: sebras: odd that GCC allows it without warning | 14:13.27 |
| the spec does forbid it though | 14:13.35 |
| "A return statement with an expression shall not appear in a function whose return type is void." | 14:13.45 |
sebras | clang doesn't say anything either. | 14:13.45 |
| clang 9.0.1-14 that is. | 14:13.57 |
ator | though that wording is new in C11 | 14:14.07 |
sebras | ator: no, jni_rethrow() returns void | 14:14.30 |
ator | if the function returns void, you can't use return <expr>; | 14:14.51 |
sebras | ator: and then I do return jni_rethrow(), -1; where -1 is the actual return value. | 14:14.53 |
| jni_rethrow() is just to do the exception conversion. | 14:15.26 |
ator | jni/nativedevice.c: return jni_rethrow(env, ctx); | 14:15.39 |
| does not return anything | 14:15.48 |
| Buffer_writeByte is another example | 14:16.09 |
| C++ allows it, https://en.cppreference.com/w/cpp/language/return | 14:16.54 |
| "In a function returning (possibly cv-qualified) void, the return statement with expression can be used, if the expression type is (possibly cv-qualified) void. " | 14:17.17 |
sebras | why does everything I touch turn in to a cesspool of shit. | 14:18.00 |
ator | I thought we could use return (like in C++) in C too | 14:18.38 |
| but if they're taking away that in C11 we better adjust | 14:19.03 |
Robin_Watts_ | sebras: It's not you. It's "computers" :) | 14:21.18 |
ator | a shame though, I live the regularity of being able to return a void-expression from a void-returning function :( | 14:22.10 |
| like* | 14:22.13 |
sebras | Robin_Watts_: yeah, I bloody hate them. | 14:22.34 |
ator | Yeah. Sometimes I get the urge to go analog... | 14:22.44 |
Robin_Watts_ | ator: likewise. | 14:22.46 |
ator | never lasts long though... | 14:22.54 |
Robin_Watts_ | Do I fix this by defining: | 14:25.58 |
| #define return_jni_rethrow(env, ctx) { jni_rethrow(env, ctx}; return; } | 14:26.34 |
| and changing 'return jni_rethrow(...)' to 'return_jni_rethrow(...)' where appropriate? | 14:27.00 |
| Or do I write it out longhand? | 14:27.05 |
| The former is more compact, the latter clearer. | 14:27.26 |
| or do we do something whereby we do: | 14:28.37 |
| return jni_rethrow(env, ctx, type); ? | 14:28.53 |
ator | I was wondering if we could do return jni_rethrow(env, ctx, ) with just a blank third argument for the return value | 14:29.30 |
| #define jni_rethrow(E,C,value) { jni_rethrow...; return value; } | 14:29.54 |
Robin_Watts_ | So we can do: return jni_rethrow(env, ctx, void); or return jni_rethrow(env, ctx, int) or return jni_rethrow(env, ctx, ptr) | 14:29.54 |
| not sure the CPP will accept that? | 14:30.04 |
ator | but emtpy argument may cause cpp to have a hissy fit? | 14:30.06 |
| return_jni_rethrow_void() and return_jni_rethrow(value) maybe? | 14:30.57 |
| I'd rather the macro here, TBH | 14:31.15 |
| than writing it out longhand | 14:31.20 |
sebras | ator: is this would be a macro including the return keyword then..? | 14:31.27 |
| or not in the case of void? | 14:31.33 |
Robin_Watts_ | zoomtime. | 14:32.19 |
ator | it would have to in case of void | 14:32.23 |
Robin_Watts_ | It'd be { jni_rethrow(whatever); return; } I think. | 14:34.55 |
| or { jni_rethrow(whatever); return <whatever value>; } | 14:35.16 |
ator | sounds good to me | 14:35.41 |
sebras | I'm working on it. | 14:35.57 |
| if you trust me to unfu...nction what I functioned up. | 14:36.26 |
Robin_Watts_ | sebras: Sure. Go for it. | 14:37.28 |
| And it's really not your fault if your compiler is not giving you warnings. | 14:37.47 |
malc_ | sebras: you are very selective with your ellipsises :) | 14:44.34 |
sebras | malc_: :) | 14:49.56 |
ator | I thought for manual duplex you print odd, then flip the stack over and print evens in reverse order | 15:01.15 |
sebras | Robin_Watts_: http://git.ghostscript.com/?p=user/sebras/mupdf.git;a=commitdiff;h=2a12ef6f46b7b7fad85dd442f1538e6fdc006d50 | 15:05.08 |
| ator: wrong channel? | 15:05.13 |
| ator: oh, did noticed. :) | 15:05.33 |
malc_ | mini stroke? | 15:05.37 |
ator | sebras: LGTM | 15:06.26 |
sebras | ator: does it build without warnings at your place too? | 15:07.26 |
| ator: I tested with gcc 10.2.0 and clang 10.0.1-5 | 15:07.46 |
malc_ | sebras: i'm impressed, dorothy i don't think you run Debian any more | 15:09.40 |
ator | sebras: I never got warnings | 15:10.16 |
sebras | ator: maybe we should look up what switch would enable those. | 15:10.43 |
ator | probably ones that would throw more problems our way | 15:11.04 |
| -pedantic or some such | 15:11.10 |
| or -std=c99 (or something strict non-"gnu") | 15:11.27 |
sebras | -Wreturn-type doens't seem to handle this. /me continues reading the manpage. | 15:11.44 |
ator | -Wpedantic throws it | 15:12.21 |
| warning: void function 'Java_com_artifex_mupdf_fitz_StructuredText_walk' should not return void expression [-Wpedantic] | 15:12.35 |
sebras | with -Wpedantic I saw more instances of this. ok. | 15:13.54 |
Robin_Watts_ | sebras: Why not just jni_rethrow_void(env,ctx); ? | 15:36.15 |
| jni_rethrow_int(env, ctx); (cos it's always 0). | 15:36.49 |
ator | Robin_Watts_: rule #1, don't hide control flow. it would be shorter though. | 15:36.50 |
sebras | Robin_Watts_: ator suggested return. | 15:36.53 |
Robin_Watts_ | Well, "rethrow" implies the control flow. | 15:37.10 |
ator | it does, I have no preference either way | 15:37.25 |
Robin_Watts_ | I agree normally, that hiding control flow is bad, but I feel rethrow and throw have the flow implicit. | 15:37.50 |
sebras | ok. I can rename it. | 15:37.52 |
ator | jni_rethrow_int() makes sense, because the return value will never be used | 15:39.08 |
| and jni_rethrow_jobject maybe | 15:39.27 |
Robin_Watts_ | jni_rethrow_ptr ? | 15:39.36 |
ator | or ptr | 15:39.41 |
sebras | what about return_jni_throw_uoe_value(env, "stream closed", -1); | 15:39.42 |
| and return_jni_rethrow_value(env, ctx, -1); in e.g. FitzInputStream_available | 15:40.04 |
Robin_Watts_ | s/return_// | 15:40.10 |
ator | sebras: why is it -1? | 15:40.14 |
| the return value will never be read on the java end, so what it is doesn't really matter does it? | 15:40.27 |
sebras | I think -1 comes from the InputStream interface. | 15:40.42 |
Robin_Watts_ | but if we've rethrown, the InputStream will never read the value, right? | 15:41.01 |
| cos as soon as we return it checks to see if we've thrown an exception, and if so, shoots off to handle it without looking at the value. Or have I misunderstood? | 15:41.46 |
sebras | no, wait. I'm confused. we throw an exception anyway. | 15:41.55 |
| Robin_Watts_: correct. | 15:42.10 |
Robin_Watts_ | So, 4 commits on robin/master, clustering now. ator has looked at the first. | 16:00.17 |
| I'm going to test the last one on unix now. | 16:00.34 |
| ator: I think there is a potential problem in parse_declaration_list in css-parse.c | 16:22.35 |
| If parse_declaration returns NULL (because of memory exhaustion, presumably) and we go into the while and get to the following parse_declaration(), and that works, tail = tail->next will access tail, which may not be set. | 16:23.29 |
| Oh, it's not memory exhaustion. It's a lookup for a css property that doesn't find anything. | 16:26.44 |
sebras | Robin_Watts_: I see no errors using -Wpedantic with what is on sebras/fix-java now. | 16:42.03 |
Robin_Watts_ | sebras: Fab. Will grab that. | 16:42.15 |
sebras | Robin_Watts_: I'm sure you'll find something, but I can't. :) | 16:42.45 |
Robin_Watts_ | sebras: Did you express an opinion on my rewritten jpx stuff? | 16:43.16 |
sebras | Robin_Watts_: not yet, not on the encoded byte align stuff either. | 16:43.38 |
| ator: asked me to take a look at both before the release. | 16:43.48 |
Robin_Watts_ | Ta. | 16:44.19 |
| sebras: You have a few: return_jni_rethrow_value(env, ctx, 0) in there. | 16:49.06 |
| shouldn't those be: jni_rethrow_int(env, ctx) ? | 16:49.19 |
| line 17 of buffer.c jni_throw_arg(env, "n cannot be negative"); | 16:51.02 |
| That needs to be... something else. jni_throw_arg_int(env, "n cannot be negative"); or something? | 16:51.27 |
| yeah, lots of stuff like that. | 16:52.03 |
malc_ | sebras: btw a) -Wpedantic is not the same as -pedantic b) warnings in all the compilers i know (regardless of the source language) are a mess c) try clangs -Weverything for size! | 16:57.10 |
sebras | malc_: I was looking for a switch that triggers the warning to be emitted in gcc. | 17:01.18 |
| malc_: -Wpedantic worked fine. | 17:01.25 |
malc_ | sebras: when it triggers nowadays it prints the name of the individual switch that enables it, that however is fragile on many levels | 17:02.26 |
sebras | Robin_Watts_: I wonder why I don't see any of those. | 17:03.15 |
| I'm going of for a bit but I'll return and try to fix that. | 17:03.25 |
Robin_Watts_ | sebras: It's possible that there is a git fubar between you and me somewhere and i'm looking at something old. | 17:03.54 |
| sebras: I'm going to have a bash at the commits. Yell when you come back so we don't both waste time on this. | 17:46.07 |
| sebras: OK, I've pushed a modified version of your commit on robin/master. See what you think. | 18:51.05 |
| sebras: OK, updated with a whole bunch of stuff. I think it's right, but I've got to step away now. | 19:10.48 |
sebras | just got back. | 19:17.37 |
| Robin_Watts_: platform/java/jni/android/androiddrawdevice.c and platform/java/jni/android/androidimage.c still look strange in you patch. | 19:28.23 |
| ther eare still comma expressions and return keywords..? return jni_rethrow(env, ctx), 0; | 19:28.38 |
| and in android_AndroidDrawDevice_invertLuminance rethrow_ | 19:29.28 |
| void has become jni_rethrow(). | 19:29.35 |
Robin_Watts_ | sebras: Bugger. | 19:29.37 |
sebras | Robin_Watts_: besides that, renaming bonanza, but that's ok. | 19:29.58 |
Robin_Watts_ | sebras: Oh, I missed all the android ones :( | 19:30.22 |
sebras | I guess you don't build the android .c-files so you didn';t see it. | 19:30.25 |
Robin_Watts_ | Will fix. | 19:30.32 |
| sebras: Urm.... | 19:35.22 |
| in android_AndroidDrawDevice_invertLuminance it *should* be jni_rethrow_void() ? | 19:35.47 |
sebras | yes, but your commit changes it to jni_rethrow()...? | 19:36.50 |
| at least according to my diff: git diff bb76d6223 749a2ddbf | 19:37.02 |
Robin_Watts_ | Let me commit what I have now, and we can look again. | 19:37.39 |
| New version pushed. | 19:39.00 |
sebras | Robin_Watts_: ok, android_AndroidDrawDevice_invertLuminance() now looks sane. | 19:41.59 |
Robin_Watts_ | Oh, ass. In all this, I've lost my PDFGraftMap_graftPage java changes :( | 19:45.17 |
| I'll redo those tomorrow. | 19:45.51 |
sebras | oh, sorry about that. :-/ | 19:47.58 |
| I never saw a commit about that, so presumably it was only in your working tree. | 19:48.23 |
Robin_Watts_ | I committed it, I think, but it probably got lost in a rebase. | 19:52.16 |
| Absolutely my fault, and NOT a big deal. | 19:52.28 |
sebras | Robin_Watts_: maybe you can find it in "git reflog"..? | 19:53.40 |
| Robin_Watts_: re: the commit -- it looks fine. | 19:53.51 |
| Robin_Watts_: LGTM "Wrap jni_rethrow() and family in macros." | 19:56.21 |
| Robin_Watts_: LGTM "java: Enable pedantic compilation warnings." | 19:56.27 |
| Robin_Watts_: LGTM "Fix warning from linux java build." | 19:57.11 |
| I'm not sure how that got there. | 19:57.17 |
| I'll head to bed now, but I'll go back to reviewing your and ator's patches tomorrow. | 19:57.45 |
Robin_Watts_ | sebras: Ta. | 23:00.44 |
| <<<Back 1 day (to 2020/09/21) | Forward 1 day (to 2020/09/23)>>> | |