| | <<<Back 1 day (to 2017/06/11) | 20170612 |
| tor8 | sebras: ping. | 13:27.16 |
| | I've looked over your -mini and -old commits | 13:28.02 |
| | 'adb shell pm clear' is not good. it zaps all settings, including saved bookmarks. | 13:28.23 |
| | there are a few string resources you can remove in -old to match the cleanup (strings that are no longer used) | 13:28.59 |
| | and the applicationVariants stuff looks weird | 13:30.07 |
| | is the namespace really android.android.applicationVariants? | 13:30.16 |
| sebras | tor8: what are the strings you mention? | 13:36.09 |
| | I tested without pm clear and it works ok. | 13:36.31 |
| tor8 | R.string.picker_title_App_Ver_Dir, and maybe R.string.app_name, R.string.version | 13:36.40 |
| | yes, pm clear just deletes all the settings and locally saved files | 13:36.53 |
| | it has nothing to do with 'power management' or restarting the app | 13:37.06 |
| | we talked about this a few weeks ago :) | 13:37.17 |
| sebras | tor8: hm.. app- | 13:41.33 |
| | napp_ | 13:41.36 |
| | app_name is used in AndroidManifest.xml | 13:41.45 |
| | tor8: I added pm clear because I wanted a way to kill the app back when I started looking at -nui | 13:42.24 |
| | tor8: long before I barely knew what I was doing. | 13:42.46 |
| | tor8: regarding the android namespace... I'm not sure how to deduce the namespace from the android DSL reference: https://google.github.io/android-gradle-dsl/current/com.android.build.gradle.AppExtension.html | 13:47.56 |
| tor8 | sebras: from that it looks like it should be android.applicationVariants (and not android.android.applicationVariants) | 13:53.07 |
| | you've got the other stuff like android.sourceSets and android.splits at that level | 13:53.31 |
| | does the code currently on your branch do what is intended? | 13:53.54 |
| sebras | tor8: ok, I just tested without the android. prefix to applicationVariants and it seems to work fine too. | 13:58.00 |
| tor8 | odd that it works with double prefixes | 13:58.11 |
| sebras | tor8: I decompiled the resulting .apks and checked the versionCode/versionName and it seems to work. | 13:58.15 |
| | tor8: yeah, no errors! | 13:58.27 |
| | tor8: what code were you referring to that might not work as intended? | 13:58.55 |
| tor8 | I wonder how they really intend to do with multiple apks and versionCode | 13:58.55 |
| | because there's a 'universalApk' setting, etc | 13:59.03 |
| | for doing split apk:s | 13:59.10 |
| | android.android.foo | 13:59.23 |
| | the versionCode numbering scheme differs between -mini and -old | 13:59.46 |
| sebras | https://developer.android.com/studio/build/configure-apk-splits.html#configure-APK-versions | 13:59.49 |
| tor8 | -mini has 11100 where -old has 111 | 13:59.54 |
| sebras | tor8: it is multiplied by 100 in the override at the end of build.gradle | 14:01.08 |
| tor8 | ah, you multiply in the output.versionCodeOverride | 14:01.14 |
| sebras | yes. | 14:01.16 |
| | I contemplated having stating that multiplication explicitly in defaultConfig and then simply summing the two in the output.versionCodeOverride | 14:01.42 |
| tor8 | I worry that this stuff is going to break in the future, and be cocked up by customers | 14:01.45 |
| sebras | tor8: absolutely! | 14:01.53 |
| | tor8: but it replaces release.sh | 14:02.01 |
| | tor8: which is just another way to state the same mess. | 14:02.21 |
| tor8 | is there a way to make all variants hard coded in the build.gradle file, like a section for each apk? | 14:02.22 |
| sebras | tor8: I haven't found a way. | 14:02.46 |
| tor8 | sebras: yes, I understand the reasoning behind it and removing the dependency on a bunch of shell scripts and sed is nice. | 14:02.47 |
| | google "helpfully" has a sentence like "To distribute your app using Multiple APK Support in Google Play, assign the same applicationId value to all variants and give each variant a different versionCode." in its build-variants page | 14:03.22 |
| | with no further instructions | 14:03.27 |
| sebras | tor8: I primarily tried to write it using gradle so that I can later move the documentations of how to compile for android from libmupdf to mupdf-android-viewer-{old,mini,nui} | 14:03.46 |
| tor8 | android.productFlavors maybe? | 14:03.46 |
| sebras | tor8: naah.. productflavors are separate. | 14:04.10 |
| | tor8: that's the demo vs full version of an app. | 14:04.22 |
| | tor8: we want to assign versionCodes based on ABI, which is not the same thing. | 14:04.46 |
| tor8 | couldn't we (ab)use it for intel vs arm vs mips versions of an app? | 14:05.11 |
| | agh, it *feels* like there should be a way ... given that you can tell it to generate one apk per abi, how on earth do they intend us to use that? | 14:05.53 |
| sebras | semantic error in symbol on line 1: intend | 14:06.39 |
| tor8 | indeed. | 14:07.00 |
| | I would be a bit happier with the * 100 being in the versionCode line | 14:07.16 |
| | and just add the offset to the versionCode in the versionCodeOverride | 14:07.30 |
| sebras | tor8: is it ok for me to say that gradle is not necessarily working _with_ me, but rather against me? | 14:11.12 |
| | tor8: wrt versionCode I have something on -old:sebras/master that seems to work. | 14:12.19 |
| | tor8: how did you know that you can use abi in a splits block? | 14:13.55 |
| | tor8: oh. I'm daft, nvm. | 14:16.18 |
| tor8 | any particular reason you write 111 * 100 instead of 11100? | 14:20.01 |
| sebras | tor8: because I want to stress that the 100 are there just to save space for the ABIs. | 14:20.47 |
| tor8 | 111 is just a magic number too... | 14:20.58 |
| | unless you parse and crack the equation below it for formatting the versionName | 14:21.12 |
| sebras | tor8: I guess we could give the versionName such as 1.11 and then compute the versionCode based on that..? | 14:21.15 |
| tor8 | I sort of liked having the versionName be "1.11 git" for non-release builds | 14:21.40 |
| | but I guess it's too hard to get it to do a $(shell git describe --tags) for the versionName? | 14:21.59 |
| sebras | tor8: I haven't even attempted that. | 14:22.10 |
| tor8 | it would fail for building from a tarball | 14:22.30 |
| | that is also something the release.sh script did (remove the " git" suffix to the versionName) | 14:22.50 |
| sebras | tor8: right, I missed that part. | 14:23.09 |
| | also I didn't see it being very valuable given that we don't really see the versionname being that valuable | 14:23.50 |
| | http://ryanharter.com/blog/2013/07/30/automatic-versioning-with-git-and-gradle/ | 14:24.05 |
| | how did release.sh work in a tarball? | 14:29.03 |
| tor8 | sebras: that looks like a nice option | 14:30.40 |
| | maybe with a fallback for when it fails | 14:30.48 |
| | release.sh was not intended to be used from a tarball :) | 14:31.09 |
| sebras | tor8: I'm thinking that maybe we should fishout FZ_VERSION from libmupdf/include/mupdf/fitz/version.h? | 14:31.10 |
| | unless we have a .git directory in which case we use the result of git describe --tags | 14:31.55 |
| tor8 | sebras: the app version does not necessarily match the library version | 14:32.26 |
| | we may want to let them diverge eventually | 14:32.34 |
| | so no, let's not use FZ_VERSION | 14:32.50 |
| sebras | tor8: ok, by using if ((new File('.git')).exists()) { } we know we are in a git repo and so can safely run git describe -tags | 14:33.24 |
| | tor8: what do I do if there is no .git? | 14:33.32 |
| | i.e. we are being compiled from a tarball. | 14:33.41 |
| | tor8: we could conceivably create a version.properties containing VERSION="1.11" when being built in a git repo and this is then included in the tarball? | 14:34.36 |
| tor8 | versionName = getVersionName("1.11") and use the default if no .git? | 14:35.10 |
| sebras | tor8: ok, so basically we moved the version number from res/values/strings.xml to build.gradle. | 14:35.49 |
| tor8 | I suspect we're digging too deep here just to save a bit of typing :) | 14:35.50 |
| sebras | no, no. I enjoy looking a build system files for weeks at end. | 14:36.10 |
| tor8 | I would just settle for hardwiring the versionCode and versionName in the build.gradle and not confuse our future selves and customers :) | 14:36.27 |
| sebras | tor8: so I remove the ABI-versionnumering too? | 14:36.56 |
| tor8 | losing the " git" version name suffix is a hit I'm willing to take to not complicate matters | 14:37.02 |
| | I think that complexity is needed though, at least until google gets their act together | 14:37.24 |
| sebras | tor8: if we use universalApks then we wouldn't need that compilcation, at the expense of users having to download unnecessarily big apps perhaps. | 14:38.09 |
| tor8 | sebras: yes. we want to avoid that (huge app bloat) | 14:38.27 |
| sebras | maybe google play allows devices to download partial apks..? | 14:38.33 |
| tor8 | sebras: I wonder, can we add *all* known ABIs to the abiVersionOffsets list, and list the used ABIs separately in ndk.abiFilters? | 14:39.06 |
| | that should work with the versionCodeOverride, and doesn't mean clueless customers ask questions about the offsets hash-map | 14:39.44 |
| sebras | tor8: I had it that way originally but opted against that because then the two lists would have to match. | 14:40.10 |
| tor8 | sebras: would they really? | 14:40.20 |
| sebras | tor8: that's why I did the keySet() trick. | 14:40.22 |
| tor8 | you just look up the offset based on the OutputFile.ABI string near the end | 14:40.49 |
| sebras | tor8: I believe so, because com.android.build.OutputFile.ABI evaluates to one of the ABIS. | 14:40.55 |
| | so whatever the OutputFile.ABI is it has to be a key in the map. | 14:41.29 |
| tor8 | yes. I don't see the problem. | 14:41.39 |
| sebras | what I'm trying to say is that the elements in ndk.abiFilters ought to match the keys in the map. | 14:42.08 |
| tor8 | if we have abiVersionOffsets = ["x86":1, "armeabi":2, "armeabi-v7":3, "mips":4, "x86_64":5, "mips64":6] | 14:42.26 |
| sebras | and it felt silly to list them twice. | 14:42.27 |
| tor8 | then whatever combination of abi:s listed for the final build in the ndk.abiFilter we can find them in the set | 14:42.49 |
| | and clueless customers can change the APIs they build for using bog standard gradle stackoverflow answers | 14:43.59 |
| | or is this something more than just a hash table mapping abi-name strings to integers? | 14:44.47 |
| sebras | no. | 14:44.58 |
| tor8 | I certainly like to tweak the list of ABIs to build. for debugging and development I only want to build for one ABI | 14:49.29 |
| | but for releases I need them all | 14:49.36 |
| | so making edits to the list of ABIs more work makes me unhappier | 14:50.10 |
| sebras | tor8: fortunately you will need to edit build.gradle to do that. | 14:50.15 |
| tor8 | yes, I don't mind doing that | 14:50.25 |
| | but I would appreciate not having to edit a full hash table mapping :) | 14:50.42 |
| | hm. is it possible to have separate ndk.abiFilters for release and debug builds? | 14:51.15 |
| | I guess you want both arm and x86 for debugging on emulator and phone, whereas I only really do debugging on the emulator | 14:51.35 |
| sebras | that's alos something I looked into a while back and I couldn't find a way to do that. | 14:51.47 |
| tor8 | then let's ignore it, and get this stuff commited so you can get back to more exciting things like SSL... :P | 14:52.22 |
| sebras | tor8: not in an intended way at least. | 14:52.23 |
| | yeah.. | 14:52.32 |
| tor8 | mbed TLS (formerly known as PolarSSL) was the last thing we talked about looking into using, IIRC | 14:53.12 |
| sebras | tor8: how does -old:sebras/master look now? | 14:53.26 |
| tor8 | good, except maybe we should double check the list of ABIs available (and sort the list?) | 14:54.43 |
| sebras | tor8: do we care about how they are numbered? | 14:55.04 |
| tor8 | not in the least | 14:55.14 |
| sebras | tor8: as I read https://developer.android.com/google/play/publishing/multiple-apks.html#Rules that is not the case. | 14:55.22 |
| tor8 | there's an arm64-v8 architecture as well | 14:55.46 |
| sebras | tor8: do we want to get the full list programmatically, sort it and simply enumerate them? | 14:56.15 |
| | tor8: even if a new arch is introduced then it wouldn't matter. | 14:56.47 |
| tor8 | is it possible to get them programmatically? | 14:57.00 |
| | they don't even need to be sorted (I just suggested that for ease of future additions) | 14:57.19 |
| sebras | I think so, this why you need to reset | 14:58.06 |
| tor8 | AbiSplitOptions.applicableFilters? | 14:58.13 |
| sebras | before doing include in the split.abi-block | 14:58.16 |
| tor8 | split.abi.applicableFilters() maybe? | 14:58.42 |
| | nah, let's just hard code the list | 14:59.14 |
| sebras | that list is empty. | 14:59.54 |
| tor8 | huh. oh well, guess we don't really have that option then. | 15:01.25 |
| sebras | tor8: from the implementation (thanks procyon) it looks like the android gradle plugin's getApplicableFilters() should result the default list. | 15:05.39 |
| tor8 | do we want to rely on undocumented functions though? | 15:06.15 |
| sebras | tor8: no, I'm just trying to understand how this works. | 15:06.47 |
| | tor8: the DSL documentation isn't really helpful. | 15:07.01 |
| | tor8: I mean "Returns a list of all applicable filters for this dimension." what is dimension here? | 15:07.38 |
| | filter? | 15:07.43 |
| tor8 | splits.abi or splits.density, I reckon | 15:08.17 |
| | so splits.density.applicableFilters() would return the set of "hdpi", "mdpi", etc | 15:08.44 |
| | or that was what I expect, the documentation is not very forthcoming | 15:08.57 |
| sebras | ah, or language. | 15:08.58 |
| | I guess simply listing them all and hardcoding the numbers is the easiest way forward. | 15:09.31 |
| | sebras/master updated again. | 15:10.09 |
| tor8 | sebras: fab. LGTM. | 15:10.36 |
| sebras | tor8: how about -mini:sebras/master? | 15:16.33 |
| | tor8: I also have a question about source/pdf/pdf-op-run.c::begin_softmask() | 15:19.47 |
| tor8 | -mini is also LGTM | 15:20.13 |
| | okay. | 15:20.33 |
| | ask and I shall confuse you (and myself) | 15:20.42 |
| sebras | if fz_begin_mask() throws should we call fz_end_mask()? | 15:21.27 |
| | always..? | 15:21.41 |
| | OTOH fz_begin_mask() seems like it cannot throw. | 15:22.24 |
| | in which case it really shouldn't be inside that fz_try(). | 15:22.40 |
| tor8 | do you mean fz_begin_mask or the code between fz_begin_mask and fz_end_mask? | 15:22.47 |
| sebras | fz_begin_mask() itself to start with. | 15:23.02 |
| tor8 | if fz_begin_mask itself throws, I would expect that the mask was never pushed and therefore we shouldn't be calling fz_end_mask | 15:23.12 |
| sebras | ok. | 15:23.21 |
| tor8 | but if the stuff in between throws, it would be safest to call end_mask anyway | 15:23.25 |
| sebras | but we do call end_mask. | 15:23.34 |
| tor8 | in case we try to recover we shouldn't be leaving the clip/mask stack unbalanced | 15:23.37 |
| | yes, that is probably incorrect | 15:23.53 |
| sebras | so in tht case end_mask reallt should be inside the fz_try()? | 15:24.14 |
| tor8 | a lot of this complexity is down to FZ_ERROR_TRYLATER | 15:24.21 |
| | for progressive rendering | 15:24.24 |
| sebras | now, what if pdf_run_xobject() throws? | 15:24.33 |
| | in that case we should call fz_end_mask. | 15:24.42 |
| tor8 | yeah | 15:24.51 |
| sebras | how would we know if fz_begin_mask() succeeded? | 15:25.02 |
| tor8 | so putting it in the try-block is going to fail in the latter case | 15:25.04 |
| sebras | unless we also put one in fz_catch()...? | 15:25.20 |
| tor8 | I can't remember how robust our error recovery is and where/if we try to do any | 15:25.46 |
| sebras | tor8: so error handling was one of the things I was adressing on my old branches. | 15:26.08 |
| | this is the reason for me asking about this. | 15:26.12 |
| tor8 | but I suspect we can get away with doing whatever we want in the case of errors | 15:26.24 |
| | but calling end_mask if begin_mask fails is definitely wrong | 15:26.33 |
| | so putting both begin_mask, run_xobject, and end_mask in the try-block sounds reasonable to me | 15:26.47 |
| sebras | tor8: if fz_begin_mask() returned a sucess/fail value | 15:28.34 |
| | tor8: then that coudl be used to determine if fz_end_mask should be called in fz_always(). | 15:28.48 |
| tor8 | if fz_begin_mask fails, it throws... | 15:29.28 |
| | if you want to determing such things, I think you'll need two try-blocks | 15:29.48 |
| | right, so I've dug through the interpreter looking for how we handle exceptions | 15:30.29 |
| sebras | tor8: I need more context to understand what you are writing about. | 15:31.08 |
| tor8 | and there is a possibility we get ourselves into a bit of a twist IFF we have a cookie and cookie->incomplete_ok is set | 15:31.22 |
| | in the interpreter we call a bunch of pdf_processor callbacks for each operator | 15:31.39 |
| | if one of these operators throws, what do we do? | 15:31.49 |
| | what if the 'q' (gsave) operator throws, for instance | 15:32.00 |
| | how do we continue from there, do we pop back to a normal state and try to resume interpreting | 15:32.17 |
| | or do we abort and stop drawing on the page | 15:32.26 |
| | etc | 15:32.28 |
| sebras | I would assume we pop back to normal state. | 15:32.34 |
| tor8 | this code is a bit of a mess, I would be surprised if anyone knows what exactly it is we do :( | 15:32.55 |
| | pdf_process_stream() | 15:33.20 |
| | in the big fat fz_catch block at the bottom of the function | 15:33.35 |
| | depending on the type of error we do different things | 15:33.54 |
| | anyway, if we get a random error we abort interpreting the page | 15:34.15 |
| | it's only in the case of syntax and trylater errors we try to carry on | 15:34.35 |
| | and fz_begin_mask would throw neither of those | 15:34.53 |
| sebras | tor8: only the test device throws FZ_ERROR_ABORT. | 15:35.01 |
| tor8 | it would probably throw an out of memory or generic error | 15:35.02 |
| | yes, that's one for a device asking for an early abortion of interpreting | 15:35.27 |
| | the problem is one of what happens when run_xobject fails | 15:36.24 |
| | that could throw due to a TRYLATER error (such as an image not being found yet) | 15:36.34 |
| | where we would try to carry on rendering the rest of the page as much as we can | 15:36.49 |
| | in that case we would want to match up the begin/endmask calls | 15:37.04 |
| | but I suspect there are a lot of places where we don't cope very gracefully with this | 15:37.16 |
| | it's not very extensively tested | 15:37.26 |
| | we only use it in mupdf-curl | 15:38.25 |
| | and I'm not convinced it's a useful feature | 15:38.45 |
| sebras | TRYLATER? | 15:39.23 |
| tor8 | y. | 15:39.30 |
| | it's the progressive loading code | 15:39.38 |
| sebras | tor8: I know. | 15:39.42 |
| | so on sebras/wip I have 20 or so potentially fz_throw() issues. | 15:40.02 |
| | this is one that I have yet to fxi. | 15:40.21 |
| | basically I'm askin fz_push_try() to fake throw once it has been called a certain number of times. | 15:40.45 |
| | and then look for memory leaks. | 15:40.59 |
| | I have found a few... | 15:41.03 |
| | though in this base when fz_begin_mask() was triggered to throw I got a message about begin/end not being in synch. | 15:42.20 |
| tor8 | I'm not in the least surprised :) | 15:42.26 |
| | yeah, the code looks definitely wrong in that it calls end_mask if begin_mask fails | 15:43.16 |
| sebras | tor8: I'll move the end_mask into the fz_try. | 15:43.30 |
| tor8 | that's break the trylater case. i'd do two fz_try statements | 15:44.42 |
| | one with begin_mask that if it throws, unconditionally rethrows | 15:44.50 |
| | and a second one for the run_xobject with the end_mask in fz_always | 15:45.01 |
| sebras | tor8: how do you feel about origin:sebras/master? | 16:00.29 |
| | I attempted to fix a few problems in the commits you prepared. | 16:00.43 |
| | I verified that the installation commands work as described so I think it is an improvement. | 16:02.42 |
| | need to sleep. | 16:09.37 |
| | Forward 1 day (to 2017/06/13)>>> | |
Log of #mupdf at irc.freenode.net.